method for dynamic secret signing rather than globally configured secret

[tomsiwik]

I'm submitting a...

[ ] Regression
[ ] Bug report
[x] Feature request
[x] Documentation issue or request
[ ] Support request

Current behavior

Currently there is no way to sign the token dynamically. (see user.secret comment). So far based on the documentation I can't seem to find a good solution. Maybe more docs are missing? In case somebody asks the same question I have created a SO post:

https://stackoverflow.com/questions/55354102/using-nestjs-jwt-signing-with-dynamic-user-related-secret/55354309#55354309

@Injectable()
export class AuthService {
  public constructor(private readonly jwtService: JwtService, private readonly userService: UserService) {}

  public async createToken(email: string): Promise<JwtReply> {
    const expiresIn = 60 * 60 * 24;
    const user = await this.userService.user({ where: { email } });
    const accessToken = await this.jwtService.signAsync({ email: user.email }, /* user.secret ,*/ { expiresIn });

    return {
      accessToken,
      expiresIn,
    };
  }
}

Expected behavior

Option 1:

Sign should work similar to jwt-token libraries sign method. Re-add second parameter as secret key rather than options.

Option 2:

Add a secretOrPrivateKeyProvider similar to PassportStrategy of @nestjs/passport

JwtModule.register({
      secretOrPrivateKeyProvider: userService.provideSecret,
      signOptions: {
        expiresIn: 3600,
      },
    })

Minimal reproduction of the problem with instructions

See first code example above

What is the motivation / use case for changing the behavior?

I'm new to nestjs and cautious about security. This might be opinionated and I apologise if this is the case here. For security purposes rather than relying on a single secret, if stolen, the whole infrastructure is compromised. When stealing a single secret a single user is compromised and the damage lowered when using dynamic secret signing. "stealing" may sound vague, but I hope it simplifies what I mean in general.

[kamilmysliwiec]

Would you be interested in creating a PR for this feature? :)

[tomsiwik]

Ofc, I'll try over the weekend. Barely any time left.

[kamilmysliwiec]

Awesome, looking forward.

[tomsiwik]

I'm confused as to where my PR and this issue/feature is going. Do I need to reply/ inform somebody about it? Am I missing something in the contribution guide what might cause this block? I have added tests, as far as I know this is not a mono-repo build dependancy (or is it?).

How or should I perform (unclear if this is relevant for external packages): https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md#commonly-used-npm-scripts

Some questions, no further notes and a pending PR which I am forced to watch weekly (since end of march) as the renovate task races me to fix the conflicts in the package.jsondue to version missmatches.

[kamilmysliwiec]

I'm not a robot @tomsiwik. I have already pulled your branch and after a few changes, will very likely merge it soon. I'm sorry for inconveniences.

[tomsiwik]

No problem... maybe I should rephrase it, I did not want to sound negative. I am just confused where this was going. A simple note would have been enough. I don't mind to update the deps. For me it just wasn't clear if the package.json conflicts were causing this delay. Thanks for answering so quickly though.

[kamilmysliwiec]

Published as 6.1.0

[cheelahim]

@tomsiwik @kamilmysliwiec Hey guys! Thanks for this feature. I just had a change to play with the new secretOrKeyProvider option and I have a question. Here is current implementation:

  sign(payload: string | Buffer | object, options?: jwt.SignOptions): string {
    const signOptions = this.mergeJwtOptions(
      options,
      'signOptions'
    ) as jwt.SignOptions;
    const secret = this.getSecretKey(
      payload,
      options,
      'privateKey',
      JwtSecretRequestType.SIGN
    );

    return jwt.sign(payload, secret, signOptions);
  }

Don't you think it would be better to call getSecretKey with signOptions (merged) instead of options? Otherwise verifyOrSignOrOptions argument of secretOrKeyProvider doesn't have access to the default options. Basically it affects all methods where getSecretKey is used.

As a temporary workaround all options can merged outside of JwtService and passed as the desired method argument, but that is not very convenient of course.