Passport-jwt will soon get an major update with better intergration with nestjs and breaking changes. (https://docs.nestjs.com/security/authentication)

[Outternet]

I'm submitting a...

• [ ] Regression
• [ ] Bug report
• [ ] Feature request
• [x] Documentation issue or request (new chapter/page)
• [ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

https://docs.nestjs.com/security/authentication#implementing-passport-jwt The current nestjs documentation describes the behavior of passport-jwt v4.0.0, this will soon no longer be valid as v5.0.0 (breaking change) is very close to release. This update includes better intergration with @nestjs/jwt, which should be the new default behavior described in the docs.

Expected behavior

The example from NestJs should work with the latest version.

 @Injectable()
export class JwtStrategy extends PassportStrategy(Strategy) {
  constructor() {
    super({
      jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
      ignoreExpiration: false,
      secretOrKey: jwtConstants.secret,
    });
  }
// Returns error because no jwtDriver has been provided.

it currently doesn't work with v5.x.x.

Minimal reproduction of the problem with instructions (includes recomendation)

• follow the documentation of NestJs
• fix the above code example with the patch below

// ... nest js imports
import {JwtStrategyOptions, Strategy, ExtractJwt} from "passport-jwt";
import {NestJsJwtDriver} from "passport-jwt/platform-nestjsjwt";

@Injectable()
export class JwtStrategy extends PassportStrategy(Strategy) {
    constructor(jwtCore: JwtService) {
      const opts: JwtStrategyOptions = {
        jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
        jwtDriver: new NestJsJwtDriver(jwtCore)
      }
      super(opts);
    }

https://github.com/mikenicholson/passport-jwt/pull/238 https://github.com/mikenicholson/passport-jwt/blob/Outternet/master/docs/migrating.md https://github.com/mikenicholson/passport-jwt/blob/Outternet/master/docs/nestjs.md

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

The motivation is to have an accurate documentation.

[Outternet]

I can make a pull request if necessary.

[jmcdo29]

jwtDriver: new NestJsJwtDriver(jwtCore)

Can I just say thank you for the re-write for this so we no longer have people complaining about having to "configure" the jwt secret twice?

if you would like to make a PR to open the docs, feel free :) I'll read over it once you've got one made

[Outternet]

https://github.com/nestjs/nest/tree/master/sample/19-auth-jwt, what is the proccess to update this sample?

[Outternet]

Consider this the first version, I'd love to hear feedback on how I can make it clearer, and your welcome ;)

[kamilmysliwiec]

Honestly, a breaking change introduced in this library just gives me another reason to actually get rid of Passport for JWT-driven authentication guide in the docs

Is there any reason why "jwtDriver" is now required?

[Outternet]

:(, it helped me a lot in the first few years of nest.

was a point of discussion, its mainly done to remove the dependency on jsonwebtoken, of course the old version just keeps working for people who have locked it, besides if nest wants to change this, there is surely something to discuss. It is always possible to automatically load the driver from the new auto entry point.

[kamilmysliwiec]

Ahh don't get me wrong, the change you've introduced is indeed helpful and as you & @jmcdo29 mentioned, it finally makes things somewhat more straightforward as you don't have to configure JWT twice!!

The problem here is that introducing a breaking change hurts every other existing project (for those who already figured that out & wired things up correctly), so I think it should at least be backward compatible (so it doesn't break existing code).

[Outternet]

The only problem we have then is what do we do if it is not provided, then we have to load the package jsonwebtoken ourself. this causes it to always have to be installed with the package as a peer dependency (even if an other driver is used) or as a hard dependency and that broke the libary in v3.0.0 where there was an exploit in jsonwebtoken, in addition, it seemed that most people had ^4.0.0 in there package.json without jsonwebtoken and that is also a breaking change if we no longer want a hard dependency.

For most projects it should continue to work well, even if they don't want to go to all the trouble of updating and stay on v4 (last update was 5 years anyway so they probably weren't expecting any new features and we can continue to provide security updates for v4) Also there is always the new passport-jwt/auto migration (which doesn't require a jwtDriver). If there is something wrong with this logic, please explain, more feedback from experienced devs is always needed. of course I don't like breaking updates either but it seemed really necessary in this case.

The only other things we were considering was the ES2020: import() and then making an optional import, or turn it around and then have something in the form passport-jwt/skeleton, to load without an driver. but the fact remains that just by changing the depency jsonwebtoken to a peerdepency is also a breaking change, and heaving it as a hard dependencies means that this library needs a (breaking) update if an exploit is found in jsonwebtoken (which is now 4 years old) and therefore the decision was to just do it both at once and be done with it. (some legacy v1 stuff could be removed now as well)

The nestjs docs update situation does not change much even if we decide to keep the dependencies and not do a breaking change. This is because the driver method is the new recommended way to use it with nest. As a final note more feedback is always appreciated, especially considering the great impact this has. We really appreciate all the help we can get with these difficult choices.