thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.53k stars 1.11k forks source link

Domain events do not provide information about tokens #1145

Closed halfpastfouram closed 3 years ago

halfpastfouram commented 4 years ago

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

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

halfpastfouram commented 3 years ago

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

GDmac commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

halfpastfouram commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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