feat(acl): ApiKey entity + key format, and put Core on the server's graph (#520)
Stage 1b. Still no enforcement wired — this adds the credential model the identity cutover (stage 2) will read. ApiKey holds one principal per key, deliberately. The reason to separate keys from principals is so many keys share a grant set and rotating a key never touches an ACL row, and that holds with one principal per key since nothing stops two keys naming the same principal. A key belonging to several principals at once buys little and costs a join table. UserAuthorizationState.Principals stays a list, so adding it later needs no ACL-side change. ApiKeySecret is the security-critical half and is pure, so it is tested on its own: otua_<keyId>_<secret>, peppered HMAC-SHA256, FixedTimeEquals. Notes why a plain HMAC rather than an iterated KDF (the secret is 32 bytes of CSPRNG, and the call runs inline on session activation, which the SDK serializes server-wide behind its SessionManager event lock), and fails closed on an unset pepper rather than hashing under an empty key. Found and fixed a bug in my own first draft: the key id was url-safe base64, whose alphabet maps '/' to '_' — the very character TryParse splits on. Roughly one key in five would have had its id truncated at a random position, an intermittent auth failure reproducing only for SOME keys. The id is hex now, and the guard is falsified: reverting the alphabet reds both The_generated_key_id_never_contains_the_separator and, as a bonus signal, Two_generated_keys_never_collide, because truncated ids collide. ApiKeyId is NOT NULL, unlike the nullable NodeAclId precedent whose filtered unique index would admit two NULL ids. Fine for a config row, not for a credentials lookup key. OpcUaServer.csproj now references Core. Until now it did not, so IPermissionEvaluator was literally uncallable from the node manager — the place enforcement has to happen. No new transitive package surface. Schema doc updated for both, and its NodeAcl block flagged for pre-existing drift (it still documents the GenerationId column v3 dropped) without fixing it here. Core 247, OpcUaServer 364, Configuration 128 — all green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
@@ -32,7 +32,9 @@ public sealed class SchemaComplianceTests
|
||||
// v3 Raw-tree tables (Namespace retired; RawFolder/TagGroup/UnsTagReference added)
|
||||
"RawFolder", "TagGroup", "UnsTagReference",
|
||||
"DriverInstance", "Device", "Equipment", "Tag", "PollGroup", "VirtualTag",
|
||||
"NodeAcl", "ExternalIdReservation",
|
||||
// ApiKey added in Gitea #520 — the OPC UA data plane is machine-to-machine, so a consuming
|
||||
// system authenticates with a key resolving to a Principal that NodeAcl rows grant to.
|
||||
"NodeAcl", "ApiKey", "ExternalIdReservation",
|
||||
// DriverHostStatus was dropped in Gitea #521 — nothing ever wrote a row, and per-cluster
|
||||
// mesh Phase 4 made the publisher its entity doc described unbuildable (a driver-only node
|
||||
// has no ConfigDb connection). Per-host connectivity now rides DriverHealthChanged to /hosts.
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Authorization;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Authorization;
|
||||
|
||||
/// <summary>
|
||||
/// Covers the pure API-key format helpers (#520). These are the security-critical half of the
|
||||
/// data-plane identity change, so they are pinned independently of any store or DI.
|
||||
/// </summary>
|
||||
public sealed class ApiKeySecretTests
|
||||
{
|
||||
private const string Pepper = "test-pepper-not-a-real-one";
|
||||
|
||||
[Fact]
|
||||
public void A_generated_key_round_trips_through_parse_and_verify()
|
||||
{
|
||||
var (presented, apiKeyId, hash) = ApiKeySecret.Generate(Pepper);
|
||||
|
||||
ApiKeySecret.TryParse(presented, out var parsedId, out var secret).ShouldBeTrue();
|
||||
parsedId.ShouldBe(apiKeyId);
|
||||
ApiKeySecret.Verify(secret, Pepper, hash).ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void The_generated_key_id_never_contains_the_separator()
|
||||
{
|
||||
// Regression guard. The secret is url-safe base64, whose alphabet maps '/' to '_'. If the key id
|
||||
// used that same alphabet it could contain the character TryParse splits on, and the id would be
|
||||
// silently truncated at a random position for roughly 1 key in 5 — an intermittent auth failure
|
||||
// that only ever reproduces for SOME keys. Hence hex. 400 draws makes a '_' near-certain if the
|
||||
// alphabet ever regresses to base64.
|
||||
for (var i = 0; i < 400; i++)
|
||||
{
|
||||
var (_, apiKeyId, _) = ApiKeySecret.Generate(Pepper);
|
||||
apiKeyId.ShouldNotContain("_", Case.Sensitive,
|
||||
$"key id '{apiKeyId}' contains the separator — TryParse would truncate it");
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_secret_containing_the_separator_still_round_trips()
|
||||
{
|
||||
// The secret IS allowed to contain '_' (it is the whole remainder), so prove the split is
|
||||
// first-two-underscores rather than split-on-every-underscore.
|
||||
const string secret = "aa_bb_cc__dd";
|
||||
var presented = $"otua_deadbeef_{secret}";
|
||||
|
||||
ApiKeySecret.TryParse(presented, out var id, out var parsed).ShouldBeTrue();
|
||||
id.ShouldBe("deadbeef");
|
||||
parsed.ShouldBe(secret);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(null)]
|
||||
[InlineData("")]
|
||||
[InlineData("otua")]
|
||||
[InlineData("otua_")]
|
||||
[InlineData("otua_onlyid")]
|
||||
[InlineData("_noprefix_secret")]
|
||||
[InlineData("wrong_id_secret")]
|
||||
[InlineData("otua__emptyid")]
|
||||
public void A_malformed_key_is_rejected_without_throwing(string? presented)
|
||||
{
|
||||
// These arrive from the network, so malformed input must be an ordinary auth failure.
|
||||
ApiKeySecret.TryParse(presented, out _, out _).ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_wrong_secret_does_not_verify()
|
||||
{
|
||||
var (_, _, hash) = ApiKeySecret.Generate(Pepper);
|
||||
ApiKeySecret.Verify("not-the-secret", Pepper, hash).ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void The_same_secret_under_a_different_pepper_does_not_verify()
|
||||
{
|
||||
// This is what the pepper buys: a leaked ApiKey table cannot be verified against offline.
|
||||
var (presented, _, hash) = ApiKeySecret.Generate(Pepper);
|
||||
ApiKeySecret.TryParse(presented, out _, out var secret).ShouldBeTrue();
|
||||
|
||||
ApiKeySecret.Verify(secret, "a-different-pepper", hash).ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
[InlineData(null)]
|
||||
public void Hashing_under_an_absent_pepper_throws_rather_than_failing_open(string? pepper)
|
||||
{
|
||||
// Fail closed. An empty pepper would still produce a stable hash, so a misconfigured deployment
|
||||
// would authenticate normally while offering none of the protection it believes it has.
|
||||
Should.Throw<ArgumentException>(() => ApiKeySecret.ComputeHash("secret", pepper!));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_non_base64_stored_hash_is_rejected_rather_than_throwing()
|
||||
{
|
||||
// A corrupted or hand-edited row must deny, not crash session activation for everyone.
|
||||
ApiKeySecret.Verify("secret", Pepper, "!!!not base64!!!").ShouldBeFalse();
|
||||
ApiKeySecret.Verify("secret", Pepper, null).ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Two_generated_keys_never_collide()
|
||||
{
|
||||
var ids = new HashSet<string>(StringComparer.Ordinal);
|
||||
var secrets = new HashSet<string>(StringComparer.Ordinal);
|
||||
for (var i = 0; i < 200; i++)
|
||||
{
|
||||
var (presented, apiKeyId, _) = ApiKeySecret.Generate(Pepper);
|
||||
ApiKeySecret.TryParse(presented, out _, out var secret).ShouldBeTrue();
|
||||
ids.Add(apiKeyId).ShouldBeTrue($"duplicate key id '{apiKeyId}'");
|
||||
secrets.Add(secret).ShouldBeTrue("duplicate secret");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user