panva / openid-client

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

Strategy changes the params object #177

Closed texels closed 5 years ago

texels commented 5 years ago

Describe the bug The params object passed into the Strategy is modified. I ran into a situation where i was using the same param object for two different Strategies. This resulted in param.redirect_url being set, which caused issues when using it to construct the next Strategy ( there were different redirect urls for each strategy )

Easy to work around now that I know it does that - just dont reuse the param object.

To Reproduce Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.


const params = { scope : 'openid email profile' }
new Strategy( { client, params  }, ( t, u, done ) => { return done( null, u ) } )
console.log( params ); // this will have keys other than scope.

To fix this, you could change the constructor in passport_strategy.js

  this._params = Object.assign( { }, params ); 

  // ... other code...

  if (!this._params.response_type) this._params.response_type = _.get(client, 'response_types[0]', 'code');
  if (!this._params.redirect_uri) this._params.redirect_uri = _.get(client, 'redirect_uris[0]'); // TODO: only default if there's one
  if (!this._params.scope) this._params.scope = 'openid';

Steps to reproduce the behaviour:

  1. See above

Expected behaviour I was expecting params to remain unchanged.

Environment:

Additional context Add any other context about the problem here.

panva commented 5 years ago

Thank you for bringing this up, i've fixed it and released v3.2.3