dohertj2/scadaproj
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(auth): canonical username from the entry; refuse empty passwords in both bind modes

Browse Source 
- Request UserNameAttribute in the search (de-duplicated, case-insensitive) and return its
  first value as LdapAuthResult.Username, falling back to the typed name (I2).
- The empty-password guard now applies in ServiceAccount mode too: AD treats an empty-password
  simple bind as an unauthenticated bind and reports success (I3).
- Restore the dropped explanatory comments in CompleteWithServiceAccount / SearchUser.
This commit is contained in:
Joseph Doherty
2026-10-01 01:27:27 -04:00
parent 177881c423
commit e3d146aa21
5 changed files with 162 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/Internal/LdapUserName.cs
+5 -2
View File
@@ -2,8 +2,11 @@ namespace ZB.MOM.WW.Auth.Ldap.Internal;
/// <summary>
/// Reduces a typed login to the bare account name used by <see cref="Abstractions.Ldap.LdapBindMode.User"/>:
/// one spelling flows into the bind template, the search filter and the returned username, so audit
/// rows never carry <c>DOMAIN\jdoe</c> for one sign-in and <c>jdoe</c> for the next.
/// the qualifier is stripped before the name reaches the bind template and the search filter, so audit
/// rows never carry <c>DOMAIN\jdoe</c> for one sign-in and <c>jdoe</c> for the next. Case is NOT
/// normalised here: the returned username is canonicalised from the directory entry's
/// <c>UserNameAttribute</c> value, so <c>JDoe</c> and <c>jdoe</c> also yield one identity; this name is
/// only the fallback when the entry has no such value.
/// </summary>
internal static class LdapUserName
{
ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs
+59 -20
View File
@@ -145,16 +145,19 @@ public sealed class LdapAuthService : ILdapAuthService
// filter and the returned result (avoids two identities for one person).
username = username.Trim();
// 3b. User-bind mode guards, before any network call. An empty password must never reach
// the directory: AD answers a simple bind with an empty password as an UNAUTHENTICATED
// bind and reports success, which would admit anyone. The name is reduced to the bare
// account name so the bind template, the search filter and the result share one spelling.
if (_options.BindMode == LdapBindMode.User)
// 3b. An empty password must never reach the directory, in EITHER bind mode: AD answers a
// simple bind with an empty password as an UNAUTHENTICATED bind and reports success —
// the user bind (or the ServiceAccount-mode re-bind as the user's DN) would admit anyone.
// Refused here, before any network call, so no consumer depends on its own edge check.
if (string.IsNullOrEmpty(password))
return LdapAuthResult.Fail(LdapAuthFailure.BadCredentials);
// 3c. User-bind mode: reduce the name to the bare account name the bind template and the
// search filter share. (The returned username is canonicalised from the entry later.)
if (_options.BindMode == LdapBindMode.User
&& !LdapUserName.TryNormalize(username, _options.AcceptDomainQualifiedNames, out username))
{
if (string.IsNullOrEmpty(password))
return LdapAuthResult.Fail(LdapAuthFailure.BadCredentials);
if (!LdapUserName.TryNormalize(username, _options.AcceptDomainQualifiedNames, out username))
return LdapAuthResult.Fail(LdapAuthFailure.BadCredentials);
return LdapAuthResult.Fail(LdapAuthFailure.BadCredentials);
}
// 4. Walk the endpoint list, preferred one first. A single-endpoint configuration
@@ -278,7 +281,8 @@ public sealed class LdapAuthService : ILdapAuthService
private LdapAuthResult CompleteWithServiceAccount(ZB.MOM.WW.Auth.Ldap.Internal.ILdapConnection conn, string username, string password)
{
// 6. Service-account bind so we can search for the user's DN. A bind failure here is a
// service-account misconfiguration — DISTINCT from a user-credential failure.
// service-account misconfiguration — DISTINCT from a user-credential failure — so it
// maps to ServiceAccountBindFailed.
try
{
conn.Bind(_options.ServiceAccountDn, _options.ServiceAccountPassword);
@@ -288,7 +292,9 @@ public sealed class LdapAuthService : ILdapAuthService
throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed, "service-bind");
}
// 7–8. Exactly one match. Zero -> UserNotFound; two or more -> AmbiguousUser.
// 7–8. Require exactly one match. Zero -> UserNotFound; two or more -> AmbiguousUser.
// Both are authoritative answers from a healthy directory, so they end the walk.
// We never attempt a user bind against an ambiguous DN.
var entries = SearchUser(conn, username);
if (entries.Count == 0)
return LdapAuthResult.Fail(LdapAuthFailure.UserNotFound);
@@ -297,7 +303,9 @@ public sealed class LdapAuthService : ILdapAuthService
var entry = entries[0];
// 9. User bind verifies the password; a failure is the user's bad credentials and final.
// 9. User bind: re-bind as the resolved DN to verify the password. A bind failure here is
// the end user's bad credentials — the directory answered, so this is returned directly
// and never retried against another endpoint.
try
{
conn.Bind(entry.Dn, password);
@@ -344,6 +352,10 @@ public sealed class LdapAuthService : ILdapAuthService
/// <summary>Step 7: search for the user entry; a search fault is system-side.</summary>
private IReadOnlyList<LdapSearchEntry> SearchUser(ZB.MOM.WW.Auth.Ldap.Internal.ILdapConnection conn, string username)
{
// A search failure is infrastructure (directory unreachable / unhealthy) — system-side.
// NOTE: the LdapAuthFailure enum has no DirectoryUnavailable value, so a failed search maps
// to ServiceAccountBindFailed, the closest system-side bucket (same limitation as
// Connect), until a DirectoryUnavailable value exists.
try
{
var filter = $"({_options.UserNameAttribute}={LdapEscaping.Filter(username)})";
@@ -355,7 +367,12 @@ public sealed class LdapAuthService : ILdapAuthService
}
}
/// <summary>Steps 10–11: fail-closed group extraction, then success.</summary>
/// <summary>
/// Steps 10–11: fail-closed group extraction, then success. The returned username is the
/// entry's own <see cref="LdapOptions.UserNameAttribute"/> value (the directory's spelling), so
/// <c>JDoe</c> and <c>jdoe</c> sign in as one identity; the typed (normalised) name is the
/// fallback when the entry carries no such value.
/// </summary>
private LdapAuthResult SucceedFromEntry(LdapSearchEntry entry, string username)
{
IReadOnlyList<string> groups;
@@ -371,7 +388,8 @@ public sealed class LdapAuthService : ILdapAuthService
if (groups.Count == 0)
return LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed);
return LdapAuthResult.Success(username, ExtractDisplayName(entry, username), groups);
var canonical = ExtractUserName(entry, username);
return LdapAuthResult.Success(canonical, ExtractDisplayName(entry, canonical), groups);
}
/// <summary>
@@ -393,16 +411,37 @@ public sealed class LdapAuthService : ILdapAuthService
}
/// <summary>
/// Builds the distinct attribute list requested from the directory. The display-name
/// and group attributes are de-duplicated so we never request the same attribute twice
/// when an operator configures them to the same value.
/// Builds the distinct attribute list requested from the directory: display name, groups and
/// the username attribute (read back as the canonical username). They are de-duplicated,
/// case-insensitively, so we never request the same attribute twice when an operator
/// configures two of them to the same value.
/// </summary>
private IReadOnlyList<string> BuildSearchAttributes()
{
if (string.Equals(_options.DisplayNameAttribute, _options.GroupAttribute, StringComparison.OrdinalIgnoreCase))
return new[] { _options.DisplayNameAttribute };
var attributes = new List<string>(3);
foreach (var name in new[] { _options.DisplayNameAttribute, _options.GroupAttribute, _options.UserNameAttribute })
{
if (!attributes.Contains(name, StringComparer.OrdinalIgnoreCase))
attributes.Add(name);
}
return new[] { _options.DisplayNameAttribute, _options.GroupAttribute };
return attributes;
}
/// <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.
/// </summary>
private string ExtractUserName(LdapSearchEntry entry, string username)
{
if (entry.Attributes.TryGetValue(_options.UserNameAttribute, out var values)
&& values.Count > 0
&& !string.IsNullOrWhiteSpace(values[0]))
{
return values[0];
}
return username;
}
/// <summary>