fix(secrets-ui): components re-check their own policy; pin the real policy matrix (scadaproj#45)
Secrets-016: RevealButton (RevealPolicy), SecretEditor and ConfirmDeleteModal (ManagePolicy) now evaluate their policy through IAuthorizationService at the top of the revealing/mutating handler and fail closed (no auth state, no IAuthorizationService, or a failed policy), instead of relying on the host to wrap them in AuthorizeView. New tests build the REAL AddSecretsAuthorization policies and evaluate each role against each policy, so secrets-manager satisfying RevealPolicy would now fail a test. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -81,6 +81,12 @@
|
||||
private async Task ConfirmAsync()
|
||||
{
|
||||
_error = null;
|
||||
|
||||
if (!await SecretsComponentAuthorization.IsAuthorizedAsync(Services, AuthState, SecretsAuthorization.ManagePolicy))
|
||||
{
|
||||
_error = "You are not authorized to delete secrets.";
|
||||
return;
|
||||
}
|
||||
_busy = true;
|
||||
bool deleted = false;
|
||||
try
|
||||
|
||||
@@ -56,6 +56,14 @@ else
|
||||
// value costs only a retry.
|
||||
private async Task RevealAsync()
|
||||
{
|
||||
if (!await SecretsComponentAuthorization.IsAuthorizedAsync(Services, AuthState, SecretsAuthorization.RevealPolicy))
|
||||
{
|
||||
_value = null;
|
||||
_error = "You are not authorized to reveal secrets.";
|
||||
_revealed = true;
|
||||
return;
|
||||
}
|
||||
|
||||
_busy = true;
|
||||
// Captured now: if the component is disposed mid-reveal, reading _cts.Token later would throw
|
||||
// ObjectDisposedException; a captured token just reports cancellation, which fails closed.
|
||||
|
||||
@@ -110,6 +110,12 @@
|
||||
{
|
||||
_error = null;
|
||||
|
||||
if (!await SecretsComponentAuthorization.IsAuthorizedAsync(Services, AuthState, SecretsAuthorization.ManagePolicy))
|
||||
{
|
||||
_error = "You are not authorized to manage secrets.";
|
||||
return;
|
||||
}
|
||||
|
||||
SecretName name;
|
||||
try
|
||||
{
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
using Microsoft.AspNetCore.Authorization;
|
||||
using Microsoft.AspNetCore.Components.Authorization;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
|
||||
namespace ZB.MOM.WW.Secrets.Ui;
|
||||
|
||||
/// <summary>
|
||||
/// The in-component policy check every revealing or mutating handler runs before it touches the
|
||||
/// store (Secrets-016). The public components used to rely entirely on the host rendering them
|
||||
/// inside an <c>AuthorizeView</c>; a host that composed <c><RevealButton/></c> into a
|
||||
/// lower-privilege page handed plaintext to anyone who could see that page. Now each component
|
||||
/// re-checks its own policy and fails closed — no authentication state, no
|
||||
/// <see cref="IAuthorizationService"/>, or a failed policy all refuse.
|
||||
/// </summary>
|
||||
internal static class SecretsComponentAuthorization
|
||||
{
|
||||
public static async Task<bool> IsAuthorizedAsync(
|
||||
IServiceProvider services, Task<AuthenticationState>? cascadedState, string policy)
|
||||
{
|
||||
IAuthorizationService? authorization = services.GetService<IAuthorizationService>();
|
||||
if (authorization is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
Task<AuthenticationState>? stateTask = cascadedState
|
||||
?? services.GetService<AuthenticationStateProvider>()?.GetAuthenticationStateAsync();
|
||||
if (stateTask is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
AuthenticationState state = await stateTask.ConfigureAwait(false);
|
||||
AuthorizationResult result = await authorization.AuthorizeAsync(state.User, resource: null, policy)
|
||||
.ConfigureAwait(false);
|
||||
return result.Succeeded;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,160 @@
|
||||
using System.Security.Claims;
|
||||
using Bunit;
|
||||
using Bunit.TestDoubles;
|
||||
using Microsoft.AspNetCore.Authorization;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using ZB.MOM.WW.Audit;
|
||||
using ZB.MOM.WW.Auth.AspNetCore;
|
||||
using ZB.MOM.WW.Secrets.Abstractions;
|
||||
using ZB.MOM.WW.Secrets.Ui.Components;
|
||||
using ZB.MOM.WW.Secrets.Ui.Tests.Fakes;
|
||||
|
||||
namespace ZB.MOM.WW.Secrets.Ui.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Secrets-016: each public component re-checks its own policy, so a host that renders one outside
|
||||
/// an <c>AuthorizeView</c> cannot hand its action to a user without the right; and the REAL policy
|
||||
/// definitions in <see cref="SecretsAuthorization.AddSecretsAuthorization"/> are pinned (the bUnit
|
||||
/// tests elsewhere use <c>SetPolicies</c>, which bypasses them).
|
||||
/// </summary>
|
||||
public class ComponentAuthorizationTests : TestContext
|
||||
{
|
||||
private const string PlainValue = "the-plaintext-value-42";
|
||||
|
||||
private (RecordingSecretStore Store, CapturingAuditWriter Audit, FakeCipher Cipher) Setup(params string[] grantedPolicies)
|
||||
{
|
||||
var store = new RecordingSecretStore();
|
||||
var audit = new CapturingAuditWriter();
|
||||
var cipher = new FakeCipher();
|
||||
Services.AddSingleton<ISecretStore>(store);
|
||||
Services.AddSingleton<IAuditWriter>(audit);
|
||||
Services.AddSingleton<ISecretCipher>(cipher);
|
||||
var auth = this.AddTestAuthorization();
|
||||
auth.SetAuthorized("eve");
|
||||
auth.SetPolicies(grantedPolicies);
|
||||
return (store, audit, cipher);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void RevealButton_outside_an_AuthorizeView_refuses_a_manager_without_reveal()
|
||||
{
|
||||
// A host composed the button into a page gated only by ManagePolicy.
|
||||
var (store, audit, cipher) = Setup(SecretsAuthorization.ManagePolicy);
|
||||
var name = new SecretName("prod/db");
|
||||
store.Seed(cipher.Encrypt(name, PlainValue, SecretContentType.Text) with
|
||||
{
|
||||
Revision = 1, CreatedUtc = DateTimeOffset.UnixEpoch, UpdatedUtc = DateTimeOffset.UnixEpoch,
|
||||
});
|
||||
|
||||
var cut = RenderComponent<RevealButton>(p => p.Add(c => c.Name, name));
|
||||
cut.Find("button.reveal-secret").Click();
|
||||
|
||||
Assert.DoesNotContain(PlainValue, cut.Markup);
|
||||
Assert.Contains("not authorized", cut.Find("[data-testid=reveal-error]").TextContent);
|
||||
Assert.DoesNotContain(audit.Events, e => e.Outcome == AuditOutcome.Success);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void RevealButton_still_reveals_for_a_holder_of_the_reveal_policy()
|
||||
{
|
||||
var (store, _, cipher) = Setup(SecretsAuthorization.RevealPolicy);
|
||||
var name = new SecretName("prod/db");
|
||||
store.Seed(cipher.Encrypt(name, PlainValue, SecretContentType.Text) with
|
||||
{
|
||||
Revision = 1, CreatedUtc = DateTimeOffset.UnixEpoch, UpdatedUtc = DateTimeOffset.UnixEpoch,
|
||||
});
|
||||
|
||||
var cut = RenderComponent<RevealButton>(p => p.Add(c => c.Name, name));
|
||||
cut.Find("button.reveal-secret").Click();
|
||||
|
||||
Assert.Contains(PlainValue, cut.Markup);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SecretEditor_refuses_a_user_without_the_manage_policy()
|
||||
{
|
||||
var (store, audit, _) = Setup(/* no policies */);
|
||||
|
||||
var cut = RenderComponent<SecretEditor>(p => p.Add(c => c.OnSaved, () => { }));
|
||||
cut.Find("[data-testid=secret-name]").Change("app/thing");
|
||||
cut.Find("[data-testid=secret-value]").Change("v");
|
||||
cut.Find("[data-testid=editor-submit]").Click();
|
||||
|
||||
Assert.Empty(store.Upserts);
|
||||
Assert.Empty(audit.Events);
|
||||
Assert.Contains("not authorized", cut.Find("[data-testid=editor-error]").TextContent);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConfirmDeleteModal_refuses_a_user_without_the_manage_policy()
|
||||
{
|
||||
var (store, _, cipher) = Setup(/* no policies */);
|
||||
var name = new SecretName("app/thing");
|
||||
store.Seed(cipher.Encrypt(name, "v", SecretContentType.Text) with
|
||||
{
|
||||
Revision = 1, CreatedUtc = DateTimeOffset.UnixEpoch, UpdatedUtc = DateTimeOffset.UnixEpoch,
|
||||
});
|
||||
bool deleted = false;
|
||||
|
||||
var cut = RenderComponent<ConfirmDeleteModal>(p => p
|
||||
.Add(c => c.Visible, true)
|
||||
.Add(c => c.Name, name)
|
||||
.Add(c => c.OnDeleted, () => deleted = true));
|
||||
cut.Find("[data-testid=confirm-delete]").Click();
|
||||
|
||||
Assert.Empty(store.Deletes);
|
||||
Assert.False(deleted);
|
||||
}
|
||||
|
||||
// ---------- the real policy definitions ----------
|
||||
|
||||
public static TheoryData<string?, bool, bool> RoleMatrix => new()
|
||||
{
|
||||
// role, satisfies ManagePolicy, satisfies RevealPolicy
|
||||
{ SecretsAuthorization.ManageRole, true, false },
|
||||
{ SecretsAuthorization.RevealRole, true, true },
|
||||
{ SecretsAuthorization.AdminRole, true, true },
|
||||
{ "Viewer", false, false },
|
||||
{ null, false, false },
|
||||
};
|
||||
|
||||
[Theory]
|
||||
[MemberData(nameof(RoleMatrix))]
|
||||
public async Task The_policy_definitions_grant_exactly_the_documented_roles(string? role, bool manage, bool reveal)
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddAuthorization(o => o.AddSecretsAuthorization());
|
||||
await using ServiceProvider sp = services.BuildServiceProvider();
|
||||
var authorization = sp.GetRequiredService<IAuthorizationService>();
|
||||
|
||||
var claims = new List<Claim> { new(ClaimTypes.Name, "u") };
|
||||
if (role is not null)
|
||||
{
|
||||
claims.Add(new Claim(ZbClaimTypes.Role, role));
|
||||
}
|
||||
|
||||
var user = new ClaimsPrincipal(new ClaimsIdentity(claims, "test", ClaimTypes.Name, ZbClaimTypes.Role));
|
||||
|
||||
Assert.Equal(manage, (await authorization.AuthorizeAsync(user, SecretsAuthorization.ManagePolicy)).Succeeded);
|
||||
Assert.Equal(reveal, (await authorization.AuthorizeAsync(user, SecretsAuthorization.RevealPolicy)).Succeeded);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task An_unauthenticated_principal_satisfies_neither_policy_even_with_the_role_claim()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddAuthorization(o => o.AddSecretsAuthorization());
|
||||
await using ServiceProvider sp = services.BuildServiceProvider();
|
||||
var authorization = sp.GetRequiredService<IAuthorizationService>();
|
||||
|
||||
// No authentication type -> IsAuthenticated == false.
|
||||
var user = new ClaimsPrincipal(new ClaimsIdentity(
|
||||
[new Claim(ZbClaimTypes.Role, SecretsAuthorization.AdminRole)], authenticationType: null,
|
||||
ClaimTypes.Name, ZbClaimTypes.Role));
|
||||
|
||||
Assert.False((await authorization.AuthorizeAsync(user, SecretsAuthorization.ManagePolicy)).Succeeded);
|
||||
Assert.False((await authorization.AuthorizeAsync(user, SecretsAuthorization.RevealPolicy)).Succeeded);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user