Skip to content
This repository was archived by the owner on Jul 25, 2024. It is now read-only.

MongoDB #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

MongoDB #11

wants to merge 3 commits into from

Conversation

EduardoReolon
Copy link

Changed the least possible, to include persisted token in mongoDB.

I tested attempt, generate, loginViaRefreshToken and revoke. All worked well in mongoDB.

I also changed 2 functions to be public: getBearerToken and verifyToken. I use them because sometimes I only need the user_id, and I can take it from this functions.

computador PRC added 3 commits May 19, 2022 11:25
@maxgalbu
Copy link
Owner

I @EduardoReolon, thanks!

I have some questions before merging it:

  • by the looks of it, it should be possible to avoid requiring "mongoose" from this library? Just like there is no import "redis"
  • the requirements to pass model in MongoTokenProviderConfig config is a bit wierd. Can you show me an example on how you initialize MongoTokenProviderConfig? Do you need to initialize the connection to mongodb in config file (which is not something I would do)?

I also changed 2 functions to be public: getBearerToken and verifyToken. I use them because sometimes I only need the user_id, and I can take it from this functions.

You can obtain the payload you've set by doing this:

await auth.use("jwt").authenticate();
const userPayload = auth.use("jwt").user!;

@EduardoReolon
Copy link
Author

EduardoReolon commented May 30, 2022

  • by the looks of it, it should be possible to avoid requiring "mongoose" from this library? Just like there is no import "redis"

I started using mongoose, then I realised it's not needed. I commented de lines e removed it from package.json

  • the requirements to pass model in MongoTokenProviderConfig config is a bit wierd. Can you show me an example on how you initialize MongoTokenProviderConfig? Do you need to initialize the connection to mongodb in config file (which is not something I would do)?

Idk if it is the right approach. In adonis 5 mongoose they stated the connection outside config, in start folder. So I kept the same structure, just sending the model already connected.

import Token from 'App/Models/Token';
...
jwt: {
      driver: "jwt",
      publicKey: Env.get('JWT_PUBLIC_KEY', '').replace(/\\n/g, '\n'),
      privateKey: Env.get('JWT_PRIVATE_KEY', '').replace(/\\n/g, '\n'),
      persistJwt: true,
      jwtDefaultExpire: '3h',
      refreshTokenDefaultExpire: '3y',
      tokenProvider: {
        type: 'api',
        driver: 'mongo',
        foreignKey: 'user_id',
        model: Token,
      },
      provider: {
        driver: "mongo",
        identifierKey: "id",
        uids: [],
        model: () => import('App/Models/User')
      }
    }

You can obtain the payload you've set by doing this:

About this. authenticate method retrieves data from jwt_token and user tables. I think this approach kind of mess with the need of having a giant privateKey/publicKey. If the idea is to access database everytime, only refreshed token would be enough, isn't it? So that's what I did in my auth middleware:

public async handle (
    ctx: any,
    next: () => Promise<void>,
    customGuards: (keyof GuardsList)[]
  ) {
    /**
     * Uses the user defined guards or the default guard mentioned in
     * the config file
     */
    const guards = customGuards.length ? customGuards : [ctx.auth.name]
    const token = ctx.auth.use('jwt').getBearerToken(ctx.request.header('authorization'));
    const payload = await ctx.auth.use('jwt').verifyToken(token);
    if (!payload) await this.authenticate(ctx.auth, guards);
    ctx.getUser = async (complete = false) => {
      if (complete) {
        await ctx.auth.use("jwt").authenticate();
        return ctx.auth.use("jwt").user!;
      }
      return {
        id: payload.data.userId,
        ...payload.data.user,
      };
    }
    await next()
    // const guards = customGuards.length ? customGuards : [auth.name]
    // await this.authenticate(auth, guards)
    // await next()
  }

And I call getUser user to get the user_id. If I need the full user, I pass the parameter "complete=true"

But again. Let me know what u think, I'm not an expert.

@maxgalbu
Copy link
Owner

maxgalbu commented May 31, 2022

I started using mongoose, then I realised it's not needed. I commented de lines e removed it from package.json

whoops my mistake, I looked at one commit

Idk if it is the right approach. In adonis 5 mongoose they stated the connection outside config, in start folder. So I kept the same structure, just sending the model already connected.

it looks weird to me - but ok. It's not so messy as I thought

About this. authenticate method retrieves data from jwt_token and user tables. I think this approach kind of mess with the need of having a giant privateKey/publicKey. If the idea is to access database everytime, only refreshed token would be enough, isn't it? So that's what I did in my auth middleware:

The library already supports the recommended way of using JWT, that is, not storing in DB (persistJwt = false). You're right it doesn't make sense to store JWT in db because it makes the JWT completely useless, it makes sense if you need to control logged-in/logged-out state, but not in your case.

If you don't need that, then just set persistJwt to false, refresh token will be persisted instead 😄

@EduardoReolon
Copy link
Author

EduardoReolon commented May 31, 2022

If you don't need that, then just set persistJwt to false, refresh token will be persisted instead

I haven't realized that. I didn't look into this, by the name I thought that persistJwt = false was to have no token in DB at all. My mistake was assuming it instead of going into the codes.

I think this is working any way. this or next week I'll test changing persistJwt to false, to see if it still fetchs the user, if not, it's gonna be perfect.

Thanks.

@EduardoReolon
Copy link
Author

EduardoReolon commented May 31, 2022

I implemented the non persistJwt with mongoDB. The same methods worded out for me.

It fetches the user when authenticate. It's better if u want to deactivate the user at some point. But I prefer having a shorter jwt life time and don't depend on database at all.

See if I need to pull a new request or u can access my last one

@EduardoReolon
Copy link
Author

Hello,

Do I still have to do something?

I've never done a pull request.

@maxgalbu
Copy link
Owner

I'm working on trying to merge it, don't worry, thanks for your work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants