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.

Sign up for GitHub

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

Auth code flow update to fix issue #70 #86

Merged
merged 2 commits into from
Jan 15, 2024
Merged

Auth code flow update to fix issue #70 #86

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75a2ee2
Auth code flow update to fix issue #70
jpengar Nov 20, 2023
1ead9a5
added Eric's rewording suggestion
jpengar Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions documentation/CAMARA-API-access-and-user-consent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,16 @@ alt Standard OIDC Auth Code Flow between Invoker and API Exposure Platform
alt If Consent is Granted or Consent not needed for legal basis
ExpO-->>FE: 302<br>Location: invoker_callback?code=Operatorcode
else If Consent is NOT granted - Consent Capture within AuthCode Flow
Note over FE,ExpO: Consent capture
alt when user grants consent / otherwise flow fails
ExpO-->>FE: 302<br>Location: invoker_callback?code=Operatorcode
end
end
Note over FE,ExpO: Consent capture
Copy link
Collaborator

Choose a reason for hiding this comment

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

The description of the process as "Consent Capture" is ambiguous here. It could be interpreted to mean "wait until user consent is obtained before issuing the operatorCode", but that is not what is intended here. A fuller description, such as "Start asynchronous end user consent capture process" would be better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@eric-murray Thank you for your comment! But actually I don't see how it applies to Auth code flow. In the case of Auth code flow, the consent capture is not an asynchronous process like in, say, CIBA. In fact, in the case of Auth code flow, the application is redirected to a "consent capture page" where the user can browse and grant or revoke consent for the requested resources.
And what is particularly clarified in this PR is that if the user doesn't consent to the access (where explicit consent is required by the legal basis), the code is still provided in the GET /authorize response.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @jpengar

It was precisely because an authorisation code is returned whether or not the end user consents that I had assumed the "Consent Capture" did not need to be synchronous.

Let's see what the OpenId Connect core specification has to say about this:

Once the End-User is authenticated, the Authorization Server MUST obtain an authorization decision before releasing information to the Relying Party. When permitted by the request parameters used, this MAY be done through an interactive dialogue with the End-User that makes it clear what is being consented to or by establishing consent via conditions for processing the request or other means (for example, via previous administrative consent). Sections 2 and 5.3 describe information release mechanisms.

So "an interactive dialogue" is but one option for OpenId Connect. But I now understand that CAMARA are mandating this approach, and that "Consent Capture" consequently must be synchronous?

This is an important point of difference from the OpenId Connect specifications which needs to be properly documented by CAMARA. Specifying this as "Consent Capture" is insufficient.

Copy link
Collaborator Author

@jpengar jpengar Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@eric-murray To date, the agreed upon and defined Auth code flow captures user consent (when required) via an interactive browsing dialog to which the user is redirected (as documented).
Also, there is nothing preventing an application from executing the Auth code flow at any time, not necessarily at the same time the network resource (CAMARA API) is consumed. So, for example, consent could be obtained at the time of user onboarding.

In any case, this topic is not related to the specific change being made is this PR. "Consent Capture" was also specified before this change. So please, let's not block this PR change because of this parallel discussion. Would you please open a new issue and contribute with your documentation improvement suggestion to avoid confusion?

Copy link
Collaborator Author

@jpengar jpengar Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@bigludo7 @rartych Could you kindly provide your review? Thank you so much in advance!
This PR has been open for weeks :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jpengar
The other changes have changed the meaning of "Consent capture" here. Previously the rule was that, if the user had not consented and the "Consent capture" process was then started, the OperatorCode would be returned "when user grants consent / otherwise flow fails".

In other words, if end user consent could not be obtained (for whatever reason), then the flow could be terminated with an appropriate error.

But now - it appears the OperatorCode must always be returned once the end user identifier is validated, but only once "Consent capture" is complete. Subsequent text states:

  • Once the user has given consent, the authorization code flow continues by redirecting to the API invoker redirect_uri (invoker_callback) and including the authorization code (OperatorCode).

Previously, if the end user did not consent, that could be interpreted as "otherwise flow fails". But now the flow cannot fail. In that case, the above text could now be interpreted as we must wait until we know whether the end user has consented or has refused consent, and then proceed to return the OperatorCode.

But what if the end user does nothing? Or if prompt=none (as it will be for Number Verification)? Or they are using a device which cannot support interactive dialogues and a slower method of consent capture must be initiated? We just ..... wait? This cannot be the intention. Asynchronous consent capture must be an option if the /authorize request now cannot fail because consent cannot be obtained in good time.

Whether intentional or not, the proposed changes appear to remove the option to terminate the flow in the event that end user consent cannot be completed synchronously, without allowing it to be completed asynchronously. I would suggest to clarify the "Consent capture" process by rewording this to "Start end user consent capture process following Section 3.1.2.4 of the OpenID Connect Core 1.0 specification".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would suggest to clarify the "Consent capture" process by rewording this to "Start end user consent capture process following Section 3.1.2.4 of the OpenID Connect Core 1.0 specification".

@eric-murray I'm fine with your rewording suggestion. Added in commit 1ead9a5

ExpO-->>FE: 302<br>Location: aggregator_callback?code=Operatorcode
end
FE-->>-BE: GET invoker_callback?code=OperatorCode
BE->>ExpO: POST /token<br> code=OperatorCode
ExpO->>BE: 200 OK <br> {OperatorAccessToken}
alt If Consent is Granted or Consent not needed for legal basis
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am NOT OK with this PR because I think that (10)-(12) are not needed in https://github.com/camaraproject/IdentityAndConsentManagement/blob/75a2ee2ac4e30e4f07783f7059941ed46a83e257/documentation/CAMARA-API-access-and-user-consent.md ?
I think that user authentication and consent collection happen in "[Standard OIDC Auth Code Flow between Invoker and API Exposure Platform]" (7) - (8).
If consent is not given then the AZ does not return a code but OIDC error.
Having a code means "The AZ is OK with exchanging the code for an access token". No further consent checks are needed on the token endpoint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AxelNennker I understand that you might disagree. But the documentation update is only intended to document in CAMARA the solution already agreed and discussed in GSMA to address the front-end flow PII leakage issue originally raised.

This is the problem of overlapping docs in GSMA and CAMARA as raised by @bigludo7 in issue #82. But for now, we would need to keep the documentation in sync in both places, GSMA & CAMARA, when it applies. And just moving this agreed change from GSMA to CAMARA is already taking weeks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should Camara copy something here that deviates from the OIDC standard?

OIDC is defined so that when you have the code all requirements to get the access token were checked - the user was authenticated and gave consent. If the user failed to authenticate or did not give consent then there is no code. So, there is no "alt"-section in OIDC when the code is being exchanged for the access token.

As I understand this, we/Camara would deviate from the OIDC standard.
Sorry, that GSMA OGW discussed and agreed upon something that is not acceptable.
I would not do this even if we see this as an intermediate step.
This PR is the place where we can and should fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to amend my comment. My interpretation is based on OIDC core which handles user authentication and consent collection, and OIDC does not discuss the case where the user declines consent and still some other scopes are allowed by an access token.
I think that users are going to be surprised when the client uses an API that they "think" they declined. Surprises are harmful for the public perception of Camara business.

GDPR requires, that "consent must be freely given, specific, informed and unambiguous". If a user declines consent and we create an access token afterwards anyway, then they will question all four points.

So, I think a privacy respecting flow would be to ask for consent and create an access token if consent is granted.
And ask for a separate access token using client credentials flow for those APIs that do not require consent.

But my opinion that this flow violates OIDC was too strong. OIDC is a profile of OAuth2 and in OAuth2 it is OK that the AZ issues an access token for a subset of the requested scopes.

I am glad that we discussed this and that we did not just accept this PR because it was discussed and agreed upon somewhere else.

ExpO->>BE: 200 OK <br> {OperatorAccessToken}
else If Consent is NOT granted - Flow fails if there is no other granted scope
ExpO->>BE: 400 Bad Request <br> {error: invalid_request}
end
end

BE->>ExpO: Access Operator CAMARA API <br> Authorization: Bearer {OperatorAccessToken}
Expand Down Expand Up @@ -190,7 +192,7 @@ Then, two alternatives may occur:
- The operator performs the consent capture. Since the authorization code grant involves the frontend, the consent can be captured directly from the user.
- Once the user has given consent, the authorization code flow continues by redirecting to the API invoker redirect_uri (invoker_callback) and including the authorization code (OperatorCode).

Once the API invoker receives the redirect with the authorization code (OperatorCode - Step 9), it will retrieve the access token from the operator's API exposure platform (OperatorAccessToken) (Steps 10-11). The OperatorAccessToken issued is encrypted so that no relevant information is exposed.
Once the API invoker receives the redirect with the authorization code (OperatorCode - Step 9), it will retrieve the access token from the operator's API exposure platform (OperatorAccessToken) (Steps 10-11). The OperatorAccessToken issued is encrypted so that no relevant information is exposed. If the user has not given consent, the access token will not contain the appropriate scopes, and if no other scopes are granted, the flow will fail.

Now the API invoker has a valid access token that can be used to invoke the CAMARA API offered by the operator (Step 12).

Expand Down