maxgalbu / adonis5-jwt

JWT Authentication service for Adonisjs v5
MIT License
86 stars 15 forks source link

MongoDB #11

Open EduardoReolon opened 2 years ago

EduardoReolon commented 2 years ago

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.

maxgalbu commented 2 years ago

I @EduardoReolon, thanks!

I have some questions before merging it:

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 commented 2 years ago
  • 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Hello,

Do I still have to do something?

I've never done a pull request.

maxgalbu commented 2 years ago

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