fix(external-authentication): scope role-deletion impact to the role's tenant (#8036)
* fix(external-authentication): scope role-deletion impact to the role's tenant ExternalAuthenticationRoleDeletionDependencyContributor scanned every stored connection with an empty ConnectionFilter and every configured connection regardless of its tenant, so a role ID that exists in two tenants could report another tenant's references as its own impact -- and a configuration entry owned by another tenant could block a role deletion outright. Remediation had the same reach: it loaded a dependency's connection by the caller-supplied owner ID without checking which tenant owned it. Impact, prevalidation and remediation now only see connections in the role's tenant context, which is the tenant active on ITenantAccessor while the role-deletion coordinator runs. Host-scoped connections stay in scope for every tenant, because the connection registry resolves the host scope for every signing-in tenant and the provisioner resolves a connection's default role IDs in the signing-in user's tenant, so a host connection naming a role ID really does reference that tenant's role. Configuration entries that leave the tenant blank are host-scoped for the same reason the configuration source materializes them there. A connection carrying another tenant's ID is out of scope in both directions, and a connection loaded for remediation that is not in the role's tenant is treated as absent, which fails the request rather than mutating it. The stored connections are fetched per applicable scope so another tenant's rows are never materialized, and both connection stores already honor ConnectionFilter.Scope; the durable store now has a test pinning that, since the tenant boundary rests on it. Refs #8013 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): scan every tenant when deleting a tenant-agnostic role Role stores expose tenant-agnostic roles (TenantId == "*") from every tenant, but the role-deletion contributor derived its dependency scan boundary from the ambient tenant only, so deleting an agnostic role while tenant A was active left references from other tenants dangling. Resolve the role being deleted once per operation, through the active role store, and scan every connection and configuration entry regardless of tenant when it is agnostic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor(external-authentication): share the active role store lookup Extract the duplicated "active role store is the last registration" resolution into a single ActiveRoleStore accessor and rename ToScope to ToConnectionScope for clarity. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): read one connection snapshot and prefer the agnostic role Reading the host and tenant scopes as two separate store queries let a connection whose TenantId changed mid-flight fall between the reads and escape both, letting role deletion proceed while a reference remained. FindConnectionsInRoleTenantScopeAsync now reads one snapshot and filters it in memory. IsAgnosticRoleAsync resolved a role by an unqualified ID lookup, which could return the ambient tenant's role instead of an agnostic role sharing its ID, silently narrowing impact scanning and leaving JIT-policy references in other tenants dangling; it now checks every role sharing the ID and gives the agnostic scope deterministic precedence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(external-authentication): correct the scope-filter test comment Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): fail closed when a role ID resolves to more than one role A same-ID collision between a tenant-scoped role and an agnostic role can only occur in MemoryRoleStore (durable persistence keys roles by ID alone). In that case the coordinator's own deletion target is already ambiguous, so widening or narrowing the scope by guessing is wrong in either direction; throw instead of picking a side. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): scope role-deletion impact by the resolved role's tenant Replace the isAgnosticRole flag with ResolveRoleTenantIdAsync, which returns the resolved role's own TenantId and falls back to the ambient tenant only when the role cannot be resolved. With multitenancy disabled the EF role store installs no tenant query filter and can resolve a tenant-owned role by ID regardless of the ambient tenant, so scoping by the ambient tenant alone left that role's connection references out of scan while the coordinator deleted it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): require an agnostic replacement when remediating an agnostic role Authorization for a replacement role still resolves through the ambient tenant's role services, so a deletion initiated in tenant A could authorize a tenant-A-only replacement and then write it into tenant B's connection policy, where that role does not exist. When the deletion target is agnostic, require the replacement role to be agnostic too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): require agnostic replacements for host connections and reject ambiguous ones Extend the agnostic-replacement requirement to host-scoped connections, since a host connection is served to every signing-in tenant and a tenant-scoped replacement would resolve in the authorizing tenant but fail to resolve in every other tenant it serves. Recheck the replacement at removal time through the same agnostic-role resolution used at validation, instead of trusting whichever same-ID role a plain FindAsync happens to return, so a replacement collision introduced between validation and mutation is rejected. Resolve IsAgnosticRoleAsync's candidate directly and return true only when exactly one matching role is agnostic, so an ambiguous replacement ID is reported as replacement_role_unavailable_or_unauthorized instead of escaping as an exception. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(external-authentication): keep host-connection replacements allowed for default-tenant roles Revert the host-scope replacement guard added for host-scoped connections. IdentityProviderConnectionManagementService forces every managed connection to host scope, and in a deployment without multitenancy roles are created scoped to the default tenant rather than agnostic, so requiring an agnostic replacement for host-scoped connections would make every replacement remediation impossible in the default deployment. The replacement guard applies only when the deletion target itself is agnostic, as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Sipke Schoorstra
GitHub
parent
54c8fda65f
commit
85e5fc083c
|
|
@ -1,3 +1,4 @@
|
|||
using Elsa.Common.Multitenancy;
|
||||
using Elsa.Extensions;
|
||||
using Elsa.ExternalAuthentication.Contracts;
|
||||
using Elsa.ExternalAuthentication.Options;
|
||||
|
|
@ -58,6 +59,11 @@ public static class ServiceCollectionExtensions
|
|||
services.AddRateLimiter(_ => { });
|
||||
services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<RateLimiterOptions>, ConfigureExternalAuthenticationRateLimiterOptions>());
|
||||
|
||||
// The module reads the ambient tenant outside the multitenancy feature -- connection scoping and the
|
||||
// role-deletion contributor's tenant boundary -- so it depends on an accessor whether or not a host
|
||||
// enabled multitenancy. TryAdd keeps a host's own registration.
|
||||
services.TryAddSingleton<ITenantAccessor, DefaultTenantAccessor>();
|
||||
|
||||
services.TryAddSingleton<ConnectionRevisionCalculator>();
|
||||
services.TryAddSingleton<FinalLoginPathGuard>();
|
||||
services.TryAddSingleton<ExternalAuthenticationSecurityNotifier>();
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ using System.Text;
|
|||
using System.Text.Json;
|
||||
using System.Text.Json.Nodes;
|
||||
using Elsa.Authorization;
|
||||
using Elsa.Common.Multitenancy;
|
||||
using Elsa.ExternalAuthentication.Contracts;
|
||||
using Elsa.ExternalAuthentication.Models;
|
||||
using Elsa.ExternalAuthentication.Notifications;
|
||||
|
|
@ -11,12 +12,43 @@ using Elsa.ExternalAuthentication.Options;
|
|||
using Elsa.ExternalAuthentication.Permissions;
|
||||
using Elsa.ExternalAuthentication.Policies;
|
||||
using Elsa.Identity.Contracts;
|
||||
using Elsa.Identity.Entities;
|
||||
using Elsa.Identity.Models;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace Elsa.ExternalAuthentication.Services;
|
||||
|
||||
/// <summary>Guards Elsa Role deletion against JIT-policy default-role references.</summary>
|
||||
/// <remarks>
|
||||
/// Roles are tenant-scoped, so impact and remediation only ever consider connections in the role's own tenant
|
||||
/// context: the resolved role's own <c>TenantId</c>. The tenant active on <see cref="ITenantAccessor"/> is used
|
||||
/// only as a fallback when the role cannot be resolved, because with multitenancy disabled (the default) the EF
|
||||
/// Core role store installs no tenant query filter and can resolve a tenant-owned role by ID regardless of the
|
||||
/// ambient tenant; trusting the ambient tenant instead of the resolved role's tenant would then let this
|
||||
/// contributor scan the wrong tenant's connections while the coordinator deletes a role belonging to another
|
||||
/// tenant. A connection carrying another tenant's ID is out of scope in both directions, for impact and for
|
||||
/// remediation.
|
||||
/// Host-scoped connections (<see cref="ConnectionScope.HostTenantId"/>, and configuration entries that leave the
|
||||
/// tenant blank, which are materialized at host scope) stay in scope for every tenant. The connection registry
|
||||
/// resolves the host scope for every signing-in tenant, and a connection's default role IDs are then resolved by
|
||||
/// <c>ExternalIdentityUserProvisioningService</c> through <see cref="IRoleProvider"/> in the signing-in user's
|
||||
/// tenant, so a host connection naming role ID X really does reference tenant A's role X.
|
||||
/// A tenant-agnostic role (<see cref="Tenant.AgnosticTenantId"/>) is visible from every tenant, so its tenant
|
||||
/// context is every tenant: impact and remediation for such a role scan every stored connection and every
|
||||
/// configuration entry regardless of tenant, instead of the single active tenant plus host scope. Authorizing a
|
||||
/// replacement role, however, is still performed through the ambient tenant's role services, so when the
|
||||
/// deletion target is agnostic the replacement role must itself be agnostic; a tenant-scoped replacement is
|
||||
/// rejected rather than being authorized in one tenant and written into every tenant's connections.
|
||||
/// In EF Core persistence a role's primary key is its ID alone, so a role ID is unique across all tenants there
|
||||
/// and an agnostic/tenant-scoped collision cannot exist. Only <c>MemoryRoleStore</c> can hold two roles that
|
||||
/// share an ID (its storage key includes the tenant); resolving a role ID against it can then be genuinely
|
||||
/// ambiguous. That ambiguity is never resolved by guessing: widening a tenant-scoped deletion would expose
|
||||
/// another tenant's references, and narrowing an agnostic deletion would leave an agnostic role's references
|
||||
/// dangling. It fails closed instead. The same collision makes a replacement candidate ambiguous too: a
|
||||
/// replacement ID that resolves to more than one role (an ambient match and an agnostic match, under
|
||||
/// <c>MemoryRoleStore</c>) is rejected as not agnostic rather than guessed at, so it is reported as
|
||||
/// <c>replacement_role_unavailable_or_unauthorized</c> instead of surfacing as an exception.
|
||||
/// </remarks>
|
||||
public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
||||
IIdentityProviderConnectionStore store,
|
||||
IOptionsMonitor<ExternalAuthenticationOptions> options,
|
||||
|
|
@ -25,24 +57,34 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
IConnectionRegistryVersionStore registryVersions,
|
||||
ConnectionRevisionCalculator revisionCalculator,
|
||||
ExternalAuthenticationSecurityNotifier notifier,
|
||||
IPermissionEvaluator permissionEvaluator) : IRoleDeletionDependencyContributor
|
||||
IPermissionEvaluator permissionEvaluator,
|
||||
ITenantAccessor tenantAccessor) : IRoleDeletionDependencyContributor
|
||||
{
|
||||
public const string SourceName = "external-authentication";
|
||||
public string Source => SourceName;
|
||||
|
||||
/// <summary>
|
||||
/// Match the default DI container's direct-service semantics: when persistence replaces the in-memory
|
||||
/// store, the last registration is the active store.
|
||||
/// </summary>
|
||||
private IRoleStore? ActiveRoleStore => roleStores.LastOrDefault();
|
||||
|
||||
public async ValueTask<RoleDeletionDependencySnapshot> InspectAsync(string roleId, CancellationToken cancellationToken = default)
|
||||
{
|
||||
var roleTenantId = await ResolveRoleTenantIdAsync(roleId, cancellationToken);
|
||||
var dependencies = new List<RoleDeletionDependency>();
|
||||
var configuredConnections = options.CurrentValue.ConfigurationConnections ?? [];
|
||||
var configurationIndex = 0;
|
||||
foreach (var connection in configuredConnections)
|
||||
{
|
||||
dependencies.AddRange(GetConfigurationDependencies(connection, configurationIndex, roleId));
|
||||
// The index is part of the configuration path an operator edits, so out-of-scope entries are
|
||||
// skipped without renumbering the entries that remain.
|
||||
if (IsInRoleTenantScope(GetConfigurationScopeTenantId(connection), roleTenantId))
|
||||
dependencies.AddRange(GetConfigurationDependencies(connection, configurationIndex, roleId));
|
||||
configurationIndex++;
|
||||
}
|
||||
|
||||
var databaseConnections = await store.FindAsync(new(), cancellationToken);
|
||||
foreach (var connection in databaseConnections.Items)
|
||||
foreach (var connection in await FindConnectionsInRoleTenantScopeAsync(roleTenantId, cancellationToken))
|
||||
{
|
||||
if (!TryGetRoleReference(connection.UnlinkedPolicy, roleId, out var policyBranch, out _, out var removesLastDefaultRole))
|
||||
continue;
|
||||
|
|
@ -91,9 +133,10 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
if (!expectedOwners.IsSubsetOf(currentOwners))
|
||||
return new RoleReferenceRemovalValidationResult.Conflict("dependency_changed");
|
||||
|
||||
var roleTenantId = await ResolveRoleTenantIdAsync(request.RoleId, cancellationToken);
|
||||
foreach (var dependency in request.Dependencies)
|
||||
{
|
||||
var connection = await store.FindByIdAsync(dependency.OwnerId, cancellationToken);
|
||||
var connection = await FindConnectionInRoleTenantScopeAsync(dependency.OwnerId, roleTenantId, cancellationToken);
|
||||
if (connection is null || connection.Revision != dependency.ExpectedRevision ||
|
||||
!TryGetRoleReference(connection.UnlinkedPolicy, request.RoleId, out _, out var roleIds, out _))
|
||||
return new RoleReferenceRemovalValidationResult.Conflict("connection_revision_changed");
|
||||
|
|
@ -104,6 +147,19 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
string.Equals(request.ReplacementRoleId, request.RoleId, StringComparison.Ordinal)))
|
||||
return new RoleReferenceRemovalValidationResult.Forbidden("replacement_role_unavailable_or_unauthorized");
|
||||
|
||||
// Authorization below still resolves through the ambient tenant's role services, so an agnostic
|
||||
// deletion target may only be replaced by another agnostic role; a tenant-scoped replacement would
|
||||
// otherwise be authorized in this tenant and then written into every other tenant's connections.
|
||||
// This does not extend to host-scoped connections: IdentityProviderConnectionManagementService
|
||||
// forces every managed connection to host scope, and in a deployment without multitenancy roles
|
||||
// are created scoped to the default tenant rather than agnostic, so requiring an agnostic
|
||||
// replacement for host-scoped connections would make every replacement remediation impossible in
|
||||
// the default deployment.
|
||||
if (requiresReplacement &&
|
||||
string.Equals(roleTenantId, Tenant.AgnosticTenantId, StringComparison.Ordinal) &&
|
||||
!await IsAgnosticRoleAsync(request.ReplacementRoleId, cancellationToken))
|
||||
return new RoleReferenceRemovalValidationResult.Forbidden("replacement_role_unavailable_or_unauthorized");
|
||||
|
||||
var rolesToAssign = requiresReplacement
|
||||
? new[] { request.ReplacementRoleId! }
|
||||
: remainingRoleIds;
|
||||
|
|
@ -128,12 +184,13 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
if (validation is RoleReferenceRemovalValidationResult.Conflict conflict)
|
||||
return new RoleReferenceRemovalResult.Conflict(conflict.Code, []);
|
||||
|
||||
var roleTenantId = await ResolveRoleTenantIdAsync(request.RoleId, cancellationToken);
|
||||
var changedOwnerIds = new List<string>();
|
||||
try
|
||||
{
|
||||
foreach (var dependency in request.Dependencies.OrderBy(x => x.OwnerId, StringComparer.Ordinal))
|
||||
{
|
||||
var connection = await store.FindByIdAsync(dependency.OwnerId, cancellationToken);
|
||||
var connection = await FindConnectionInRoleTenantScopeAsync(dependency.OwnerId, roleTenantId, cancellationToken);
|
||||
if (connection is null || connection.Revision != dependency.ExpectedRevision ||
|
||||
!TryGetRoleReference(connection.UnlinkedPolicy, request.RoleId, out _, out _, out var removesLastDefaultRole))
|
||||
return new RoleReferenceRemovalResult.Conflict("connection_revision_changed", changedOwnerIds);
|
||||
|
|
@ -147,15 +204,26 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
|
||||
if (request.SelectedReferences is not null && removesLastDefaultRole)
|
||||
{
|
||||
// Match the default DI container's direct-service semantics: when persistence
|
||||
// replaces the in-memory store, the last registration is the active store.
|
||||
var roleStore = roleStores.LastOrDefault();
|
||||
var roleStore = ActiveRoleStore;
|
||||
if (roleStore is null)
|
||||
return new RoleReferenceRemovalResult.Failed("replacement_role_unavailable_or_unauthorized", changedOwnerIds);
|
||||
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);
|
||||
|
||||
// Authorization above still resolves through the ambient tenant's role services, so an
|
||||
// agnostic deletion target may only be replaced by another agnostic role; a tenant-scoped
|
||||
// replacement would otherwise be authorized in this tenant and then written into every other
|
||||
// tenant's connections. This does not extend to host-scoped connections: see the matching
|
||||
// guard in ValidateRemovalAsync for why. The check is re-run through IsAgnosticRoleAsync
|
||||
// rather than trusting the TenantId on `replacement` from the FindAsync call above, because
|
||||
// a same-ID tenant-scoped role added after validation could make that lookup ambiguous;
|
||||
// IsAgnosticRoleAsync resolves the candidate itself and rejects an ambiguous match instead
|
||||
// of accepting whichever role FindAsync happened to return.
|
||||
if (string.Equals(roleTenantId, Tenant.AgnosticTenantId, StringComparison.Ordinal) &&
|
||||
!await IsAgnosticRoleAsync(request.ReplacementRoleId, cancellationToken))
|
||||
return new RoleReferenceRemovalResult.Failed("replacement_role_unavailable_or_unauthorized", changedOwnerIds);
|
||||
}
|
||||
|
||||
var update = await store.UpdateAsync(candidate, connection.Revision, cancellationToken);
|
||||
|
|
@ -191,6 +259,90 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
|
|||
return new RoleReferenceRemovalResult.Success(changedOwnerIds);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the tenant context for the role being deleted: the role's own <c>TenantId</c>
|
||||
/// (<see cref="Tenant.AgnosticTenantId"/> normalized when the role is tenant-agnostic, in which case its
|
||||
/// tenant context is every tenant). In EF Core persistence a role ID is unique across all tenants (the
|
||||
/// <c>Roles</c> table keys on <c>Id</c> alone), so this lookup resolves to at most one role there. Only
|
||||
/// <c>MemoryRoleStore</c> can hold two roles that share an ID because its storage key includes the tenant;
|
||||
/// if the ID resolves to more than one role, which tenant's role the coordinator's own delete actually
|
||||
/// targets is already ambiguous, and this method cannot make the operation consistent by guessing in either
|
||||
/// direction -- widening would expose another tenant's references for what may be a tenant-scoped deletion,
|
||||
/// and narrowing would leave an agnostic role's references dangling. It fails closed instead.
|
||||
/// A missing store or no matching role falls back to the ambient tenant on <see cref="ITenantAccessor"/>,
|
||||
/// which is the only case where the ambient tenant is trusted: the role cannot be resolved at all, so there
|
||||
/// is no resolved tenant to prefer over it.
|
||||
/// </summary>
|
||||
private async ValueTask<string> ResolveRoleTenantIdAsync(string roleId, CancellationToken cancellationToken)
|
||||
{
|
||||
var roles = await FindRolesByIdAsync(roleId, cancellationToken);
|
||||
return roles.Length switch
|
||||
{
|
||||
0 => tenantAccessor.TenantId.NormalizeTenantId(),
|
||||
1 => roles[0].TenantId.NormalizeTenantId(),
|
||||
_ => throw new InvalidOperationException(
|
||||
$"Role '{roleId}' resolves to {roles.Length} roles across tenant scopes; the deletion target is ambiguous and its external-authentication dependencies cannot be determined.")
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves whether a candidate role ID (typically a replacement role) is itself tenant-agnostic. Unlike
|
||||
/// <see cref="ResolveRoleTenantIdAsync"/>, an ambiguous candidate -- a role ID that resolves to more than one
|
||||
/// role, which only <c>MemoryRoleStore</c> can produce -- is not the coordinator's own deletion target, so
|
||||
/// there is no operation to fail closed on by throwing; it is instead treated the same as an unresolved
|
||||
/// candidate and reported as not agnostic, since there is no single resolved role to trust as safe to write
|
||||
/// into every tenant's connections.
|
||||
/// </summary>
|
||||
private async ValueTask<bool> IsAgnosticRoleAsync(string? roleId, CancellationToken cancellationToken)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(roleId))
|
||||
return false;
|
||||
var roles = await FindRolesByIdAsync(roleId, cancellationToken);
|
||||
return roles.Length == 1 && string.Equals(roles[0].TenantId.NormalizeTenantId(), Tenant.AgnosticTenantId, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
/// <summary>Loads every role matching the given ID from the active role store, or an empty result if none is configured.</summary>
|
||||
private async ValueTask<Role[]> FindRolesByIdAsync(string roleId, CancellationToken cancellationToken)
|
||||
{
|
||||
var roleStore = ActiveRoleStore;
|
||||
if (roleStore is null)
|
||||
return [];
|
||||
return (await roleStore.FindManyAsync(new() { Id = roleId }, cancellationToken)).ToArray();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Loads every stored connection in a single snapshot and filters it in memory to the role's tenant context.
|
||||
/// Composing the result from separate per-scope reads instead would let a connection's <c>TenantId</c> change
|
||||
/// between those reads (<see cref="RemoveEditableReferencesAsync"/> permits it via <c>UpdateAsync</c>), so the
|
||||
/// connection could fall between the reads and appear in neither result. A single snapshot has no gap to fall
|
||||
/// through.
|
||||
/// </summary>
|
||||
private async ValueTask<IReadOnlyCollection<IdentityProviderConnection>> FindConnectionsInRoleTenantScopeAsync(string roleTenantId, CancellationToken cancellationToken)
|
||||
{
|
||||
var connections = (await store.FindAsync(new(), cancellationToken)).Items;
|
||||
return connections.Where(x => IsInRoleTenantScope(x.TenantId, roleTenantId)).ToArray();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Loads one dependency's connection, reporting a connection outside the role's tenant context as absent so
|
||||
/// that a caller-supplied owner ID cannot reach across a tenant boundary. An agnostic role's tenant context
|
||||
/// is every tenant, so any connection loaded by owner ID qualifies.
|
||||
/// </summary>
|
||||
private async ValueTask<IdentityProviderConnection?> FindConnectionInRoleTenantScopeAsync(string ownerId, string roleTenantId, CancellationToken cancellationToken)
|
||||
{
|
||||
var connection = await store.FindByIdAsync(ownerId, cancellationToken);
|
||||
return connection is not null && IsInRoleTenantScope(connection.TenantId, roleTenantId) ? connection : null;
|
||||
}
|
||||
|
||||
private static bool IsInRoleTenantScope(string? connectionTenantId, string roleTenantId) =>
|
||||
string.Equals(roleTenantId, Tenant.AgnosticTenantId, StringComparison.Ordinal) ||
|
||||
string.Equals(connectionTenantId, ConnectionScope.HostTenantId, StringComparison.Ordinal) ||
|
||||
string.Equals(connectionTenantId.NormalizeTenantId(), roleTenantId, StringComparison.Ordinal);
|
||||
|
||||
/// <summary>A configuration entry that leaves the tenant blank is materialized at host scope.</summary>
|
||||
private static string GetConfigurationScopeTenantId(IdentityProviderConnection connection) =>
|
||||
string.IsNullOrWhiteSpace(connection.TenantId) ? ConnectionScope.HostTenantId : connection.TenantId;
|
||||
|
||||
private IEnumerable<RoleDeletionDependency> GetConfigurationDependencies(IdentityProviderConnection connection, int connectionIndex, string roleId)
|
||||
{
|
||||
if (!TryGetRoleReference(connection.UnlinkedPolicy, roleId, out var policyBranch, out var roleIds, out var removesLastDefaultRole))
|
||||
|
|
|
|||
|
|
@ -150,6 +150,25 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime
|
|||
Assert.Equal(2, Assert.IsType<ConnectionMutationResult.RevisionConflict>(await store.UpdateAsync(created.Connection, 1)).CurrentRevision);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ConnectionStoreReturnsOnlyTheRequestedScope()
|
||||
{
|
||||
var store = new EFCoreIdentityProviderConnectionStore(_leaseFactory);
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await store.CreateAsync(CreateConnection()));
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await store.CreateAsync(CreateConnection("connection-b", "tenant-b")));
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await store.CreateAsync(CreateConnection("connection-host", ConnectionScope.HostTenantId)));
|
||||
|
||||
var tenantScoped = await store.FindAsync(new() { Scope = new(ConnectionScopeKind.Tenant, "tenant-a") });
|
||||
var hostScoped = await store.FindAsync(new() { Scope = ConnectionScope.Host });
|
||||
var unscoped = await store.FindAsync(new());
|
||||
|
||||
// The store honors ConnectionFilter.Scope, so callers that query by scope get only that scope's rows;
|
||||
// a store that accepted and ignored the filter would silently widen their reach.
|
||||
Assert.Equal(["connection-a"], tenantScoped.Items.Select(x => x.Id).ToArray());
|
||||
Assert.Equal(["connection-host"], hostScoped.Items.Select(x => x.Id).ToArray());
|
||||
Assert.Equal(3, unscoped.Items.Count);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DurableStateGrantSessionAndRegistryVersionOperationsAreSingleUseOrCompareAndSwap()
|
||||
{
|
||||
|
|
@ -754,10 +773,10 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime
|
|||
|
||||
private static IReadOnlyDictionary<string, IReadOnlyCollection<string>> EmptyClaims { get; } = new Dictionary<string, IReadOnlyCollection<string>>();
|
||||
|
||||
private static IdentityProviderConnection CreateConnection(string id = "connection-a") => new()
|
||||
private static IdentityProviderConnection CreateConnection(string id = "connection-a", string tenantId = "tenant-a") => new()
|
||||
{
|
||||
Id = id,
|
||||
TenantId = "tenant-a",
|
||||
TenantId = tenantId,
|
||||
Key = "contoso",
|
||||
AdapterType = "openid-connect",
|
||||
AdapterSettingsVersion = 1,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,10 @@ using Elsa.Authorization;
|
|||
using Elsa.Testing.Shared.Multitenancy;
|
||||
using System.Security.Claims;
|
||||
using System.Text.Json;
|
||||
using Elsa.Common.Models;
|
||||
using Elsa.Common.Multitenancy;
|
||||
using Elsa.Common.Services;
|
||||
using Elsa.ExternalAuthentication.Contracts;
|
||||
using Elsa.ExternalAuthentication.Models;
|
||||
using Elsa.ExternalAuthentication.Options;
|
||||
using Elsa.ExternalAuthentication.Permissions;
|
||||
|
|
@ -165,7 +168,8 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
new InMemoryConnectionRegistryVersionStore(),
|
||||
new ConnectionRevisionCalculator(),
|
||||
new ExternalAuthenticationSecurityNotifier(services),
|
||||
new PermissionEvaluator());
|
||||
new PermissionEvaluator(),
|
||||
TestTenantAccessor.Default);
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
var request = new RoleReferenceRemovalRequest(
|
||||
"workflow-user",
|
||||
|
|
@ -236,7 +240,8 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
versions,
|
||||
new ConnectionRevisionCalculator(),
|
||||
new ExternalAuthenticationSecurityNotifier(services),
|
||||
new PermissionEvaluator());
|
||||
new PermissionEvaluator(),
|
||||
TestTenantAccessor.Default);
|
||||
var securityNotifier = new RoleSecurityNotifier(Substitute.For<INotificationSender>(), TestTenantAccessor.Default, new SystemClock());
|
||||
var coordinator = new RoleDeletionCoordinator(roleStore, roleAuthorizationService, [contributor], securityNotifier);
|
||||
var impact = Assert.IsType<RoleDeletionInspectionResult.Success>(await coordinator.InspectAsync("workflow-user", Administrator())).Impact;
|
||||
|
|
@ -311,6 +316,351 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
Assert.IsType<RoleReferenceRemovalValidationResult.Forbidden>(result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ImpactExcludesConnectionsOwnedByAnotherTenant()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("workflow-user"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("workflow-user"), TenantB);
|
||||
var otherTenantConfiguration = Connection("other-tenant-configured", CreateUserPolicy("workflow-user"), TenantB);
|
||||
var (contributor, _, _) = await CreateContributorAsync(
|
||||
[otherTenantConfiguration],
|
||||
[ownConnection, otherTenantConnection],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
|
||||
// Neither tenant B's stored connection nor its configuration entry -- which would block deletion
|
||||
// outright -- is reported, while the tenant's own reference still is, so the filter is not simply
|
||||
// reporting nothing.
|
||||
var dependency = Assert.Single(snapshot.Dependencies);
|
||||
Assert.Equal(ownConnection.Id, dependency.OwnerId);
|
||||
Assert.Equal(RoleDeletionDependencyOwnership.Database, dependency.Ownership);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ImpactIncludesHostScopedConnectionsForEveryTenant()
|
||||
{
|
||||
var hostConnection = Connection("host-connection", CreateUserPolicy("workflow-user"));
|
||||
var hostConfiguration = Connection("host-configured", CreateUserPolicy("workflow-user"), ConnectionScope.DefaultTenantId);
|
||||
var (contributor, _, _) = await CreateContributorAsync(
|
||||
[hostConfiguration],
|
||||
[hostConnection],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
|
||||
// A host connection is served to every signing-in tenant and its default roles are resolved in that
|
||||
// tenant, so it references this tenant's role. A blank configuration tenant is materialized at host scope.
|
||||
Assert.Equal(
|
||||
[hostConfiguration.Id, hostConnection.Id],
|
||||
snapshot.Dependencies.Select(x => x.OwnerId).Order(StringComparer.Ordinal).ToArray());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationCannotReachAnotherTenantsConnection()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("workflow-user", "other-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("workflow-user", "other-role"), TenantB);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
var request = new RoleReferenceRemovalRequest(
|
||||
"workflow-user",
|
||||
Administrator(),
|
||||
snapshot.Version,
|
||||
[
|
||||
..snapshot.Dependencies,
|
||||
// The owner ID of another tenant's connection, as a caller could supply it.
|
||||
new RoleDeletionDependency(
|
||||
ExternalAuthenticationRoleDeletionDependencyContributor.SourceName,
|
||||
otherTenantConnection.Id,
|
||||
otherTenantConnection.Key,
|
||||
"create-user",
|
||||
RoleDeletionDependencyOwnership.Database,
|
||||
null,
|
||||
1,
|
||||
false)
|
||||
]);
|
||||
|
||||
Assert.IsType<RoleReferenceRemovalValidationResult.Conflict>(await contributor.ValidateRemovalAsync(request));
|
||||
var result = Assert.IsType<RoleReferenceRemovalResult.Conflict>(await contributor.RemoveEditableReferencesAsync(request));
|
||||
|
||||
// Nothing may half-run: neither the foreign connection nor the tenant's own connection is touched.
|
||||
Assert.Empty(result.ChangedOwnerIds);
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "workflow-user", "other-role");
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "workflow-user", "other-role");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationStopsWhenTheConnectionLeavesTheRoleTenantAfterValidation()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("workflow-user", "other-role"), TenantA);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA),
|
||||
decorateStore: inner => new ConnectionStoreThatMovesConnectionToAnotherTenant(inner, ownConnection.Id, TenantB, lookupsBeforeMove: 1));
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
var request = new RoleReferenceRemovalRequest("workflow-user", Administrator(), snapshot.Version, snapshot.Dependencies);
|
||||
|
||||
var result = Assert.IsType<RoleReferenceRemovalResult.Conflict>(await contributor.RemoveEditableReferencesAsync(request));
|
||||
|
||||
Assert.Equal("connection_revision_changed", result.Code);
|
||||
Assert.Empty(result.ChangedOwnerIds);
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "workflow-user", "other-role");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ImpactForAnAgnosticRoleIncludesAConnectionOwnedByAnotherTenant()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
|
||||
var (contributor, _, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles: [new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] }],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
|
||||
var snapshot = await contributor.InspectAsync("agnostic-role");
|
||||
|
||||
// The role is visible from every tenant, so its tenant context is every tenant: tenant B's reference is
|
||||
// reported alongside tenant A's, unlike a tenant-scoped role (see ImpactExcludesConnectionsOwnedByAnotherTenant).
|
||||
Assert.Equal(
|
||||
[otherTenantConnection.Id, ownConnection.Id],
|
||||
snapshot.Dependencies.Select(x => x.OwnerId).Order(StringComparer.Ordinal).ToArray());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ImpactIsScopedByTheResolvedRolesTenantRatherThanTheAmbientTenant()
|
||||
{
|
||||
// The ambient tenant is the default tenant (no tenant pushed), which is what an EF host runs as with
|
||||
// multitenancy disabled: the EF role store installs no tenant query filter there and can resolve a
|
||||
// tenant-owned role by ID regardless of the ambient tenant, unlike MemoryRoleStore, which always
|
||||
// filters by the ambient tenant itself. RoleStoreWithoutAmbientTenantFilter stands in for that EF
|
||||
// behavior. The role being deleted belongs to tenant A, so impact must be scoped by that resolved
|
||||
// tenant, not by the unrelated ambient one.
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("tenant-a-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("tenant-a-role"), TenantB);
|
||||
var connectionStore = new InMemoryIdentityProviderConnectionStore();
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await connectionStore.CreateAsync(ownConnection));
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await connectionStore.CreateAsync(otherTenantConnection));
|
||||
var roleStore = new RoleStoreWithoutAmbientTenantFilter(
|
||||
[new Role { Id = "tenant-a-role", Name = "Tenant A role", TenantId = TenantA, Permissions = [] }]);
|
||||
var roleAuthorizationService = new RoleAuthorizationService(new StoreBasedRoleProvider(roleStore), new PermissionEvaluator());
|
||||
var services = new ServiceCollection().BuildServiceProvider();
|
||||
var contributor = new ExternalAuthenticationRoleDeletionDependencyContributor(
|
||||
connectionStore,
|
||||
new MutableOptionsMonitor<ExternalAuthenticationOptions>(new ExternalAuthenticationOptions()),
|
||||
[roleAuthorizationService],
|
||||
[roleStore],
|
||||
new InMemoryConnectionRegistryVersionStore(),
|
||||
new ConnectionRevisionCalculator(),
|
||||
new ExternalAuthenticationSecurityNotifier(services),
|
||||
new PermissionEvaluator(),
|
||||
TestTenantAccessor.Default);
|
||||
|
||||
var snapshot = await contributor.InspectAsync("tenant-a-role");
|
||||
|
||||
// Scoped by the role's own tenant (A): tenant A's connection is reported, tenant B's is not. Scoping by
|
||||
// the ambient default tenant instead -- what this test is guarding against -- would report neither.
|
||||
var dependency = Assert.Single(snapshot.Dependencies);
|
||||
Assert.Equal(ownConnection.Id, dependency.OwnerId);
|
||||
Assert.Equal(RoleDeletionDependencyOwnership.Database, dependency.Ownership);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RoleIdThatResolvesToMoreThanOneRoleAcrossTenantScopesFailsClosed()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("workflow-user"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("workflow-user"), TenantB);
|
||||
var (contributor, _, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles: [new Role { Id = "workflow-user", Name = "Agnostic workflow user", TenantId = Tenant.AgnosticTenantId, Permissions = [] }],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
|
||||
// Tenant A's own "workflow-user" role and an agnostic role sharing that same ID both exist in the
|
||||
// in-memory role store, so the deletion target is ambiguous: the contributor cannot determine whether
|
||||
// to scope its inspection and remediation to tenant A alone or to every tenant, and must fail closed
|
||||
// rather than guess in either direction.
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() => contributor.InspectAsync("workflow-user").AsTask());
|
||||
|
||||
var request = new RoleReferenceRemovalRequest(
|
||||
"workflow-user",
|
||||
Administrator(),
|
||||
"irrelevant-version",
|
||||
[
|
||||
new RoleDeletionDependency(
|
||||
ExternalAuthenticationRoleDeletionDependencyContributor.SourceName,
|
||||
ownConnection.Id,
|
||||
ownConnection.Key,
|
||||
"create-user",
|
||||
RoleDeletionDependencyOwnership.Database,
|
||||
null,
|
||||
1,
|
||||
false)
|
||||
]);
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() => contributor.ValidateRemovalAsync(request).AsTask());
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() => contributor.RemoveEditableReferencesAsync(request).AsTask());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationOfAnAgnosticRoleCanRemoveTheReferenceFromAnotherTenantsConnection()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles: [new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] }],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("agnostic-role");
|
||||
var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies);
|
||||
|
||||
Assert.IsType<RoleReferenceRemovalValidationResult.Valid>(await contributor.ValidateRemovalAsync(request));
|
||||
var result = Assert.IsType<RoleReferenceRemovalResult.Success>(await contributor.RemoveEditableReferencesAsync(request));
|
||||
|
||||
Assert.Equal(
|
||||
[otherTenantConnection.Id, ownConnection.Id],
|
||||
result.ChangedOwnerIds.Order(StringComparer.Ordinal).ToArray());
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id));
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationOfAnAgnosticRoleRejectsATenantScopedReplacementRole()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles:
|
||||
[
|
||||
new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] },
|
||||
new Role { Id = "tenant-a-replacement", Name = "Tenant A replacement", TenantId = TenantA, Permissions = [] }
|
||||
],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("agnostic-role");
|
||||
var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies)
|
||||
{
|
||||
SelectedReferences = snapshot.Dependencies
|
||||
.Select(x => new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, x.OwnerId))
|
||||
.ToArray(),
|
||||
ReplacementRoleId = "tenant-a-replacement"
|
||||
};
|
||||
|
||||
// Remediation is initiated in tenant A and would resolve the replacement role through tenant A's role
|
||||
// authorization service alone, even though tenant B's connection is also in scope for this agnostic
|
||||
// role. Admitting a tenant-A-only replacement would write a role into tenant B's policy that does not
|
||||
// exist there, so it must be rejected rather than authorized in one tenant and applied to every tenant.
|
||||
var validation = await contributor.ValidateRemovalAsync(request);
|
||||
var forbidden = Assert.IsType<RoleReferenceRemovalValidationResult.Forbidden>(validation);
|
||||
Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
|
||||
|
||||
var result = await contributor.RemoveEditableReferencesAsync(request);
|
||||
var failed = Assert.IsType<RoleReferenceRemovalResult.Failed>(result);
|
||||
Assert.Equal("replacement_role_unavailable_or_unauthorized", failed.Code);
|
||||
Assert.Empty(failed.ChangedOwnerIds);
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "agnostic-role");
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "agnostic-role");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationOfAnAgnosticRoleAcceptsAnAgnosticReplacementRole()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles:
|
||||
[
|
||||
new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] },
|
||||
new Role { Id = "agnostic-replacement", Name = "Agnostic replacement", TenantId = Tenant.AgnosticTenantId, Permissions = [] }
|
||||
],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("agnostic-role");
|
||||
var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies)
|
||||
{
|
||||
SelectedReferences = snapshot.Dependencies
|
||||
.Select(x => new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, x.OwnerId))
|
||||
.ToArray(),
|
||||
ReplacementRoleId = "agnostic-replacement"
|
||||
};
|
||||
|
||||
// An agnostic replacement exists identically in every tenant, so it is safe to write into tenant B's
|
||||
// policy even though remediation was authorized through tenant A's role services.
|
||||
Assert.IsType<RoleReferenceRemovalValidationResult.Valid>(await contributor.ValidateRemovalAsync(request));
|
||||
var result = Assert.IsType<RoleReferenceRemovalResult.Success>(await contributor.RemoveEditableReferencesAsync(request));
|
||||
|
||||
Assert.Equal(
|
||||
[otherTenantConnection.Id, ownConnection.Id],
|
||||
result.ChangedOwnerIds.Order(StringComparer.Ordinal).ToArray());
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "agnostic-replacement");
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "agnostic-replacement");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RemediationRemovesTheRoleFromAHostScopedConnectionForATenant()
|
||||
{
|
||||
var hostConnection = Connection("host-connection", CreateUserPolicy("workflow-user", "other-role"));
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[hostConnection],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("workflow-user");
|
||||
var request = new RoleReferenceRemovalRequest("workflow-user", Administrator(), snapshot.Version, snapshot.Dependencies);
|
||||
|
||||
Assert.IsType<RoleReferenceRemovalValidationResult.Valid>(await contributor.ValidateRemovalAsync(request));
|
||||
var result = Assert.IsType<RoleReferenceRemovalResult.Success>(await contributor.RemoveEditableReferencesAsync(request));
|
||||
|
||||
Assert.Equal([hostConnection.Id], result.ChangedOwnerIds);
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(hostConnection.Id), "other-role");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReplacementRoleIdThatResolvesToBothATenantRoleAndAnAgnosticRoleIsRejectedRatherThanThrowing()
|
||||
{
|
||||
var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
|
||||
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
|
||||
var (contributor, store, _) = await CreateContributorAsync(
|
||||
[],
|
||||
[ownConnection, otherTenantConnection],
|
||||
additionalRoles:
|
||||
[
|
||||
new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] },
|
||||
new Role { Id = "ambiguous-replacement", Name = "Ambiguous replacement (tenant A)", TenantId = TenantA, Permissions = [] },
|
||||
new Role { Id = "ambiguous-replacement", Name = "Ambiguous replacement (agnostic)", TenantId = Tenant.AgnosticTenantId, Permissions = [] }
|
||||
],
|
||||
tenantAccessor: new TestTenantAccessor(TenantA));
|
||||
var snapshot = await contributor.InspectAsync("agnostic-role");
|
||||
var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies)
|
||||
{
|
||||
SelectedReferences = snapshot.Dependencies
|
||||
.Select(x => new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, x.OwnerId))
|
||||
.ToArray(),
|
||||
ReplacementRoleId = "ambiguous-replacement"
|
||||
};
|
||||
|
||||
// The replacement ID resolves to two roles in the in-memory store (a tenant-A role and an agnostic role
|
||||
// sharing the same ID), which is exactly the collision ResolveRoleTenantIdAsync fails closed on for a
|
||||
// deletion target. A replacement candidate is not the coordinator's own deletion target, so this must be
|
||||
// reported as an ordinary validation failure rather than escape as an exception.
|
||||
var validation = await contributor.ValidateRemovalAsync(request);
|
||||
var forbidden = Assert.IsType<RoleReferenceRemovalValidationResult.Forbidden>(validation);
|
||||
Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
|
||||
|
||||
var result = await contributor.RemoveEditableReferencesAsync(request);
|
||||
var failed = Assert.IsType<RoleReferenceRemovalResult.Failed>(result);
|
||||
Assert.Equal("replacement_role_unavailable_or_unauthorized", failed.Code);
|
||||
Assert.Empty(failed.ChangedOwnerIds);
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "agnostic-role");
|
||||
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "agnostic-role");
|
||||
}
|
||||
|
||||
private static Task<(ExternalAuthenticationRoleDeletionDependencyContributor Contributor, InMemoryIdentityProviderConnectionStore Store, InMemoryConnectionRegistryVersionStore Versions)> CreateContributorAsync(
|
||||
IReadOnlyCollection<IdentityProviderConnection> configuredConnections,
|
||||
params IdentityProviderConnection[] databaseConnections) =>
|
||||
|
|
@ -319,35 +669,39 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
private static async Task<(ExternalAuthenticationRoleDeletionDependencyContributor Contributor, InMemoryIdentityProviderConnectionStore Store, InMemoryConnectionRegistryVersionStore Versions)> CreateContributorAsync(
|
||||
IReadOnlyCollection<IdentityProviderConnection> configuredConnections,
|
||||
IdentityProviderConnection[] databaseConnections,
|
||||
IReadOnlyCollection<Role>? additionalRoles = null)
|
||||
IReadOnlyCollection<Role>? additionalRoles = null,
|
||||
ITenantAccessor? tenantAccessor = null,
|
||||
Func<InMemoryIdentityProviderConnectionStore, IIdentityProviderConnectionStore>? decorateStore = null)
|
||||
{
|
||||
var store = new InMemoryIdentityProviderConnectionStore();
|
||||
foreach (var connection in databaseConnections)
|
||||
Assert.IsType<ConnectionMutationResult.Created>(await store.CreateAsync(connection));
|
||||
|
||||
var roleStore = new MemoryRoleStore(new MemoryStore<Role>(), 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 = [] });
|
||||
var accessor = tenantAccessor ?? TestTenantAccessor.Default;
|
||||
var roleStore = new MemoryRoleStore(new MemoryStore<Role>(), accessor);
|
||||
await roleStore.SaveAsync(new Role { Id = "workflow-user", Name = "Workflow user", TenantId = accessor.TenantId, Permissions = [] });
|
||||
await roleStore.SaveAsync(new Role { Id = "other-role", Name = "Other role", TenantId = accessor.TenantId, Permissions = [] });
|
||||
foreach (var role in additionalRoles ?? [])
|
||||
await roleStore.SaveAsync(role);
|
||||
var versions = new InMemoryConnectionRegistryVersionStore();
|
||||
var services = new ServiceCollection().BuildServiceProvider();
|
||||
var contributor = new ExternalAuthenticationRoleDeletionDependencyContributor(
|
||||
store,
|
||||
decorateStore?.Invoke(store) ?? store,
|
||||
new MutableOptionsMonitor<ExternalAuthenticationOptions>(new ExternalAuthenticationOptions { ConfigurationConnections = configuredConnections.ToList() }),
|
||||
[new RoleAuthorizationService(new StoreBasedRoleProvider(roleStore), new PermissionEvaluator())],
|
||||
[roleStore],
|
||||
versions,
|
||||
new ConnectionRevisionCalculator(),
|
||||
new ExternalAuthenticationSecurityNotifier(services),
|
||||
new PermissionEvaluator());
|
||||
new PermissionEvaluator(),
|
||||
accessor);
|
||||
return (contributor, store, versions);
|
||||
}
|
||||
|
||||
private static IdentityProviderConnection Connection(string id, PolicySelection policy) => new()
|
||||
private static IdentityProviderConnection Connection(string id, PolicySelection policy, string? tenantId = null) => new()
|
||||
{
|
||||
Id = id,
|
||||
TenantId = ConnectionScope.HostTenantId,
|
||||
TenantId = tenantId ?? ConnectionScope.HostTenantId,
|
||||
Key = id,
|
||||
AdapterType = "oidc",
|
||||
AdapterSettingsVersion = 1,
|
||||
|
|
@ -359,6 +713,18 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
UpdatedAt = DateTimeOffset.UnixEpoch
|
||||
};
|
||||
|
||||
private static PolicySelection CreateUserPolicy(params string[] defaultRoleIds) => new(
|
||||
CreateUserUnlinkedIdentityPolicy.PolicyType,
|
||||
1,
|
||||
JsonSerializer.SerializeToElement(new { defaultRoleIds }));
|
||||
|
||||
private static void AssertDefaultRoleIds(IdentityProviderConnection? connection, params string[] expectedRoleIds) =>
|
||||
Assert.Equal(
|
||||
expectedRoleIds,
|
||||
Assert.IsType<IdentityProviderConnection>(connection).UnlinkedPolicy!.Settings.GetProperty("defaultRoleIds").EnumerateArray().Select(x => x.GetString()!).ToArray());
|
||||
|
||||
private const string TenantA = "tenant-a";
|
||||
private const string TenantB = "tenant-b";
|
||||
private const string ConnectionsUpdate = $"{ExternalAuthenticationResourcePermissions.Connections}:{CoreVerbs.Update}";
|
||||
private const string PoliciesUpdate = $"{ExternalAuthenticationResourcePermissions.Policies}:{CoreVerbs.Update}";
|
||||
private const string DefaultRolesUpdate = $"{ExternalAuthenticationResourcePermissions.PolicyDefaultRoles}:{CoreVerbs.Update}";
|
||||
|
|
@ -378,6 +744,57 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
|
|||
.Select(x => new Claim(PermissionNames.ClaimType, x))));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reassigns a connection to another tenant once the contributor has read it, which puts the connection
|
||||
/// outside the role's tenant context between prevalidation and the remediation write.
|
||||
/// </summary>
|
||||
private sealed class ConnectionStoreThatMovesConnectionToAnotherTenant(
|
||||
InMemoryIdentityProviderConnectionStore inner,
|
||||
string connectionId,
|
||||
string tenantId,
|
||||
int lookupsBeforeMove) : IIdentityProviderConnectionStore
|
||||
{
|
||||
private int _lookups;
|
||||
|
||||
public ValueTask<Page<IdentityProviderConnection>> FindAsync(ConnectionFilter filter, CancellationToken cancellationToken = default) =>
|
||||
inner.FindAsync(filter, cancellationToken);
|
||||
|
||||
public async ValueTask<IdentityProviderConnection?> FindByIdAsync(string id, CancellationToken cancellationToken = default)
|
||||
{
|
||||
var connection = await inner.FindByIdAsync(id, cancellationToken);
|
||||
if (connection is not null && string.Equals(id, connectionId, StringComparison.Ordinal) && Interlocked.Increment(ref _lookups) > lookupsBeforeMove)
|
||||
connection.TenantId = tenantId;
|
||||
return connection;
|
||||
}
|
||||
|
||||
public ValueTask<ConnectionMutationResult> CreateAsync(IdentityProviderConnection connection, CancellationToken cancellationToken = default) =>
|
||||
inner.CreateAsync(connection, cancellationToken);
|
||||
|
||||
public ValueTask<ConnectionMutationResult> UpdateAsync(IdentityProviderConnection connection, long expectedRevision, CancellationToken cancellationToken = default) =>
|
||||
inner.UpdateAsync(connection, expectedRevision, cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves roles by ID alone, regardless of the ambient tenant, standing in for the EF Core role store
|
||||
/// with multitenancy disabled: it installs no tenant query filter and can resolve a tenant-owned role by
|
||||
/// ID no matter which tenant is ambient. <see cref="MemoryRoleStore"/> cannot exercise that scenario
|
||||
/// because it always filters by the ambient tenant itself.
|
||||
/// </summary>
|
||||
private sealed class RoleStoreWithoutAmbientTenantFilter(IReadOnlyCollection<Role> roles) : IRoleStore
|
||||
{
|
||||
public Task AddAsync(Role role, CancellationToken cancellationToken = default) => throw new NotSupportedException();
|
||||
|
||||
public Task DeleteAsync(RoleFilter filter, CancellationToken cancellationToken = default) => throw new NotSupportedException();
|
||||
|
||||
public Task SaveAsync(Role role, CancellationToken cancellationToken = default) => throw new NotSupportedException();
|
||||
|
||||
public Task<Role?> FindAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
|
||||
Task.FromResult(roles.FirstOrDefault(x => x.Id == filter.Id));
|
||||
|
||||
public Task<IEnumerable<Role>> FindManyAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
|
||||
Task.FromResult(roles.Where(x => x.Id == filter.Id));
|
||||
}
|
||||
|
||||
private sealed class RoleStoreThatRemovesReplacementAfterContributorValidation(
|
||||
MemoryRoleStore inner,
|
||||
string replacementRoleId) : IRoleStore
|
||||
|
|
|
|||
Loading…
Reference in a new issue