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 all commits
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
18 changes: 10 additions & 8 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: Start user consent capture process<br>following Section 3.1.2.4 of the OIDC Core 1.0 spec.
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 @@ -187,10 +189,10 @@ Then, two alternatives may occur:

**Scenario 2**: Consent is required and not yet provided by user (Step 8)

- The operator performs the consent capture. Since the authorization code grant involves the frontend, the consent can be captured directly from the user.
- The operator performs the consent capture following Section 3.1.2.4 of the OpenID Connect Core 1.0 specification. 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