diff --git a/packages/backend/src/core/RoleService.ts b/packages/backend/src/core/RoleService.ts index 0fc6883922..9713e6274c 100644 --- a/packages/backend/src/core/RoleService.ts +++ b/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; private roleAssignmentByUserIdCache: MemoryKVCache; - public static AlreadyAssignedError = class extends Error {}; - public static NotAssignedError = class extends Error {}; - constructor( @Inject(DI.redis) private redisClient: Redis.Redis, @@ -386,50 +384,53 @@ export class RoleService implements OnApplicationShutdown { public async assign(userId: MiUser['id'], roleId: MiRole['id'], expiresAt: Date | null = null): Promise { const now = new Date(); - const existing = await this.roleAssignmentsRepository.findOneBy({ - roleId: roleId, - userId: userId, - }); - - if (existing) { - if (existing.expiresAt && (existing.expiresAt.getTime() < now.getTime())) { - await this.roleAssignmentsRepository.delete({ - roleId: roleId, - userId: userId, - }); - } else { - throw new RoleService.AlreadyAssignedError(); - } + let existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId }); + if (existing?.expiresAt && (existing.expiresAt.getTime() < now.getTime())) { + await this.roleAssignmentsRepository.delete({ + roleId: roleId, + userId: userId, + }); + existing = null; } - const created = await this.roleAssignmentsRepository.insert({ - id: this.idService.genId(), - createdAt: now, - expiresAt: expiresAt, - roleId: roleId, - userId: userId, - }).then(x => this.roleAssignmentsRepository.findOneByOrFail(x.identifiers[0])); + if (!existing) { + const created = await this.roleAssignmentsRepository.insert({ + id: this.idService.genId(), + createdAt: now, + expiresAt: expiresAt, + roleId: roleId, + 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 { const now = new Date(); - const existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId }); - if (existing == null) { - throw new RoleService.NotAssignedError(); - } else if (existing.expiresAt && (existing.expiresAt.getTime() < now.getTime())) { + let existing = await this.roleAssignmentsRepository.findOneBy({ roleId, userId }); + 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); diff --git a/packages/backend/src/server/api/endpoints/admin/roles/assign.ts b/packages/backend/src/server/api/endpoints/admin/roles/assign.ts index 78ab13a773..4878cfd05c 100644 --- a/packages/backend/src/server/api/endpoints/admin/roles/assign.ts +++ b/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', diff --git a/packages/backend/src/server/api/endpoints/admin/roles/unassign.ts b/packages/backend/src/server/api/endpoints/admin/roles/unassign.ts index 1b3c49571b..ebddd46b52 100644 --- a/packages/backend/src/server/api/endpoints/admin/roles/unassign.ts +++ b/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', },