Skip to content

Commit

Permalink
fix(strategy): do not modify the params argument, clone it instead
Browse files Browse the repository at this point in the history
fixes #177
  • Loading branch information
panva committed Jul 18, 2019
1 parent 61f21b6 commit 4731d29
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
10 changes: 6 additions & 4 deletions lib/passport_strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
const url = require('url');
const { format } = require('util');

const { cloneDeep } = require('lodash');

const { RPError, OPError } = require('./errors');
const { BaseClient } = require('./client');
const { random, codeChallenge } = require('./helpers/generators');
Expand Down Expand Up @@ -48,7 +50,7 @@ function OpenIDConnectStrategy({
this._passReqToCallback = passReqToCallback;
this._usePKCE = usePKCE;
this._key = sessionKey || `oidc:${url.parse(this._issuer.issuer).hostname}`;
this._params = params;
this._params = cloneDeep(params);

if (this._usePKCE === true) {
const supportedMethods = this._issuer.code_challenge_methods_supported;
Expand All @@ -68,9 +70,9 @@ function OpenIDConnectStrategy({

this.name = url.parse(client.issuer.issuer).hostname;

if (!params.response_type) params.response_type = resolveResponseType.call(client);
if (!params.redirect_uri) params.redirect_uri = resolveRedirectUri.call(client);
if (!params.scope) params.scope = 'openid';
if (!this._params.response_type) this._params.response_type = resolveResponseType.call(client);
if (!this._params.redirect_uri) this._params.redirect_uri = resolveRedirectUri.call(client);
if (!this._params.scope) this._params.scope = 'openid';
}

OpenIDConnectStrategy.prototype.authenticate = function authenticate(req, options) {
Expand Down
4 changes: 3 additions & 1 deletion test/passport/passport_strategy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ describe('OpenIDConnectStrategy', () => {

describe('initate', function () {
it('starts authentication requests for GETs', function () {
const strategy = new Strategy({ client: this.client }, () => {});
const params = { foo: 'bar' };
const strategy = new Strategy({ client: this.client, params }, () => {});

const req = new MockRequest('GET', '/login/oidc');
req.session = {};

strategy.redirect = sinon.spy();
strategy.authenticate(req);

expect(params).to.eql({ foo: 'bar' });
expect(strategy.redirect.calledOnce).to.be.true;
const target = strategy.redirect.firstCall.args[0];
expect(target).to.include('redirect_uri=');
Expand Down

0 comments on commit 4731d29

Please sign in to comment.