Added refresh token functionality

[tsukimi2]

1) Added refresh token functionality (for single device per user at a time only for now) 2) Added Logout route 3)Added validateJwt middleware to replace the passport middleware using PassportJWTStrategy 4)Added cookie session npm package

Note: When I was testing manually, the login, token validation, and logout functions mostly work. However, there's a specific case where the logout will throw an exception. I think it has something to do with typeorm connection, but since it's my first time using typeorm, I can't really figure out the reason why the code will throw this exception. Please help if you are able to figure out the cause. Anyway, here's the step to reproduce the exception: 1) Log in successfully with http://localhost:3000/api/v1/auth/google/callback 2) http://localhost:3000/api/v1/user 3) Stop the server (i.e. control-c on the server) 4) Restart the server 5) http://localhost:3000/api/v1/logout

The following exception will then be thrown: ConnectionNotFoundError: Connection "default" was not found. at new ConnectionNotFoundError (/Users/osiris/repos/MBOne-Backend/src/error/ConnectionNotFoundError.ts:8:9) at ConnectionManager.get (/Users/osiris/repos/MBOne-Backend/src/connection/ConnectionManager.ts:40:19) at getConnection (/Users/osiris/repos/MBOne-Backend/src/index.ts:252:35) at updateRefreshToken (/Users/osiris/repos/MBOne-Backend/.webpack/service/webpack:/src/services/user.service.js:85:22) at logout (/Users/osiris/repos/MBOne-Backend/.webpack/service/webpack:/src/controllers/auth.controller.js:87:35) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/route.js:137:13) at Route.dispatch (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/route.js:112:3) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:281:22 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at Function.handle (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:174:3) at router (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:47:12) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at initialize (/Users/osiris/repos/MBOne-Backend/node_modules/passport/lib/middleware/initialize.js:53:5) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at cookieParser (/Users/osiris/repos/MBOne-Backend/node_modules/cookie-parser/index.js:71:5) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at _cookieSession (/Users/osiris/repos/MBOne-Backend/node_modules/cookie-session/index.js:126:5) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at jsonParser (/Users/osiris/repos/MBOne-Backend/node_modules/body-parser/lib/types/json.js:119:7) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at expressInit (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/middleware/init.js:40:5) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at query (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/middleware/query.js:45:5) at Layer.handle [as handle_request] (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:317:13) at /Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:335:12) at next (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:275:10) at Function.handle (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/router/index.js:174:3) at Function.handle (/Users/osiris/repos/MBOne-Backend/node_modules/express/lib/application.js:174:10) at /Users/osiris/repos/MBOne-Backend/node_modules/serverless-http/lib/framework/get-framework.js:22:11 at /Users/osiris/repos/MBOne-Backend/node_modules/serverless-http/lib/framework/get-framework.js:9:5 at /Users/osiris/repos/MBOne-Backend/node_modules/serverless-http/serverless-http.js:19:28 at processTicksAndRejections (internal/process/task_queues.js:95:5) at /Users/osiris/repos/MBOne-Backend/node_modules/serverless-http/lib/provider/aws/index.js:11:22 at request (/Users/osiris/repos/MBOne-Backend/.webpack/service/webpack:/handler.js:43:18)

[ryanolee]

Looking good 😎 For the note I think it might have something to do with https://github.com/Multiplied-By-One/MBOne-Backend/issues/13 not properly handling database connections. Though will double check.

[ryanolee]

Was re reading against https://hasura.io/blog/best-practices-of-using-jwt-with-graphql/#refresh_token and was just checking against what we are currently doing and I agree this looks good! Will need to properly test in a sec but using https://github.com/expressjs/cookie-session seems ok. The only thing I worry about is we will be exposing PII in cookie data if we are serializing user data into the cookie variable / would that be GDPR compliant. Will double check when I functionally test but the general gist of it looks really quite good! (Though if it saves us a backend fetch per request might work out to be optimal 🤔. Might ask some people I know if this is ok )

[tsukimi2]

Was re reading against https://hasura.io/blog/best-practices-of-using-jwt-with-graphql/#refresh_token and was just checking against what we are currently doing and I agree this looks good! Will need to properly test in a sec but using https://github.com/expressjs/cookie-session seems ok. The only thing I worry about is we will be exposing PII in cookie data if we are serializing user data into the cookie variable / would that be GDPR compliant. Will double check when I functionally test but the general gist of it looks really quite good! (Though if it saves us a backend fetch per request might work out to be optimal 🤔. Might ask some people I know if this is ok )

@ryanolee Regarding serializing user data into a session cookie, after pondering a bit more it does feel a bit insecure to put user data into cookie as you suggested in your last reply. In lieu of this, I wonder whether it'll be better from a security/data privacy standpoint that instead of using cookie-session, we simply create a hash for a user every time a user logs in and store this hash in the User table of the database and also store it in a cookie? And when user is requesting an API from the resource server, it can use the hash stored in the cookie to query the User table for user info when needed? This way there'll be an additional query against user database, but it does seem better from a security / data privacy standpoint. What do you think? Should I modify the current PR based on this? Or do you have better suggestion?

[ryanolee]

@ryanolee Regarding serializing user data into a session cookie, after pondering a bit more it does feel a bit insecure to put user data into cookie as you suggested in your last reply. In lieu of this, I wonder whether it'll be better from a security/data privacy standpoint that instead of using cookie-session, we simply create a hash for a user every time a user logs in and store this hash in the User table of the database and also store it in a cookie? And when user is requesting an API from the resource server, it can use the hash stored in the cookie to query the User table for user info when needed? This way there'll be an additional query against user database, but it does seem better from a security / data privacy standpoint. What do you think? Should I modify the current PR based on this? Or do you have better suggestion?

Hi @tsukimi2, This sounds good. Out of interest what OS / version of node have you got running? I remember updating deps to node v15 on the other open PR given I cannot seem to run this branch on windows 10 node v15 / 14🤔 . Though I have a osx and linux computers so should be good to test on either.

In terms of loading the user the JWT stores the user ID so we should be able to use that? 🤔

[tsukimi2]

@ryanolee Ok, I'll begin to implement the aforementioned idea suggested above this weekend then.

I'm using node v14.17.0 on osx for this particular branch. (I usually develop with this node version on linux in virtualbox hosted on windows or sometimes on my osx) Sorry I must have missed seeing the open PR you sent before concerning using node v15, so I was just using node v14.17.0, which is the latest long-term stable version. I think I should install node v15 on my osx and linux machines and try out my code on this branch then?

Also, do you think it's a good idea to consider about using docker in our project (and maybe some instructions on the git and docker commands to use afterward) so as to provide all involved developers a consistent development environment?

[tsukimi2]

@ryanolee Also, I would like to double-confirm is it node version 15 that we should upgrade to? 'cause when I checked the nodejs website, it says odd-numbered version of nodejs becomes unsupported after 6 months of release (https://nodejs.org/en/about/releases/). So are we moving to a node version that will become unsupported after July? Just want to double-check before proceeding with the node upgrade on my development platform. Thanks.

[tsukimi2]

In terms of loading the user the JWT stores the user ID so we should be able to use that? 🤔

@ryanolee Yes, the jwt access token stores the user ID so we can use it to query for the user, which is what I intend to do in auth/jwt.js line 63

const decodedJwtPayload = jwt.verify(accessToken, process.env.JWT_SECRET)

in the code in this branch.

However, the problem arises in the case when access token is expired (line 67 in jwt.js), in which case jwt.verify() throws an error and the access token payload is not being decoded, which means we can no longer make use of the user id stored in the access token. Hence the need for a separate user hash to be stored in a separate cookie (not user id, since it doesn't seem like we should store user id in a place where it's visible to the browser for security reason).

Is there a better solution for this? I do feel it's kind of hacky to need to make a separate cookie for storing a user hash just because we can't get the user id from the access token when the token has expired, but at the moment I can't come up with a better solution yet. Great if you could have a better suggestion for this. ^_^

[ryanolee]

Hi @tsukumi2, I think we should be able to rotate the JWT once it has expired using the refresh token to find the user and generate a new one?🤔 After which we cycle the refresh token. Out at the moment but will verify in more detail on back. Thanks, -Ryan

On Sun, 13 Jun 2021, 02:15 tsukimi2, @.***> wrote:

In terms of loading the user the JWT stores the user ID so we should be able to use that? 🤔

@ryanolee https://github.com/ryanolee Yes, the jwt access token stores the user ID so we can use it to query for the user, which is what I intend to do in auth/jwt.js line 63

const decodedJwtPayload = jwt.verify(accessToken, process.env.JWT_SECRET)

in the code in this branch.

However, the problem arises in the case when access token is expired (line 67 in jwt.js), in which case jwt.verify() throws an error and the access token payload is not being decoded, which means we can no longer make use of the user id stored in the access token. Hence the need for a separate user hash to be stored in a separate cookie (not user id, since it doesn't seem like we should store user id in a place where it's visible to the browser for security reason).

Is there a better solution for this? I do feel it's kind of hacky to need to make a separate cookie for storing a user hash just because we can't get the user id from the access token when the token has expired, but at the moment I can't come up with a better solution yet. Great if you could have a better suggestion for this. ^_^

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Multiplied-By-One/MBOne-Backend/pull/12#issuecomment-860134179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHRVIN7YAA643GR5QMSWFDTSQBEFANCNFSM452N4XAA .

[tsukimi2]

Hi @tsukumi2, I think we should be able to rotate the JWT once it has expired using the refresh token to find the user and generate a new one?🤔 After which we cycle the refresh token. Out at the moment but will verify in more detail on back. Thanks, -Ryan

@ryanolee Oh yes, you are right. I forgot we have user id in the refresh token payload also. Thanks.

[tsukimi2]

@ryanolee Sorry, made some git command mistakes when trying to push some new code and ended up creating a new branch and a new push request. I'll close this current push request and the newer code will be in the following branch https://github.com/Multiplied-By-One/MBOne-Backend/pull/15