Authentication: Improve registration tests and fix register/login overlap

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<void> {
+        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);
+            // Register
-            if (!user) { // Register
+            if (isRegistration) return await method.attemptRegister(req, res, identifier);
-                if (!isRegistration) return await this.redirectToRegistration(req, res, identifier);
 
-                await method.attemptRegister(req, res, identifier);
+            const user = await method.findUserByIdentifier(identifier);
-                return;
+
-            }
+            // 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
+        // Redirect to registration if user not found
-            if (!isRegistration) return await this.redirectToRegistration(req, res, identifier);
+        if (methods.length === 0) return await this.redirectToRegistration(req, res, identifier);
-
-            await authGuard.getRegistrationMethod().attemptRegister(req, res, identifier);
-            return;
-        }
 
+        // Login
         const {user, method} = methods[0];
         return await method.attemptLogin(req, res, user);
     }

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<supertest.Test>;
 
-describe('Authentication system', () => {
+beforeAll(() => {
-    test('Obtain session cookies', async () => {
+    agent = supertest(app.getExpressApp());
-        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');
+        cookies = res.get('Set-Cookie');
-        const csrf = res.text;
+        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 () => {
+    test('Can\'t register when logged in', async () => {
-        let res = await agent.get('/csrf').expect(200);
+        await agent.post('/auth/register')
-        const cookies = res.get('Set-Cookie');
-        const csrf = res.text;
-
-        expect(cookies).toBeDefined();
-        res = await agent.post('/auth/register')
             .set('Cookie', cookies)
             .send({
                 csrf: csrf,
-                auth_method: 'magic_link',
+                auth_method: 'password',
-                identifier: 'glimmer@example.org',
+                identifier: 'entrapta2',
-                name: 'glimmer',
+                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<string, unknown> | null = await popEmail();
+        const user2 = await User.select()
-        expect(mail).not.toBeNull();
+            .where('name', 'entrapta2')
+            .first();
+        expect(user2).toBeNull();
+    });
 
-        const query = (mail?.text as string).split('/magic/link?')[1].split('\n')[0];
+    test('Can\'t register taken username', async () => {
-        expect(query).toBeDefined();
+        // Check that there is no hordak in DB
+        expect(await User.select()
+            .where('name', 'hordak')
+            .count()).toStrictEqual(0);
 
-        // .expect('Location', '/');
+        const res1 = await agent.get('/csrf').expect(200);
-        res = await agent.get('/magic/link?' + query)
+
-            .expect(200);
+        // Register user
-        res = await agent.get('/magic/lobby')
+        await agent.post('/auth/register')
-            .set('Cookie', cookies)
+            .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()
+        expect(await User.select()
-            .with('mainEmail')
+            .where('name', 'hordak')
-            .where('name', 'glimmer')
+            .count()).toStrictEqual(1);
-            .first();
 
-        expect(user).toBeDefined();
 
-        const email = user?.mainEmail.getOrFail();
+        const res2 = await agent.get('/csrf').expect(200);
-        expect(email).toBeDefined();
-        expect(email?.email).toStrictEqual('glimmer@example.org');
 
-        expect(user?.as(UserNameComponent).name).toStrictEqual('glimmer');
+        // Attempt register same user
-        await expect(user?.as(UserPasswordComponent).verifyPassword('')).resolves.toStrictEqual(false);
+        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<string, unknown> | 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);
+});