FE / BE - Refresh tokens

[ajhollid]

Currently if you have a valid JWT token that has expired you are logged out and booted to the login page.

It would be preferrable to have use long lived refresh tokens to renew the JWT rather than logout completely. The final logout would happen when the long lived refresh token expires.

This would allow users to stay logged in without having long lived JWT tokens in the wild.

[Rushi1109]

Hello @ajhollid I would like to work on this issue. Can you assign it to me?

[sankettank66]

Hello @ajhollid I would like to contribute on this issue. Can you assign to me?

[ajhollid]

Hello @ajhollid I would like to contribute on this issue. Can you assign to me?

Hi @sankettank66 ,

Thanks for your interest in in the issue!

As this is a core security issue that requires work on both these front and back end I would need to see a proposal with clearly defined plan and steps to achieve the goal before assigning the issue.

We can then go back and forth and refine the plan together.

If that's something you you're interested in doing please go ahead and post your plan and we can discuss!

[Rushi1109]

Hello @ajhollid,

Here’s a plan for implementing the refresh token system:

1. Database Schema: We’ll either extend the current User schema or create a new table to store refresh tokens, associated with user IDs.

2. Token Generation: Upon a successful login, we’ll generate two tokens:

• A short-lived JWT access token (for authenticated requests).
• A long-lived refresh token (stored securely on the server).

3. Client-side Storage: The Refresh token will be stored in the client side cookies.

4. Token Renewal: When the JWT access token expires, the frontend will send a request to a new endpoint (/auth/refresh_token) to obtain a new JWT, using the refresh token. This will ensure users remain logged in seamlessly.

5. Logout on Expired Refresh Token: If the refresh token has expired, the user will be logged out and redirected to the login page. All tokens (JWT and refresh) will be invalidated at server-side.

Let me know your thoughts on this approach, and if you’d like any further ajdustments!

[ajhollid]

Hey @Rushi1109,

Thanks for taking the time to put a proposal together. Here's my thoughts:

1. Database Schema - I don't believe there's any need to store the refresh token. It should function in the same way as a regular JWT, all we need to do is verify that it is correctly signed and has not expired.

2. Token generation - This sounds good to me, I don't see any issues here, aside from the fact that we don't need to store the refresh token

3. Client Side Storage -We use Redux on the client side, so we can store the refersh token in the user's redux store along with the JWT.

4. Token Renewal - The backend API, specifically the JWT verification middleware, will have to be modified to send an identifiable response for when a JWT has expired. When the client receives this response it can then make a request for a new JWT with it's refresh token. The client can then proceed with its original request if the refersh token is valid and a new JWT token is received. I believe that is how the authorization flow should go.

5. Logout on Expired Refresh Token - This would occur when the client attempts to make a request with an expired JWT, then attempts to renew with an expired refersh token. The client would have to be set up to recognize this response.

Those are my thoughts, let me know what you think and if we're on the same page I think we could start working on this.

I think it would be best to break this down into a series of PRs, working from the backend to the fronted without breaking any current functionality.

[ajhollid]

@sankettank66 if you'd like to contribute as well to this issue please feel free to weigh in and share your thoughts!

[Rushi1109]

Hi @ajhollid,

Thank you for your feedback! I appreciate your insights and agree with most of your points. However, I do have some concerns regarding storing the refresh token in the Redux store.

1. Is Redux store a secure place to store tokens?
2. How would we invalidate the session when a user logs out? Would simply erasing the tokens from the store be sufficient?
3. Redux store isn't persistent storage. I researched a bit and found that we can use react-persist for this purpose. Is react-persist library a good choice for achieving persistent storage?

I believe it would be beneficial to discuss these points.

[sankettank66]

[User Login] 
     |
     v
[Access Token + Refresh Token] 
     |
     v
[Make API Request]
     |
     v
[Is Access Token valid?] -- Yes --> [Proceed to API Call]
     |
    No
     |
     v
[Token Renewal]
     |
     v
[Is Refresh Token valid?] -- Yes --> [Generate New Access Token] --> [Proceed to API Call]
     |
    No
     |
     v
[Logout]
     |
     v
[Clear Redux Store + Redirect to Login]

---

Summary of Chart Elements User Login: Initial action to authenticate and receive tokens. API Requests: Use the access token for making requests. Token Checks: Validations to determine if tokens are still valid. Token Renewal: Logic for refreshing the access token using the refresh token. Logout Process: Handling expired tokens to redirect the user to login.

Let me know if there any suggestion you would like to add. @ajhollid

[ajhollid]

Hi @ajhollid,

Thank you for your feedback! I appreciate your insights and agree with most of your points. However, I do have some concerns regarding storing the refresh token in the Redux store.

1. Is Redux store a secure place to store tokens?
2. How would we invalidate the session when a user logs out? Would simply erasing the tokens from the store be sufficient?
3. Redux store isn't persistent storage. I researched a bit and found that we can use react-persist for this purpose. Is react-persist library a good choice for achieving persistent storage?

I believe it would be beneficial to discuss these points.

1. There is no such thing as secure storage client side :joy: That's really the whole reason for short lived access tokens and long lived refresh tokens. We should assume that anything client side is compromised.

2. Yes, clearing the tokens from storage is sufficient for logout purposes. Since our backend is a REST api it is stateless, so there is no concept of being logged in on the back end. You either have access or you do not, based on your token.

3. We do already use redux-persist actually for persistent storage actually :+1:

As a side note security is not the paramount concern as this application is generally meant to be used on a local network by a small team. We should follow best practices, but we don't need to be 100% bulletproof.

It sounds like you and @sankettank66 are on the same page generally, if you'd like to cooperate and work on this together that's great, if not then feel free to each work on your own, the softeware is open source and we'll be willing to look at any contributions.

To make this a process that we can reasonably review I suggest making small granular PRs that follow the general structure we outlined above.

My suggestion for the order of PRs would be something like this:

Server Side

1. Generation of Refresh Token on Back End
2. Add refresh token to server resposne when user logs in.
3. Modify JWT verification middleware to check for expired access tokens 3a. Send an identifiable response to the Client if access token has expired
4. Add a route to listen for requests for new access tokens
5. Add controller method to deal with request for new access tokens (verify refresh token valid) 5a. Send 401 if both access and refresh tokens are invalid 5b. Send new access token if refresh token is not expried

We can deal with the client side after that's all done :joy:

[Rushi1109]

I’ll focus on the server-side implementation. @sankettank66, will you be handling the client-side?

@ajhollid, just to clarify, should I create a single pull request for the entire server-side implementation (with multiple commits), or would you prefer smaller, more frequent PRs for each part of the server-side?

[sankettank66]

I’ll focus on the server-side implementation. @sankettank66, will you be handling the client-side?

@ajhollid, just to clarify, should I create a single pull request for the entire server-side implementation (with multiple commits), or would you prefer smaller, more frequent PRs for each part of the server-side?

Sure, I will Handle Client Side.

[ajhollid]

I’ll focus on the server-side implementation. @sankettank66, will you be handling the client-side?

@ajhollid, just to clarify, should I create a single pull request for the entire server-side implementation (with multiple commits), or would you prefer smaller, more frequent PRs for each part of the server-side?

Smaller, more frequent PRs will help the process move along at a reasonable pace :+1: If the PRs are too large they are difficult and time consuming to review.

[ajhollid]

I’ll focus on the server-side implementation. @sankettank66, will you be handling the client-side? @ajhollid, just to clarify, should I create a single pull request for the entire server-side implementation (with multiple commits), or would you prefer smaller, more frequent PRs for each part of the server-side?

Sure, I will Handle Client Side.

Sounds good, once you have a better idea of what @Rushi1109 is going to return to you from the BE you can start planning out the FE side of the issue.

[Rushi1109]

Hey @ajhollid,

Should I update AppSettings schema to store the refresh token secret and TTL, or using environment variable is sufficient as of now?

[ajhollid]

Hey @ajhollid,

Should I update AppSettings schema to store the refresh token secret and TTL, or using environment variable is sufficient as of now?

Sure you can update the app settings but also please do provide an env var as the backend is set up so that env vars take presences over AppSettings.

Please update the project documentation as well as you go RE env vars and endpoints.

[Rushi1109]

Hey @sankettank66 & @ajhollid, Could you please review my PR? I would appreciate any feedback or suggestions you might have.

[Rushi1109]

I have few questions @ajhollid,

I am creating a middleware for verifying whether the refresh token is valid.

• Is it okay if I retrieve both the tokens from request body? (Frontend will send tokens in body of request.)
• What happens when I only get refresh token? (JWT access token is not sent) Do I proceed with the request? (I won't be able to get the payload for generating new token.)

You can check my closed pull request #964. I had implemented the middleware in that PR.

[ajhollid]

I have few questions @ajhollid,

I am creating a middleware for verifying whether the refresh token is valid.

• Is it okay if I retrieve both the tokens from request body? (Frontend will send tokens in body of request.)
• What happens when I only get refresh token? (JWT access token is not sent) Do I proceed with the request? (I won't be able to get the payload for generating new token.)

You can check my closed pull request #964. I had implemented the middleware in that PR.

Hi @Rushi1109 ,

1. Access to tokens are sent in the headers of the request. Refresh token can be sent in the body.

2. There should never be a case where you only have a refresh token but no access token. All requests to the backend are made with an access token or are rejected outright

[Rushi1109]

@ajhollid could you please review the PR #973.

[ajhollid]

@ajhollid could you please review the PR #973.

Yep, just had a look at it. You can just use the "request review" feature, I keep a close eye on the PRs anyways and get to them as soon as I possibly can :smile: You can also request reviews from the other team members, @shemy-gan and @marcelluscaio

By the way, just a heads up we just merged a large PR to migrate the backend to use more modern javascript, please make sure to merge the latest develop branch into your codebase before making any further PRs, or there will be a great many merge conflicts to resolve :joy:

[Rushi1109]

Should I merge the develop branch in my currently open PR? or Should I keep it as it is?

[ajhollid]

Should I merge the develop branch in my currently open PR? or Should I keep it as it is?

I always recommend merging develop into your feature branches before requesting PR review, then you can resolve conficts on your end first

[Rushi1109]

I'll now make a new PR for the new endpoint that generates new auth token.

[Rushi1109]

@sankettank66 For your reference,

• Both tokens are expected in request header.
• Auth token is expected as Bearer token.
• Refresh token is expected in header with key being "x-refresh-token"

[Rushi1109]

@ajhollid, I think backend part of this issue is completed.

[ajhollid]

@ajhollid, I think backend part of this issue is completed.

I believe so!

[sankettank66]

Okay, @Rushi1109 Now I will start work on Front-end part of this issue If there will be any query I will let you know.

[Rushi1109]

Okay, @Rushi1109 Now I will start work on Front-end part of this issue If there will be any query I will let you know.

Sure.