Allow users to change their username every configurable period of time

Closes #22
This commit is contained in:
Alice Gaudon 2021-02-23 17:42:25 +01:00
parent dcfb0b8f1c
commit caae753d74
13 changed files with 209 additions and 14 deletions

View File

@ -50,5 +50,8 @@
magic_link: { magic_link: {
validity_period: 20, validity_period: 20,
}, },
approval_mode: false, auth: {
approval_mode: false, // Registered accounts need to be approved by an administrator
name_change_wait_period: 2592000000, // 30 days
},
} }

View File

@ -31,6 +31,7 @@ import MakeMagicLinksSessionNotUniqueMigration from "./auth/magic_link/MakeMagic
import AddUsedToMagicLinksMigration from "./auth/magic_link/AddUsedToMagicLinksMigration"; import AddUsedToMagicLinksMigration from "./auth/magic_link/AddUsedToMagicLinksMigration";
import packageJson = require('./package.json'); import packageJson = require('./package.json');
import PreviousUrlComponent from "./components/PreviousUrlComponent"; import PreviousUrlComponent from "./components/PreviousUrlComponent";
import AddNameChangedAtToUsersMigration from "./auth/migrations/AddNameChangedAtToUsersMigration";
export const MIGRATIONS = [ export const MIGRATIONS = [
CreateMigrationsTable, CreateMigrationsTable,
@ -40,6 +41,7 @@ export const MIGRATIONS = [
AddNameToUsersMigration, AddNameToUsersMigration,
MakeMagicLinksSessionNotUniqueMigration, MakeMagicLinksSessionNotUniqueMigration,
AddUsedToMagicLinksMigration, AddUsedToMagicLinksMigration,
AddNameChangedAtToUsersMigration,
]; ];
export default class TestApp extends Application { export default class TestApp extends Application {

View File

@ -12,6 +12,8 @@ import MagicLinkController from "./magic_link/MagicLinkController";
import {MailTemplate} from "../mail/Mail"; import {MailTemplate} from "../mail/Mail";
import {ADD_EMAIL_MAIL_TEMPLATE, REMOVE_PASSWORD_MAIL_TEMPLATE} from "../Mails"; import {ADD_EMAIL_MAIL_TEMPLATE, REMOVE_PASSWORD_MAIL_TEMPLATE} from "../Mails";
import AuthMagicLinkActionType from "./magic_link/AuthMagicLinkActionType"; import AuthMagicLinkActionType from "./magic_link/AuthMagicLinkActionType";
import UserNameComponent from "./models/UserNameComponent";
import Time from "../Time";
export default class AccountController extends Controller { export default class AccountController extends Controller {
@ -29,6 +31,10 @@ export default class AccountController extends Controller {
public routes(): void { public routes(): void {
this.get('/', this.getAccount, 'account', RequireAuthMiddleware); this.get('/', this.getAccount, 'account', RequireAuthMiddleware);
if (ModelFactory.get(User).hasComponent(UserNameComponent)) {
this.post('/change-name', this.postChangeName, 'change-name', RequireAuthMiddleware);
}
if (ModelFactory.get(User).hasComponent(UserPasswordComponent)) { if (ModelFactory.get(User).hasComponent(UserPasswordComponent)) {
this.post('/change-password', this.postChangePassword, 'change-password', RequireAuthMiddleware); this.post('/change-password', this.postChangePassword, 'change-password', RequireAuthMiddleware);
this.post('/remove-password', this.postRemovePassword, 'remove-password', RequireAuthMiddleware); this.post('/remove-password', this.postRemovePassword, 'remove-password', RequireAuthMiddleware);
@ -43,15 +49,47 @@ export default class AccountController extends Controller {
const user = req.as(RequireAuthMiddleware).getUser(); const user = req.as(RequireAuthMiddleware).getUser();
const passwordComponent = user.asOptional(UserPasswordComponent); const passwordComponent = user.asOptional(UserPasswordComponent);
res.render('auth/account', {
const nameComponent = user.asOptional(UserNameComponent);
const nameChangeWaitPeriod = config.get<number>('auth.name_change_wait_period');
const nameChangedAt = nameComponent?.getNameChangedAt()?.getTime() || Date.now();
const nameChangeRemainingTime = new Date(nameChangedAt + nameChangeWaitPeriod);
res.render('auth/account/account', {
main_email: await user.mainEmail.get(), main_email: await user.mainEmail.get(),
emails: await user.emails.get(), emails: await user.emails.get(),
display_email_warning: config.get('app.display_email_warning'), display_email_warning: config.get('app.display_email_warning'),
has_password_component: !!passwordComponent, has_password_component: !!passwordComponent,
has_password: passwordComponent?.hasPassword(), has_password: passwordComponent?.hasPassword(),
has_name_component: !!nameComponent,
name_change_wait_period: Time.humanizeDuration(nameChangeWaitPeriod, false, true),
can_change_name: nameComponent?.canChangeName(),
can_change_name_in: Time.humanizeTimeTo(nameChangeRemainingTime),
}); });
} }
protected async postChangeName(req: Request, res: Response): Promise<void> {
await Validator.validate({
'name': new Validator().defined(),
}, req.body);
const user = req.as(RequireAuthMiddleware).getUser();
const userNameComponent = user.as(UserNameComponent);
if (!userNameComponent.setName(req.body.name)) {
const nameChangedAt = userNameComponent.getNameChangedAt()?.getTime() || Date.now();
const nameChangeWaitPeriod = config.get<number>('auth.name_change_wait_period');
req.flash('error', `Your can't change your name until ${new Date(nameChangedAt + nameChangeWaitPeriod)}.`);
res.redirect(Controller.route('account'));
return;
}
await user.save();
req.flash('success', `Your name was successfully changed to ${req.body.name}.`);
res.redirect(Controller.route('account'));
}
protected async postChangePassword(req: Request, res: Response): Promise<void> { protected async postChangePassword(req: Request, res: Response): Promise<void> {
const validationMap = { const validationMap = {
'new_password': new Validator().defined(), 'new_password': new Validator().defined(),

View File

@ -141,7 +141,7 @@ export default class AuthGuard {
if (!user.isApproved()) { if (!user.isApproved()) {
await new Mail(this.app.as(NunjucksComponent).getEnvironment(), PENDING_ACCOUNT_REVIEW_MAIL_TEMPLATE, { await new Mail(this.app.as(NunjucksComponent).getEnvironment(), PENDING_ACCOUNT_REVIEW_MAIL_TEMPLATE, {
username: user.asOptional(UserNameComponent)?.name || username: user.asOptional(UserNameComponent)?.getName() ||
(await user.mainEmail.get())?.getOrFail('email') || (await user.mainEmail.get())?.getOrFail('email') ||
'Could not find an identifier', 'Could not find an identifier',
link: config.get<string>('public_url') + Controller.route('accounts-approval'), link: config.get<string>('public_url') + Controller.route('accounts-approval'),

View File

@ -65,7 +65,12 @@ export default class MagicLinkController<A extends Application> extends Controll
return await req.as(AuthMiddleware).getAuthGuard().authenticateOrRegister( return await req.as(AuthMiddleware).getAuthGuard().authenticateOrRegister(
session, magicLink, !!session.wantsSessionPersistence, undefined, async (connection, user) => { session, magicLink, !!session.wantsSessionPersistence, undefined, async (connection, user) => {
const userNameComponent = user.asOptional(UserNameComponent); const userNameComponent = user.asOptional(UserNameComponent);
if (userNameComponent) userNameComponent.name = magicLink.as(MagicLinkUserNameComponent).username; const magicLinkUserNameComponent = magicLink.asOptional(MagicLinkUserNameComponent);
if (userNameComponent && magicLinkUserNameComponent?.username) {
userNameComponent.setName(magicLinkUserNameComponent.username);
}
return []; return [];
}, async (connection, user) => { }, async (connection, user) => {
const callbacks: RegisterCallback[] = []; const callbacks: RegisterCallback[] = [];
@ -192,7 +197,7 @@ export default class MagicLinkController<A extends Application> extends Controll
if (!res.headersSent && user) { if (!res.headersSent && user) {
// Auth success // Auth success
const name = user.asOptional(UserNameComponent)?.name; const name = user.asOptional(UserNameComponent)?.getName();
req.flash('success', `Authentication success. Welcome${name ? `, ${name}` : ''}`); req.flash('success', `Authentication success. Welcome${name ? `, ${name}` : ''}`);
res.redirect(req.getIntendedUrl() || Controller.route('home')); res.redirect(req.getIntendedUrl() || Controller.route('home'));
} }

View File

@ -0,0 +1,12 @@
import Migration from "../../db/Migration";
export default class AddNameChangedAtToUsersMigration extends Migration {
public async install(): Promise<void> {
await this.query(`ALTER TABLE users
ADD COLUMN name_changed_at DATETIME`);
}
public async rollback(): Promise<void> {
await this.query('ALTER TABLE users DROP COLUMN IF EXISTS name_changed_at');
}
}

View File

@ -9,7 +9,7 @@ import UserNameComponent from "./UserNameComponent";
export default class User extends Model { export default class User extends Model {
public static isApprovalMode(): boolean { public static isApprovalMode(): boolean {
return config.get<boolean>('approval_mode') && return config.get<boolean>('auth.approval_mode') &&
MysqlConnectionManager.hasMigration(AddApprovedFieldToUsersTableMigration); MysqlConnectionManager.hasMigration(AddApprovedFieldToUsersTableMigration);
} }
@ -42,10 +42,10 @@ export default class User extends Model {
protected getPersonalInfoFields(): { name: string, value: string }[] { protected getPersonalInfoFields(): { name: string, value: string }[] {
const fields: { name: string, value: string }[] = []; const fields: { name: string, value: string }[] = [];
const nameComponent = this.asOptional(UserNameComponent); const nameComponent = this.asOptional(UserNameComponent);
if (nameComponent && nameComponent.name) { if (nameComponent && nameComponent.hasName()) {
fields.push({ fields.push({
name: 'Name', name: 'Name',
value: nameComponent.name, value: nameComponent.getName(),
}); });
} }
return fields; return fields;

View File

@ -1,12 +1,44 @@
import ModelComponent from "../../db/ModelComponent"; import ModelComponent from "../../db/ModelComponent";
import User from "../models/User"; import User from "../models/User";
import config from "config";
export const USERNAME_REGEXP = /^[0-9a-z_-]+$/; export const USERNAME_REGEXP = /^[0-9a-z_-]+$/;
export default class UserNameComponent extends ModelComponent<User> { export default class UserNameComponent extends ModelComponent<User> {
public name?: string = undefined; private name?: string = undefined;
private name_changed_at?: Date | null = undefined;
public init(): void { public init(): void {
this.setValidation('name').defined().between(3, 64).regexp(USERNAME_REGEXP).unique(this._model); this.setValidation('name').defined().between(3, 64).regexp(USERNAME_REGEXP).unique(this._model);
} }
public hasName(): boolean {
return !!this.name;
}
public getName(): string {
if (!this.name) throw new Error('User has no name.');
return this.name;
}
public setName(newName: string): boolean {
if (!this.canChangeName()) return false;
this.name = newName;
this.name_changed_at = new Date();
return true;
}
public getNameChangedAt(): Date | null {
return this.name_changed_at || null;
}
public forgetNameChangeDate(): void {
this.name_changed_at = null;
}
public canChangeName(): boolean {
return !this.name_changed_at ||
Date.now() - this.name_changed_at.getTime() >= config.get<number>('auth.name_change_wait_period');
}
} }

View File

@ -113,7 +113,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
await user.as(UserPasswordComponent).setPassword(req.body.password); await user.as(UserPasswordComponent).setPassword(req.body.password);
// Username // Username
user.as(UserNameComponent).name = req.body.identifier; user.as(UserNameComponent).setName(req.body.identifier);
return callbacks; return callbacks;
}, async (connection, user) => { }, async (connection, user) => {
@ -132,7 +132,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
const user = await passwordAuthProof.getResource(); const user = await passwordAuthProof.getResource();
req.flash('success', `Your account was successfully created! Welcome, ${user?.as(UserNameComponent).name}.`); req.flash('success', `Your account was successfully created! Welcome, ${user?.as(UserNameComponent).getName()}.`);
res.redirect(req.getIntendedUrl() || Controller.route('home')); res.redirect(req.getIntendedUrl() || Controller.route('home'));
} }

View File

@ -72,7 +72,7 @@ describe('Register with username and password (password)', () => {
.first(); .first();
expect(user).toBeDefined(); expect(user).toBeDefined();
expect(user?.as(UserNameComponent).name).toStrictEqual('entrapta'); expect(user?.as(UserNameComponent).getName()).toStrictEqual('entrapta');
await expect(user?.as(UserPasswordComponent).verifyPassword('darla_is_cute')).resolves.toStrictEqual(true); await expect(user?.as(UserPasswordComponent).verifyPassword('darla_is_cute')).resolves.toStrictEqual(true);
}); });
@ -186,7 +186,7 @@ describe('Register with email (magic_link)', () => {
expect(email).toBeDefined(); expect(email).toBeDefined();
expect(email?.email).toStrictEqual('glimmer@example.org'); expect(email?.email).toStrictEqual('glimmer@example.org');
expect(user?.as(UserNameComponent).name).toStrictEqual('glimmer'); expect(user?.as(UserNameComponent).getName()).toStrictEqual('glimmer');
await expect(user?.as(UserPasswordComponent).verifyPassword('')).resolves.toStrictEqual(false); await expect(user?.as(UserPasswordComponent).verifyPassword('')).resolves.toStrictEqual(false);
}); });
@ -575,7 +575,7 @@ describe('Authenticate with email and password (password)', () => {
expect(email).toBeDefined(); expect(email).toBeDefined();
expect(email?.email).toStrictEqual('double-trouble@example.org'); expect(email?.email).toStrictEqual('double-trouble@example.org');
expect(user?.as(UserNameComponent).name).toStrictEqual('double-trouble'); expect(user?.as(UserNameComponent).getName()).toStrictEqual('double-trouble');
await expect(user?.as(UserPasswordComponent).verifyPassword('trick-or-treat')).resolves.toStrictEqual(true); await expect(user?.as(UserPasswordComponent).verifyPassword('trick-or-treat')).resolves.toStrictEqual(true);
}); });
@ -871,6 +871,89 @@ describe('Change password', () => {
}); });
}); });
describe('Change username', () => {
let cookies: string[], csrf: string;
test('Prepare user', async () => {
const res = await agent.get('/csrf').expect(200);
cookies = res.get('Set-Cookie');
csrf = res.text;
await agent.post('/auth/register')
.set('Cookie', cookies)
.send({
csrf: csrf,
auth_method: 'password',
identifier: 'richard',
password: 'a_very_strong_password',
password_confirmation: 'a_very_strong_password',
terms: 'on',
})
.expect(302)
.expect('Location', '/');
});
test('Initial value of name_changed_at should be the same as created_at', async () => {
const user = await User.select().where('name', 'richard').first();
expect(user).toBeDefined();
expect(user?.as(UserNameComponent).getNameChangedAt()?.getTime()).toBe(user?.created_at?.getTime());
});
test('Cannot change username just after registration', async () => {
expect(await User.select().where('name', 'richard').count()).toBe(1);
await agent.post('/account/change-name')
.set('Cookie', cookies)
.send({
csrf: csrf,
name: 'robert',
})
.expect(302)
.expect('Location', '/account/');
expect(await User.select().where('name', 'richard').count()).toBe(1);
});
test('Can change username after hold period', async () => {
// Set last username change to older date
const user = await User.select().where('name', 'richard').first();
if (user) {
user.as(UserNameComponent).forgetNameChangeDate();
await user.save();
}
expect(await User.select().where('name', 'richard').count()).toBe(1);
expect(await User.select().where('name', 'robert').count()).toBe(0);
await agent.post('/account/change-name')
.set('Cookie', cookies)
.send({
csrf: csrf,
name: 'robert',
})
.expect(302)
.expect('Location', '/account/');
expect(await User.select().where('name', 'richard').count()).toBe(0);
expect(await User.select().where('name', 'robert').count()).toBe(1);
});
test('Cannot change username just after changing username', async () => {
expect(await User.select().where('name', 'robert').count()).toBe(1);
expect(await User.select().where('name', 'bebert').count()).toBe(0);
await agent.post('/account/change-name')
.set('Cookie', cookies)
.send({
csrf: csrf,
name: 'bebert',
})
.expect(302)
.expect('Location', '/account/');
expect(await User.select().where('name', 'robert').count()).toBe(1);
expect(await User.select().where('name', 'bebert').count()).toBe(0);
});
});
describe('Manage email addresses', () => { describe('Manage email addresses', () => {

View File

@ -22,6 +22,10 @@
{% endif %} {% endif %}
</div> </div>
{% if has_name_component %}
{% include './name_panel.njk' %}
{% endif %}
{% if has_password_component %} {% if has_password_component %}
{% include './password_panel.njk' %} {% include './password_panel.njk' %}
{% endif %} {% endif %}

View File

@ -0,0 +1,16 @@
<section class="panel">
<h2><i data-feather="key"></i> Change name</h2>
{% if can_change_name %}
<form action="{{ route('change-name') }}" method="POST">
{{ macros.field(_locals, 'text', 'name', null, 'New name', null, 'required') }}
{{ macros.field(_locals, 'checkbox', 'terms', null, 'I understand that I can only change my name once every ' + name_change_wait_period, '', 'required') }}
<button type="submit"><i data-feather="save"></i> Confirm</button>
{{ macros.csrf(getCsrfToken) }}
</form>
{% else %}
{{ macros.message('info', 'You can change your name in ' + can_change_name_in) }}
{% endif %}
</section>