Repository navigation
Implement OIDC token renewal #641
Description
Activity
hmm good idea
I recommend only doing this when
process.env.NODE_ENV !== "production"or, better yet, check fordevelopment. Basically avoid doing so in a way that is prioritizing developer experience over end-user security.@gobengo thanks for the comment, good suggestion!
My thought was to increase 20 minutes to 20,000 minutes.
Certainly, having longer sessions would improve user and developer experience. Do you think this would impact user security? If so, Im trying to understand better how, so the right trade offs can be made.
@melvincarvalho Usually you have very very short access tokens, but they can be refreshed with a refresh token. Just because the oauth client's access token expires quickly, that doesn't mean the end-user's session need's to expire at the same interval.
So with short access tokens, often times when you go to use it to request an API, things go ike this:
- I want to make a request to /endpoint
- Look up access token A for the current session
- Make request with access token A
- Request might fail because expired access token (if this client knows the expiry date, it doesn't even need tomake this failed requesT)
- Get access token B by requesting /refresh, providing Refresh token with access token A (and client credentials)
- Save accesstoken B and throw away A
- Make original Request
- Other requests for the next 20minutes just work. After that repeat.
This sounds like a lot of work, but really it's not. And you can the benefit that if the client's access tokens get hacked/leaked, the user is only at risk for 20minutes. (The client credentials are much more important to keep secret)
@gobengo - I agree, we should put in config flags (and reasonable defaults) for token timeouts (for the various oauth2 flows) based on PRODUCTION / DEVELOPMENT env variables.
@gobengo thanks for the detailed explanation!
And you can the benefit that if the client's access tokens get hacked/leaked, the user is only at risk for 20minutes
Am I right to say, that this is a similar risk profile to hacking someone's cookie?
I agree, we should put in config flags (and reasonable defaults) for token timeouts (for the various oauth2 flows) based on PRODUCTION / DEVELOPMENT env variables.
@dmitrizagidulin sounds like a good idea! We should probably refactor the solid-test executable to align with DEVELOPMENT and PRODUCTION profiles. I'd suggest that this refectoring be part of a new issue.
As for this patch, I think it's just changing the default while we are in pre-alpha stage. The current issue is that there are so many errors and moving parts, that it's impractical for either developers or users to really get started. A longer default would remove one of those moving parts and would be a relatively simple patch.
Am I right to say, that this is a similar risk profile to hacking someone's cookie?
No. Often times (in my experience) access tokens can do way more with the upstream API than you can with a session for the Client Application. But in the simplest of apps, I guess they could have a 1:1 correspondence of priveleges (e.g. the Client Application developer just has a proxy endpoint to the upstream API). For this reason I prefer to never actually let an access token get into the frontend of a browser where e.g. a malicious browser extension could sniff it.
Also to perform like really serious operations, apps often don't even trust the cookie/session. The end-user is asked to re-authenticate, maybe even with MFA, before the sensitive operation is allowed.
@gobengo great info, thanks! I've added a security tag, so that this doesnt get lost and we can in future do a full security audit.
Another pointer to code (thanks @webr3 !) is :
https://github.com/anvilresearch/oidc-rs/blob/master/src/AccessToken.js#L69
I believe the default access token for OIDC expires after 20 minutes.
That's not correct.
If you look in the code, you'll see that thisTokenServiceis only used for password reset purposes.I only find evidence for a default access token expiry of 60 minutes.
@RubenVerborgh - that's exactly it (re 60 mins default expiry).
Line 103 a bit lower down points to the various config layers where token expiration can be changed.
So, there's
DEFAULT_MAX_AGEconstant (both on the OP and RP side), which is the fallback if no other param is specified. (So, we can patch this in the -op and -rp libs.)A client, as part of Dynamic Registration, can pass in
default_max_age- the default expiration for tokens issued to that client. (Which the OP can override, but there's no real reason to most times.)
The auth client passes in this param (so, a fix for this layer would need to happen on the auth client level.)Lastly, an auth client can override its max age that it requested on each individual auth session, using the
max_ageparameter in the authentication request. Similarly, the fix for that would need to happen in the auth client lib.2 remaining items
I have tried the following steps:
- Start node-solid-server 4.0.9
- In a new private window, go to /inbox/
- Log in with a local account through OIDC
- You are redirected to /inbox/
- Wait 60 minutes
- Refresh /inbox/
Performing the above steps did not trigger a 401, so these are not the right steps to reproduce. Does anyone know which of the steps above need to be changed? @melvincarvalho? I'll tag this issue cannot-reproduce until we have the right steps. Thanks in advance for your help.
- addedpriority-highto be used for important issues and pull requests that need to be addressed soonto be used for important issues and pull requests that need to be addressed soon
on May 25, 2018 Nice work @RubenVerborgh !
my experiences have been as follows
when using vanilla node solid server, I very frequently have 401 logout / login issues, tho as yet have been unable to reproduce exact steps, or the cause
when patching the AccessToken file on the server (https://github.com/solid/oidc-rs/tree/emergency-fix-token-expiry), these issues disappear for me, tested over several days, getting to weeks
A little update. These are not reproducible steps, as they can be hard to establish in distributed systems with many moving parts but.
Having been logged out from my inbox (401) I tried changing the following file :
https://github.com/anvilresearch/oidc-op/blob/master/src/AccessToken.js#L8
const DEFAULT_MAX_AGE = 3600 // Default Access token expiration, in secondsI changed 3600 to a larger number.
And bingo! I was able to use the system again. So perhaps this is a clue, if not a solution.
I changed 3600 to a larger number.
And bingo! I was able to use the system again. So perhaps this is a clue, if not a solution.
Yes indeed, we established that above. The only question remaining is: what is the right place to change this value? And to find out, we need the steps.
So what I suggest is to change this into a smaller number like 60, so the 401 happens sooner. That should make the bug more frequent and hence reproducing easier.
- changed the title
[-]OIDC Access token expires quickly[/-][+]Implement OIDC token renewal[/+]on Jul 19, 2018 - removedpriority-highto be used for important issues and pull requests that need to be addressed soonto be used for important issues and pull requests that need to be addressed soon
on Aug 8, 2018
I believe the default access token for OIDC expires after 20 minutes.
While this helps in case it is leaked to another system in that period, it can be confusing for new users, developers and apps who are not prepared for short lived access.
I suggest during the developer / alpha phase, either non expiring or 2 week long access tokens as outlined in :
https://www.oauth.com/oauth2-servers/access-tokens/access-token-lifetime/