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

Refresh token flow handler does not set the original request ID in the handler early enough #754

Open
4 of 6 tasks
vivshankar opened this issue Jul 15, 2023 · 0 comments
Open
4 of 6 tasks
Labels
bug Something is not working.

Comments

@vivshankar
Copy link
Contributor

Preflight checklist

Describe the bug

The requester ID for the Requester used in the refresh token flow should use the same ID as the original requester object. This is currently set just before the CreateRefreshTokenSession is called in the "Populate" step.

Problems -

  1. Implies that anything outside the refresh token handler is unaware of the original request's ID, which may play a part in populating audit records, other token sessions - such as the upcoming device secret, etc.
  2. Inconsistent with the auth code flow, where the token handler populates the ID in the right location based on the AuthorizeRequester.

Alternatives -

  • Where needed, call GetRefreshTokenSession again. This is potentially expensive, especially given the implementations in Hydra and others that call into the DB whenever this is invoked. This can be worked around if the implementer caches the refresh token session in the Go context and looks it up if the same function is invoked within the same request.

Resolution -

Reproducing the bug

This is not a bug that can be recreated without adding new handlers that consume the original request ID at specific places, such as at the end of token generation or request validation (NewAccessRequest).

Relevant log output

N/A

Relevant configuration

N/A

Version

N/A

On which operating system are you observing this issue?

macOS

In which environment are you deploying?

Binary

Additional Context

No response

@vivshankar vivshankar added the bug Something is not working. label Jul 15, 2023
vivshankar added a commit to vivshankar/fosite that referenced this issue Jul 15, 2023
vivshankar added a commit to vivshankar/fosite that referenced this issue Jul 15, 2023
vivshankar added a commit to vivshankar/fosite that referenced this issue Jul 15, 2023
vivshankar added a commit to vivshankar/fosite that referenced this issue Jul 15, 2023
aeneasr pushed a commit to vivshankar/fosite that referenced this issue Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

No branches or pull requests

1 participant