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

Clarify what should happen to authorization codes on an error response #82

Closed
aaronpk opened this issue Jun 12, 2021 · 6 comments
Closed
Assignees
Milestone

Comments

@aaronpk
Copy link
Member

aaronpk commented Jun 12, 2021

If there was a problem with the token request that otherwise contained a valid authorization code, clarify whether the authorization code should remain valid or should be treated as if it had been used successfully.

@Zegnat
Copy link

Zegnat commented Jun 14, 2021

From the current OAuth RFC 6749 § 10.5 Authorization Codes, emphasis mine:

Authorization codes MUST be short lived and single-use. If the authorization server observes multiple attempts to exchange an authorization code for an access token, the authorization server SHOULD attempt to revoke all access tokens already granted based on the compromised authorization code.

I always read this to mean that any attempt to exchange, whether failing or not, counts as a use. As soon as the token endpoint sees any request coming in with the authorization code, the code can be marked as having had its single use, no matter what the endpoint decides to respond to the request.

This makes sense to me too. As soon as the first request has gone over the wire we start being susceptible to replay attacks, so even if that first request could not be exchanged correctly we would not want to see the same code again.

My 2¢ after missing the discussion on IRC. But this has always been my reading, whether valid or not.

@aaronpk aaronpk added the editors discussion Items to discuss in the next editors' call label Sep 8, 2021
@dickhardt
Copy link
Collaborator

I don't think the AS should revoke previously valid access tokens. The authorization code should only be presented once, subsequent presentations should be an error.

Some more color on the problems that might occur would help this discussion!

@jogu
Copy link

jogu commented Jul 27, 2022

To throw a few considerations in here:

It is useful if a successful authorization code use can be attempted again in the event that the client fails to receive/successfully process the response. (i.e. the same problem many folks have seen with refresh token rotation where the new refresh token isn't received by the client and, if the old refresh token was immediately invalidated, then the client has lost access to the data and can't regain access until the user next interacts with the client - but in the case of the authorization code this is a smaller problem as the client is likely interacting with the user at the same and can just send them through the authorization endpoint again. Also to add this isn't a theoretical problem, multiple Openbanking ecosystems have had issues with clients losing access to data due to refresh token rotation failing.)

For the purposes of testing an OAuth implementation, it'd be really handy if it was clearly defined that an erroneous use of an authorization code did NOT count as a use. Defining it like that means a large number of tests can be done against a production system, without requiring that a manual authorization endpoint interaction (potentially with 2FA) happens for each test. (Currently the OpenID Foundation FAPI test suite is a bit constrained in the testing it can do against the authorization code grant as we have to assume the authorization code has been consumed even when we use it unsuccessfully, meaning we have to carefully weigh up the benefits of performing an extra test vs the "cost" of the test user having to do extra manual interactions.

A further point is that I believe the DPoP spec assumes (or should assume) an authorization code is NOT consumed if the token endpoint call fails because the server wants a newly provided nonce to be used. I don't recall this being mentioned before so opened danielfett/draft-dpop#164

@aaronpk
Copy link
Member Author

aaronpk commented Jul 27, 2022

From the side meeting

  • Allow the request to be rejected without touching the authorization code if it failed because of client authentication
  • Specify in the authorization code extension how to reject the request and when to invalidate the authorization code
  • Put this in the error response section in the authorization code flow

@bc-pi
Copy link

bc-pi commented Jul 27, 2022

concur with the side meeting decisions FWIW

@aaronpk aaronpk modified the milestones: version -07, version -08 Oct 24, 2022
@aaronpk aaronpk removed the interim Items to discuss in the next WG interim meeting label Nov 4, 2022
@aaronpk aaronpk self-assigned this Nov 4, 2022
aaronpk added a commit that referenced this issue Jul 27, 2023
@aaronpk aaronpk modified the milestones: version -09, version -10 Aug 20, 2023
@aaronpk
Copy link
Member Author

aaronpk commented Jan 9, 2024

I went with describing the intended outcome instead of all the details:

The authorization server MUST return an access token only once for a given authorization code.

This allows DPoP to have the client send the authorization code multiple times, since it doesn't place a restriction on clients "using" authorization codes.

@aaronpk aaronpk closed this as completed in 404f464 Jan 9, 2024
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

No branches or pull requests

5 participants