panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Double encoding authenticate url #143

Closed mirkobobanac closed 5 years ago

mirkobobanac commented 5 years ago

Hi, i have situation with expressJs and custom authentication server. Everything works fine except multiple scopes url formatting.

Scope part of retUrl is formatted as scope%2520scope%2520scope instead of scope%20scope%20scope

Is there any quick workaround?

Environment:

panva commented 5 years ago

Hi @mirkobobanac

that's too vague of an explanation, can you provide a sample app that's redirect to double-encoded url? I'm not entirely sure what is it that you have a problem with - the authorizationUrl prototype function already returns properly encoded url to redirect to, anything encoding it again is beyond openid-client's control and should be avoided.

mirkobobanac commented 5 years ago

Code in short:

app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: false }));

app.use(session);
app.use(passport.initialize());
app.use(passport.session());
require('./config')(passport);

'config.js:
const issuer = new Issuer({
    issuer: '**',
    authorization_endpoint: '**',
    token_endpoint: '**',
    userinfo_endpoint: '**',
    jwks_uri: '**',
});
const client = new issuer.Client({
    client_id: config('client_id'),
    client_secret:  config('client_secret')
});
  passport.use('oidc', new Strategy({
        client,
        params: {
            redirect_uri: '',
            scope: 'openid email profile',
        }
    }, (tokenset, userinfo, done) => {
        return done(null, userinfo);
    }));

app.get('/auth', function (req, res, next) {
    passport.authenticate('oidc', {
        redirect_uri: 'redirect route'
    })(req, res, next);
});

Can this be (expressJs + node-openid-client) issue?

panva commented 5 years ago

Can you NOT remove the actual values (apart from secrets)? if you need to sanitize them just replace the hostname, but context matters and you're changing it.

mirkobobanac commented 5 years ago

Can you NOT remove the actual values (apart from secrets)? if you need to sanitize them just replace the hostname, but context matters and you're changing it.

Sorry but i need to remove them, this is company openId implementation, it is not public one.

panva commented 5 years ago

If you need to sanitize them just replace the hostname. I can't help if there's no way for me to reproduce. I've got plenty working express and passport examples so it's likely your setup or use issue.

mirkobobanac commented 5 years ago

It seems that company implementation differs a bit, it expect + instead of space when setting scopes.

Sorry for troubling you.