From b28e2b75b74c1ae8d17cb0a71579d56b86fbf1ac Mon Sep 17 00:00:00 2001 From: Alice Gaudon Date: Sun, 15 Nov 2020 12:20:11 +0100 Subject: [PATCH] Authentication: Improve registration tests and fix register/login overlap --- src/auth/AuthController.ts | 28 +++---- test/Authentication.test.ts | 152 ++++++++++++++++++++++++++---------- 2 files changed, 125 insertions(+), 55 deletions(-) diff --git a/src/auth/AuthController.ts b/src/auth/AuthController.ts index e66698e..52bf6f1 100644 --- a/src/auth/AuthController.ts +++ b/src/auth/AuthController.ts @@ -6,7 +6,6 @@ import ModelFactory from "../db/ModelFactory"; import User from "./models/User"; import UserPasswordComponent from "./password/UserPasswordComponent"; import UserNameComponent from "./models/UserNameComponent"; -import {log} from "../Logger"; export default class AuthController extends Controller { public getRoutesPrefix(): string { @@ -48,6 +47,11 @@ export default class AuthController extends Controller { } protected async handleAuth(req: Request, res: Response, isRegistration: boolean): Promise { + if (isRegistration && !req.body.auth_method) { + throw new BadRequestError('Cannot register without specifying desired auth_method.', + 'Please specify auth_method.', req.url); + } + const authGuard = this.getApp().as(AuthComponent).getAuthGuard(); const identifier = req.body.identifier; @@ -61,28 +65,24 @@ export default class AuthController extends Controller { 'Available methods are: ' + authGuard.getAuthMethodNames(), req.url); } - const user = await method.findUserByIdentifier(identifier); - if (!user) { // Register - if (!isRegistration) return await this.redirectToRegistration(req, res, identifier); + // Register + if (isRegistration) return await method.attemptRegister(req, res, identifier); - await method.attemptRegister(req, res, identifier); - return; - } + const user = await method.findUserByIdentifier(identifier); + + // Redirect to registration if user not found + if (!user) return await this.redirectToRegistration(req, res, identifier); // Login return await method.attemptLogin(req, res, user); } - const methods = await authGuard.getAuthMethodsByIdentifier(identifier); - if (methods.length === 0) { // Register - if (!isRegistration) return await this.redirectToRegistration(req, res, identifier); - - await authGuard.getRegistrationMethod().attemptRegister(req, res, identifier); - return; - } + // Redirect to registration if user not found + if (methods.length === 0) return await this.redirectToRegistration(req, res, identifier); + // Login const {user, method} = methods[0]; return await method.attemptLogin(req, res, user); } diff --git a/test/Authentication.test.ts b/test/Authentication.test.ts index 92a2465..d48f7e1 100644 --- a/test/Authentication.test.ts +++ b/test/Authentication.test.ts @@ -5,7 +5,6 @@ import supertest from "supertest"; import CsrfProtectionComponent from "../src/components/CsrfProtectionComponent"; import MysqlConnectionManager from "../src/db/MysqlConnectionManager"; import config from "config"; -import {log} from "../src/Logger"; import User from "../src/auth/models/User"; import UserNameComponent from "../src/auth/models/UserNameComponent"; import UserPasswordComponent from "../src/auth/password/UserPasswordComponent"; @@ -37,17 +36,20 @@ useApp(async (addr, port) => { let agent: supertest.SuperTest; -describe('Authentication system', () => { - test('Obtain session cookies', async () => { - agent = supertest(app.getExpressApp()); - }); +beforeAll(() => { + agent = supertest(app.getExpressApp()); +}); - test('Register with email with username', async () => { +describe('Register with username', () => { + let cookies: string[]; + let csrf: string; + + test('General case', async () => { const res = await agent.get('/csrf').expect(200); - const cookies = res.get('Set-Cookie'); - const csrf = res.text; + cookies = res.get('Set-Cookie'); + csrf = res.text; - expect(cookies).toBeDefined(); + // Register user await agent.post('/auth/register') .set('Cookie', cookies) .send({ @@ -72,52 +74,120 @@ describe('Authentication system', () => { await expect(user?.as(UserPasswordComponent).verifyPassword('darla_is_cute')).resolves.toStrictEqual(true); }); - test('Register with email with email (magic_link)', async () => { - let res = await agent.get('/csrf').expect(200); - const cookies = res.get('Set-Cookie'); - const csrf = res.text; - - expect(cookies).toBeDefined(); - res = await agent.post('/auth/register') + test('Can\'t register when logged in', async () => { + await agent.post('/auth/register') .set('Cookie', cookies) .send({ csrf: csrf, - auth_method: 'magic_link', - identifier: 'glimmer@example.org', - name: 'glimmer', + auth_method: 'password', + identifier: 'entrapta2', + password: 'darla_is_cute', + password_confirmation: 'darla_is_cute', + terms: 'on', }) .expect(302) - .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf'); + .expect('Location', '/csrf'); - const mail: Record | null = await popEmail(); - expect(mail).not.toBeNull(); + const user2 = await User.select() + .where('name', 'entrapta2') + .first(); + expect(user2).toBeNull(); + }); - const query = (mail?.text as string).split('/magic/link?')[1].split('\n')[0]; - expect(query).toBeDefined(); + test('Can\'t register taken username', async () => { + // Check that there is no hordak in DB + expect(await User.select() + .where('name', 'hordak') + .count()).toStrictEqual(0); - // .expect('Location', '/'); - res = await agent.get('/magic/link?' + query) - .expect(200); - res = await agent.get('/magic/lobby') - .set('Cookie', cookies) + const res1 = await agent.get('/csrf').expect(200); + + // Register user + await agent.post('/auth/register') + .set('Cookie', res1.get('Set-Cookie')) + .send({ + csrf: res1.text, + auth_method: 'password', + identifier: 'hordak', + password: 'horde_prime_will_rise', + password_confirmation: 'horde_prime_will_rise', + terms: 'on', + }) .expect(302) .expect('Location', '/'); - log.debug(res.status, res.headers, res.body, res.text); - // Verify saved user - const user = await User.select() - .with('mainEmail') - .where('name', 'glimmer') - .first(); + expect(await User.select() + .where('name', 'hordak') + .count()).toStrictEqual(1); - expect(user).toBeDefined(); - const email = user?.mainEmail.getOrFail(); - expect(email).toBeDefined(); - expect(email?.email).toStrictEqual('glimmer@example.org'); + const res2 = await agent.get('/csrf').expect(200); - expect(user?.as(UserNameComponent).name).toStrictEqual('glimmer'); - await expect(user?.as(UserPasswordComponent).verifyPassword('')).resolves.toStrictEqual(false); + // Attempt register same user + const res = await agent.post('/auth/register') + .set('Cookie', res2.get('Set-Cookie')) + .send({ + csrf: res2.text, + auth_method: 'password', + identifier: 'hordak', + password: 'horde_prime_will_rise', + password_confirmation: 'horde_prime_will_rise', + terms: 'on', + }) + .expect(401); + // username field should be translated from identifier + expect(res.body.messages?.username?.name).toStrictEqual('AlreadyExistsValidationError'); + + // Verify nothing changed + expect(await User.select() + .where('name', 'hordak') + .count()).toStrictEqual(1); }); }); + +test('Register with email (magic_link)', async () => { + const res = await agent.get('/csrf').expect(200); + const cookies = res.get('Set-Cookie'); + const csrf = res.text; + + expect(cookies).toBeDefined(); + await agent.post('/auth/register') + .set('Cookie', cookies) + .send({ + csrf: csrf, + auth_method: 'magic_link', + identifier: 'glimmer@example.org', + name: 'glimmer', + }) + .expect(302) + .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf'); + + const mail: Record | null = await popEmail(); + expect(mail).not.toBeNull(); + + const query = (mail?.text as string).split('/magic/link?')[1].split('\n')[0]; + expect(query).toBeDefined(); + + await agent.get('/magic/link?' + query) + .expect(200); + await agent.get('/magic/lobby') + .set('Cookie', cookies) + .expect(302) + .expect('Location', '/'); + + // Verify saved user + const user = await User.select() + .with('mainEmail') + .where('name', 'glimmer') + .first(); + + expect(user).toBeDefined(); + + const email = user?.mainEmail.getOrFail(); + expect(email).toBeDefined(); + expect(email?.email).toStrictEqual('glimmer@example.org'); + + expect(user?.as(UserNameComponent).name).toStrictEqual('glimmer'); + await expect(user?.as(UserPasswordComponent).verifyPassword('')).resolves.toStrictEqual(false); +});