diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs index 2f69026..e86d26c 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs @@ -2,6 +2,31 @@ using ZB.MOM.WW.Auth.Abstractions.Ldap; namespace ZB.MOM.WW.MxGateway.Server.Configuration; +/// +/// Gateway-side view of the MxGateway:Ldap section. This is a SHADOW of the +/// shared type and is NOT +/// used to perform LDAP authentication at runtime — runtime bind/search is done by the +/// shared ZB.MOM.WW.Auth.Ldap provider, whose options are bound directly from the +/// same MxGateway:Ldap section by AddZbLdapAuth (see +/// ). +/// +/// This shadow exists for three things only: (1) startup validation via +/// ; (2) the redacted effective-config display +/// ( / ); +/// and (3) it is the single home of the gateway's dev/default LDAP values, which the +/// integration live-test helper copies onto the shared options. +/// +/// +/// Review C2 — DRIFT WARNING: this class MUST stay field-compatible with the shared +/// so the one config section +/// binds cleanly onto both. The two are intentionally NOT merged because their defaults +/// differ on purpose: this shadow ships dev-friendly defaults (plaintext localhost, +/// AllowInsecure=true, populated SearchBase/ServiceAccount*), whereas +/// the shared type is secure-by-default (Transport=Ldaps, AllowInsecure=false, +/// empty DN fields). If you add/rename/remove a field on the shared type, mirror it here +/// (and in the validator + effective-config) so the section keeps binding to both. +/// +/// public sealed class LdapOptions { /// Gets a value indicating whether LDAP authentication is enabled. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs index 93efe70..47c211a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs @@ -72,6 +72,23 @@ public sealed class DashboardAuthenticator( roles)); } + /// + /// Builds the dashboard from the LDAP outcome. + /// + /// The (trimmed) login name → . + /// The user's display name → . + /// + /// The user's LDAP groups, as returned by . NOTE + /// (review C1): these are already-normalized short RDN names (e.g. + /// GwAdmin), not raw distinguished names. The shared + /// ZB.MOM.WW.Auth.Ldap provider strips each group DN to its first RDN + /// value before returning it, so the + /// claim carries the short name. This differs from the pre-cutover behaviour, + /// which surfaced the raw memberOf values (full DNs) on the claim; the + /// claim is informational only (no policy or UI reads its value — authorization + /// is role-based), so the shape change is non-breaking for dashboard consumers. + /// + /// The dashboard roles resolved from . private static ClaimsPrincipal CreatePrincipal( string username, string displayName, @@ -85,6 +102,8 @@ public sealed class DashboardAuthenticator( ]; claims.AddRange(roles.Select(role => new Claim(ClaimTypes.Role, role))); + // Groups are short RDN names from ILdapAuthService (see param doc above), so + // this claim value is the short group name, not the original DN. claims.AddRange(groups.Select(group => new Claim( DashboardAuthenticationDefaults.LdapGroupClaimType, group))); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs index f02b9b7..2fa1029 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs @@ -36,6 +36,19 @@ internal static class DashboardGroupRoleMapping // "ou=GwAdmin,ou=groups,..."). The map's comparer is // OrdinalIgnoreCase (see DashboardOptions.GroupToRole), so e.g. // "GwAdmin" and "gwadmin" both match. + // + // Review C1: with the shared ZB.MOM.WW.Auth.Ldap provider, groups + // arrive here already stripped to short RDN names (the library calls + // FirstRdnValue before returning them). So through the live login path + // the full-string branch only ever sees short names and the RDN + // fallback is effectively a no-op — they collapse to the same key. + // The fallback is retained because this mapping is also reachable + // directly via the IGroupRoleMapper seam (DashboardGroupRoleMapper), + // where a caller could still pass a full DN. CONSEQUENCE: configuring a + // full-DN GroupToRole *key* (e.g. "ou=GwAdmin,ou=groups,...") is + // UNSUPPORTED with the shared library — the incoming group is a short + // name, so it will never equal a full-DN key. Keep GroupToRole keys as + // short group names. if (groupToRole.TryGetValue(normalizedGroup, out string? mapped) || groupToRole.TryGetValue(ExtractFirstRdnValue(normalizedGroup), out mapped)) { diff --git a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj index fdeaacb..7fc4ca6 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj +++ b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj @@ -6,9 +6,9 @@ - - - + + + diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthenticatorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthenticatorTests.cs index a7598fe..49ba362 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthenticatorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthenticatorTests.cs @@ -81,9 +81,10 @@ public sealed class DashboardAuthenticatorTests } /// - /// On success the principal carries the resolved roles, the raw LDAP-group claims, - /// the display name (ClaimTypes.Name), and the username (ClaimTypes.NameIdentifier), - /// under the dashboard authentication scheme — the exact shape produced before the cutover. + /// On success the principal carries the resolved roles, the LDAP-group claims + /// (short RDN names as returned by ), the display + /// name (ClaimTypes.Name), and the username (ClaimTypes.NameIdentifier), under the + /// dashboard authentication scheme. /// [Fact] public async Task AuthenticateAsync_Success_BuildsPrincipalWithExpectedClaims() @@ -115,7 +116,8 @@ public sealed class DashboardAuthenticatorTests Assert.True(principal.IsInRole(DashboardRoles.Admin)); Assert.True(principal.IsInRole(DashboardRoles.Viewer)); - // Raw LDAP groups are surfaced under the dedicated group claim type. + // LDAP groups (already short RDN names from the service) are surfaced under + // the dedicated group claim type. IReadOnlyList groupClaims = principal.FindAll( DashboardAuthenticationDefaults.LdapGroupClaimType) .Select(claim => claim.Value) @@ -146,11 +148,22 @@ public sealed class DashboardAuthenticatorTests } /// - /// A group supplied as a full distinguished name still resolves to its role via the - /// mapper's leading-RDN fallback, and the original DN is preserved on the group claim. + /// Direct-injection path only (review C1): when an + /// implementation hands the authenticator a full distinguished name as a group, the + /// mapper's leading-RDN fallback still resolves the role, and whatever string the + /// service supplied is surfaced verbatim on the group claim. + /// + /// This is NOT the real production flow. The shared ZB.MOM.WW.Auth.Ldap + /// provider strips each group DN to its short RDN name before returning it, so the + /// authenticator never receives a full DN from the real library and the group claim + /// in production carries the short name (e.g. GwAdmin), not the DN. This test + /// uses a fake service to exercise only the authenticator's own pass-through of group + /// values; see + /// for the realistic (already-short) group shape. + /// /// [Fact] - public async Task AuthenticateAsync_GroupAsDistinguishedName_ResolvesRoleAndPreservesGroupClaim() + public async Task AuthenticateAsync_GroupAsDistinguishedNameFromService_ResolvesRoleAndSurfacesServiceValue() { const string groupDn = "ou=GwAdmin,ou=groups,dc=lmxopcua,dc=local"; FakeLdapAuthService ldap = new(LdapAuthResult.Success( @@ -166,7 +179,10 @@ public sealed class DashboardAuthenticatorTests Assert.True(result.Succeeded); ClaimsPrincipal principal = Assert.IsType(result.Principal); + // Role resolves via the leading-RDN fallback in DashboardGroupRoleMapping. Assert.True(principal.IsInRole(DashboardRoles.Admin)); + // The authenticator surfaces the value the (fake) service returned, verbatim. + // With the real library this value would already be the short RDN ("GwAdmin"). Assert.Contains( principal.FindAll(DashboardAuthenticationDefaults.LdapGroupClaimType), claim => claim.Value == groupDn);