-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
proposal/discussion: OAuth: requirement for refresh_token lifetime #1968
Comments
I move discussion from #2040 (comment) to this issue. The latest proposal:
|
Proposed update:
|
We are in OAuth, so we are talking about authorization but …
Do we need to talk about re-authentication here or about re-authorization? Would it be considered bad practice to re-authorize without going through all the OAuth flow dance? Would it be OK if the authorization server was sending you a notification saying "authorization XXX which was granted for application YYYY is about to expire" and then you could expand its lifetime from the authorization server instead? One downside of this approach is that there might be a higher chance that a rogue application/instance would impersonate a legitimate one. |
I believe this is correct, at some point the user should re-authenticate doing the full "OAuth dance" to prove that the user is in control of the users credentials (in example MFA), not allowing "forever sliding" refresh tokens (or sessions). But as @randomstuff noted, the way this was written it is unclear (or if it also forbids) the use of re-authorization, which was not intended...perhaps this will make it clearer?
|
Even in this case sliding can not go over refresh token expiration time.
At the same time I'm not convinced we should cover to extension in the same requirement, Maybe the focus should be only on the expiration time. |
I do not think refresh-refresh or expiration is enough of a control for refresh tokens in L 2/3 apps.
FAPI mandates sender-constrained tokens using Mutual TLS (MTLS), Proof of Possession (PoP) or similar - to avoid any kind of token theft reuse - which is still possible even with refresh-refresh or tight expiration windows. I think is a very wise constraint from FAPI that we should emulate.
|
@TobiasAhnoff, This is actually not what I meant, sorry :) |
@jmanico, I don't think anyone was saying that refresh token expiration was in any way an alternative to sender-constrained tokens. The idea here is that is that all grants should eventually expire after a given duration even if it tokens are being refreshed periodically. The proposal from #2040 (comment) was “user re-authentication should be forced at some point in time» i.e. OAuth 2.0 code flow should be done again after this duration. My argument was that the authorization server might accept other methods for extending the duration of the grant through user interaction involving verification of the user consent. |
my apologies :)
@randomstuff maybe a dumb question, but could you give an example on verification methods (if not using the same method as the initial user authentication)? |
@TobiasAhnoff, actually I understood “user re-authentication” as “the user must use the client application to restart the OAuth flow in order to obtain new refresh token” which is actually not exactly what is being said in the text. I retract the comments on this topic which were useless :) (My point was, it might (maybe) be OK for the user to extend the lifetime of the existing grant by going directly from the authorization server without going through the client application.) |
@randomstuff not useless at all, now I understand! And yes, I was actually thinking of "using the same client", but as you noted the way it is written it allows for other scenarios as well (like "going directly from the authorization server"). |
I'm a bit lost here in comments, what is the latest proposal to discuss? |
I think the last comments are on
But, as you noted, maybe it is good to just focus on expiration time? Then it could be like this
or
|
I think it makes sense just to deliver the point of absolute expiration. ping @randomstuff - any comments? |
Last version from @TobiasAhnoff looks good to me. I guess it should be enough (?). Could this be formulated as?
What worries me somewhat with this formulation is that it's not the refresh token which has a maximum expiration time but the grant. When you do the token refresh token request you (may) obtain a new refresh token but the expiration delay is not restarted i.e. the expiration deadline is shared for all associated refresh tokens. Would something like this be OK?
|
@randomstuff - for example Keycloak extending (sliding?) the refresh_token exp but still has maximum lifespan set. I would like to have wording to cover / not restricting that. I agree that terminology "refresh token" vs " grant" requires some work. |
I think the suggested requirement from @randomstuff is good and address that sliding must not pass the expiration deadline, or is something missing @elarlang ? Using the right terminology is a bit challenging...this is the best guidance I know of https://aaronparecki.com/2024/03/29/3/oauth-terminology. Aaron notes that
Perhaps add "authorized" to make it a little more clear to ASVS readers what "grant" means?
One more note, since "grant" is more general than "refresh-token lifetime", and applies to both code and client-credentials, maybe remove "user" or add "and client"? |
@TobiasAhnoff, "authorized grant" is probably more explicit although "authorized grant" seems somewhat tautological to me 😆 |
I try to keep it as simple as possible with only one clear focus - if a new refresh token is provided, it does not extend the max lifetime, including when sliding exp is used. Whatever business logical decision needs to be made before the expiration happens, is not in the scope of this requirement.
The more interesting part is, should we cover somehow the 'offline_access' in this requirement? |
I don't think this is really related, is it? This should probably go into the OpenID Connect chapter, instead. |
Yes, but this is an option to remove expires time for refresh token JWT. |
Goes in via #2140
If it requires update, we can do that. It's more to get the requirement in and having a number for it. |
spin-off from #1925 "proposal 7", from @TobiasAhnoff
Proppoal from Elar:
The refresh_token topic requires more attention. But there is more than one point of view.
It is different attitude, is the OAuth used as first-party or 3rd party solution. For the first-party and "session management replacement" (which should be disallowed or not recommended) I would say we can apply V3.3 Session Timeout and V3.8 Session Termination requirements.
For 3rd party solution, we have a requirement currently located in 3.5.1 (it is subject to change via #1917 (comment))
I think we need to be really precise, about what architecture and solution we address with the requirement.
One extra issue to cover with the refresh_token topic expiration is that with new refresh_token the AS must keep the
exp
value like it was before (and not extend it).The text was updated successfully, but these errors were encountered: