fix(auth): canonical username is the attribute value the user matched (Auth-009)

ExtractUserName returned the first value of a possibly multi-valued
UserNameAttribute (default cn), so jsmith could sign in as "John Smith"
depending on server ordering. It now prefers the value equal to the typed
name ignoring case (still canonicalising case) and falls back to the first
value only when none matches.

(scadaproj#46)

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 15:18:25 -05:00
parent b9a096c53a
commit 9ec0da1b2c
3 changed files with 81 additions and 9 deletions
@@ -541,19 +541,29 @@ public sealed class LdapAuthService : ILdapAuthService
}
/// <summary>
/// Returns the first non-blank value of the configured username attribute — the directory's
/// canonical spelling — falling back to the (already normalised) typed username.
/// Returns the directory's spelling of the name the user matched on: the value of the configured
/// username attribute that equals the typed (already normalised) name ignoring case. Only when no
/// value does is the first non-blank value used, and only when there is none the typed name.
/// </summary>
/// <remarks>
/// The attribute can be multi-valued — the default <c>cn</c> is, and OpenLDAP / 389-DS entries
/// routinely carry both <c>cn: John Smith</c> and <c>cn: jsmith</c>. 0.4.1 returned whichever
/// value the server listed first, so <c>jsmith</c> could sign in as <c>John Smith</c> depending on
/// attribute ordering (Auth-009). Matching ordinal-ignore-case still canonicalises case
/// (<c>JSMITH</c> → <c>jsmith</c>), which is what 0.4.1 set out to do.
/// </remarks>
private string ExtractUserName(LdapSearchEntry entry, string username)
{
if (entry.Attributes.TryGetValue(_options.UserNameAttribute, out var values)
&& values.Count > 0
&& !string.IsNullOrWhiteSpace(values[0]))
if (!entry.Attributes.TryGetValue(_options.UserNameAttribute, out var values) || values.Count == 0)
return username;
foreach (var value in values)
{
return values[0];
if (string.Equals(value, username, StringComparison.OrdinalIgnoreCase))
return value;
}
return username;
return string.IsNullOrWhiteSpace(values[0]) ? username : values[0];
}
/// <summary>
@@ -174,6 +174,54 @@ public class LdapAuthServiceTests
Assert.Equal("ALICE", r.Username);
}
// ---- Auth-009 (scadaproj#46): a multi-valued UserNameAttribute ----------------------------
[Fact]
public async Task Username_FromAMultiValuedAttribute_IsTheValueTheUserMatched()
{
// OpenLDAP / 389-DS entries routinely carry both cn values; the server's ordering decides
// which comes first. The search matched "jsmith", so that is the identity, not "John Smith".
var fake = new FakeLdapConnection().WithUserEntry(
"cn=jsmith,dc=x", new[] { "cn=g,dc=x" }, "John",
new Dictionary<string, string[]> { ["cn"] = new[] { "John Smith", "jsmith" } });
var svc = new LdapAuthService(Opts(), new FakeLdapConnectionFactory(fake));
var r = await svc.AuthenticateAsync("jsmith", "pw", default);
Assert.True(r.Succeeded);
Assert.Equal("jsmith", r.Username);
}
[Fact]
public async Task Username_FromAMultiValuedAttribute_StillTakesTheDirectorysCase()
{
var fake = new FakeLdapConnection().WithUserEntry(
"cn=jsmith,dc=x", new[] { "cn=g,dc=x" }, "John",
new Dictionary<string, string[]> { ["cn"] = new[] { "John Smith", "JSmith" } });
var svc = new LdapAuthService(Opts(), new FakeLdapConnectionFactory(fake));
var r = await svc.AuthenticateAsync("jsmith", "pw", default);
Assert.True(r.Succeeded);
Assert.Equal("JSmith", r.Username);
}
[Fact]
public async Task Username_FallsBackToTheFirstValue_WhenNoValueMatchesTheTypedName()
{
// The directory matched on something the attribute values do not spell the same way
// (a matching rule beyond case, say): keep 0.4.1's first-value behaviour.
var fake = new FakeLdapConnection().WithUserEntry(
"cn=jsmith,dc=x", new[] { "cn=g,dc=x" }, "John",
new Dictionary<string, string[]> { ["cn"] = new[] { "J. Smith", "John Smith" } });
var svc = new LdapAuthService(Opts(), new FakeLdapConnectionFactory(fake));
var r = await svc.AuthenticateAsync("jsmith", "pw", default);
Assert.True(r.Succeeded);
Assert.Equal("J. Smith", r.Username);
}
[Theory]
[InlineData("")]
[InlineData(null)]
+16 -2
View File
@@ -10,7 +10,7 @@
| Last reviewed | 2026-11-09 (re-review; baseline 2026-06-01 at `5f75cd4`) |
| Reviewer | Claude (2026-11-09 re-review, scadaproj#15; baseline: Claude automated) |
| Commit reviewed | `91fb2b4` |
| Open findings | 3 |
| Open findings | 2 |
## Summary
@@ -429,7 +429,7 @@ I verified this live against a SQL Server 2022 container:
| Severity | Low |
| Priority | P3 |
| Category | Correctness & logic bugs |
| Status | Open |
| Status | Resolved |
| Location | `ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs:437` |
**Description**
@@ -454,6 +454,20 @@ fake connection: typed `jsmith` returned `Username == "John Smith"`.
Prefer the attribute value that equals the typed (normalised) name under `OrdinalIgnoreCase`, which still
canonicalises case. Fall back to `values[0]` only when no value matches. Add a multi-valued-attribute test.
**Resolution**
Resolved 2026-11-09 (scadaproj#46, Auth `0.4.4`, unpublished). Re-verified through the fake connection: with
`cn: ["John Smith", "jsmith"]`, typed `jsmith` returned `Username == "John Smith"`. `ExtractUserName` now returns the
attribute value that equals the typed (normalised) name under `OrdinalIgnoreCase`. That keeps 0.4.1's case
canonicalisation, so typed `jsmith` against `JSmith` still returns `JSmith`. Only when no value matches does it fall
back to the first non-blank value, and then to the typed name, as 0.4.1 did. Both bind modes go through
`SucceedFromEntry`, so both are covered.
Tests (`LdapAuthServiceTests`):
- `Username_FromAMultiValuedAttribute_IsTheValueTheUserMatched` fails pre-fix.
- `..._StillTakesTheDirectorysCase` fails pre-fix.
- `Username_FallsBackToTheFirstValue_WhenNoValueMatchesTheTypedName` is a guard that passes both ways.
### Auth-010 — `ApiKeyVerifier` lets store exceptions escape despite its "only exception path is cancellation" contract
| | |