passport: Extending `PassportStrategy` does not take provider specific OAuth2 options in account

I’m submitting a…


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

Current behavior

Extending PassportStrategy does not work as expected. In case of extending PassportStrategy(Strategy, 'google') additional OAuth2 options, respectively provider specific options like e.g. approval_prompt passed to Superconstructor are NOT applied. So it is not possible to obtain a REFRESH_TOKEN.

Expected behavior

Additional, provider specific OAuth2 options can be passed through the Superconstructor and become effective.

Minimal reproduction of the problem with instructions

My Google OAuth2 strategy implementation:

import { Injectable } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { Strategy } from 'passport-google-oauth20';
import { AuthService, Provider } from './auth.service';
import { ConfigService } from '../config/config.service';
import { User } from '../api/user/user.interface';

@Injectable()
export class GoogleStrategy extends PassportStrategy(Strategy, 'google') {

  constructor(configService: ConfigService, private readonly authService: AuthService) {
    super({
      clientID: configService.get('OAUTH_CLIENT_ID'),
      clientSecret: configService.get('OAUTH_CLIENT_SECRET'),
      callbackURL: `${configService.baseUrl}/auth/google/callback`,
      passReqToCallback: true,
      scope: ['email', 'profile'],
      // NOT WORKING
      approval_prompt: 'force',
      access_type: 'offline',
    });
  }

  async validate(request: any, accessToken: string, refreshToken: string, profile: any, done: (err: any, result: any) => void) {
    // ...
  }

  // WORKAROUND: pass options to superclass auth call by overriding superclass method
  authorizationParams(options: any): any {
    return Object.assign(options, {
      approval_prompt: 'force',
      access_type: 'offline',
    });
  }
}

Environment


@nestjs/passport: 6.0.0

 
For Tooling issues:
- Node version: 10.11.0
- Platform:  Mac

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Reactions: 12
  • Comments: 31 (12 by maintainers)

Most upvoted comments

While that does work it skips the validation and sanitization the original function does. You’re able to achieve the same by overriding the authenticate function and it retains all original behavior of the strategy.

@Injectable()
export class GoogleStrategy extends PassportStrategy(Strategy, 'google') {
    authenticate(req, options: any): any {
        super.authenticate(req, Object.assign(options, {
            hd: '<HD_PARAM>'
        });
    }
}

Edit: You can achieve similar results by calling super.authorizationParams, however if anyone stumbled upon custom_params or custom parameters this would be the way it’d work across providers (potentially).

https://github.com/nestjs/passport/blob/master/lib/auth.guard.ts#L82

We pass AuthModuleOptions to the authenticate() function, so you should be able to put them here:

PassportModule.register({
      approval_prompt: 'force',
      access_type: 'offline',
})

@iveselin just try with:

@Injectable()
export class GoogleStrategy extends PassportStrategy(Strategy, 'google') {
  // ...
  // WILL be passed to authenticate()
  authorizationParams(options: any): any {
    return Object.assign(options, {
      hd: '<HD_PARAM>'
    });
  }
}

The problem is, that non-standard params passed through constructor super() call are ignored, which normally should work (https://github.com/jaredhanson/passport-google-oauth2/blob/master/lib/strategy.js#L159). Just use authorizationParams as in the example above.

According to Facebook docs for api 2.4 (current is 3.2)

Fewer default fields for faster performance: To help improve performance on mobile network connections, we've reduced the number of fields that the API returns by default. You should now use the ?fields=field1,field2syntax to declare all the fields you want the API to return.

https://developers.facebook.com/blog/post/2015/07/08/graph-api-v2.4/

This means even if I use profileFields in Strategy constructor

      profileFields: ["id", "email", "name"],

I still have to pass additional params to authorize function like this

passport.use(new FacebookStrategy({
          clientID: "CLIENT_ID",
          clientSecret: "CLIENT_SECRET",
          callbackURL: "http://www.example.com/auth/facebook/callback"
          passReqToCallback : true,
          profileFields: ["id", "emails", "name"] 
    })
);

app.get("/connect/facebook", passport.authorize("facebook", { scope : ["email"] }));

Thanks to @csidell-earny his workaround works fine for me

  authenticate(req: Request, options?: Record<string, any>): void {
    super.authenticate(req, Object.assign(options, {
      scope: ["email"]
    }));
  }

@csidell-earny I tried your PR with my app. Identical code with only swapping in/out your version of @nestjs/passport works with current version, fails as follows.

I can’t make this repo public, but if it matters, I could see if I can strip it down. But it looks like it has to do with the fact that I’m also using Passport sessions, a la stuff like this in my main.ts:

app.use(session(...

app.use(passport.session())
Error: Failed to serialize user into session
[0]     at pass (/home/john/code/dwserver5/node_modules/passport/lib/authenticator.js:281:19)
[0]     at Authenticator.serializeUser (/home/john/code/dwserver5/node_modules/passport/lib/authenticator.js:299:5)
[0]     at SessionManager.logIn (/home/john/code/dwserver5/node_modules/passport/lib/sessionmanager.js:14:8)
[0]     at IncomingMessage.req.login.req.logIn (/home/john/code/passport-1/node_modules/passport/lib/http/request.js:50:33)
[0]     at Promise (/home/john/code/passport-1/dist/auth.guard.js:58:64)
[0]     at new Promise (<anonymous>)
[0]     at GoogleLoginGuard.<anonymous> (/home/john/code/passport-1/dist/auth.guard.js:58:23)
[0]     at Generator.next (<anonymous>)
[0]     at /home/john/code/passport-1/dist/auth.guard.js:19:71
[0]     at new Promise (<anonymous>)

Ahhh, got you now. That might be tricky. You could theoretically extend the guard and strip options properties depending on the provider in the constructor.

I am not sure why we need a separate method just as @kamilmysliwiec said. The above suggestion makes perfect sense to me.

I have Google, Twitter & Facebook working altogether with separate Strategy & Guards. An authenticate method with customParameters seems un-necessary to me as well. All I am doing is passing different options to constructor of the new strategies.

Also, just to be clear, according to passport-google-oauth2’s strategy.js,

approval_prompt: 'force',  // should be "approvalPrompt" - strategy.js#L183
access_type: 'offline', // should be "accessType" - strategy.js#L138

CC: @johnbiundo