Skip to content

Commit

Permalink
Scope all cookies (skipSilentLogin, transient and appSession) to the …
Browse files Browse the repository at this point in the history
…app session cookie path and domain config (if specified) (#125)
  • Loading branch information
adamjmcgrath committed Aug 13, 2020
1 parent a2d10e9 commit b7e394d
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 22 deletions.
6 changes: 6 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ interface SessionConfigParams {
*/
domain?: string;

/**
* Path for the cookie.
* Passed to the [Response cookie](https://expressjs.com/en/api.html#res.cookie) as `path`
*/
path?: string;

/**
* Set to true to use a transient cookie (cookie without an explicit expiration).
* Default is `false`
Expand Down
5 changes: 4 additions & 1 deletion lib/appSession.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ module.exports = (config) => {
);
for (const cookieName of Object.keys(req[COOKIES])) {
if (cookieName.match(`^${sessionName}(?:\\.\\d)?$`)) {
res.clearCookie(cookieName, { domain: cookieOptions.domain });
res.clearCookie(cookieName, {
domain: cookieOptions.domain,
path: cookieOptions.path,
});
}
}
} else {
Expand Down
1 change: 1 addition & 0 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const paramsSchema = Joi.object({
.optional()
.default('Lax'),
secure: Joi.boolean().optional(),
path: Joi.string().uri({ relativeOnly: true }).optional(),
})
.default()
.unknown(false),
Expand Down
17 changes: 10 additions & 7 deletions lib/transientHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ class TransientCookieHandler {
}
this.currentKey = current;
this.keyStore = keystore;
this.secureCookieConfig =
session && session.cookie && session.cookie.secure;
this.sessionCookieConfig = (session && session.cookie) || {};
this.legacySameSiteCookie = legacySameSiteCookie;
}

Expand All @@ -92,12 +91,12 @@ class TransientCookieHandler {
{ sameSite = 'None', value = this.generateNonce() } = {}
) {
const isSameSiteNone = sameSite === 'None';
const { domain, path, secure } = this.sessionCookieConfig;
const basicAttr = {
httpOnly: true,
secure:
typeof this.secureCookieConfig === 'boolean'
? this.secureCookieConfig
: req.secure,
secure: typeof secure === 'boolean' ? secure : req.secure,
domain,
path,
};

{
Expand Down Expand Up @@ -191,7 +190,11 @@ class TransientCookieHandler {
* @param {Object} res Express Response object
*/
deleteCookie(name, res) {
res.clearCookie(name);
const { domain, path } = this.sessionCookieConfig;
res.clearCookie(name, {
domain,
path,
});
}
}

Expand Down
21 changes: 11 additions & 10 deletions middleware/attemptSilentLogin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,31 @@ const COOKIE_NAME = 'skipSilentLogin';
const cancelSilentLogin = (req, res) => {
const {
config: {
session: { cookie: cookieConfig },
session: {
cookie: { secure, domain, path },
},
},
} = weakRef(req.oidc);
res.cookie(COOKIE_NAME, true, {
httpOnly: true,
secure:
typeof cookieConfig.secure === 'boolean'
? cookieConfig.secure
: req.secure,
secure: typeof secure === 'boolean' ? secure : req.secure,
domain,
path,
});
};

const resumeSilentLogin = (req, res) => {
const {
config: {
session: { cookie: cookieConfig },
session: {
cookie: { domain, path },
},
},
} = weakRef(req.oidc);
res.clearCookie(COOKIE_NAME, {
httpOnly: true,
secure:
typeof cookieConfig.secure === 'boolean'
? cookieConfig.secure
: req.secure,
domain,
path,
});
};

Expand Down
32 changes: 28 additions & 4 deletions test/logout.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ const defaultConfig = {
authRequired: false,
};

const baseUrl = 'http://localhost:3000';

const login = async () => {
const login = async (baseUrl = 'http://localhost:3000') => {
const jar = request.jar();
await request.post({
uri: '/session',
Expand All @@ -35,7 +33,7 @@ const login = async () => {
return { jar, session };
};

const logout = async (jar) => {
const logout = async (jar, baseUrl = 'http://localhost:3000') => {
const response = await request.get({
uri: '/logout',
baseUrl,
Expand Down Expand Up @@ -173,10 +171,36 @@ describe('logout route', async () => {
);
});

it('should logout when scoped to a sub path', async () => {
server = await createServer(
auth({
...defaultConfig,
session: {
cookie: {
path: '/foo',
},
},
}),
null,
'/foo'
);
const baseUrl = 'http://localhost:3000/foo';

const { jar, session: loggedInSession } = await login(baseUrl);
assert.ok(loggedInSession.id_token);
const sessionCookie = jar
.getCookies('http://localhost:3000/foo')
.find(({ key }) => key === 'appSession');
assert.equal(sessionCookie.path, '/foo');
const { session: loggedOutSession } = await logout(jar, baseUrl);
assert.notOk(loggedOutSession.id_token);
});

it('should cancel silent logins when user logs out', async () => {
server = await createServer(auth(defaultConfig));

const { jar } = await login();
const baseUrl = 'http://localhost:3000';
assert.notOk(
jar.getCookies(baseUrl).find(({ key }) => key === 'skipSilentLogin')
);
Expand Down

0 comments on commit b7e394d

Please sign in to comment.