-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(authentication): refreshing/replacing the token #4573
Comments
Adding token refresh would add a bit of polish. 👍 |
@sformisano To make this working i would like to extend the E.g. by adding a /**
* Renews, refreshes or extends ttl a given token string.
*
* @param token A current valid token.
* @param secret A secret, e.g. refreshtoken to secure the operation.
* @returns A promise which resolves to a new or refreshed token
*/
refreshToken(token: string, secret?: string): Promise<string>; The
Anything i missed? |
@derdeka, since you have a PoC, let me assign this to you. Thanks. |
While we are at it, I would also suggest that the tokens are @derdeka I already developed the authentication flow you described above but in a microservice context. I also think that JWT in general makes more sense in that context, if your application is a monolith I would rather go for session cookies |
@nflaig 👎 for cookies. LocalStorage is better suited to the task at hand. Cookies break RESTful best practices as it requires the server to store session information. |
@dougal83 I guess you didn't understand my proposal correctly. We are not storing any session information on the server, the only thing that changes is how the JWT is stored on the client which is localStorage vs HttpOnly cookie. |
You could be right but I'm just not convinced atm. I'll read more about it and come back if I change my mind.
Furthermore, I don't really see the above statement being a concern with regard to inclusion of the refresh token that this PR proposes. The short lived access token contains the permissions/scopes. The longer lived refresh token simply requests/refreshes the access token and at the point of reissue we can check if access has been revoked or altered. |
There are a lot of great resources with information on web security best practices for example OWASP cheat sheets. Also if you take a look how websites store sensitive information you will not find a single one that uses local storage.
The PR listed 3 different options. This was just another argument against option 1 and why it is not a suitable approach. |
You're right that LocalStorage shouldn't be used to store sensitive information. However the OWASP Docs don't highlight a particular issue. The JWT token, is fine in LocalStorage as it is really just a verifiable claim and not the actual credentials for instance.
Fair enough, I boiled down the post in my head to option 3 in agreement with @derdeka. |
They are highlighting some issues like XSS in the chapter about local storage. I think the highest risk might be a malicious npm module you are using in your UI code which just gets all items in local storage and sends those to the attacker. I can't really tell how likely it is that such an attack happens but I guess when it comes to security one should try to follow best practices to avoid potential exploits. |
@nflaig I don't make any asumptions how the tokens are stored (in cookies or in localstorage). I think this is in the responsibility of the frontend and is out of scope of my POC. But i'm open to discussions about best practices and feedback how other developers handle this. In my case i have a angular frontend which should periodically refresh the |
@derdeka yes it probably makes sense to discuss this topic in another issue.
This can not be decided by the frontend since the backend sets the cookies in the response. In case the cookies are Http only it is even impossible for the UI javascript code to interact with the cookies at all. But I guess the first step is anyways to do the PoC implementation with refresh tokens and the next step would be security considerations. In my opinion even though this is just a demo application it should still showcase best practices. |
@nflaig I don't see any cookies in the login process set by |
@derdeka |
I agree that tokens should be stored as a However if I'm not mistaken, This seems to be more of an issue on how |
Agreed. Client side implementation is out of scope of PR. |
Just to clarify; are we simply adding a If so, we should land this before the |
Is there any documentation about how to make this work with the example? |
Closing as done. |
Suggestion
TTL of AccessToken (
JWT
,LB3 legacy token
or any other token system) should be extendable or token should be replaced after some time of beeing used. AFAIK the@loopback/authentication
component does not support this yet.Use Cases
Cross-Posting @sformisano 's comment from #3673 (comment) as discussion point:
Acceptance criteria
TBD - will be filled by the team.
The text was updated successfully, but these errors were encountered: