Skip to content

Commit

Permalink
Update new secure cookie behaviour form auth0/express-openid-connect#159
Browse files Browse the repository at this point in the history
  • Loading branch information
newcodestar7 committed Dec 8, 2020
1 parent baaab40 commit c70fa4c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 25 deletions.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@
"text",
"text-summary"
],
"preset": "ts-jest"
"preset": "ts-jest",
"setupFilesAfterEnv": [
"./tests/setup.ts"
]
}
}
2 changes: 1 addition & 1 deletion src/auth0-session/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export interface CookieConfig {
/**
* Marks the cookie to be used over secure channels only.
* Passed to the [Response cookie](https://expressjs.com/en/api.html#res.cookie) as `secure`.
* Defaults to {@link Request.secure}.
* Defaults to the protocol of {@link Config.baseURL}.
*/
secure?: boolean;

Expand Down
3 changes: 1 addition & 2 deletions src/auth0-session/cookie-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ export default class CookieStore {

const cookieOptions = {
...cookieConfig,
maxAge: transient ? undefined : exp,
secure: 'secure' in cookieConfig ? cookieConfig.secure : this.config.baseURL.startsWith('https:')
maxAge: transient ? undefined : exp
};

debug('found session, creating signed session cookie(s) with name %o(.i)', sessionName);
Expand Down
58 changes: 47 additions & 11 deletions src/auth0-session/get-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Joi from '@hapi/joi';
import { defaultState as getLoginState } from './hooks/get-login-state';
import { Config } from './config';

const isHttps = /^https:/i;

const paramsSchema = Joi.object({
secret: Joi.alternatives([
Joi.string().min(8),
Expand Down Expand Up @@ -36,7 +38,22 @@ const paramsSchema = Joi.object({
transient: Joi.boolean().optional().default(false),
httpOnly: Joi.boolean().optional().default(true),
sameSite: Joi.string().valid('lax', 'strict', 'none').optional().default('lax'),
secure: Joi.boolean().optional(),
secure: Joi.when(Joi.ref('/baseURL'), {
is: Joi.string().pattern(isHttps),
then: Joi.boolean()
.default(true)
.custom((value, { warn }) => {
if (!value) warn('insecure.cookie');
return value;
})
.messages({
'insecure.cookie':
"Setting your cookie to insecure when over https is not recommended, I hope you know what you're doing."
}),
otherwise: Joi.boolean().valid(false).default(false).messages({
'any.only': 'Cookies set with the `Secure` property wont be attached to http requests'
})
}),
path: Joi.string().uri({ relativeOnly: true }).optional()
})
.default()
Expand All @@ -55,14 +72,27 @@ const paramsSchema = Joi.object({
.optional()
.when('response_type', {
is: 'code',
then: Joi.valid('query', 'query'),
then: Joi.valid('query', 'form_post'),
otherwise: Joi.valid('form_post').default('form_post')
})
})
.optional()
.unknown(true)
.default(),
baseURL: Joi.string().uri().required(),
baseURL: Joi.string()
.uri()
.required()
.when(Joi.ref('authorizationParams.response_mode'), {
is: 'form_post',
then: Joi.string()
.pattern(isHttps)
.rule({
warn: true,
message:
"Using 'form_post' for response_mode may cause issues for you logging in over http, " +
'see https://github.com/auth0/express-openid-connect/blob/master/FAQ.md'
})
}),
clientID: Joi.string().required(),
clientSecret: Joi.string()
.when(
Expand All @@ -89,8 +119,6 @@ const paramsSchema = Joi.object({
),
clockTolerance: Joi.number().optional().default(60),
enableTelemetry: Joi.boolean().optional().default(true),
errorOnRequiredAuth: Joi.boolean().optional().default(false), // ?
attemptSilentLogin: Joi.boolean().optional().default(false), // ?
getLoginState: Joi.function()
.optional()
.default(() => getLoginState),
Expand All @@ -103,15 +131,20 @@ const paramsSchema = Joi.object({
idTokenSigningAlg: Joi.string().insensitive().not('none').optional().default('RS256'),
issuerBaseURL: Joi.string().uri().required(),
legacySameSiteCookie: Joi.boolean().optional().default(true),
authRequired: Joi.boolean().optional().default(true), // ?
routes: Joi.object({
login: Joi.alternatives([Joi.string().uri({ relativeOnly: true }), Joi.boolean().valid(false)]).default('/login'),
logout: Joi.alternatives([Joi.string().uri({ relativeOnly: true }), Joi.boolean().valid(false)]).default('/logout'),
callback: Joi.string().uri({ relativeOnly: true }).default('/callback'),
postLogoutRedirect: Joi.string().uri({ allowRelative: true }).default('')
})
.default()
.unknown(false) // ?
.unknown(false),
clientAuthMethod: Joi.string()
.valid('client_secret_basic', 'client_secret_post', 'none')
.optional()
.default((parent) => {
return parent.authorizationParams.response_type === 'id_token' ? 'none' : 'client_secret_basic';
})
});

export type DeepPartial<T> = {
Expand All @@ -130,10 +163,13 @@ export const get = (params: ConfigParameters = {}): Config => {
...params
};

const paramsValidation = paramsSchema.validate(config);
if (paramsValidation.error) {
throw new TypeError(paramsValidation.error.details[0].message);
const { value, error, warning } = paramsSchema.validate(config);
if (error) {
throw new TypeError(error.details[0].message);
}
if (warning) {
console.warn(warning.message);
}

return paramsValidation.value;
return value;
};
2 changes: 1 addition & 1 deletion src/auth0-session/transient-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export default class TransientStore {
const { domain, path, secure } = this.config.session.cookie;
const basicAttr = {
httpOnly: true,
secure: typeof secure === 'boolean' ? secure : this.config.baseURL.startsWith('https:'), // @TODO check
secure,
domain,
path
};
Expand Down
11 changes: 4 additions & 7 deletions tests/auth0-session/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ describe('Config', () => {
response_type: 'id_token',
response_mode: 'form_post',
scope: 'openid profile email'
},
authRequired: true
}
});
});

Expand All @@ -41,8 +40,7 @@ describe('Config', () => {
response_type: 'id_token',
response_mode: 'form_post',
scope: 'openid profile email'
},
authRequired: true
}
});
process.env = _env;
});
Expand All @@ -59,8 +57,7 @@ describe('Config', () => {
authorizationParams: {
response_type: 'code',
scope: 'openid profile email'
},
authRequired: true
}
});
});

Expand Down Expand Up @@ -473,7 +470,7 @@ describe('Config', () => {
response_type: 'code',
response_mode: ('' as unknown) as undefined
})
).toThrowError(new TypeError('"authorizationParams.response_mode" must be [query]'));
).toThrowError(new TypeError('"authorizationParams.response_mode" must be one of [query, form_post]'));
});

it('should not allow response_type id_token and response_mode query', () => {
Expand Down
3 changes: 1 addition & 2 deletions tests/auth0-session/fixture/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ const clientId = '__test_client_id__';
export const defaultConfig: Omit<ConfigParameters, 'baseURL'> = {
secret,
clientID: clientId,
issuerBaseURL: 'https://op.example.com',
authRequired: false
issuerBaseURL: 'https://op.example.com'
};

export const toSignedCookieJar = (cookies: { [key: string]: string }, url: string): CookieJar => {
Expand Down
9 changes: 9 additions & 0 deletions tests/setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
beforeEach(() => {
jest.spyOn(console, 'warn').mockImplementation(() => {
// noop
});
});

afterEach(() => {
jest.restoreAllMocks();
});

0 comments on commit c70fa4c

Please sign in to comment.