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

V51 - OAuth - confidential client #2109

Closed
elarlang opened this issue Sep 23, 2024 · 7 comments · Fixed by #2118
Closed

V51 - OAuth - confidential client #2109

elarlang opened this issue Sep 23, 2024 · 7 comments · Fixed by #2118
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

Spin-off from #2040 (comment)

Verify that confidential clients must use client authentication in refresh token requests. (L1, L2, L3)

Isn't it the point for confidential client to use client authentication?

If we require it from Level 1 from confidential client but allow public clients without any restrictions, I would say there is a conflict.

Is it required only for refresh token request or to any request to token endpoint?

@elarlang elarlang added the V51 Group issues related to OAuth label Sep 23, 2024
@randomstuff
Copy link
Contributor

I believe the idea here is that that some authorization server implementations could assume that presenting the refresh token is enough for refreshing the token (as presenting the access token is enough for using the service). This is requirement is indeed valid for all requests to the token endpoint.

A more precise wording of this requirement could be something like: ”Verify that, for using refresh tokens emitted to a confidential client, the authorization server requires authentication of this client.“

@elarlang
Copy link
Collaborator Author

Isn't it the point for confidential client to use client authentication?

My point is - can you use confidential client without authentication for the token endpoint?

... for using refresh tokens emitted

but asking the refresh token with token request?

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Sep 23, 2024
@TobiasAhnoff
Copy link

My point is - can you use confidential client without authentication for the token endpoint?

It should not be possible, but not all AS implementations follows the RFC which states that confidential clients must be authenticated, both when using refresh-tokens, authorization codes or client-credentials.
See in example https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.3

This is important for a secure OAuth implementation, but perhaps not a widespread issue, except if using a custom built AS or flow (not "well-known industry standard"). Still, I think it is a good level 1 requirement for ASVS.

It could also me written to not just focus on refresh-tokens, like this

Verify that all confidential clients are authenticated for all token requests. (L1, L2, L3)

There is no conflict with public clients, since the requirement only applies to confidential clients.

@elarlang
Copy link
Collaborator Author

elarlang commented Sep 24, 2024

Ok, the definition says "capable of", not doing so https://datatracker.ietf.org/doc/html/rfc6749#section-2.1

confidential - Clients capable of maintaining the confidentiality of their credentials (e.g., client implemented on a secure server with restricted access to the client credentials), or capable of secure client authentication using other means.

So from that perspective it makes sense.

Should we add "authorization server" in the requirement?

Verify that all confidential clients are authenticated for all token requests to the authorization server.

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Sep 25, 2024
elarlang pushed a commit to elarlang/ASVS that referenced this issue Sep 25, 2024
@elarlang
Copy link
Collaborator Author

At the moment it goes to the document as:

# Description L1 L2 L3
51.2.7 [ADDED] Verify that all confidential clients are authenticated for all token requests to the authorization server.

@elarlang elarlang added 6) PR awaiting review and removed 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Sep 25, 2024
@elarlang elarlang linked a pull request Sep 25, 2024 that will close this issue
@randomstuff
Copy link
Contributor

Coming from #2042, should we extend this to include other backend requests (such as PAR, token revocation, token introspection). Something in the line:

Verify that all confidential clients are authenticated for client-to-authorized server backchannel request such as token requests, PAR requests, token revocation requests and token introspection requests.

@elarlang elarlang reopened this Sep 26, 2024
@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something and removed 6) PR awaiting review labels Sep 26, 2024
@elarlang
Copy link
Collaborator Author

elarlang commented Sep 27, 2024

Update - from "all confidential clients" to "confidential client"

# Description L1 L2 L3
51.2.7 [ADDED] Verify that confidential client is authenticated for client-to-authorized server backchannel requests such as token requests, PAR requests, token revocation requests, and token introspection requests.

@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants