ロールのアサインAPIで有効期間が変わった場合は期限だけを更新するように (MisskeyIO#188)

packages/backend/src/core/RoleService.ts

@@ -8,6 +8,7 @@ import * as Redis from 'ioredis';
 import { In } from 'typeorm';
 import type { MiRole, MiRoleAssignment, RoleAssignmentsRepository, RolesRepository, UsersRepository } from '@/models/index.js';
 import { MemoryKVCache, MemorySingleCache } from '@/misc/cache.js';
+import { IdentifiableError } from '@/misc/identifiable-error.js';
 import type { MiUser } from '@/models/entities/User.js';
 import { DI } from '@/di-symbols.js';
 import { bindThis } from '@/decorators.js';
@@ -80,9 +81,6 @@ export class RoleService implements OnApplicationShutdown {
 	private rolesCache: MemorySingleCache<MiRole[]>;
 	private roleAssignmentByUserIdCache: MemoryKVCache<MiRoleAssignment[]>;
 
-	public static AlreadyAssignedError = class extends Error {};
-	public static NotAssignedError = class extends Error {};
-
 	constructor(
 		@Inject(DI.redis)
 		private redisClient: Redis.Redis,
@@ -386,22 +384,16 @@ export class RoleService implements OnApplicationShutdown {
 	public async assign(userId: MiUser['id'], roleId: MiRole['id'], expiresAt: Date | null = null): Promise<void> {
 		const now = new Date();
 
-		const existing = await this.roleAssignmentsRepository.findOneBy({
+		let existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId });
-			roleId: roleId,
+		if (existing?.expiresAt && (existing.expiresAt.getTime() < now.getTime())) {
-			userId: userId,
-		});
-
-		if (existing) {
-			if (existing.expiresAt && (existing.expiresAt.getTime() < now.getTime())) {
 			await this.roleAssignmentsRepository.delete({
 				roleId: roleId,
 				userId: userId,
 			});
-			} else {
+			existing = null;
-				throw new RoleService.AlreadyAssignedError();
-			}
 		}
 
+		if (!existing) {
 			const created = await this.roleAssignmentsRepository.insert({
 				id: this.idService.genId(),
 				createdAt: now,
@@ -410,26 +402,35 @@ export class RoleService implements OnApplicationShutdown {
 				userId: userId,
 			}).then(x => this.roleAssignmentsRepository.findOneByOrFail(x.identifiers[0]));
 
+			this.globalEventService.publishInternalEvent('userRoleAssigned', created);
+		} else if (existing.expiresAt !== expiresAt) {
+			await this.roleAssignmentsRepository.update(existing.id, {
+				expiresAt: expiresAt,
+			});
+		} else {
+			throw new IdentifiableError('67d8689c-25c6-435f-8ced-631e4b81fce1', 'User is already assigned to this role.');
+		}
+
 		this.rolesRepository.update(roleId, {
 			lastUsedAt: new Date(),
 		});
-
-		this.globalEventService.publishInternalEvent('userRoleAssigned', created);
 	}
 
 	@bindThis
 	public async unassign(userId: MiUser['id'], roleId: MiRole['id']): Promise<void> {
 		const now = new Date();
 
-		const existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId });
+		let existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId });
-		if (existing == null) {
+		if (existing?.expiresAt && (existing.expiresAt.getTime() < now.getTime())) {
-			throw new RoleService.NotAssignedError();
-		} else if (existing.expiresAt && (existing.expiresAt.getTime() < now.getTime())) {
 			await this.roleAssignmentsRepository.delete({
 				roleId: roleId,
 				userId: userId,
 			});
-			throw new RoleService.NotAssignedError();
+			existing = null;
+		}
+
+		if (!existing) {
+			throw new IdentifiableError('b9060ac7-5c94-4da4-9f55-2047c953df44', 'User was not assigned to this role.');
 		}
 
 		await this.roleAssignmentsRepository.delete(existing.id);

packages/backend/src/server/api/endpoints/admin/roles/assign.ts

@@ -29,6 +29,12 @@ export const meta = {
 			id: '558ea170-f653-4700-94d0-5a818371d0df',
 		},
 
+		alreadyAssigned: {
+			message: 'User is already assigned to this role.',
+			code: 'ALREADY_ASSIGNED',
+			id: '67d8689c-25c6-435f-8ced-631e4b81fce1',
+		},
+
 		accessDenied: {
 			message: 'Only administrators can edit members of the role.',
 			code: 'ACCESS_DENIED',

packages/backend/src/server/api/endpoints/admin/roles/unassign.ts

@@ -30,7 +30,7 @@ export const meta = {
 		},
 
 		notAssigned: {
-			message: 'Not assigned.',
+			message: 'User was not assigned to this role.',
 			code: 'NOT_ASSIGNED',
 			id: 'b9060ac7-5c94-4da4-9f55-2047c953df44',
 		},