fosite icon indicating copy to clipboard operation
fosite copied to clipboard

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

Open vivshankar opened this issue 2 years ago • 0 comments

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 -

  • Add request.SetID(originalRequest.GetID()) before https://github.com/ory/fosite/blob/master/handler/oauth2/flow_refresh.go#L80

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 avatar Jul 15 '23 01:07 vivshankar