Skip to content
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

Refresh token #30

Closed
wants to merge 12 commits into from
Closed

Refresh token #30

wants to merge 12 commits into from

Conversation

guanacone
Copy link
Owner

Backend implementation to attribute and save a refresh token to the DB.

The idea is to save a refresh token when a user logs in that can be checked and used to emit a new token if the user makes a request with an expired token.

@guanacone
Copy link
Owner Author

Seems that the issue is coming form gatsby-plugin-node but can point why... Can you assist?

Copy link

@edwmurph edwmurph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few comments that might fix the problem you're seeing. also is there a particular guide you're following for setting up refresh tokens that you could share? looks like you want the server to automatically refresh the token?

server/auth/index.js Show resolved Hide resolved
server/auth/index.js Show resolved Hide resolved
@@ -1 +1 @@
v12.18.3
v14.15.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice this looks like the new LTS for node but now you should also make sure the Dockerfile and github workflows are updated to also use this node version

you could update your github actions to automatically use the value in the nvmrc

or it looks like nvm was added to the github action virtual environments so you could also just update your github actions to use nvm directly

@guanacone
Copy link
Owner Author

left a few comments that might fix the problem you're seeing. also is there a particular guide you're following for setting up refresh tokens that you could share? looks like you want the server to automatically refresh the token?

The idea is that when a user logs in a refresh token is saved to the DB and deleted when he logs out. On the front end if a response comes back with a status code 404 and if there's a expired token it will retrieve the refresh token from the DB and if there's a refresh token hit a refresh route to get a new token. (Not sure where exactly on the front end, maybe in the getContent()?)

I did find several tutorials on how to implement the refresh token on the backend but could't find anything good on the client side...

Comment on lines -26 to +29
"gatsby": "2.24.52",
"gatsby": "2.25.2",
"gatsby-plugin-create-client-paths": "2.3.11",
"gatsby-plugin-nodejs": "0.7.0",
"gatsby-plugin-nodejs": "0.6.4",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the build issue you were seeing was caused by other issues and these downgrades can probably be reverted

},
{ useFindAndModify: false,
new: true },
);

return res.json({ token, info });
Copy link

@edwmurph edwmurph Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that when a user logs in a refresh token is saved to the DB and deleted when he logs out. On the front end if a response comes back with a status code 404 and if there's a expired token it will retrieve the refresh token from the DB and if there's a refresh token hit a refresh route to get a new token.

it might help to get a complete picture to update the logout route to delete the refresh token in this iteration?

also it looks like this implementation doesnt pass the refresh token back to the user but i read a few articles on this and it looks like they all suggest passing the refresh token back to the user. and then when the user needs to get a fresh token, it passes that refresh token in the request. otherwise anyone could get a refreshed token without auth?

also it would help if you also implemented the route for refreshing the token in this iteration (assuming you're implementing refresh in that way) so we can make sure refreshing a token automatically works. it's probably good tho to ya at least defer the frontend implementation for this to a separate PR

Comment on lines +117 to +120
const refreshToken = jwt.sign(
{ user: body },
process.env.REFRESH_TOKEN_SECRET,
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this refresh token should probably have some expiration like 24 hours or something

@guanacone
Copy link
Owner Author

Just to let you know that I couldn't do much over the weekend. We got hit by a major hurricane (cat. 4) on Wednesday and it rained over a foot in 4days. We're pretty much without power and internet till today. Will get back to it ASAP.

@edwmurph
Copy link

edwmurph commented Nov 9, 2020

wow that's crazy hope you guys are doing ok. thanks for the update

@guanacone
Copy link
Owner Author

Yeah, we had some intense moments, but we're fine.

I spoke yesterday with a friend who is also a developer and he recommended the following approach:

  1. API emits JWT at login with a expiration date of a couple days (shorter for an API with sensible data),
  2. Frontend sets an interval to hit the refresh route of the API a couple of seconds before the JWT expires. Logic should be implemented to automatically to refresh the JWT on page reload (i.E. if the user closes the browser and comes back later, if by then the JWT is expired, user will need to login agin).
  3. At logout JWT will be saved to blacklist.

What do you think?

@edwmurph
Copy link

ya i think something along those lines should work but only thing is how is the refresh route authenticated? doesn't the refresh token need to be given to the client?

wouldn't the flow need to be something like:

  1. login returns jwt token for standard requests and also refresh token to the client
  2. client holds both tokens in local storage
  3. on page reload, fetch fresh jwt token using refresh token and setTimeout to refresh token again when it will expire
  4. on logout, blacklist both tokens
    ?

@guanacone
Copy link
Owner Author

how is the refresh route authenticated
with the same JWT that was emitted at login.

Where in gatsby would the the logic for setTimeout an on page load go? I believe this would need to go at toplevel of the app and not in a specific component or page?

@edwmurph
Copy link

there's a couple ways you could do this:

@guanacone
Copy link
Owner Author

I will close this PR and start fresh.

@guanacone guanacone closed this Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants