From c285f72e12ca015eb1b02eaa13908f37ee33a6fe Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Tue, 11 Oct 2022 10:42:30 -0400 Subject: [PATCH] fix: bug that allowed for bypass of GDPR interstitial on SSO registrations simply by cancelling the form --- src/controllers/authentication.js | 24 ++++++----- test/controllers.js | 68 +++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index 25df05afc3..885088dd26 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -209,18 +209,22 @@ authenticationController.registerComplete = async function (req, res) { } }; -authenticationController.registerAbort = function (req, res) { +authenticationController.registerAbort = async (req, res) => { if (req.uid) { - // Clear interstitial data and continue on... - delete req.session.registration; - res.redirect(nconf.get('relative_path') + (req.session.returnTo || '/')); - } else { - // End the session and redirect to home - req.session.destroy(() => { - res.clearCookie(nconf.get('sessionKey'), meta.configs.cookie.get()); - res.redirect(`${nconf.get('relative_path')}/`); - }); + // Email is the only cancelable interstitial + delete req.session.registration.updateEmail; + + const { interstitials } = await user.interstitials.get(req, req.session.registration); + if (!interstitials.length) { + return res.redirect(nconf.get('relative_path') + (req.session.returnTo || '/')); + } } + + // End the session and redirect to home + req.session.destroy(() => { + res.clearCookie(nconf.get('sessionKey'), meta.configs.cookie.get()); + res.redirect(`${nconf.get('relative_path')}/`); + }); }; authenticationController.login = async (req, res, next) => { diff --git a/test/controllers.js b/test/controllers.js index f6c642a00b..ab2f7202c1 100644 --- a/test/controllers.js +++ b/test/controllers.js @@ -600,6 +600,74 @@ describe('Controllers', () => { assert.strictEqual(res.headers.location, `${nconf.get('relative_path')}/`); }); }); + + describe('abort behaviour', () => { + let jar; + let token; + + beforeEach(async () => { + jar = await helpers.registerUser({ + username: utils.generateUUID().slice(0, 10), + password: utils.generateUUID(), + }); + token = await helpers.getCsrfToken(jar); + }); + + it('should terminate the session and send user back to index if interstitials remain', async () => { + const res = await requestAsync(`${nconf.get('url')}/register/abort`, { + method: 'post', + jar, + json: true, + followRedirect: false, + simple: false, + resolveWithFullResponse: true, + headers: { + 'x-csrf-token': token, + }, + }); + + assert.strictEqual(res.statusCode, 302); + assert.strictEqual(res.headers['set-cookie'][0], 'express.sid=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax'); + assert.strictEqual(res.headers.location, `${nconf.get('relative_path')}/`); + }); + + it('should preserve the session and send user back to user profile if no interstitials remain (e.g. GDPR OK + email change cancellation)', async () => { + // Submit GDPR consent + await requestAsync(`${nconf.get('url')}/register/complete`, { + method: 'post', + jar, + json: true, + followRedirect: false, + simple: false, + resolveWithFullResponse: true, + headers: { + 'x-csrf-token': token, + }, + form: { + gdpr_agree_data: 'on', + gdpr_agree_email: 'on', + }, + }); + + // Start email change flow + await requestAsync(`${nconf.get('url')}/me/edit/email`, { jar }); + + const res = await requestAsync(`${nconf.get('url')}/register/abort`, { + method: 'post', + jar, + json: true, + followRedirect: false, + simple: false, + resolveWithFullResponse: true, + headers: { + 'x-csrf-token': token, + }, + }); + + assert.strictEqual(res.statusCode, 302); + assert(res.headers.location.match(/\/uid\/\d+$/)); + }); + }); }); it('should load /robots.txt', (done) => {