Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove msisdn from request body #59

Conversation

fernandopradocabrillo
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

Remove phone number (MSISDN) from request body

Which issue(s) this PR fixes:

Fixes (#58)

Special notes for reviewers:

There are other PRs with changes that will be applied together. Once previous PRs are merged I will update and align this one.

Changelog input

N/A

Additional documentation

N/A

@bigludo7
Copy link
Collaborator

Hello @fernandopradocabrillo
Sorry if my question is stupid but if we remove the msisdn the API will be unuseable in Wifi or Tetheering situation right ?

@fernandopradocabrillo
Copy link
Collaborator Author

Hi @bigludo7
So, AFAIK, I don't see the relation with the wifi or tethering case. If you are in the situation of using one of those you won't be able to use AuthCode since it uses network auth, so you will have to use CIBA passing the phone number as login_hint. This will associate the phone_number to the token so you won't have to pass the phone number to the endpoint.

@gregory1g
Copy link
Collaborator

as far as I understand, when an application does not have an access token it will be redirected to OIDC endpoint, there it will send auth request together with MSISDN as login_hint (with needed scopes). OIDC server will check if this application has a permission to requested scope and given MSISDN. If "yes" (pre-configured or granted by the user interactively - does not matter), OIDC server returns an access token (which is MSISDN specific). If no, and only in this case, OIDC could need to contact user to request their consent.
So, if we assume that SimSwap is opt-in by default (based on a legal agreement between user and application provider), OIDC server will not need to contact a user. And if user explicitly opted-out, again, OIDC sees this and simply reject authorization without contacting the user.

@DT-DawidWroblewski
Copy link
Collaborator

Based on current Local Champion activities in Germany it has been agreed to move forward with API release that contains client credentials security scheme. Therefore, it is required to have MSISDN passed inside resource request. Removing MSISDN makes sense where 3-legged-token is in place. Suggestion: keep phone_number as a option in a resource request (add proper description that refers to client credentials flow).

@bigludo7
Copy link
Collaborator

@DT-DawidWroblewski the decision in Germany is only for the check endpoint or for both endpoints?

I'm wondering if we should not split this API in 2 for simplification.

  • one yaml for the check endpoint and could be client credential as it seem that what the market has
  • one yaml for the get simswap date and this one in three legged and probably not require the msisdn

@gregory1g
Copy link
Collaborator

Based on current Local Champion activities in Germany it has been agreed to move forward with API release that contains client credentials security scheme. Therefore, it is required to have MSISDN passed inside resource request. Removing MSISDN makes sense where 3-legged-token is in place. Suggestion: keep phone_number as a option in a resource request (add proper description that refers to client credentials flow).

So, we keep support for both options, right? If so, does it make sense to make MSISDN optional, so it can be skipped when 3-legged approach is used?

Copy link
Collaborator

@DT-DawidWroblewski DT-DawidWroblewski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx @fernandopradocabrillo !

all good - according to our last meeting

@DT-DawidWroblewski DT-DawidWroblewski merged commit adb6b81 into camaraproject:main Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants