Skip to content

Commit

Permalink
feat(users): delete inactive users with expired tokens on create/update
Browse files Browse the repository at this point in the history
When creating or updating a user, the system checks for existing users with the same username or
email. If such users exist but have never activated their accounts and their tokens are expired,
they are deleted automatically. This avoids conflicts and frees up old, unused usernames and emails.
  • Loading branch information
Rafatcb committed Jan 13, 2025
1 parent 6d43977 commit 5d2a0cf
Show file tree
Hide file tree
Showing 4 changed files with 536 additions and 7 deletions.
44 changes: 37 additions & 7 deletions models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ async function findOneById(userId, options = {}) {
async function create(postedUserData) {
const validUserData = validatePostSchema(postedUserData);
await validateUniqueUser(validUserData);
await deleteExpiredUsers(validUserData);
await hashPasswordInObject(validUserData);

validUserData.features = ['read:activation_token'];
Expand Down Expand Up @@ -295,6 +296,7 @@ async function update(targetUser, postedUserData, options = {}) {
if (shouldValidateUsername || shouldValidateEmail) {
try {
await validateUniqueUser({ ...validPostedUserData, id: currentUser.id }, { transaction: options.transaction });
await deleteExpiredUsers(validPostedUserData);
if (shouldValidateEmail && !options.skipEmailConfirmation) {
await emailConfirmation.createAndSendEmail(currentUser, validPostedUserData.email, {
transaction: options.transaction,
Expand Down Expand Up @@ -375,30 +377,36 @@ 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
${where}
(
COALESCE(aat.used, true)
OR aat.expires_at > NOW()
OR 'nuked' = ANY(u.features)
)
AND ${where}
`,
values: queryValues,
};
Expand Down Expand Up @@ -428,6 +436,28 @@ async function validateUniqueUser(userData, options) {
});
}

async function deleteExpiredUsers(userObject) {
const query = {
text: `
DELETE FROM
users u
USING activate_account_tokens aat
WHERE
aat.user_id = u.id
AND COALESCE(aat.used, false) = false
AND aat.expires_at < NOW()
AND NOT ('nuked' = ANY(u.features))
AND (
LOWER(username) = LOWER($2)
OR LOWER(email) = LOWER($1)
)
;`,
values: [userObject.email, userObject.username],
};

await database.query(query);
}

async function hashPasswordInObject(userObject) {
userObject.password = await authentication.hashPassword(userObject.password);
return userObject;
Expand Down
83 changes: 83 additions & 0 deletions tests/integration/api/v1/users/[username]/patch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,89 @@ describe('PATCH /api/v1/users/[username]', () => {
expect(foundUser.email).toBe(defaultUser.email);
expect(foundUser.updated_at).toStrictEqual(defaultUser.updated_at);
});

test('Patching itself with a duplicate "username" for an inactive user with expired activation token', async () => {
const inactiveUser = await orchestrator.createUser({ username: 'existentInactiveUser' });
await orchestrator.updateActivateAccountTokenByUserId(inactiveUser.id, {
expires_at: new Date(Date.now() - 1000),
});

const usersRequestBuilder = new RequestBuilder('/api/v1/users/');
const defaultUser = await usersRequestBuilder.buildUser();

const { response, responseBody } = await usersRequestBuilder.patch(defaultUser.username, {
username: 'existentInactiveUser',
});
expect.soft(response.status).toBe(200);

expect(responseBody).toStrictEqual({
id: defaultUser.id,
username: 'existentInactiveUser',
email: defaultUser.email,
description: defaultUser.description,
features: defaultUser.features,
notifications: true,
tabcoins: 0,
tabcash: 0,
created_at: defaultUser.created_at.toISOString(),
updated_at: responseBody.updated_at,
});

expect(responseBody.updated_at).not.toBe(defaultUser.updated_at.toISOString());
expect(Date.parse(responseBody.updated_at)).not.toBeNaN();

await expect(user.findOneById(inactiveUser.id)).rejects.toThrow(
`O id "${inactiveUser.id}" não foi encontrado no sistema.`,
);
});

test('Patching itself with "username" and "email" duplicated from different users, both inactive with expired tokens', async () => {
const firstInactiveUser = await orchestrator.createUser({ username: 'firstInactiveUser' });
const secondInactiveUser = await orchestrator.createUser({ email: '[email protected]' });
await orchestrator.updateActivateAccountTokenByUserId(firstInactiveUser.id, {
expires_at: new Date(Date.now() - 1000),
});
await orchestrator.updateActivateAccountTokenByUserId(secondInactiveUser.id, {
expires_at: new Date(Date.now() - 1000),
});

const usersRequestBuilder = new RequestBuilder('/api/v1/users/');
const defaultUser = await usersRequestBuilder.buildUser();

const { response, responseBody } = await usersRequestBuilder.patch(defaultUser.username, {
username: 'firstInactiveUser',
email: '[email protected]',
});
expect.soft(response.status).toBe(200);

expect(responseBody).toStrictEqual({
id: defaultUser.id,
username: 'firstInactiveUser',
email: defaultUser.email,
description: defaultUser.description,
features: defaultUser.features,
notifications: true,
tabcoins: 0,
tabcash: 0,
created_at: defaultUser.created_at.toISOString(),
updated_at: responseBody.updated_at,
});

expect(responseBody.updated_at).not.toBe(defaultUser.updated_at.toISOString());
expect(Date.parse(responseBody.updated_at)).not.toBeNaN();

const confirmationEmail = await orchestrator.getLastEmail();

expect(confirmationEmail.recipients).toStrictEqual(['<[email protected]>']);
expect(confirmationEmail.subject).toBe('Confirme seu novo email');

await expect(user.findOneById(firstInactiveUser.id)).rejects.toThrow(
`O id "${firstInactiveUser.id}" não foi encontrado no sistema.`,
);
await expect(user.findOneById(secondInactiveUser.id)).rejects.toThrow(
`O id "${secondInactiveUser.id}" não foi encontrado no sistema.`,
);
});
});

describe('TEMPORARY BEHAVIOR', () => {
Expand Down
Loading

0 comments on commit 5d2a0cf

Please sign in to comment.