Skip to content

Implement OIDC token renewal #641

Description

@melvincarvalho

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/

Activity

  1. dmitrizagidulin commented on Mar 6, 2018

    @dmitrizagidulin
    Contributor

    hmm good idea

  2. gobengo commented on Mar 7, 2018

    @gobengo
    Contributor

    I recommend only doing this when process.env.NODE_ENV !== "production" or, better yet, check for development. Basically avoid doing so in a way that is prioritizing developer experience over end-user security.

  3. melvincarvalho commented on Mar 9, 2018

    @melvincarvalho
    ContributorAuthor

    @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.

  4. gobengo commented on Mar 9, 2018

    @gobengo
    Contributor

    @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)

  5. dmitrizagidulin commented on Mar 9, 2018

    @dmitrizagidulin
    Contributor

    @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.

  6. melvincarvalho commented on Mar 13, 2018

    @melvincarvalho
    ContributorAuthor

    @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.

  7. gobengo commented on Mar 13, 2018

    @gobengo
    Contributor

    @melvincarvalho

    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.

  8. melvincarvalho commented on Mar 13, 2018

    @melvincarvalho
    ContributorAuthor

    @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.

  9. melvincarvalho commented on May 10, 2018

    @melvincarvalho
    ContributorAuthor
  10. RubenVerborgh commented on May 19, 2018

    @RubenVerborgh
    Contributor

    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 this TokenService is only used for password reset purposes.

    I only find evidence for a default access token expiry of 60 minutes.

  11. dmitrizagidulin commented on May 20, 2018

    @dmitrizagidulin
    Contributor

    @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_AGE constant (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_age parameter in the authentication request. Similarly, the fix for that would need to happen in the auth client lib.

  12. 2 remaining items

  13. RubenVerborgh commented on May 25, 2018

    @RubenVerborgh
    Contributor

    I have tried the following steps:

    1. Start node-solid-server 4.0.9
    2. In a new private window, go to /inbox/
    3. Log in with a local account through OIDC
    4. You are redirected to /inbox/
    5. Wait 60 minutes
    6. 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.

  14. melvincarvalho commented on May 25, 2018

    @melvincarvalho
    ContributorAuthor

    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

  15. melvincarvalho commented on May 25, 2018

    @melvincarvalho
    ContributorAuthor

    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 seconds
    

    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.

  16. RubenVerborgh commented on May 25, 2018

    @RubenVerborgh
    Contributor

    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.

  17. changed the title [-]OIDC Access token expires quickly[/-] [+]Implement OIDC token renewal[/+] on Jul 19, 2018
  18. added this to the 6.0.0 milestone on Aug 8, 2018
  19. removed
    priority-highto be used for important issues and pull requests that need to be addressed soon
    on Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions