Skip to content

Commit c0e7e71

Browse files
test(#1695): cover identity-form vs username branching in ResolveHumanGuidAsync (#1711)
The PermissionService fix landed in PR #1691 as one of several changes; #1695 remained open because the test coverage that locks in the regression behaviour hadn't followed. This adds the four facts the issue called for so the bug cannot silently come back: - ResolveHumanGuidAsync_IdentityFormGuid_ResolvesDirectly: 32-hex "N" form short-circuits, IHumanIdentityResolver is never invoked. - ResolveHumanGuidAsync_IdentityFormGuid_DashedForm_ResolvesDirectly: GuidFormatter.TryParse accepts both "N" and "D" forms, so dashed callers also bypass the resolver. - ResolveHumanGuidAsync_LegacyUsername_FallsThroughToResolver: legacy username-form callers (e.g. local-dev-user) still hit the resolver and the upsert-on-first-contact path is preserved. - ResolveHumanGuidAsync_IdentityForm_DoesNotCreatePhantomRow: regression test against the spring.humans phantom-row symptom — asserts ResolveByUsernameAsync is never invoked across both identity-form shapes, which is what guarantees the resolver's upsert-on-miss can no longer create a phantom row keyed by GUID-hex. Tests inject a real ServiceCollection-backed IServiceScopeFactory so the service exercises the production scope-resolution path and the substituted IHumanIdentityResolver records every call (matching the pattern used in UnitCreationServiceTests). Closes #1695. Co-authored-by: savasp-agent[bot] <275188714+savasp-agent[bot]@users.noreply.github.com>
1 parent 688b748 commit c0e7e71

1 file changed

Lines changed: 108 additions & 0 deletions

File tree

tests/Cvoya.Spring.Dapr.Tests/Auth/PermissionServiceTests.cs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ namespace Cvoya.Spring.Dapr.Tests.Auth;
66
using Cvoya.Spring.Core.Directory;
77
using Cvoya.Spring.Core.Identifiers;
88
using Cvoya.Spring.Core.Messaging;
9+
using Cvoya.Spring.Core.Security;
910
using Cvoya.Spring.Core.Units;
1011
using Cvoya.Spring.Dapr.Actors;
1112
using Cvoya.Spring.Dapr.Auth;
1213

1314
using global::Dapr.Actors;
1415
using global::Dapr.Actors.Client;
1516

17+
using Microsoft.Extensions.DependencyInjection;
1618
using Microsoft.Extensions.Logging;
1719

1820
using NSubstitute;
@@ -330,4 +332,110 @@ public async Task ResolveEffectivePermissionAsync_DirectoryHasNoEntry_ReturnsNul
330332

331333
result.ShouldBeNull();
332334
}
335+
336+
// -- #1695 ----------------------------------------------------------
337+
// Identity-form callers (human:id:<guid>) hand the GUID-hex through
338+
// PermissionService directly. The pre-fix code blindly passed every
339+
// humanId string to IHumanIdentityResolver.ResolveByUsernameAsync,
340+
// which on miss upserted a phantom row keyed by the GUID-hex with
341+
// its own brand-new UUID — the unit's permission map then 403'd the
342+
// legitimate caller and the spring.humans table grew a leaking row
343+
// per send. The guard short-circuits the resolver when humanId
344+
// already parses as a Guid, so neither symptom can recur.
345+
346+
[Fact]
347+
public async Task ResolveHumanGuidAsync_IdentityFormGuid_ResolvesDirectly()
348+
{
349+
// Identity-form: 32-hex-char "N" form (the wire shape produced by
350+
// GuidFormatter.Format and emitted on the From address path
351+
// component). Must round-trip through the service to the unit
352+
// permission map without ever calling the resolver.
353+
var ct = TestContext.Current.CancellationToken;
354+
var resolver = Substitute.For<IHumanIdentityResolver>();
355+
var service = BuildServiceWithResolver(resolver);
356+
357+
var identityFormHex = HumanGuid.ToString("N");
358+
Unit(UnitOneId).GetHumanPermissionAsync(HumanGuid, ct).Returns(PermissionLevel.Owner);
359+
360+
var result = await service.ResolvePermissionAsync(identityFormHex, Id(UnitOneId), ct);
361+
362+
result.ShouldBe(PermissionLevel.Owner);
363+
await resolver.DidNotReceive().ResolveByUsernameAsync(
364+
Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<CancellationToken>());
365+
}
366+
367+
[Fact]
368+
public async Task ResolveHumanGuidAsync_IdentityFormGuid_DashedForm_ResolvesDirectly()
369+
{
370+
// GuidFormatter.TryParse accepts both "N" and "D" forms — assert
371+
// the dashed shape also short-circuits (callers in either form
372+
// must land on the same row, per the issue's prescription).
373+
var ct = TestContext.Current.CancellationToken;
374+
var resolver = Substitute.For<IHumanIdentityResolver>();
375+
var service = BuildServiceWithResolver(resolver);
376+
377+
Unit(UnitOneId).GetHumanPermissionAsync(HumanGuid, ct).Returns(PermissionLevel.Owner);
378+
379+
var result = await service.ResolvePermissionAsync(HumanIdString, Id(UnitOneId), ct);
380+
381+
result.ShouldBe(PermissionLevel.Owner);
382+
await resolver.DidNotReceive().ResolveByUsernameAsync(
383+
Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<CancellationToken>());
384+
}
385+
386+
[Fact]
387+
public async Task ResolveHumanGuidAsync_LegacyUsername_FallsThroughToResolver()
388+
{
389+
// Username-form callers (e.g. "local-dev-user", cloud OAuth
390+
// usernames) must still flow through the resolver so the
391+
// upsert-on-first-contact behaviour is preserved.
392+
var ct = TestContext.Current.CancellationToken;
393+
var resolver = Substitute.For<IHumanIdentityResolver>();
394+
resolver.ResolveByUsernameAsync("local-dev-user", null, Arg.Any<CancellationToken>())
395+
.Returns(HumanGuid);
396+
var service = BuildServiceWithResolver(resolver);
397+
398+
Unit(UnitOneId).GetHumanPermissionAsync(HumanGuid, ct).Returns(PermissionLevel.Owner);
399+
400+
var result = await service.ResolvePermissionAsync("local-dev-user", Id(UnitOneId), ct);
401+
402+
result.ShouldBe(PermissionLevel.Owner);
403+
await resolver.Received(1).ResolveByUsernameAsync(
404+
"local-dev-user", null, Arg.Any<CancellationToken>());
405+
}
406+
407+
[Fact]
408+
public async Task ResolveHumanGuidAsync_IdentityForm_DoesNotCreatePhantomRow()
409+
{
410+
// Regression test for the phantom-humans-row symptom in #1695.
411+
// Sending a message with an identity-form From address used to
412+
// call ResolveByUsernameAsync(<guid-hex>) → resolver upsert
413+
// created a row whose username was the GUID-hex. We assert
414+
// ResolveByUsernameAsync is never invoked for any of the input
415+
// shapes the AuthenticatedCallerAccessor produces (#1485 / #1491
416+
// shape: bare "N" form, plus the dashed "D" form for resilience).
417+
var ct = TestContext.Current.CancellationToken;
418+
var resolver = Substitute.For<IHumanIdentityResolver>();
419+
var service = BuildServiceWithResolver(resolver);
420+
421+
await service.ResolvePermissionAsync(HumanGuid.ToString("N"), Id(UnitOneId), ct);
422+
await service.ResolvePermissionAsync(HumanGuid.ToString("D"), Id(UnitOneId), ct);
423+
424+
await resolver.DidNotReceive().ResolveByUsernameAsync(
425+
Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<CancellationToken>());
426+
}
427+
428+
private PermissionService BuildServiceWithResolver(IHumanIdentityResolver resolver)
429+
{
430+
var services = new ServiceCollection();
431+
services.AddScoped(_ => resolver);
432+
var scopeFactory = services.BuildServiceProvider().GetRequiredService<IServiceScopeFactory>();
433+
434+
return new PermissionService(
435+
_actorProxyFactory,
436+
_hierarchyResolver,
437+
_directoryService,
438+
_loggerFactory,
439+
scopeFactory);
440+
}
333441
}

0 commit comments

Comments
 (0)