Resend user invites (#18147)

* Support for re-sending user invites. Close #1799

* Reuse getUserByEmail in user service.

* Add mockImplementation to spies on ItemsServices

* Signed contributors list.

* Fix invite tests

* Fix comments

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

api/src/services/users.test.ts

@@ -3,7 +3,7 @@ import knex from 'knex';
 import type { Knex } from 'knex';
 import { createTracker, MockClient, Tracker } from 'knex-mock-client';
 import { afterEach, beforeAll, beforeEach, describe, expect, it, MockedFunction, SpyInstance, vi } from 'vitest';
-import { ItemsService, UsersService } from './index.js';
+import { ItemsService, MailService, UsersService } from './index.js';
 import { ForbiddenException, InvalidPayloadException } from '../exceptions/index.js';
 import { RecordNotUniqueException } from '../exceptions/database/record-not-unique.js';
 
@@ -12,6 +12,13 @@ vi.mock('../../src/database/index', () => ({
 	getDatabaseClient: vi.fn().mockReturnValue('postgres'),
 }));
 
+vi.mock('./mail', () => {
+	const MailService = vi.fn();
+	MailService.prototype.send = vi.fn();
+
+	return { MailService };
+});
+
 const testRoleId = '4ccdb196-14b3-4ed1-b9da-c1978be07ca2';
 
 const testSchema = {
@@ -66,6 +73,8 @@ describe('Integration Tests', () => {
 
 	describe('Services / Users', () => {
 		let service: UsersService;
+		let mailService: MailService;
+		let superCreateManySpy: SpyInstance;
 		let superUpdateManySpy: SpyInstance;
 		let checkUniqueEmailsSpy: SpyInstance;
 		let checkPasswordPolicySpy: SpyInstance;
@@ -106,7 +115,8 @@ describe('Integration Tests', () => {
 				},
 			});
 
-			superUpdateManySpy = vi.spyOn(ItemsService.prototype, 'updateMany');
+			superCreateManySpy = vi.spyOn(ItemsService.prototype as any, 'createMany');
+			superUpdateManySpy = vi.spyOn(ItemsService.prototype as any, 'updateMany');
 
 			// "as any" are needed since these are private methods
 			checkUniqueEmailsSpy = vi
@@ -121,13 +131,16 @@ describe('Integration Tests', () => {
 			checkRemainingActiveAdminSpy = vi
 				.spyOn(UsersService.prototype as any, 'checkRemainingActiveAdmin')
 				.mockResolvedValue(() => vi.fn());
+
+			vi.spyOn(UsersService.prototype as any, 'inviteUrl').mockImplementation(() => vi.fn());
+
+			mailService = new MailService({
+				schema: testSchema,
+			});
 		});
 
 		afterEach(() => {
-			checkUniqueEmailsSpy.mockClear();
-			checkPasswordPolicySpy.mockClear();
-			checkRemainingAdminExistenceSpy.mockClear();
-			checkRemainingActiveAdminSpy.mockClear();
+			vi.clearAllMocks();
 		});
 
 		describe('createOne', () => {
@@ -603,5 +616,92 @@ describe('Integration Tests', () => {
 				expect(checkRemainingAdminExistenceSpy).toBeCalledTimes(1);
 			});
 		});
+
+		describe('invite', () => {
+			it('should invite new users', async () => {
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: true },
+				});
+
+				const promise = service.inviteUser('user@example.com', 'invite-role', null);
+
+				await expect(promise).resolves.not.toThrow();
+
+				expect(superCreateManySpy.mock.lastCall![0]).toEqual([
+					expect.objectContaining({
+						email: 'user@example.com',
+						status: 'invited',
+						role: 'invite-role',
+					}),
+				]);
+
+				expect(mailService.send).toBeCalledTimes(1);
+			});
+
+			it('should re-send invites for invited users', async () => {
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: true },
+				});
+
+				// mock an invited user
+				vi.spyOn(UsersService.prototype as any, 'getUserByEmail').mockResolvedValueOnce({
+					status: 'invited',
+					role: 'invite-role',
+				});
+
+				const promise = service.inviteUser('user@example.com', 'invite-role', null);
+				await expect(promise).resolves.not.toThrow();
+
+				expect(superCreateManySpy).not.toBeCalled();
+				expect(mailService.send).toBeCalledTimes(1);
+			});
+
+			it('should not re-send invites for users in state other than invited', async () => {
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: true },
+				});
+
+				// mock an active user
+				vi.spyOn(UsersService.prototype as any, 'getUserByEmail').mockResolvedValueOnce({
+					status: 'active',
+					role: 'invite-role',
+				});
+
+				const promise = service.inviteUser('user@example.com', 'invite-role', null);
+				await expect(promise).resolves.not.toThrow();
+
+				expect(superCreateManySpy).not.toBeCalled();
+				expect(mailService.send).not.toBeCalled();
+			});
+
+			it('should update role when re-sent invite contains different role than user has', async () => {
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: true },
+				});
+
+				tracker.on.select(/select "admin_access" from "directus_roles"/).response({ admin_access: true });
+
+				// mock an invited user with different role
+				vi.spyOn(UsersService.prototype as any, 'getUserByEmail').mockResolvedValueOnce({
+					id: 1,
+					status: 'invited',
+					role: 'existing-role',
+				});
+
+				const promise = service.inviteUser('user@example.com', 'invite-role', null);
+				await expect(promise).resolves.not.toThrow();
+
+				expect(superUpdateManySpy.mock.lastCall![0]).toEqual([1]);
+				expect(superUpdateManySpy.mock.lastCall![1]).toContain({ role: 'invite-role' });
+			});
+		});
 	});
 });

api/src/services/users.ts

@@ -2,7 +2,7 @@ import { FailedValidationException } from '@directus/exceptions';
 import type { Query } from '@directus/types';
 import { getSimpleHash, toArray } from '@directus/utils';
 import jwt from 'jsonwebtoken';
-import { cloneDeep } from 'lodash-es';
+import { cloneDeep, isEmpty } from 'lodash-es';
 import { performance } from 'perf_hooks';
 import getDatabase from '../database/index.js';
 import env from '../env.js';
@@ -135,6 +135,30 @@ export class UsersService extends ItemsService {
 		}
 	}
 
+	/**
+	 * Get basic information of user identified by email
+	 */
+	private async getUserByEmail(email: string): Promise<{ id: string; role: string; status: string; password: string }> {
+		return await this.knex
+			.select('id', 'role', 'status', 'password')
+			.from('directus_users')
+			.whereRaw(`LOWER(??) = ?`, ['email', email.toLowerCase()])
+			.first();
+	}
+
+	/**
+	 * Create url for inviting users
+	 */
+	private inviteUrl(email: string, url: string | null): string {
+		const payload = { email, scope: 'invite' };
+
+		const token = jwt.sign(payload, env['SECRET'] as string, { expiresIn: '7d', issuer: 'directus' });
+		const inviteURL = url ? new Url(url) : new Url(env['PUBLIC_URL']).addPath('admin', 'accept-invite');
+		inviteURL.setQuery('token', token);
+
+		return inviteURL.toString();
+	}
+
 	/**
 	 * Create a new user
 	 */
@@ -326,26 +350,34 @@ export class UsersService extends ItemsService {
 		});
 
 		for (const email of emails) {
-			const payload = { email, scope: 'invite' };
-			const token = jwt.sign(payload, env['SECRET'] as string, { expiresIn: '7d', issuer: 'directus' });
-			const subjectLine = subject ?? "You've been invited";
-			const inviteURL = url ? new Url(url) : new Url(env['PUBLIC_URL']).addPath('admin', 'accept-invite');
-			inviteURL.setQuery('token', token);
+			// Check if user is known
+			const user = await this.getUserByEmail(email);
 
-			// Create user first to verify uniqueness
-			await this.createOne({ email, role, status: 'invited' }, opts);
+			// Create user first to verify uniqueness if unknown
+			if (isEmpty(user)) {
+				await this.createOne({ email, role, status: 'invited' }, opts);
 
-			await mailService.send({
-				to: email,
-				subject: subjectLine,
-				template: {
-					name: 'user-invitation',
-					data: {
-						url: inviteURL.toString(),
-						email,
+				// For known users update role if changed
+			} else if (user.status === 'invited' && user.role !== role) {
+				await this.updateOne(user.id, { role }, opts);
+			}
+
+			// Send invite for new and already invited users
+			if (isEmpty(user) || user.status === 'invited') {
+				const subjectLine = subject ?? "You've been invited";
+
+				await mailService.send({
+					to: email,
+					subject: subjectLine,
+					template: {
+						name: 'user-invitation',
+						data: {
+							url: this.inviteUrl(email, url),
+							email,
+						},
 					},
-				},
-			});
+				});
+			}
 		}
 	}
 
@@ -357,7 +389,7 @@ export class UsersService extends ItemsService {
 
 		if (scope !== 'invite') throw new ForbiddenException();
 
-		const user = await this.knex.select('id', 'status').from('directus_users').where({ email }).first();
+		const user = await this.getUserByEmail(email);
 
 		if (user?.status !== 'invited') {
 			throw new InvalidPayloadException(`Email address ${email} hasn't been invited.`);
@@ -376,11 +408,7 @@ export class UsersService extends ItemsService {
 		const STALL_TIME = 500;
 		const timeStart = performance.now();
 
-		const user = await this.knex
-			.select('status', 'password')
-			.from('directus_users')
-			.whereRaw('LOWER(??) = ?', ['email', email.toLowerCase()])
-			.first();
+		const user = await this.getUserByEmail(email);
 
 		if (user?.status !== 'active') {
 			await stall(STALL_TIME, timeStart);
@@ -436,7 +464,7 @@ export class UsersService extends ItemsService {
 			opts.preMutationException = err;
 		}
 
-		const user = await this.knex.select('id', 'status', 'password').from('directus_users').where({ email }).first();
+		const user = await this.getUserByEmail(email);
 
 		if (user?.status !== 'active' || hash !== getSimpleHash('' + user.password)) {
 			throw new ForbiddenException();

contributors.yml

@@ -15,5 +15,6 @@
 - denkhaus
 - joselcvarela
 - knulpi
+- david-zacharias
 - hanneskuettner
 - JoshTheDerf