Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing user specified parameter to authorizationParams() #81

Open
soichih opened this issue Mar 31, 2017 · 5 comments · May be fixed by #116
Open

Passing user specified parameter to authorizationParams() #81

soichih opened this issue Mar 31, 2017 · 5 comments · May be fixed by #116

Comments

@soichih
Copy link

soichih commented Mar 31, 2017

Hello.

My oauth2 provider handles multiple idP(s), and I can specify which one to use by setting "selected_idp" parameter to authorize URL

http://www.cilogon.org/oidc

I see that I can add arbitrary parameters to authorize URL by overriding authorizationParams() function, but this function doesn't allow accessing user request parameter.

Can you add an additional option parameter to authenticate() so that I could do something like

router.get('/signin', function(req, res, next) {
    passport.authenticate('oauth2', {
        authorize_params: function() {
            return {selected_idp: req.query.idp}
        }
    })(req, res, next);
});

??

@soichih
Copy link
Author

soichih commented Mar 31, 2017

I just realized I could do this instead

OAuth2Strategy.prototype.authorizationParams = function(options) {
    return { selected_idp: options.idp }
}
router.get('/signin', function(req, res, next) {
    passport.authenticate('oauth2', {
        //this will be used by my authorizationParams() and selected_idp will be injected to authorized url
        idp: req.query.idp
    })(req, res, next);
});

Is there any side effect from Javascript closure?

@davidjb
Copy link

davidjb commented Sep 15, 2017

+1 for a way to allow arbitrary parameters in the authorisation URL. For instance, if req could be passed to the call to authorizationParams (eg https://github.com/jaredhanson/passport-oauth2/blob/master/lib/strategy.js#L216) would allow returning params and values based upon the request, session, user etc. In my use case, my OAuth provider takes a variety of different parameters and because I'm authorize()'ing an existing user, I'd want to pass the user's attributes (from the request session).

What @soichih suggests with authenticate() is an option -- perhaps a second level of customisation -- but at least initially it having the Strategy pass the request to its authorizationParams function would make it simpler than having to create separate closures/callbacks.

@ktuukkan
Copy link

Agreed, and there is also the claims parameter which doesn't seem to be supported at the moment. The most convenient way to support it would be giving it as an option like you do with scope and state params.

@oleg-codaio
Copy link

A workaround here is to directly pass any custom params into the authorization URL. These will be merged into the authorization URL this package generates:

var parsed = url.parse(this._oauth2._authorizeUrl, true);
utils.merge(parsed.query, params);

@mdorda
Copy link

mdorda commented Sep 26, 2019

In a case you even cannot pass data as @soichih sugests, you can access the request object using this ugly nasty never-use-that way:

passportOAuth.Strategy.prototype.authorizationParams = function() {
    const req = passportOAuth.Strategy.prototype.authorizationParams.caller.arguments[0];
    // do whatever you need
}

I will create a pull request to avoid this ugly hack, hope it will be merged soon.

@mdorda mdorda linked a pull request Sep 26, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants