From 4bfb0a1f14015e59c607f1b1f183bbcf38255acb Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sun, 6 Sep 2026 02:29:43 +0200 Subject: [PATCH] Support selective role deletion remediation --- ...cationRoleDeletionDependencyContributor.cs | 35 +++- .../Endpoints/Roles/Delete/Models.cs | 2 + .../Roles/Delete/RemediateAndDelete.cs | 6 +- .../Delete/RoleDeletionEndpointSupport.cs | 1 + .../Models/RoleDeletionModels.cs | 16 +- .../Services/RoleDeletionCoordinator.cs | 100 ++++++++++- .../RoleDeletionEndpointContractTests.cs | 105 +++++++++++ ...nRoleDeletionDependencyContributorTests.cs | 149 +++++++++++++++- .../Services/RoleDeletionCoordinatorTests.cs | 165 +++++++++++++++++- 9 files changed, 559 insertions(+), 20 deletions(-) create mode 100644 test/integration/Elsa.ExternalAuthentication.IntegrationTests/Identity/RoleDeletionEndpointContractTests.cs diff --git a/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs b/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs index 320b76df3..63a6479aa 100644 --- a/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs +++ b/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs @@ -21,6 +21,7 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( IIdentityProviderConnectionStore store, IOptionsMonitor options, IRoleAuthorizationService roleAuthorizationService, + IRoleStore roleStore, IConnectionRegistryVersionStore registryVersions, ConnectionRevisionCalculator revisionCalculator, ExternalAuthenticationSecurityNotifier notifier, @@ -84,7 +85,7 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( .Where(x => x.Ownership == RoleDeletionDependencyOwnership.Database) .Select(x => x.OwnerId) .ToHashSet(StringComparer.Ordinal); - if (!expectedOwners.SetEquals(currentOwners)) + if (!expectedOwners.IsSubsetOf(currentOwners)) return new RoleReferenceRemovalValidationResult.Conflict("dependency_changed"); foreach (var dependency in request.Dependencies) @@ -94,8 +95,20 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( !TryGetRoleReference(connection.UnlinkedPolicy, request.RoleId, out _, out var roleIds, out _)) return new RoleReferenceRemovalValidationResult.Conflict("connection_revision_changed"); var remainingRoleIds = roleIds.Where(x => !string.Equals(x, request.RoleId, StringComparison.Ordinal)).ToArray(); - if (!await roleAuthorizationService.CanAssignRolesAsync(request.Actor, remainingRoleIds, cancellationToken)) - return new RoleReferenceRemovalValidationResult.Forbidden("role_assignment_denied"); + var requiresReplacement = remainingRoleIds.Length == 0 && request.SelectedReferences is not null; + if (requiresReplacement && + (string.IsNullOrWhiteSpace(request.ReplacementRoleId) || + string.Equals(request.ReplacementRoleId, request.RoleId, StringComparison.Ordinal))) + return new RoleReferenceRemovalValidationResult.Forbidden("replacement_role_unavailable_or_unauthorized"); + + var rolesToAssign = requiresReplacement + ? new[] { request.ReplacementRoleId! } + : remainingRoleIds; + if (!await roleAuthorizationService.CanAssignRolesAsync(request.Actor, rolesToAssign, cancellationToken)) + { + var code = requiresReplacement ? "replacement_role_unavailable_or_unauthorized" : "role_assignment_denied"; + return new RoleReferenceRemovalValidationResult.Forbidden(code); + } } return new RoleReferenceRemovalValidationResult.Valid(); @@ -116,16 +129,24 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( { var connection = await store.FindByIdAsync(dependency.OwnerId, cancellationToken); if (connection is null || connection.Revision != dependency.ExpectedRevision || - !TryGetRoleReference(connection.UnlinkedPolicy, request.RoleId, out _, out _, out _)) + !TryGetRoleReference(connection.UnlinkedPolicy, request.RoleId, out _, out _, out var removesLastDefaultRole)) return new RoleReferenceRemovalResult.Conflict("connection_revision_changed", changedOwnerIds); var candidate = IdentityProviderConnectionCloner.Clone(connection); candidate.UnlinkedPolicy = candidate.UnlinkedPolicy is { } policy - ? policy with { Settings = RemoveRole(policy.Settings, request.RoleId) } + ? policy with { Settings = RemoveRole(policy.Settings, request.RoleId, request.SelectedReferences is not null ? request.ReplacementRoleId : null) } : null; candidate.UpdatedAt = DateTimeOffset.UtcNow; candidate.MaterialRevision = revisionCalculator.CalculateMaterialRevision(candidate); + if (request.SelectedReferences is not null && removesLastDefaultRole) + { + var replacement = await roleStore.FindAsync(new() { Id = request.ReplacementRoleId }, cancellationToken); + if (replacement is null || + !await roleAuthorizationService.CanAssignRolesAsync(request.Actor, [replacement.Id], cancellationToken)) + return new RoleReferenceRemovalResult.Failed("replacement_role_unavailable_or_unauthorized", changedOwnerIds); + } + var update = await store.UpdateAsync(candidate, connection.Revision, cancellationToken); if (update is not ConnectionMutationResult.Updated updated) return new RoleReferenceRemovalResult.Conflict("connection_revision_changed", changedOwnerIds); @@ -207,7 +228,7 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( return true; } - private static JsonElement RemoveRole(JsonElement settings, string roleId) + private static JsonElement RemoveRole(JsonElement settings, string roleId, string? replacementRoleId = null) { var root = settings.ValueKind == JsonValueKind.Object ? JsonNode.Parse(settings.GetRawText()) as JsonObject @@ -217,6 +238,8 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor( .Where(x => !string.Equals(x, roleId, StringComparison.Ordinal)) .Distinct(StringComparer.Ordinal) .ToArray(); + if (remainingRoleIds.Length == 0 && !string.IsNullOrWhiteSpace(replacementRoleId)) + remainingRoleIds = [replacementRoleId]; var roleNodes = new JsonNode?[remainingRoleIds.Length]; for (var index = 0; index < remainingRoleIds.Length; index++) roleNodes[index] = JsonValue.Create(remainingRoleIds[index]); diff --git a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/Models.cs b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/Models.cs index ab26ffc24..48b655fc9 100644 --- a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/Models.cs +++ b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/Models.cs @@ -8,6 +8,8 @@ internal sealed class RemediateRoleDeletionRequest public bool ConfirmRemoveFromEditableJitPolicies { get; set; } public bool ConfirmEmptyDefaultRoles { get; set; } public bool ConfirmBestEffort { get; set; } + public IReadOnlyCollection? SelectedReferences { get; set; } + public string? ReplacementRoleId { get; set; } } internal sealed record RoleDeletionErrorResponse(string Error, string Message, object? Details = null); diff --git a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RemediateAndDelete.cs b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RemediateAndDelete.cs index 68cc2b3f1..6c580f5d0 100644 --- a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RemediateAndDelete.cs +++ b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RemediateAndDelete.cs @@ -35,7 +35,11 @@ internal sealed class RemediateAndDelete(IRoleDeletionCoordinator coordinator) : request.ExpectedDependencyVersion, request.ConfirmRemoveFromEditableJitPolicies, request.ConfirmEmptyDefaultRoles, - request.ConfirmBestEffort), + request.ConfirmBestEffort) + { + SelectedReferences = request.SelectedReferences, + ReplacementRoleId = request.ReplacementRoleId + }, cancellationToken); await RoleDeletionEndpointSupport.SendOperationResultAsync(HttpContext, result, cancellationToken); } diff --git a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RoleDeletionEndpointSupport.cs b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RoleDeletionEndpointSupport.cs index 5e8bc049b..bae418285 100644 --- a/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RoleDeletionEndpointSupport.cs +++ b/src/modules/Elsa.Identity/Endpoints/Roles/Delete/RoleDeletionEndpointSupport.cs @@ -12,6 +12,7 @@ internal static class RoleDeletionEndpointSupport RoleDeletionOperationResult.Forbidden => SendErrorAsync(context, StatusCodes.Status403Forbidden, "forbidden", "The caller may not delete this role or update all affected policies.", null, cancellationToken), RoleDeletionOperationResult.Blocked blocked => SendErrorAsync(context, StatusCodes.Status409Conflict, "conflict", "The role is referenced by one or more policies.", new { code = "role_referenced_by_jit_policy", deletionImpact = RoleDeletionImpactResponse.From(blocked.Impact) }, cancellationToken), RoleDeletionOperationResult.PreconditionFailed conflict => SendErrorAsync(context, StatusCodes.Status409Conflict, "conflict", "The role dependencies changed. Inspect the current impact and retry.", new { code = "role_dependency_changed", deletionImpact = RoleDeletionImpactResponse.From(conflict.Impact) }, cancellationToken), + RoleDeletionOperationResult.ValidationFailed validation => SendErrorAsync(context, StatusCodes.Status400BadRequest, "validation_failed", "The role deletion request is invalid.", new { code = validation.Code, deletionImpact = RoleDeletionImpactResponse.From(validation.Impact) }, cancellationToken), RoleDeletionOperationResult.ConfirmationRequired confirmation => SendErrorAsync(context, StatusCodes.Status400BadRequest, "confirmation_required", "Explicit confirmation is required.", new { warnings = confirmation.Warnings, deletionImpact = RoleDeletionImpactResponse.From(confirmation.Impact) }, cancellationToken), RoleDeletionOperationResult.Incomplete incomplete => SendErrorAsync(context, StatusCodes.Status409Conflict, "conflict", "Role-policy remediation did not complete; the role was not deleted.", new { code = "role_remediation_incomplete", reason = incomplete.Code, changedOwnerIds = incomplete.ChangedOwnerIds, deletionImpact = RoleDeletionImpactResponse.From(incomplete.Impact) }, cancellationToken), _ => throw new InvalidOperationException("Unknown role-deletion operation result.") diff --git a/src/modules/Elsa.Identity/Models/RoleDeletionModels.cs b/src/modules/Elsa.Identity/Models/RoleDeletionModels.cs index c4beee273..07b46f7dd 100644 --- a/src/modules/Elsa.Identity/Models/RoleDeletionModels.cs +++ b/src/modules/Elsa.Identity/Models/RoleDeletionModels.cs @@ -34,6 +34,9 @@ public sealed record RoleDeletionDependencySnapshot( bool SupportsAtomicRemoval, IReadOnlyCollection Dependencies); +/// Identifies one editable dependency reference selected for role remediation. +public sealed record RoleDeletionReferenceSelection(string Source, string OwnerId); + /// The aggregated impact of deleting a role. public sealed record RoleDeletionImpact( string RoleId, @@ -48,7 +51,11 @@ public sealed record RoleReferenceRemovalRequest( string RoleId, ClaimsPrincipal Actor, string ExpectedContributorVersion, - IReadOnlyCollection Dependencies); + IReadOnlyCollection Dependencies) +{ + public IReadOnlyCollection? SelectedReferences { get; init; } + public string? ReplacementRoleId { get; init; } +} /// Inputs for the coordinated remediation command. public sealed record RoleDeletionRemediationCommand( @@ -57,7 +64,11 @@ public sealed record RoleDeletionRemediationCommand( string ExpectedDependencyVersion, bool ConfirmRemoveFromEditablePolicies, bool ConfirmEmptyDefaultRoles, - bool ConfirmBestEffort); + bool ConfirmBestEffort) +{ + public IReadOnlyCollection? SelectedReferences { get; init; } + public string? ReplacementRoleId { get; init; } +} public abstract record RoleReferenceRemovalValidationResult { @@ -103,6 +114,7 @@ public abstract record RoleDeletionOperationResult public sealed record Forbidden : RoleDeletionOperationResult; public sealed record Blocked(RoleDeletionImpact Impact) : RoleDeletionOperationResult; public sealed record PreconditionFailed(RoleDeletionImpact Impact) : RoleDeletionOperationResult; + public sealed record ValidationFailed(RoleDeletionImpact Impact, string Code) : RoleDeletionOperationResult; public sealed record ConfirmationRequired(RoleDeletionImpact Impact, IReadOnlyCollection Warnings) : RoleDeletionOperationResult; public sealed record Incomplete(RoleDeletionImpact Impact, IReadOnlyCollection ChangedOwnerIds, string Code) : RoleDeletionOperationResult; } diff --git a/src/modules/Elsa.Identity/Services/RoleDeletionCoordinator.cs b/src/modules/Elsa.Identity/Services/RoleDeletionCoordinator.cs index 281fe85cc..ba3f9875e 100644 --- a/src/modules/Elsa.Identity/Services/RoleDeletionCoordinator.cs +++ b/src/modules/Elsa.Identity/Services/RoleDeletionCoordinator.cs @@ -59,13 +59,22 @@ public sealed class RoleDeletionCoordinator( return new RoleDeletionOperationResult.PreconditionFailed(impact); if (impact.Dependencies.Any(x => x.Ownership == RoleDeletionDependencyOwnership.Configuration)) return new RoleDeletionOperationResult.Blocked(impact); + var selectionError = ValidateSelectedReferences(impact, command.SelectedReferences); + if (selectionError is not null) + return new RoleDeletionOperationResult.ValidationFailed(impact, selectionError); + if (impact.CanDelete) { await roleStore.DeleteAsync(new() { Id = command.RoleId }, cancellationToken); return new RoleDeletionOperationResult.Deleted([]); } - var warnings = GetRequiredConfirmations(impact, command); + var selectedDependencies = SelectEditableDependencies(impact, command.SelectedReferences); + var replacementValidation = await ValidateReplacementRoleAsync(impact, command, selectedDependencies, cancellationToken); + if (replacementValidation is not null) + return replacementValidation; + + var warnings = GetRequiredConfirmations(impact, command, selectedDependencies); if (warnings.Count != 0) return new RoleDeletionOperationResult.ConfirmationRequired(impact, warnings); @@ -75,12 +84,16 @@ public sealed class RoleDeletionCoordinator( return new RoleDeletionOperationResult.PreconditionFailed(currentImpact); var requests = snapshots - .Where(x => x.Dependencies.Any(dependency => dependency.Ownership == RoleDeletionDependencyOwnership.Database)) + .Where(x => x.Dependencies.Any(dependency => dependency.Ownership == RoleDeletionDependencyOwnership.Database && IsSelected(dependency, command.SelectedReferences))) .Select(snapshot => new RoleReferenceRemovalRequest( command.RoleId, command.Actor, snapshot.Version, - snapshot.Dependencies.Where(x => x.Ownership == RoleDeletionDependencyOwnership.Database).ToArray())) + snapshot.Dependencies.Where(x => x.Ownership == RoleDeletionDependencyOwnership.Database && IsSelected(x, command.SelectedReferences)).ToArray()) + { + SelectedReferences = command.SelectedReferences, + ReplacementRoleId = command.ReplacementRoleId + }) .ToArray(); foreach (var request in requests) @@ -176,18 +189,91 @@ public sealed class RoleDeletionCoordinator( return $"role-dependencies-{Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(payload))).ToLowerInvariant()}"; } - private static IReadOnlyCollection GetRequiredConfirmations(RoleDeletionImpact impact, RoleDeletionRemediationCommand command) + private static IReadOnlyCollection GetRequiredConfirmations( + RoleDeletionImpact impact, + RoleDeletionRemediationCommand command, + IReadOnlyCollection selectedDependencies) { var warnings = new List(); - if (!command.ConfirmRemoveFromEditablePolicies) + var selective = command.SelectedReferences is not null; + var remediationRequested = !selective || selectedDependencies.Count != 0; + if (remediationRequested && !command.ConfirmRemoveFromEditablePolicies) warnings.Add("confirm_remove_from_editable_jit_policies"); - if (impact.Dependencies.Any(x => x.RemovesLastDefaultRole) && !command.ConfirmEmptyDefaultRoles) + var removesLastDefaultRole = selective + ? selectedDependencies.Any(x => x.RemovesLastDefaultRole) + : impact.Dependencies.Any(x => x.RemovesLastDefaultRole); + if (!selective && removesLastDefaultRole && !command.ConfirmEmptyDefaultRoles) warnings.Add("removes_last_default_role"); - if (impact.ExecutionMode == RoleDeletionExecutionMode.BestEffort && !command.ConfirmBestEffort) + if (remediationRequested && impact.ExecutionMode == RoleDeletionExecutionMode.BestEffort && !command.ConfirmBestEffort) warnings.Add("confirm_best_effort"); return warnings; } + private static IReadOnlyCollection SelectEditableDependencies( + RoleDeletionImpact impact, + IReadOnlyCollection? selectedReferences) => + impact.Dependencies + .Where(x => x.Ownership == RoleDeletionDependencyOwnership.Database && IsSelected(x, selectedReferences)) + .ToArray(); + + private async ValueTask ValidateReplacementRoleAsync( + RoleDeletionImpact impact, + RoleDeletionRemediationCommand command, + IReadOnlyCollection selectedDependencies, + CancellationToken cancellationToken) + { + if (command.SelectedReferences is null || !selectedDependencies.Any(x => x.RemovesLastDefaultRole)) + return null; + + if (string.IsNullOrWhiteSpace(command.ReplacementRoleId)) + return new RoleDeletionOperationResult.ValidationFailed(impact, "replacement_role_required"); + if (string.Equals(command.ReplacementRoleId, command.RoleId, StringComparison.Ordinal)) + return new RoleDeletionOperationResult.ValidationFailed(impact, "replacement_role_must_differ"); + + var replacement = await roleStore.FindAsync(new() { Id = command.ReplacementRoleId }, cancellationToken); + if (replacement is null) + return new RoleDeletionOperationResult.ValidationFailed(impact, "replacement_role_not_found"); + if (!await roleAuthorizationService.CanAssignRolesAsync(command.Actor, [replacement.Id], cancellationToken)) + return new RoleDeletionOperationResult.Forbidden(); + + return null; + } + + private static bool IsSelected(RoleDeletionDependency dependency, IReadOnlyCollection? selectedReferences) => + selectedReferences is null || selectedReferences.Any(x => + string.Equals(x.Source, dependency.Source, StringComparison.Ordinal) && + string.Equals(x.OwnerId, dependency.OwnerId, StringComparison.Ordinal)); + + private static string? ValidateSelectedReferences( + RoleDeletionImpact impact, + IReadOnlyCollection? selectedReferences) + { + if (selectedReferences is null) + return null; + + var seen = new HashSet(StringComparer.Ordinal); + foreach (var selection in selectedReferences) + { + if (selection is null || string.IsNullOrWhiteSpace(selection.Source) || string.IsNullOrWhiteSpace(selection.OwnerId)) + return "invalid_reference_selection"; + + var key = $"{selection.Source}\n{selection.OwnerId}"; + if (!seen.Add(key)) + return "duplicate_reference"; + + var matches = impact.Dependencies + .Where(x => string.Equals(x.Source, selection.Source, StringComparison.Ordinal) && + string.Equals(x.OwnerId, selection.OwnerId, StringComparison.Ordinal)) + .ToArray(); + if (matches.Length == 0) + return "unknown_reference"; + if (matches.Any(x => x.Ownership != RoleDeletionDependencyOwnership.Database)) + return "configuration_reference_not_editable"; + } + + return null; + } + private async ValueTask GetCurrentImpactAsync(string roleId, CancellationToken cancellationToken) => CreateImpact(roleId, await InspectContributorsAsync(roleId, cancellationToken)); diff --git a/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Identity/RoleDeletionEndpointContractTests.cs b/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Identity/RoleDeletionEndpointContractTests.cs new file mode 100644 index 000000000..6e5796103 --- /dev/null +++ b/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Identity/RoleDeletionEndpointContractTests.cs @@ -0,0 +1,105 @@ +using System.Net; +using System.Net.Http.Json; +using System.Security.Claims; +using Elsa.Authorization; +using Elsa.Identity.Contracts; +using Elsa.Identity.Features; +using Elsa.Identity.Models; +using FastEndpoints; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Elsa.ExternalAuthentication.IntegrationTests.Fixtures; + +namespace Elsa.ExternalAuthentication.IntegrationTests.Identity; + +[Collection(nameof(EndpointSecurityCollection))] +public sealed class RoleDeletionEndpointContractTests : IAsyncLifetime +{ + private WebApplication? _app; + private HttpClient? _client; + private bool _wasSecurityEnabled; + private readonly CapturingRoleDeletionCoordinator _coordinator = new(); + + public async Task InitializeAsync() + { + _wasSecurityEnabled = EndpointSecurityOptions.SecurityIsEnabled; + EndpointSecurityOptions.SecurityIsEnabled = false; + + var builder = WebApplication.CreateSlimBuilder(); + builder.WebHost.UseTestServer(); + builder.Services.AddFastEndpoints(options => + { + options.Assemblies = [typeof(IdentityFeature).Assembly]; + options.Filter = endpoint => endpoint.Namespace == "Elsa.Identity.Endpoints.Roles.Delete"; + }); + builder.Services.AddAuthorization(); + builder.Services.AddSingleton(_coordinator); + + _app = builder.Build(); + _app.UseAuthorization(); + _app.UseFastEndpoints(); + await _app.StartAsync(); + _client = _app.GetTestClient(); + } + + public async Task DisposeAsync() + { + EndpointSecurityOptions.SecurityIsEnabled = _wasSecurityEnabled; + _client?.Dispose(); + if (_app is not null) + { + await _app.StopAsync(); + await _app.DisposeAsync(); + } + } + + [Fact] + public async Task RemediationBindsSelectedReferencesAndReplacementRole() + { + var response = await _client!.PostAsJsonAsync( + "/identity/roles/target-role/remove-from-jit-policies-and-delete", + new + { + expectedDependencyVersion = "dependency-version", + confirmRemoveFromEditableJitPolicies = true, + confirmEmptyDefaultRoles = true, + confirmBestEffort = true, + selectedReferences = new[] { new { source = "external-authentication", ownerId = "connection-a" } }, + replacementRoleId = "replacement-role" + }); + + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + Assert.NotNull(_coordinator.Command); + Assert.Equal("target-role", _coordinator.Command.RoleId); + Assert.Equal("dependency-version", _coordinator.Command.ExpectedDependencyVersion); + Assert.Equal( + new RoleDeletionReferenceSelection("external-authentication", "connection-a"), + Assert.Single(_coordinator.Command.SelectedReferences!)); + Assert.Equal("replacement-role", _coordinator.Command.ReplacementRoleId); + } + + private sealed class CapturingRoleDeletionCoordinator : IRoleDeletionCoordinator + { + public RoleDeletionRemediationCommand? Command { get; private set; } + + public ValueTask InspectAsync(string roleId, ClaimsPrincipal actor, CancellationToken cancellationToken = default) => + ValueTask.FromResult(new RoleDeletionInspectionResult.NotFound()); + + public ValueTask DeleteAsync(string roleId, ClaimsPrincipal actor, CancellationToken cancellationToken = default) => + ValueTask.FromResult(new RoleDeletionOperationResult.NotFound()); + + public ValueTask RemediateAndDeleteAsync(RoleDeletionRemediationCommand command, CancellationToken cancellationToken = default) + { + Command = command; + var impact = new RoleDeletionImpact( + command.RoleId, + command.ExpectedDependencyVersion, + RoleDeletionExecutionMode.BestEffort, + false, + true, + []); + return ValueTask.FromResult(new RoleDeletionOperationResult.Incomplete(impact, [], "role_dependencies_remain")); + } + } +} diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs index 6096f03c5..ae3fcd053 100644 --- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs +++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs @@ -9,6 +9,7 @@ using Elsa.ExternalAuthentication.Permissions; using Elsa.ExternalAuthentication.Policies; using Elsa.ExternalAuthentication.Services; using Elsa.ExternalAuthentication.Stores.InMemory; +using Elsa.Identity.Contracts; using Elsa.Identity.Entities; using Elsa.Identity.Models; using Elsa.Identity.Providers; @@ -108,6 +109,114 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests Assert.True(await versions.GetVersionAsync() > 0); } + [Fact] + public async Task ReplacesFinalDefaultRoleWhenSelectedForRemediation() + { + var databaseConnection = Connection( + "database", + new PolicySelection( + CreateUserUnlinkedIdentityPolicy.PolicyType, + 1, + JsonSerializer.SerializeToElement(new { defaultRoleIds = new[] { "workflow-user" } }))); + var (contributor, store, _) = await CreateContributorAsync([], [databaseConnection], [new Role { Id = "replacement-role", Name = "Replacement role", Permissions = [] }]); + var snapshot = await contributor.InspectAsync("workflow-user"); + var request = new RoleReferenceRemovalRequest( + "workflow-user", + Administrator(), + snapshot.Version, + snapshot.Dependencies) + { + SelectedReferences = [new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, databaseConnection.Id)], + ReplacementRoleId = "replacement-role" + }; + + Assert.IsType(await contributor.ValidateRemovalAsync(request)); + var result = Assert.IsType(await contributor.RemoveEditableReferencesAsync(request)); + + Assert.Equal([databaseConnection.Id], result.ChangedOwnerIds); + var updated = Assert.IsType(await store.FindByIdAsync(databaseConnection.Id)); + Assert.Equal(["replacement-role"], updated.UnlinkedPolicy!.Settings.GetProperty("defaultRoleIds").EnumerateArray().Select(x => x.GetString()!).ToArray()); + } + + [Fact] + public async Task RejectsMissingReplacementForSelectedFinalDefaultRole() + { + var databaseConnection = Connection( + "database", + new PolicySelection( + CreateUserUnlinkedIdentityPolicy.PolicyType, + 1, + JsonSerializer.SerializeToElement(new { defaultRoleIds = new[] { "workflow-user" } }))); + var (contributor, store, _) = await CreateContributorAsync([], databaseConnection); + var snapshot = await contributor.InspectAsync("workflow-user"); + var request = new RoleReferenceRemovalRequest( + "workflow-user", + Administrator(), + snapshot.Version, + snapshot.Dependencies) + { + SelectedReferences = [new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, databaseConnection.Id)] + }; + + var result = await contributor.ValidateRemovalAsync(request); + + var forbidden = Assert.IsType(result); + Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code); + var current = Assert.IsType(await store.FindByIdAsync(databaseConnection.Id)); + Assert.Equal(["workflow-user"], current.UnlinkedPolicy!.Settings.GetProperty("defaultRoleIds").EnumerateArray().Select(x => x.GetString()!).ToArray()); + } + + [Fact] + public async Task ReplacementRemovedAfterCoordinatorValidationFailsClosedBeforePolicyUpdate() + { + var databaseConnection = Connection( + "database", + new PolicySelection( + CreateUserUnlinkedIdentityPolicy.PolicyType, + 1, + JsonSerializer.SerializeToElement(new { defaultRoleIds = new[] { "workflow-user" } }))); + var connectionStore = new InMemoryIdentityProviderConnectionStore(); + Assert.IsType(await connectionStore.CreateAsync(databaseConnection)); + + var backingRoleStore = new MemoryRoleStore(new MemoryStore(), TestTenantAccessor.Default); + await backingRoleStore.SaveAsync(new Role { Id = "workflow-user", Name = "Workflow user", Permissions = [] }); + await backingRoleStore.SaveAsync(new Role { Id = "replacement-role", Name = "Replacement role", Permissions = [] }); + var roleStore = new RoleStoreThatRemovesReplacementAfterContributorValidation(backingRoleStore, "replacement-role"); + var roleAuthorizationService = new RoleAuthorizationService(new StoreBasedRoleProvider(roleStore), new PermissionEvaluator()); + var versions = new InMemoryConnectionRegistryVersionStore(); + var services = new ServiceCollection().BuildServiceProvider(); + var contributor = new ExternalAuthenticationRoleDeletionDependencyContributor( + connectionStore, + new MutableOptionsMonitor(new ExternalAuthenticationOptions()), + roleAuthorizationService, + roleStore, + versions, + new ConnectionRevisionCalculator(), + new ExternalAuthenticationSecurityNotifier(services), + new PermissionEvaluator()); + var coordinator = new RoleDeletionCoordinator(roleStore, roleAuthorizationService, [contributor]); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + + var result = await coordinator.RemediateAndDeleteAsync(new RoleDeletionRemediationCommand( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = [new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, databaseConnection.Id)], + ReplacementRoleId = "replacement-role" + }); + + var incomplete = Assert.IsType(result); + Assert.Equal("replacement_role_unavailable_or_unauthorized", incomplete.Code); + Assert.True(roleStore.ReplacementRemoved); + Assert.NotNull(await roleStore.FindAsync(new() { Id = "workflow-user" })); + var current = Assert.IsType(await connectionStore.FindByIdAsync(databaseConnection.Id)); + Assert.Equal(["workflow-user"], current.UnlinkedPolicy!.Settings.GetProperty("defaultRoleIds").EnumerateArray().Select(x => x.GetString()!).ToArray()); + } + [Fact] public async Task StaleConnectionRevisionFailsPrevalidationWithoutMutation() { @@ -158,9 +267,15 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests Assert.IsType(result); } + private static Task<(ExternalAuthenticationRoleDeletionDependencyContributor Contributor, InMemoryIdentityProviderConnectionStore Store, InMemoryConnectionRegistryVersionStore Versions)> CreateContributorAsync( + IReadOnlyCollection configuredConnections, + params IdentityProviderConnection[] databaseConnections) => + CreateContributorAsync(configuredConnections, databaseConnections, null); + private static async Task<(ExternalAuthenticationRoleDeletionDependencyContributor Contributor, InMemoryIdentityProviderConnectionStore Store, InMemoryConnectionRegistryVersionStore Versions)> CreateContributorAsync( IReadOnlyCollection configuredConnections, - params IdentityProviderConnection[] databaseConnections) + IdentityProviderConnection[] databaseConnections, + IReadOnlyCollection? additionalRoles = null) { var store = new InMemoryIdentityProviderConnectionStore(); foreach (var connection in databaseConnections) @@ -169,12 +284,15 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests var roleStore = new MemoryRoleStore(new MemoryStore(), TestTenantAccessor.Default); await roleStore.SaveAsync(new Role { Id = "workflow-user", Name = "Workflow user", Permissions = [] }); await roleStore.SaveAsync(new Role { Id = "other-role", Name = "Other role", Permissions = [] }); + foreach (var role in additionalRoles ?? []) + await roleStore.SaveAsync(role); var versions = new InMemoryConnectionRegistryVersionStore(); var services = new ServiceCollection().BuildServiceProvider(); var contributor = new ExternalAuthenticationRoleDeletionDependencyContributor( store, new MutableOptionsMonitor(new ExternalAuthenticationOptions { ConfigurationConnections = configuredConnections.ToList() }), new RoleAuthorizationService(new StoreBasedRoleProvider(roleStore), new PermissionEvaluator()), + roleStore, versions, new ConnectionRevisionCalculator(), new ExternalAuthenticationSecurityNotifier(services), @@ -215,4 +333,33 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests .Where(x => !string.Equals(x, omittedPermission, StringComparison.Ordinal)) .Select(x => new Claim(PermissionNames.ClaimType, x)))); } + + private sealed class RoleStoreThatRemovesReplacementAfterContributorValidation( + MemoryRoleStore inner, + string replacementRoleId) : IRoleStore + { + private int _replacementChecks; + + public bool ReplacementRemoved { get; private set; } + + public Task AddAsync(Role role, CancellationToken cancellationToken = default) => inner.AddAsync(role, cancellationToken); + + public Task DeleteAsync(RoleFilter filter, CancellationToken cancellationToken = default) => inner.DeleteAsync(filter, cancellationToken); + + public Task SaveAsync(Role role, CancellationToken cancellationToken = default) => inner.SaveAsync(role, cancellationToken); + + public Task FindAsync(RoleFilter filter, CancellationToken cancellationToken = default) => inner.FindAsync(filter, cancellationToken); + + public async Task> FindManyAsync(RoleFilter filter, CancellationToken cancellationToken = default) + { + var roles = (await inner.FindManyAsync(filter, cancellationToken)).ToArray(); + if (!ReplacementRemoved && filter.Ids?.Contains(replacementRoleId) == true && Interlocked.Increment(ref _replacementChecks) == 2) + { + await inner.DeleteAsync(new() { Id = replacementRoleId }, cancellationToken); + ReplacementRemoved = true; + } + + return roles; + } + } } diff --git a/test/unit/Elsa.Identity.UnitTests/Services/RoleDeletionCoordinatorTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/RoleDeletionCoordinatorTests.cs index f62ca55d6..8c700a586 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/RoleDeletionCoordinatorTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/RoleDeletionCoordinatorTests.cs @@ -12,6 +12,32 @@ namespace Elsa.Identity.UnitTests.Services; public class RoleDeletionCoordinatorTests { + [Fact] + public void RoleRemediationContractsRetainTheirLegacyConstructors() + { + var removalConstructor = typeof(RoleReferenceRemovalRequest).GetConstructor([ + typeof(string), + typeof(ClaimsPrincipal), + typeof(string), + typeof(IReadOnlyCollection)]); + var commandConstructor = typeof(RoleDeletionRemediationCommand).GetConstructor([ + typeof(string), + typeof(ClaimsPrincipal), + typeof(string), + typeof(bool), + typeof(bool), + typeof(bool)]); + + Assert.NotNull(removalConstructor); + Assert.NotNull(commandConstructor); + Assert.Single(typeof(RoleReferenceRemovalRequest).GetConstructors()); + Assert.Single(typeof(RoleDeletionRemediationCommand).GetConstructors()); + Assert.NotNull(typeof(RoleReferenceRemovalRequest).GetProperty(nameof(RoleReferenceRemovalRequest.SelectedReferences))); + Assert.NotNull(typeof(RoleReferenceRemovalRequest).GetProperty(nameof(RoleReferenceRemovalRequest.ReplacementRoleId))); + Assert.NotNull(typeof(RoleDeletionRemediationCommand).GetProperty(nameof(RoleDeletionRemediationCommand.SelectedReferences))); + Assert.NotNull(typeof(RoleDeletionRemediationCommand).GetProperty(nameof(RoleDeletionRemediationCommand.ReplacementRoleId))); + } + [Fact] public async Task InspectionRequiresDeleteRolePermission() { @@ -130,6 +156,138 @@ public class RoleDeletionCoordinatorTests Assert.Single(contributor.Dependencies); } + [Fact] + public async Task SelectiveRemediationChangesOnlySelectedDependenciesAndRetainsRoleWhenOthersRemain() + { + var contributor = new StubContributor([Dependency("connection-a"), Dependency("connection-b")]); + var (store, coordinator) = await CreateCoordinatorAsync(contributor); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + + var result = await coordinator.RemediateAndDeleteAsync(new( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = [new RoleDeletionReferenceSelection(StubContributor.SourceName, "connection-a")] + }); + + var incomplete = Assert.IsType(result); + Assert.Equal(["connection-a"], incomplete.ChangedOwnerIds); + Assert.NotNull(await store.FindAsync(new() { Id = "workflow-user" })); + Assert.Equal(["connection-b"], contributor.Dependencies.Select(x => x.OwnerId).ToArray()); + } + + [Fact] + public async Task ExplicitEmptySelectionDoesNotMutateAndReturnsIncomplete() + { + var contributor = new StubContributor([Dependency("connection-a")]); + var (store, coordinator) = await CreateCoordinatorAsync(contributor); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + + var result = await coordinator.RemediateAndDeleteAsync(new( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = [] + }); + + var incomplete = Assert.IsType(result); + Assert.Empty(incomplete.ChangedOwnerIds); + Assert.NotNull(await store.FindAsync(new() { Id = "workflow-user" })); + Assert.Single(contributor.Dependencies); + } + + [Fact] + public async Task SelectedFinalDefaultRequiresCoordinatorValidatedReplacementBeforeMutation() + { + var contributor = new StubContributor([Dependency("connection-a", removesLastDefaultRole: true)]); + var (store, coordinator) = await CreateCoordinatorAsync(contributor); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + + var result = await coordinator.RemediateAndDeleteAsync(new( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = [new RoleDeletionReferenceSelection(StubContributor.SourceName, "connection-a")], + ReplacementRoleId = "replacement-role" + }); + + var validation = Assert.IsType(result); + Assert.Equal("replacement_role_not_found", validation.Code); + Assert.NotNull(await store.FindAsync(new() { Id = "workflow-user" })); + Assert.Single(contributor.Dependencies); + } + + [Fact] + public async Task ConfigurationDependencyBlocksSelectiveDatabaseRemediation() + { + var contributor = new StubContributor([ + Dependency("configuration", RoleDeletionDependencyOwnership.Configuration, configurationPath: "ExternalAuthentication:Connections:0"), + Dependency("connection-a") + ]); + var (store, coordinator) = await CreateCoordinatorAsync(contributor); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + + var result = await coordinator.RemediateAndDeleteAsync(new( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = [new RoleDeletionReferenceSelection(StubContributor.SourceName, "connection-a")] + }); + + Assert.IsType(result); + Assert.NotNull(await store.FindAsync(new() { Id = "workflow-user" })); + Assert.Equal(2, contributor.Dependencies.Count); + } + + [Theory] + [InlineData("unknown", "connection-a", "unknown_reference")] + [InlineData(StubContributor.SourceName, "connection-a", "duplicate_reference")] + public async Task InvalidSelectionFailsClosedWithoutMutation(string source, string ownerId, string expectedCode) + { + var contributor = new StubContributor([Dependency("connection-a")]); + var (store, coordinator) = await CreateCoordinatorAsync(contributor); + var impact = Assert.IsType(await coordinator.InspectAsync("workflow-user", Administrator())).Impact; + var selections = expectedCode == "duplicate_reference" + ? new[] + { + new RoleDeletionReferenceSelection(source, ownerId), + new RoleDeletionReferenceSelection(source, ownerId) + } + : new[] { new RoleDeletionReferenceSelection(source, ownerId) }; + + var result = await coordinator.RemediateAndDeleteAsync(new( + "workflow-user", + Administrator(), + impact.DependencyVersion, + true, + true, + true) + { + SelectedReferences = selections + }); + + var validation = Assert.IsType(result); + Assert.Equal(expectedCode, validation.Code); + Assert.NotNull(await store.FindAsync(new() { Id = "workflow-user" })); + Assert.Single(contributor.Dependencies); + } + private static async Task<(MemoryRoleStore Store, RoleDeletionCoordinator Coordinator)> CreateCoordinatorAsync(IRoleDeletionDependencyContributor contributor) { var store = new MemoryRoleStore(new MemoryStore(), TestTenantAccessor.Default); @@ -177,15 +335,16 @@ public class RoleDeletionCoordinatorTests public ValueTask RemoveEditableReferencesAsync(RoleReferenceRemovalRequest request, CancellationToken cancellationToken = default) { + var selectedOwnerIds = request.Dependencies.Select(x => x.OwnerId).ToHashSet(StringComparer.Ordinal); if (failAfterFirst) { - var changed = Dependencies.OrderBy(x => x.OwnerId, StringComparer.Ordinal).First(); + var changed = Dependencies.Where(x => selectedOwnerIds.Contains(x.OwnerId)).OrderBy(x => x.OwnerId, StringComparer.Ordinal).First(); Dependencies = Dependencies.Where(x => !string.Equals(x.OwnerId, changed.OwnerId, StringComparison.Ordinal)).ToArray(); return ValueTask.FromResult(new RoleReferenceRemovalResult.Failed("simulated", [changed.OwnerId])); } - var changedOwnerIds = Dependencies.Select(x => x.OwnerId).ToArray(); - Dependencies = []; + var changedOwnerIds = Dependencies.Where(x => selectedOwnerIds.Contains(x.OwnerId)).Select(x => x.OwnerId).ToArray(); + Dependencies = Dependencies.Where(x => !selectedOwnerIds.Contains(x.OwnerId)).ToArray(); return ValueTask.FromResult(new RoleReferenceRemovalResult.Success(changedOwnerIds)); }