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

Add SameSite support #39

Merged
merged 7 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Additional configuration keys that can be passed to `auth()` on initialization:
- **`errorOnRequiredAuth`** - Boolean value to throw a `Unauthorized 401` error instead of triggering the login process for routes that require authentication. Default is `false`.
- **`httpOptions`** - Default options object used for all HTTP calls made by the library ([possible options](https://github.com/sindresorhus/got/tree/v9.6.0#options)). Default is empty.
- **`idpLogout`** - Boolean value to log the user out from the identity provider on application logout. Requires the issuer to provide a `end_session_endpoint` value. Default is `false`.
- **`legacySameSiteCookie`** - Set a fallback cookie with no SameSite attribute when `authorizationParams.response_mode` is `form_post`. Default is `true`.
- **`loginPath`** - Relative path to application login. Default is `/login`.
- **`logoutPath`** - Relative path to application logout. Default is `/logout`.
- **`redirectUriPath`** - Relative path to the application callback to process the response from the authorization server. This value is combined with the `baseUrl` and sent to the authorize endpoint as the `redirectUri` parameter. Default is `/callback`.
Expand Down
1 change: 1 addition & 0 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const paramsSchema = Joi.object().keys({
redirectUriPath: Joi.string().optional().default('/callback'),
loginPath: Joi.string().optional().default('/login'),
logoutPath: Joi.string().optional().default('/logout'),
legacySameSiteCookie: Joi.boolean().optional().default(true),
idpLogout: Joi.boolean().optional().default(false)
.when('auth0Logout', { is: true, then: Joi.boolean().optional().default(true) })
});
Expand Down
38 changes: 20 additions & 18 deletions lib/context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const cb = require('cb');
const urlJoin = require('url-join');
const crypto = require('crypto');
const transient = require('./transientHandler');
const { get: getClient } = require('./client');
const { TokenSet } = require('openid-client');

Expand Down Expand Up @@ -57,30 +57,32 @@ class ResponseContext {
const next = cb(this._next).once();
const req = this._req;
const res = this._res;
const authorizeParams = this._config.authorizationParams;
const config = this._config;

try {
const redirect_uri = this.getRedirectUri();
if (typeof req.session === 'undefined') {
return next(new Error('This router needs the session middleware'));
}
const client = this._req.openid.client;
const client = req.openid.client;
const authorizeParams = config.authorizationParams;
const transientOpts = {
legacySameSiteCookie: config.legacySameSiteCookie,
sameSite: config.authorizationParams.response_mode === 'form_post' ? 'None' : 'Lax'
};

req.session.nonce = crypto.randomBytes(8).toString('hex');
req.session.state = crypto.randomBytes(10).toString('hex');

if(params.returnTo) {
req.session.returnTo = params.returnTo;
try {
let returnTo;
if (params.returnTo) {
returnTo = params.returnTo;
} else if (req.method === 'GET') {
req.session.returnTo = req.originalUrl;
returnTo = req.originalUrl;
} else {
req.session.returnTo = this._config.baseURL;
returnTo = this._config.baseURL;
}

// TODO: Store this in state
transient.store('returnTo', res, Object.assign({value: returnTo}, transientOpts));

const authParams = Object.assign({
nonce: req.session.nonce,
state: req.session.state,
redirect_uri
nonce: transient.store('nonce', res, transientOpts),
state: transient.store('state', res, transientOpts),
redirect_uri: this.getRedirectUri()
}, authorizeParams, params.authorizationParams || {});

const authorizationUrl = client.authorizationUrl(authParams);
Expand Down
90 changes: 90 additions & 0 deletions lib/transientHandler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
const crypto = require('crypto');

/**
* Set a cookie with a value or a generated nonce.
*
* @param {String} key Cookie name to use.
* @param {Object} res Express Response object.
* @param {Object} opts Options object.
* @param {String} opts.sameSite SameSite attribute of "None," "Lax," or "Strict". Default is "None."
* @param {String} opts.value Cookie value. Omit this key to store a generated value.
* @param {Boolean} opts.legacySameSiteCookie Should a fallback cookie be set? Default is true.
* @param {Boolean} opts.maxAge Cookie MaxAge value, in microseconds. Default is 600000 (10 minutes).
*
* @return {String} Cookie value that was set.
*/
function store(key, res, opts = {}) {
const sameSiteAttr = opts.sameSite || 'None';
const isSameSiteNone = sameSiteAttr === 'None';
const value = opts.value || createNonce();
const fallbackCookie = 'legacySameSiteCookie' in opts ? opts.legacySameSiteCookie : true;

const basicAttr = {
httpOnly: true,
maxAge: 'maxAge' in opts ? parseInt(opts.maxAge, 10) : 600 * 1000 // 10 minutes
};

// Set the cookie with the SameSite attribute and, if needed, the Secure flag.
res.cookie(key, value, Object.assign({}, basicAttr, {sameSite: sameSiteAttr, secure: isSameSiteNone}));

if (isSameSiteNone && fallbackCookie) {
// Set the fallback cookie with no SameSite or Secure attributes.
res.cookie('_' + key, value, basicAttr);
}

return value;
}

/**
* Get a cookie value then delete it.
*
* @param {String} key Cookie name to use.
* @param {Object} req Express Request object.
* @param {Object} res Express Response object.
* @param {Object} opts Options object.
* @param {Boolean} opts.legacySameSiteCookie Should a fallback cookie be checked? Default is true.
*
* @return {String|undefined} Cookie value or undefined if cookie was not found.
*/
function getOnce(key, req, res, opts = {}) {

if (!req.cookies) {
return undefined;
}

let value = req.cookies[key];
delete req.cookies[key];
deleteCookie(key, res);

if ('legacySameSiteCookie' in opts ? opts.legacySameSiteCookie : true) {
const fallbackKey = '_' + key;
value = value || req.cookies[fallbackKey];
delete req.cookies[fallbackKey];
deleteCookie(fallbackKey, res);
}

return value;
}

/**
* Generates a nonce value.
*
* @return {String}
*/
function createNonce() {
return crypto.randomBytes(16).toString('hex');
}

/**
* Sets a blank value and zero max age cookie.
*
* @param {String} name Cookie name
* @param {Object} res Express Response object
*/
function deleteCookie(name, res) {
res.cookie(name, '', {maxAge: 0});
}

exports.store = store;
exports.getOnce = getOnce;
exports.createNonce = createNonce;
16 changes: 7 additions & 9 deletions middleware/auth.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
const express = require('express');
const cb = require('cb');
const createError = require('http-errors');
const cookieParser = require('cookie-parser');
const { get: getConfig } = require('../lib/config');
const { get: getClient } = require('../lib/client');
const requiresAuth = require('./requiresAuth');
const transient = require('../lib/transientHandler');
const { RequestContext, ResponseContext } = require('../lib/context');

/**
Expand Down Expand Up @@ -79,23 +81,20 @@ module.exports = function (params) {
callbackMethod = 'get';
}

router[callbackMethod](config.redirectUriPath, express.urlencoded({ extended: false }), async (req, res, next) => {
router[callbackMethod](config.redirectUriPath, express.urlencoded({ extended: false }), cookieParser(), async (req, res, next) => {
next = cb(next).once();
try {
const { nonce, state } = req.session;
delete req.session.nonce;
delete req.session.state;

const redirect_uri = res.openid.getRedirectUri();
const client = req.openid.client;
const transientOpts = { legacySameSiteCookie: config.legacySameSiteCookie };

let tokenSet;

try {
const callbackParams = client.callbackParams(req);
tokenSet = await client.callback(redirect_uri, callbackParams, {
nonce,
state,
nonce: transient.getOnce('nonce', req, res, transientOpts),
state: transient.getOnce('state', req, res, transientOpts),
response_type: authorizeParams.response_type,
});
} catch (err) {
Expand All @@ -104,8 +103,7 @@ module.exports = function (params) {

req.session.openidTokens = tokenSet;

const returnTo = req.session.returnTo || '/';
delete req.session.returnTo;
const returnTo = transient.getOnce('returnTo', req, res, transientOpts) || '/';

res.redirect(returnTo);
} catch (err) {
Expand Down
19 changes: 17 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
"@hapi/joi": "^14.5.0",
"cb": "^0.1.0",
"clone": "^2.1.2",
"cookie-parser": "^1.4.4",
"http-errors": "^1.7.3",
"openid-client": "^3.7.3",
"p-memoize": "^3.1.0",
"url-join": "^4.0.1"
},
"devDependencies": {
"@types/express": "^4.17.1",
"body-parser": "^1.19.0",
"chai": "^4.2.0",
"cookie-session": "^2.0.0-beta.3",
"eslint": "^5.16.0",
Expand Down
32 changes: 26 additions & 6 deletions test/auth.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ const filterRoute = (method, path) => {
r.route.methods[method.toLowerCase()];
};

const getCookieFromResponse = (res, cookieName) => {
const cookieHeaders = res.headers['set-cookie'];

const foundHeader = cookieHeaders.filter(header => header.substring(0,6) === cookieName + '=')[0];
if (!foundHeader) {
return false;
}

const cookieValuePart = foundHeader.split('; ')[0];
if (!cookieValuePart) {
return false;
}

return cookieValuePart.split('=')[1];
};

describe('auth', function() {
describe('default', () => {

Expand Down Expand Up @@ -49,17 +65,17 @@ describe('auth', function() {
assert.equal(parsed.hostname, 'test.auth0.com');
assert.equal(parsed.pathname, '/authorize');
assert.equal(parsed.query.client_id, '__test_client_id__');

assert.equal(parsed.query.scope, 'openid profile email');
assert.equal(parsed.query.response_type, 'id_token');
assert.equal(parsed.query.response_mode, 'form_post');
assert.equal(parsed.query.redirect_uri, 'https://example.org/callback');
assert.property(parsed.query, 'nonce');
assert.property(parsed.query, 'state');

const session = (await request.get('/session', { jar, baseUrl, json: true })).body;
assert.equal(session.nonce, parsed.query.nonce);
assert.equal(session.state, parsed.query.state);
const cookies = jar.getCookies(baseUrl + '/login');

assert.equal(cookies.filter(cookie => cookie.key === '_nonce')[0].value, parsed.query.nonce);
assert.equal(cookies.filter(cookie => cookie.key === '_state')[0].value, parsed.query.state);
});

});
Expand Down Expand Up @@ -110,7 +126,7 @@ describe('auth', function() {
let router;

before(async function() {
router = router = expressOpenid.auth({
router = expressOpenid.auth({
clientID: '__test_client_id__',
clientSecret: '__test_client_secret__',
baseURL: 'https://example.org',
Expand Down Expand Up @@ -140,6 +156,10 @@ describe('auth', function() {
assert.equal(parsed.query.redirect_uri, 'https://example.org/callback');
assert.property(parsed.query, 'nonce');
assert.property(parsed.query, 'state');
assert.property(res.headers, 'set-cookie');

assert.equal(getCookieFromResponse(res, 'nonce'), parsed.query.nonce);
assert.equal(getCookieFromResponse(res, 'state'), parsed.query.state);
});

it('should contain a callback route', function() {
Expand All @@ -152,7 +172,7 @@ describe('auth', function() {
let baseUrl;

before(async function() {
router = router = expressOpenid.auth({
router = expressOpenid.auth({
clientID: '__test_client_id__',
baseURL: 'https://example.org',
issuerBaseURL: 'https://test.auth0.com',
Expand Down
Loading