diff --git a/backend/src/PnvPanel.Application/Admin/Roles/RoleErrors.cs b/backend/src/PnvPanel.Application/Admin/Roles/RoleErrors.cs index 1e2db51..aee355c 100644 --- a/backend/src/PnvPanel.Application/Admin/Roles/RoleErrors.cs +++ b/backend/src/PnvPanel.Application/Admin/Roles/RoleErrors.cs @@ -17,4 +17,8 @@ public static class RoleErrors "Roles.RoleInUse", "Роль назначена пользователям — сначала переназначьте их." ); + public static readonly Error CannotRemoveLastAdmin = Error.Conflict( + "Roles.CannotRemoveLastAdmin", + "Нельзя снять роль admin с последнего администратора." + ); } diff --git a/backend/src/PnvPanel.Infrastructure/Identity/RoleService.cs b/backend/src/PnvPanel.Infrastructure/Identity/RoleService.cs index 9c55a23..fb5e7f4 100644 --- a/backend/src/PnvPanel.Infrastructure/Identity/RoleService.cs +++ b/backend/src/PnvPanel.Infrastructure/Identity/RoleService.cs @@ -100,6 +100,16 @@ internal sealed class RoleService( return Result.Failure(RoleErrors.NotFound); var currentRoles = await userManager.GetRolesAsync(user); + var wasAdmin = currentRoles.Contains(RoleNames.Admin, StringComparer.OrdinalIgnoreCase); + var staysAdmin = role.Name!.Equals(RoleNames.Admin, StringComparison.OrdinalIgnoreCase); + + if (wasAdmin && !staysAdmin) + { + var adminCount = (await userManager.GetUsersInRoleAsync(RoleNames.Admin)).Count; + if (adminCount <= 1) + return Result.Failure(RoleErrors.CannotRemoveLastAdmin); + } + if (currentRoles.Count > 0) await userManager.RemoveFromRolesAsync(user, currentRoles); diff --git a/backend/tests/PnvPanel.Application.Tests/Admin/Support/ApproveRoleRequestCommandHandlerTests.cs b/backend/tests/PnvPanel.Application.Tests/Admin/Support/ApproveRoleRequestCommandHandlerTests.cs index 58801c4..2715c5d 100644 --- a/backend/tests/PnvPanel.Application.Tests/Admin/Support/ApproveRoleRequestCommandHandlerTests.cs +++ b/backend/tests/PnvPanel.Application.Tests/Admin/Support/ApproveRoleRequestCommandHandlerTests.cs @@ -1,4 +1,5 @@ using NSubstitute; +using PnvPanel.Application.Admin.Roles; using PnvPanel.Application.Admin.Support; using PnvPanel.Application.Common.Interfaces; using PnvPanel.Application.Common.Models; @@ -56,7 +57,12 @@ public class ApproveRoleRequestCommandHandlerTests .ChangeUserRoleAsync(userId, newRoleId, Arg.Any()); await _telegramNotifier .Received(1) - .NotifyUserAsync(userId, Arg.Any(), Arg.Any(), Arg.Any()); + .NotifyUserAsync( + userId, + Arg.Any(), + Arg.Any(), + Arg.Any() + ); } [Fact] @@ -123,4 +129,72 @@ public class ApproveRoleRequestCommandHandlerTests Assert.False(result.IsSuccess); Assert.Equal(SupportErrors.NotRoleRequest, result.Error); } + + [Fact] + public async Task Handle_WhenTicketIsOwnedByAdmin_StillApproves() + { + // Одобрить свою же заявку можно — единственный реальный риск (снять admin с последнего + // администратора) ловит RoleService.ChangeUserRoleAsync, а не этот хендлер (см. соседний тест). + using var dbContext = InMemoryDbContextFactory.Create(); + var adminId = Guid.NewGuid(); + var roleId = Guid.NewGuid(); + var ticket = SupportTicket.CreateRoleRequestForExistingRole(adminId, roleId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + _roleService + .ChangeUserRoleAsync(adminId, roleId, Arg.Any()) + .Returns(Result.Success()); + + var currentUser = FakeCurrentUser.Authenticated(adminId, "admin"); + var handler = new ApproveRoleRequestCommandHandler( + dbContext, + _roleService, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new ApproveRoleRequestCommand(ticket.Id), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Resolved, ticket.Status); + } + + [Fact] + public async Task Handle_WhenRoleServiceRefusesLastAdminDowngrade_PropagatesFailure() + { + using var dbContext = InMemoryDbContextFactory.Create(); + var adminId = Guid.NewGuid(); + var roleId = Guid.NewGuid(); + var ticket = SupportTicket.CreateRoleRequestForExistingRole(adminId, roleId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + _roleService + .ChangeUserRoleAsync(adminId, roleId, Arg.Any()) + .Returns(Result.Failure(RoleErrors.CannotRemoveLastAdmin)); + + var currentUser = FakeCurrentUser.Authenticated(adminId, "admin"); + var handler = new ApproveRoleRequestCommandHandler( + dbContext, + _roleService, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new ApproveRoleRequestCommand(ticket.Id), + CancellationToken.None + ); + + Assert.False(result.IsSuccess); + Assert.Equal(RoleErrors.CannotRemoveLastAdmin, result.Error); + // Тикет остаётся Open — можно повторить попытку после назначения второго админа. + Assert.Equal(TicketStatus.Open, ticket.Status); + } } diff --git a/backend/tests/PnvPanel.Application.Tests/Admin/Support/CloseTicketCommandHandlerTests.cs b/backend/tests/PnvPanel.Application.Tests/Admin/Support/CloseTicketCommandHandlerTests.cs new file mode 100644 index 0000000..0eab220 --- /dev/null +++ b/backend/tests/PnvPanel.Application.Tests/Admin/Support/CloseTicketCommandHandlerTests.cs @@ -0,0 +1,68 @@ +using NSubstitute; +using PnvPanel.Application.Admin.Support; +using PnvPanel.Application.Common.Interfaces; +using PnvPanel.Application.Tests.TestSupport; +using PnvPanel.Domain.Support; +using Xunit; + +namespace PnvPanel.Application.Tests.Admin.Support; + +public class CloseTicketCommandHandlerTests +{ + private readonly IRealtimeNotifier _notifier = Substitute.For(); + private readonly ITelegramNotifier _telegramNotifier = Substitute.For(); + + [Fact] + public async Task Handle_ClosesOpenTicket() + { + using var dbContext = InMemoryDbContextFactory.Create(); + var userId = Guid.NewGuid(); + var ticket = SupportTicket.CreateBugReport(userId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(Guid.NewGuid(), "admin"); + var handler = new CloseTicketCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new CloseTicketCommand(ticket.Id), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Closed, ticket.Status); + } + + [Fact] + public async Task Handle_WhenTicketIsOwnedByAdmin_StillCloses() + { + // Закрыть свой же тикет можно — Close не меняет роль, риска нет (иначе одинокий админ не + // смог бы почистить случайно созданный тикет никогда). + using var dbContext = InMemoryDbContextFactory.Create(); + var adminId = Guid.NewGuid(); + var ticket = SupportTicket.CreateBugReport(adminId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(adminId, "admin"); + var handler = new CloseTicketCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new CloseTicketCommand(ticket.Id), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Closed, ticket.Status); + } +} diff --git a/backend/tests/PnvPanel.Application.Tests/Admin/Support/RejectRoleRequestCommandHandlerTests.cs b/backend/tests/PnvPanel.Application.Tests/Admin/Support/RejectRoleRequestCommandHandlerTests.cs new file mode 100644 index 0000000..6882713 --- /dev/null +++ b/backend/tests/PnvPanel.Application.Tests/Admin/Support/RejectRoleRequestCommandHandlerTests.cs @@ -0,0 +1,67 @@ +using NSubstitute; +using PnvPanel.Application.Admin.Support; +using PnvPanel.Application.Common.Interfaces; +using PnvPanel.Application.Tests.TestSupport; +using PnvPanel.Domain.Support; +using Xunit; + +namespace PnvPanel.Application.Tests.Admin.Support; + +public class RejectRoleRequestCommandHandlerTests +{ + private readonly IRealtimeNotifier _notifier = Substitute.For(); + private readonly ITelegramNotifier _telegramNotifier = Substitute.For(); + + [Fact] + public async Task Handle_RejectsRoleRequest_ClosesTicket() + { + using var dbContext = InMemoryDbContextFactory.Create(); + var userId = Guid.NewGuid(); + var ticket = SupportTicket.CreateRoleRequestForExistingRole(userId, Guid.NewGuid()); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(Guid.NewGuid(), "admin"); + var handler = new RejectRoleRequestCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new RejectRoleRequestCommand(ticket.Id, "не подходит"), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Closed, ticket.Status); + } + + [Fact] + public async Task Handle_WhenTicketIsOwnedByAdmin_StillRejects() + { + // Отклонить свою же заявку можно — Reject не меняет роль, риска нет. + using var dbContext = InMemoryDbContextFactory.Create(); + var adminId = Guid.NewGuid(); + var ticket = SupportTicket.CreateRoleRequestForExistingRole(adminId, Guid.NewGuid()); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(adminId, "admin"); + var handler = new RejectRoleRequestCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new RejectRoleRequestCommand(ticket.Id, null), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Closed, ticket.Status); + } +} diff --git a/backend/tests/PnvPanel.Application.Tests/Admin/Support/ResolveTicketCommandHandlerTests.cs b/backend/tests/PnvPanel.Application.Tests/Admin/Support/ResolveTicketCommandHandlerTests.cs new file mode 100644 index 0000000..289560c --- /dev/null +++ b/backend/tests/PnvPanel.Application.Tests/Admin/Support/ResolveTicketCommandHandlerTests.cs @@ -0,0 +1,68 @@ +using NSubstitute; +using PnvPanel.Application.Admin.Support; +using PnvPanel.Application.Common.Interfaces; +using PnvPanel.Application.Tests.TestSupport; +using PnvPanel.Domain.Support; +using Xunit; + +namespace PnvPanel.Application.Tests.Admin.Support; + +public class ResolveTicketCommandHandlerTests +{ + private readonly IRealtimeNotifier _notifier = Substitute.For(); + private readonly ITelegramNotifier _telegramNotifier = Substitute.For(); + + [Fact] + public async Task Handle_ResolvesOpenTicket() + { + using var dbContext = InMemoryDbContextFactory.Create(); + var userId = Guid.NewGuid(); + var ticket = SupportTicket.CreateBugReport(userId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(Guid.NewGuid(), "admin"); + var handler = new ResolveTicketCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new ResolveTicketCommand(ticket.Id), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Resolved, ticket.Status); + } + + [Fact] + public async Task Handle_WhenTicketIsOwnedByAdmin_StillResolves() + { + // Решить свой же тикет можно — Resolve не меняет роль, риска нет (иначе тикет одинокого + // админа навсегда застревал бы в Open). + using var dbContext = InMemoryDbContextFactory.Create(); + var adminId = Guid.NewGuid(); + var ticket = SupportTicket.CreateBugReport(adminId); + dbContext.SupportTickets.Add(ticket); + await dbContext.SaveChangesAsync(CancellationToken.None); + + var currentUser = FakeCurrentUser.Authenticated(adminId, "admin"); + var handler = new ResolveTicketCommandHandler( + dbContext, + _notifier, + _telegramNotifier, + currentUser + ); + + var result = await handler.Handle( + new ResolveTicketCommand(ticket.Id), + CancellationToken.None + ); + + Assert.True(result.IsSuccess); + Assert.Equal(TicketStatus.Resolved, ticket.Status); + } +} diff --git a/docs/api-design.md b/docs/api-design.md index 5a940b6..452b465 100644 --- a/docs/api-design.md +++ b/docs/api-design.md @@ -190,6 +190,11 @@ Support.CannotRequestAdminRole`), либо все три поля новой р | POST | `/api/admin/support/tickets/{id}/approve` | — | `204 No Content` (только `RoleRequest`/`Open`; создаёt/назначает роль) | | POST | `/api/admin/support/tickets/{id}/reject` | `{ reason? }` | `204 No Content` (только `RoleRequest`; `reason` уходит комментарием) | +Обработать **собственный** тикет админу можно (в т.ч. одобрить свою же заявку на роль) — resolve/close/ +reject/approve владением тикета не ограничены. Единственное реальное ограничение — `approve` вернёт +`409 Roles.CannotRemoveLastAdmin` через `ChangeUserRoleAsync`, если заявка (своя или чужая) снимает +`admin` с последнего администратора в системе. + `approve`/`reject` — единственный способ решить заявку на роль (нельзя одобрить через `resolve`). При одобрении: если заявка на существующую роль — сразу `ChangeUserRoleCommand`-эквивалент; если на новую — сперва создаётся `AppRole` (`IRoleService.CreateRoleAsync`), затем назначается. То же самое @@ -242,7 +247,7 @@ Support.CannotRequestAdminRole`), либо все три поля новой р | POST | `/api/admin/roles` | admin | `{ name, maxConfigs, maxIpLimit }` | `RoleDto` | | PUT | `/api/admin/roles/{id}` | admin | `{ maxConfigs, maxIpLimit }` | `RoleDto` | | DELETE | `/api/admin/roles/{id}` | admin | — | `204 No Content` (системные `admin`/`user` удалить нельзя) | -| PATCH | `/api/admin/users/{id}/role` | admin | `{ roleId }` | `204 No Content` | +| PATCH | `/api/admin/users/{id}/role` | admin | `{ roleId }` | `204 No Content` (`409 Roles.CannotRemoveLastAdmin`, если у цели сейчас `admin`, новая роль другая, и это единственный админ) | Нет отдельного эндпоинта «активировать напрямую без запроса» — активация только через approve/reject над `ActivationRequest`. diff --git a/docs/domain-model.md b/docs/domain-model.md index c30e8f1..229b8f1 100644 --- a/docs/domain-model.md +++ b/docs/domain-model.md @@ -281,6 +281,14 @@ UI **настойчиво напоминает** привязать его (ед конфигов больше новой квоты — существующие конфиги сохраняются, но **создание новых блокируется**, пока число активных не станет меньше квоты. Форс-отзыв лишних не делаем. +**Нельзя снять `admin` с последнего администратора**: `IRoleService.ChangeUserRoleAsync` перед сменой +роли проверяет — если у пользователя сейчас `admin`, а новая роль другая, и админов в системе ровно +один — `RoleErrors.CannotRemoveLastAdmin` (409), смены не происходит. Единая точка защиты — работает +и при прямой смене роли из `/admin/users`, и при одобрении заявки на роль через `SupportTicket` +(`ApproveRoleRequestCommandHandler` вызывает тот же `ChangeUserRoleAsync`), в том числе когда админ +одобряет заявку на понижение самому себе — этот путь специально не блокируется отдельно, чтобы не +плодить тикеты, которые некому обработать, если админ единственный. + ### ActivationRequest — запрос активации Пользователь просит активацию у админа; админ одобряет/отклоняет на сайте или в Telegram. @@ -355,6 +363,12 @@ UI **настойчиво напоминает** привязать его (ед - Доступ — только активированному пользователю (`IRequiresActivation`, как и у конфигов/новостей); админские действия (resolve/close/approve/reject) идут по отдельным `/api/admin/support/*` с ролевой проверкой, без завязки на активацию. +- Resolve/close/reject **собственного** тикета админом разрешены — они не трогают роль, риска нет + (запрет ломал бы самообслуживание: тикет единственного админа застревал бы в `Open` навсегда, убрать + некому). Единственное действие с реальным риском — approve заявки на роль, потому что оно меняет + роль заявителя; его самостоятельная защита не нужна — она уже есть на уровень ниже, см. `AppRole` + (`RoleErrors.CannotRemoveLastAdmin`), и одинаково работает что для approve своей заявки, что для + прямой смены роли через `/admin/users`. - `Closed`-тикеты не удаляются автоматически — админ может подчистить их вручную (вкладка «Обслуживание», `DELETE /api/admin/maintenance/tickets/closed`), это удаляет и `TicketComment`/ `TicketAttachment` (+ файлы на диске), необратимо.