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

Domain events do not provide information about tokens #1145

Closed
halfpastfouram opened this issue Oct 29, 2020 · 8 comments
Closed

Domain events do not provide information about tokens #1145

halfpastfouram opened this issue Oct 29, 2020 · 8 comments

Comments

@halfpastfouram
Copy link

When a domain event is emitted the receiver of that event has no way to access information about for instance the access/refresh-token that was issued.

Situation

Our access tokens have a supplementary property called mfa_validated. That property's value is either true or false. When issuing a new access token that was requested via a refresh token the new access token should inherit the value of that property from the previous access token.

We're unable to find out what the previous access token or refresh token was. We are also unable to find out what the new access token is. This results in our users having to redo the MFA steps when the access token is expired but the refresh token is still valid.

Proposal

  • League\OAuth2\Server::ACCESS_TOKEN_ISSUED: Provide the new access token to the RequestEvent when a new access token was issued.
  • League\OAuth2\Server::REFRESH_TOKEN_ISSUED: Provide the new refresh token to the RequestEvent when a new refresh token was issued.

Or provide any other way to retrieve the access token or refresh token related to the event that was emitted.

@halfpastfouram
Copy link
Author

Any comments on this are very much welcome. I can create a PR if you'd like.

@GDmac
Copy link

GDmac commented Dec 17, 2020

Isn't your custom property mfa_validated something you
can store in your own implementation of OurCustomToken implements TokenInterface ?

The moment your implementation of AccessTokenRepositoryInterface
or RefreshTokenRepositoryInterface returns a new token,
it can set the property and log / notify accordingly.

@halfpastfouram
Copy link
Author

Late response, but no that won't help. The problem is that we need our logic to run AFTER the token is issued and BEFORE the http response is generated.

I'm thinking about suggesting a small change that would introduce two new event classes to make our situation possible until #1184 is fully implemented. In short, it would introduce AccessTokenIssuedEvent and RefreshTokenIssuedEvent, both extending the RequestEvent class. The first would accept an issued access token as a third parameter that can be accessed via an event listener. The second would do the same but with a refresh token.

@halfpastfouram
Copy link
Author

Just checked. It seems my issue is solved in #1211 that was released in 8.3.0. I will close this issue if I can use this solution.

@GDmac
Copy link

GDmac commented Jul 28, 2021

please tell me you are not modifying the token object thru an dispatched/submitted event

@halfpastfouram
Copy link
Author

halfpastfouram commented Jul 28, 2021

@GDmac In our case a user has to go through MFA. When MFA was succesful, the token's property mfa_validated will be set to true. That will not happen through an event.

But when the user's client requests a new access token with a refresh token, the new access token needs to have mfa_validated set to the same value that was set for the same property in the old access token. Otherwise the user would have to go through MFA again. So the what happens is that the previous token is checked (request body has access_token_id) and new access token's mfa_validated = old token's mfa_validated.

The token itself remains unmodified and the property mfa_validated is not exposed to the user or a client.

@GDmac
Copy link

GDmac commented Jul 28, 2021

So, you are setting (modifying) the mfa_validated property on the token you receive with the event?

My suggestion was that when the token expires you fetch the refreshtoken anyway from the repository.
At that moment that you can pass extra info (mfa) that has to be merged into the NEW token.
Always make it as immutable as possible $newToken = repo->fetchToken($oldToken)

@halfpastfouram
Copy link
Author

Great suggestion. I will definitely look into that, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants