-
Notifications
You must be signed in to change notification settings - Fork 52
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
💡 [REQUEST] - Allow saving token
to memory
#197
Comments
Hi, and thanks for engaging in the library!
I think we should try to understand the issue before deciding that we don't want to support something 😅 From what I understand, you're proposing saving the And this is a fair point. However, since this is all happening on the client, if we assume that we must "withstand" an XSS-attack, then we must also assume that the attacker can inject basically whatever code they want. And so we must also assume that there are ways to get a hold of this in-memory value as well (since it's stored in JS, which is the attack surface of the XSS-attacker). I think this sounds like security by obscurity, but by all means - security by obscurity is still a form of security, it's just not waterproof. I do think that in most cases, the difference in complexity of retrieving the token from Finally, I'm not yet sure if this is something we want to support, mainly because this only solves a problem that arises when the client is already compromised (e.g. by XSS), and at that point, we sort of just consider the battle lost with regards to security. From a practical standpoint, I think that storing the token outside (partially) persistent memory would require the user to login whenever they refreshed the page. Unless (!) you store e.g. the refresh token in persistant memory (Session-/LocalStorage). And to my understanding, even if you only store the refresh token in there, whatever issues you've solved by storing the access token in memory will be outshined as anyone with a valid refresh token can just request a valid access token. If you feel like I've missed something, @brumik, please add more context! Leaving this open until we've concluded 😄 |
Hey, Thank you for your fast reply. Yes, you understood it correctly. And yes, the refresh token would still be in session storage. I am not an expert on security, but working with health data our security team requires us to have the accessToken in memory and refreshToken can be in session storage or local storage but it has to be "rolling" (eg. You get a new one after using it). I see your point that when a user is compromised it is not really making a difference, but I have to adhere and trust our security team in this regard. P.S.: Ideal state is to use cookies but PKCE with the above setup should be satisfactory for everyone who cannot use http only cookies. Implementation wise either a react state or a module level global variable would be good. Neither of them would break any existing API in the context as far as I can see. |
Allowing the refresh_token to be stored in session/local storage, but not the access_token is nonsensical. If anything, it should be opposite. Please, if there are any other arguments for this feature I would be must interested in hearing them 🙂 |
Feel free to forward this thread to someone on your security team, in case they want to understand why we are reluctant to implement this. In short, our conclusion is based on our belief that such functionality would not offer any actual improvement when it comes to security, only a degredation in functionality, since one would have to wait for a new access token upon refreshing the page. |
Summary
I want to be able to save the
token
to memory instead of local storage or session storage. This allows for higher security (with rolling refresh token).As a sideeffect prevents issues like:
token
in storage(this is an issue when the lib is used outside of the given react app or using the token in global config like RTK base auth logic) - Edit: I know you do not want to support this officially, but keeping it here as an additional context for later if somebody has the same issue as me.
Basic Example
Drawbacks
Added complexity on imlementation and documentation
Unresolved questions
No response
Implementation PR
x
Reference Issues
No response
The text was updated successfully, but these errors were encountered: