Concurrent requests for token endpoint on auth-code flow with same code succeed.

[tn185075]

Preflight checklist

• [X] I could not find a solution in the existing issues, docs, nor discussions.
• [X] I agree to follow this project's Code of Conduct.
• [X] I have read and am following this repository's Contribution Guidelines.
• [ ] I have joined the Ory Community Slack.
• [ ] I am signed up to the Ory Security Patch Newsletter.

Ory Network Project

No response

Describe the bug

When an authorization code is issued to the client, if the client makes two concurrent requests for token endpoint using the same auth code, it results in two tokens, as the code is not invalidated in PopulateTokenEndpointResponse before the other request reaches the HandleTokenEndpointRequest method.

Reproducing the bug

1. Run the auth code flow with a registered client.
2. Make two concurrent requests (can use goroutines) on token endpoint with the same auth code.
3. We can get the token for both the requests.

Relevant log output

No response

Relevant configuration

No response

Version

v0.42.2

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes

Additional Context

No response

[tn185075]

Bump ‼️ ‼️

[vivshankar]

Won't this depend on the storage implementation? In my implementation, we use redis for such short term storage and treat the Get as a pop. The choice of storage doesn't matter too much.

I expect that is the only way to atomically implement this.

[james-d-elliott]

There are quite a few options in Postgres to deal with this from memory.

[mitar]

I think you need at least repeatable read isolation levels, but you can just go with serializable.

[vivshankar]

@tn185075 are you concerned about the implementation in fosite that uses the memory store? At the moment, the InvalidateAuthorizeCodeSession is a no-op for me. I expect in any practical implementation, that would be the case. The GetAuthorizeCodeSession should pop, i.e. get and delete atomically.

I haven't looked at Hydra.

[tn185075]

popping it out on GET is a probable solution, but has it's own limitations. As we need to keep that record, in case if we fail to reach the populateTokenEndpointResponse, client should be able to use the auth code again to fetch the token.

Also it's one of the key info, which we can use to detect malicious activities against our providers I believe.

I do not think it is a storage problem, as we are making a get in HandletokenEndpointRequest, but invalidation happens in PopulateTokenEndpointResponse. This is a clear case of race/timing conditions, in my opinion, even if we manage to wrap the block invalidating auth code over mutex :).

[tn185075]

If I remember rightly, I believe I was able to replicate this with Hydra :).

[tn185075]

Any updates here? ‼️