From 772a72b2fde0a67f3302bd179719667c7d2f0303 Mon Sep 17 00:00:00 2001 From: Rafael Tavares <26308880+Rafatcb@users.noreply.github.com> Date: Tue, 3 Sep 2024 21:53:45 -0300 Subject: [PATCH] fix(user): release username and email after activation account token expires There is an extra safeguard in `ON CONFLICT` SQL to prevent duplicate users from being created if this has not been validated before. --- models/user.js | 97 ++++++--- tests/integration/api/v1/users/post.test.js | 214 ++++++++++++++++++++ tests/orchestrator.js | 32 +++ 3 files changed, 317 insertions(+), 26 deletions(-) diff --git a/models/user.js b/models/user.js index 85ead5ae3..03eb853f2 100644 --- a/models/user.js +++ b/models/user.js @@ -234,30 +234,69 @@ async function create(postedUserData) { validUserData.features = ['read:activation_token']; - const newUser = await runInsertQuery(validUserData); - return newUser; + const query = getInsertQuery(validUserData, 'LOWER(username)'); + + let results; + try { + results = await database.query(query); + } catch (error) { + if (error.databaseErrorCode !== database.errorCodes.UNIQUE_CONSTRAINT_VIOLATION) { + throw error; + } - async function runInsertQuery(validUserData) { - const query = { - text: ` - INSERT INTO - users (username, email, password, features) - VALUES - ($1, $2, $3, $4) - RETURNING - * - ;`, - values: [validUserData.username, validUserData.email, validUserData.password, validUserData.features], - }; + const query = getInsertQuery(validUserData, 'email'); + results = await database.query(query); + } - const results = await database.query(query); - const newUser = results.rows[0]; + const newUser = results.rows[0]; - newUser.tabcoins = 0; - newUser.tabcash = 0; + newUser.tabcoins = 0; + newUser.tabcash = 0; - return newUser; - } + return newUser; +} + +function getInsertQuery(validUserData, constraint) { + return { + text: ` + INSERT INTO + users (username, email, password, features) + VALUES + ($1, $2, $3, $4) + ON CONFLICT (${constraint}) DO + UPDATE SET + username = $1, + email = $2, + password = $3, + features = $4, + created_at = NOW(), + updated_at = NOW(), + rewarded_at = NOW() + WHERE + ( + SELECT + COUNT(*) + FROM + users u + LEFT JOIN + activate_account_tokens aat ON u.id = aat.user_id + WHERE + ( + LOWER(u.username) = LOWER($1) OR + LOWER(u.email) = LOWER($2) + ) + AND + ( + COALESCE(aat.used, true) OR + aat.expires_at > NOW() OR + 'nuked' = ANY(u.features) + ) + ) = 0 + RETURNING + * + ;`, + values: [validUserData.username, validUserData.email, validUserData.password, validUserData.features], + }; } function createAnonymous() { @@ -375,29 +414,35 @@ async function validateUniqueUser(userData, options) { if (userData.username) { queryValues.push(userData.username); - orConditions.push(`LOWER(username) = LOWER($${queryValues.length})`); + orConditions.push(`LOWER(u.username) = LOWER($${queryValues.length})`); } if (userData.email) { queryValues.push(userData.email); - orConditions.push(`LOWER(email) = LOWER($${queryValues.length})`); + orConditions.push(`LOWER(u.email) = LOWER($${queryValues.length})`); } let where = `(${orConditions.join(' OR ')})`; if (userData.id) { queryValues.push(userData.id); - where += ` AND id <> $${queryValues.length}`; + where += ` AND u.id <> $${queryValues.length}`; } const query = { text: ` SELECT - username, - email + u.username, + u.email FROM - users + users u + LEFT JOIN activate_account_tokens aat ON user_id = u.id WHERE + ( + COALESCE(aat.used, true) OR + aat.expires_at > NOW() OR + 'nuked' = ANY(u.features) + ) AND ${where} `, values: queryValues, diff --git a/tests/integration/api/v1/users/post.test.js b/tests/integration/api/v1/users/post.test.js index e31740e1a..4a980cd84 100644 --- a/tests/integration/api/v1/users/post.test.js +++ b/tests/integration/api/v1/users/post.test.js @@ -3,6 +3,7 @@ import { version as uuidVersion } from 'uuid'; import password from 'models/password.js'; import user from 'models/user.js'; import orchestrator from 'tests/orchestrator.js'; +import RequestBuilder from 'tests/request-builder'; beforeAll(async () => { await orchestrator.waitForAllServices(); @@ -11,6 +12,8 @@ beforeAll(async () => { }); describe('POST /api/v1/users', () => { + const usersRequestBuilder = new RequestBuilder('/api/v1/users'); + describe('Anonymous user', () => { test('With unique and valid data', async () => { const response = await fetch(`${orchestrator.webserverUrl}/api/v1/users`, { @@ -208,6 +211,43 @@ describe('POST /api/v1/users', () => { expect(secondResponseBody.key).toBe('username'); }); + test('With "username" and "email" duplicated', async () => { + const { response: firstResponse, responseBody: firstResponseBody } = await usersRequestBuilder.post({ + username: 'SaMeUsErNaMe', + email: 'SaMeEmAiL@example.com', + password: 'validpassword', + }); + + expect.soft(firstResponse.status).toBe(201); + + await orchestrator.nukeUser(firstResponseBody); + + const activateAccountToken = await orchestrator.getLastActivateAccountToken(); + await orchestrator.updateActivateAccountToken(activateAccountToken.id, { + expires_at: new Date(Date.now() - 1000), + }); + + const { response: secondResponse, responseBody: secondResponseBody } = await usersRequestBuilder.post({ + username: 'sameUsername', + email: 'sameEmail@example.com', + password: 'new-password', + }); + + expect.soft(secondResponse.status).toBe(400); + expect(secondResponseBody).toStrictEqual({ + status_code: 400, + name: 'ValidationError', + message: 'O "username" informado já está sendo usado.', + action: 'Ajuste os dados enviados e tente novamente.', + error_location_code: 'MODEL:USER:VALIDATE_UNIQUE_USERNAME:ALREADY_EXISTS', + key: 'username', + error_id: secondResponseBody.error_id, + request_id: secondResponseBody.request_id, + }); + expect(uuidVersion(secondResponseBody.error_id)).toBe(4); + expect(uuidVersion(secondResponseBody.request_id)).toBe(4); + }); + test('With "username" missing', async () => { const response = await fetch(`${orchestrator.webserverUrl}/api/v1/users`, { method: 'POST', @@ -762,4 +802,178 @@ describe('POST /api/v1/users', () => { expect(responseBody.key).toBe('object'); }); }); + + test('With a duplicate "username" for a user with expired activation token', async () => { + const { response: firstResponse, responseBody: firstResponseBody } = await usersRequestBuilder.post({ + username: 'ARepeatedUsername', + email: 'a-repeated-username-1@gmail.com', + password: 'validpassword', + }); + + expect.soft(firstResponse.status).toBe(201); + + const activateAccountToken = await orchestrator.getLastActivateAccountToken(); + await orchestrator.updateActivateAccountToken(activateAccountToken.id, { + expires_at: new Date(Date.now() - 1000), + }); + + const { response: secondResponse, responseBody: secondResponseBody } = await usersRequestBuilder.post({ + username: 'ARepeatedUSERNAME', + email: 'a-repeated-username-2@example.com', + password: 'new-password', + }); + + expect.soft(secondResponse.status).toBe(201); + expect(secondResponseBody).toStrictEqual({ + id: firstResponseBody.id, + username: 'ARepeatedUSERNAME', + description: '', + features: ['read:activation_token'], + tabcoins: 0, + tabcash: 0, + created_at: secondResponseBody.created_at, + updated_at: secondResponseBody.updated_at, + }); + + expect(new Date(secondResponseBody.created_at).getTime()).toBeGreaterThan( + new Date(firstResponseBody.created_at).getTime(), + ); + expect(new Date(secondResponseBody.updated_at).getTime()).toBeGreaterThan( + new Date(firstResponseBody.updated_at).getTime(), + ); + + const userInDatabase = await user.findOneByUsername('ARepeatedUSERNAME'); + const passwordsMatch = await password.compare('new-password', userInDatabase.password); + + expect(passwordsMatch).toBe(true); + expect(userInDatabase.email).toBe('a-repeated-username-2@example.com'); + }); + + test('With a duplicate "email" for a user with expired activation token', async () => { + const { response: firstResponse, responseBody: firstResponseBody } = await usersRequestBuilder.post({ + username: 'ARepeatedEmail', + email: 'a-repeated-email@gmail.com', + password: 'validpassword', + }); + + expect.soft(firstResponse.status).toBe(201); + + const activateAccountToken = await orchestrator.getLastActivateAccountToken(); + await orchestrator.updateActivateAccountToken(activateAccountToken.id, { + expires_at: new Date(Date.now() - 1000), + }); + + const { response: secondResponse, responseBody: secondResponseBody } = await usersRequestBuilder.post({ + username: 'ARepeatedEmail2', + email: 'A-Repeated-Email@gmail.com', + password: 'new-password', + }); + + expect.soft(secondResponse.status).toBe(201); + expect(secondResponseBody).toStrictEqual({ + id: firstResponseBody.id, + username: 'ARepeatedEmail2', + description: '', + features: ['read:activation_token'], + tabcoins: 0, + tabcash: 0, + created_at: secondResponseBody.created_at, + updated_at: secondResponseBody.updated_at, + }); + + expect(new Date(secondResponseBody.created_at).getTime()).toBeGreaterThan( + new Date(firstResponseBody.created_at).getTime(), + ); + expect(new Date(secondResponseBody.updated_at).getTime()).toBeGreaterThan( + new Date(firstResponseBody.updated_at).getTime(), + ); + + const userInDatabase = await user.findOneByUsername('ARepeatedEmail2'); + const passwordsMatch = await password.compare('new-password', userInDatabase.password); + + expect(passwordsMatch).toBe(true); + expect(userInDatabase.email).toBe('a-repeated-email@gmail.com'); + }); + + test('With a duplicate "username" for a nuked user with expired activation token', async () => { + const { response: firstResponse, responseBody: firstResponseBody } = await usersRequestBuilder.post({ + username: 'NukedSameUsername', + email: 'nuked-same-username-1@gmail.com', + password: 'validpassword', + }); + + expect.soft(firstResponse.status).toBe(201); + + await orchestrator.nukeUser(firstResponseBody); + + const activateAccountToken = await orchestrator.getLastActivateAccountToken(); + await orchestrator.updateActivateAccountToken(activateAccountToken.id, { + expires_at: new Date(Date.now() - 1000), + }); + + const { response: secondResponse, responseBody: secondResponseBody } = await usersRequestBuilder.post({ + username: 'NukedSameUsername', + email: 'nuked-same-username-2@example.com', + password: 'new-password', + }); + + expect.soft(secondResponse.status).toBe(400); + expect(secondResponseBody).toStrictEqual({ + status_code: 400, + name: 'ValidationError', + message: 'O "username" informado já está sendo usado.', + action: 'Ajuste os dados enviados e tente novamente.', + error_location_code: 'MODEL:USER:VALIDATE_UNIQUE_USERNAME:ALREADY_EXISTS', + key: 'username', + error_id: secondResponseBody.error_id, + request_id: secondResponseBody.request_id, + }); + expect(uuidVersion(secondResponseBody.error_id)).toBe(4); + expect(uuidVersion(secondResponseBody.request_id)).toBe(4); + }); + + test('With "username" and "email" duplicated from different users, one with expired token and the other nuked', async () => { + const { response: firstResponse, responseBody: firstResponseBody } = await usersRequestBuilder.post({ + username: 'firstUserNuked', + email: 'first-user@example.com', + password: 'validpassword', + }); + + expect.soft(firstResponse.status).toBe(201); + + await orchestrator.nukeUser(firstResponseBody); + + const { response: secondResponse } = await usersRequestBuilder.post({ + username: 'secondUserExpiredToken', + email: 'second-user@example.com', + password: 'validpassword', + }); + + expect.soft(secondResponse.status).toBe(201); + + const activateAccountToken = await orchestrator.getLastActivateAccountToken(); + await orchestrator.updateActivateAccountToken(activateAccountToken.id, { + expires_at: new Date(Date.now() - 1000), + }); + + const { response: thirdResponse, responseBody: thirdResponseBody } = await usersRequestBuilder.post({ + username: 'firstUserNuked', + email: 'second-user@example.com', + password: 'new-password', + }); + + expect.soft(thirdResponse.status).toBe(400); + expect(thirdResponseBody).toStrictEqual({ + status_code: 400, + name: 'ValidationError', + message: 'O "username" informado já está sendo usado.', + action: 'Ajuste os dados enviados e tente novamente.', + error_location_code: 'MODEL:USER:VALIDATE_UNIQUE_USERNAME:ALREADY_EXISTS', + key: 'username', + error_id: thirdResponseBody.error_id, + request_id: thirdResponseBody.request_id, + }); + expect(uuidVersion(thirdResponseBody.error_id)).toBe(4); + expect(uuidVersion(thirdResponseBody.request_id)).toBe(4); + }); }); diff --git a/tests/orchestrator.js b/tests/orchestrator.js index 7d29461bb..1ad12400a 100644 --- a/tests/orchestrator.js +++ b/tests/orchestrator.js @@ -10,6 +10,7 @@ import migrator from 'infra/migrator.js'; import webserver from 'infra/webserver.js'; import activation from 'models/activation.js'; import balance from 'models/balance.js'; +import ban from 'models/ban'; import content from 'models/content.js'; import event from 'models/event.js'; import recovery from 'models/recovery.js'; @@ -188,6 +189,10 @@ async function activateUser(userObject) { return await activation.activateUserByUserId(userObject.id); } +async function nukeUser(userObject) { + return await ban.nuke(userObject.id); +} + async function createSession(userObject) { return await session.create(userObject.id); } @@ -298,6 +303,30 @@ async function createRecoveryToken(userObject) { return await recovery.create(userObject); } +async function getLastActivateAccountToken() { + const results = await database.query('SELECT * FROM activate_account_tokens ORDER BY created_at DESC LIMIT 1;'); + return results.rows[0]; +} + +async function updateActivateAccountToken(tokenId, tokenBody) { + const query = { + text: ` + UPDATE + activate_account_tokens + SET + expires_at = $2 + WHERE + id = $1 + RETURNING + * + ;`, + values: [tokenId, tokenBody.expires_at], + }; + + const results = await database.query(query); + return results.rows[0]; +} + async function updateEmailConfirmationToken(tokenId, tokenBody) { const query = { text: ` @@ -469,11 +498,14 @@ const orchestrator = { dropAllTables, findSessionByToken, getEmails, + getLastActivateAccountToken, getLastEmail, getLastEvent, + nukeUser, parseSetCookies, removeFeaturesFromUser, runPendingMigrations, + updateActivateAccountToken, updateContent, updateEmailConfirmationToken, updateEventCreatedAt,