Skip to content

Commit

Permalink
chore: fixup 43daff3
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Jan 20, 2023
1 parent 43daff3 commit 93f788d
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
10 changes: 5 additions & 5 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -855,13 +855,13 @@ Creates a new Strategy
- `done`: `<Function>`
- Returns: `<Strategy>`

---

The strategy automatically generates `state` and `nonce` parameters when required. To provide one for a flow where it is optional (for example the `nonce` for the Authorization Code Flow), it can be passed in the optional `options` argument to `passport.authenticate()`:
Note: You can also set authorization request parameters dynamically using the `options` argument in `passport.authenticate([options])`:

```js
app.post('/auth/oidc', function(req, res, next) {
passport.authenticate('oidc', { nonce: crypto.randomBytes(16).toString('base64url') })(req, res, next);
app.get('/protected-route', function(req, res, next) {
if (shouldReConsent(req)) {
passport.authenticate('oidc', { prompt: 'consent' })(req, res, next);
}
});
```

Expand Down
2 changes: 1 addition & 1 deletion lib/passport_strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function OpenIDConnectStrategy(
this._key = sessionKey || `oidc:${url.parse(this._issuer.issuer).hostname}`;
this._params = cloneDeep(params);

// state and nonce should be provided or generated below on each authenticate()
// state and nonce are handled in authenticate()
delete this._params.state;
delete this._params.nonce;

Expand Down
35 changes: 34 additions & 1 deletion test/passport/passport_strategy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,14 @@ describe('OpenIDConnectStrategy', () => {
expect(target).to.include(`resource=${encodeURIComponent('urn:example:foo')}`);
});

it('automatically includes nonce for where it applies', function () {
it('automatically includes nonce for where it applies (and ignores one from params)', function () {
const strategy = new Strategy(
{
client: this.client,
params: {
response_type: 'code id_token token',
response_mode: 'form_post',
nonce: 'foo',
},
},
() => {},
Expand All @@ -220,6 +221,7 @@ describe('OpenIDConnectStrategy', () => {
expect(target).to.include('redirect_uri=');
expect(target).to.include('scope=');
expect(target).to.include('nonce=');
expect(target).not.to.include('nonce=foo');
expect(target).to.include('response_mode=form_post');
expect(req.session).to.have.property('oidc:op.example.com');
expect(req.session['oidc:op.example.com']).to.have.keys(
Expand All @@ -230,6 +232,37 @@ describe('OpenIDConnectStrategy', () => {
);
});

it('ignores static state coming from params', function () {
const strategy = new Strategy(
{
client: this.client,
params: {
state: 'foo',
},
},
() => {},
);

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

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

expect(strategy.redirect.calledOnce).to.be.true;
const target = strategy.redirect.firstCall.args[0];
expect(target).to.include('redirect_uri=');
expect(target).to.include('scope=');
expect(target).to.include('state=');
expect(target).not.to.include('state=foo');
expect(req.session).to.have.property('oidc:op.example.com');
expect(req.session['oidc:op.example.com']).to.have.keys(
'state',
'response_type',
'code_verifier',
);
});

describe('use pkce', () => {
it('will throw when explictly provided value is not supported', function () {
expect(() => {
Expand Down

0 comments on commit 93f788d

Please sign in to comment.