fix(auth): ScadaBridge 1.2 review fixes — secret-test repoint, checklist, Scope guard, 0.1.1 pin

This commit is contained in:
Joseph Doherty
2026-06-02 01:23:52 -04:00
parent ac34dac479
commit 4db8c373af
5 changed files with 44 additions and 19 deletions
+4 -4
View File
@@ -80,10 +80,10 @@
<PackageVersion Include="ZB.MOM.WW.MxGateway.Client" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.MxGateway.Contracts" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Configuration" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Auth.ApiKeys" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Auth.ApiKeys" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" />
</ItemGroup>
+4 -3
View File
@@ -18,9 +18,10 @@
- [ ] EF Core migrations have been applied (SQL script reviewed and executed)
- [ ] `ScadaBridge:Security:JwtSigningKey` is at least 32 characters, randomly generated
- [ ] **Both central nodes use the same JwtSigningKey** (required for JWT failover)
- [ ] `ScadaBridge:Security:LdapServer` points to the production LDAP/AD server
- [ ] `ScadaBridge:Security:LdapUseTls` is `true` (LDAPS required in production)
- [ ] `ScadaBridge:Security:AllowInsecureLdap` is `false`
- [ ] `ScadaBridge:Security:Ldap:Server` points to the production LDAP/AD server
- [ ] `ScadaBridge:Security:Ldap:Transport` is `Ldaps` (LDAPS required in production)
- [ ] `ScadaBridge:Security:Ldap:AllowInsecure` is `false`
- [ ] LDAP service-account password supplied via env var `ScadaBridge__Security__Ldap__ServiceAccountPassword` (renamed from `ScadaBridge__Security__LdapServiceAccountPassword` in the Task 1.4 nested-config cutover)
- [ ] LDAP search base DN is correct for the organization
- [ ] LDAP group-to-role mappings are configured
- [ ] Load balancer is configured in front of central UI (sticky sessions not required)
+9 -7
View File
@@ -246,13 +246,15 @@ These are clones of `docker/central-node-a/appsettings.Central.json` and `docker
"MachineDataDb": "Server=scadabridge-mssql,1433;Database=ScadaBridgeMachineData2;User Id=scadabridge_app;Password=ScadaBridge_Dev1#;TrustServerCertificate=true"
},
"Security": {
"LdapServer": "scadabridge-ldap",
"LdapPort": 3893,
"LdapUseTls": false,
"AllowInsecureLdap": true,
"LdapSearchBase": "dc=scadabridge,dc=local",
"LdapServiceAccountDn": "cn=admin,dc=scadabridge,dc=local",
"LdapServiceAccountPassword": "password",
"Ldap": {
"Server": "scadabridge-ldap",
"Port": 3893,
"Transport": "None",
"AllowInsecure": true,
"SearchBase": "dc=scadabridge,dc=local",
"ServiceAccountDn": "cn=admin,dc=scadabridge,dc=local",
"ServiceAccountPassword": "password"
},
"JwtSigningKey": "scadabridge-env2-dev-jwt-signing-key-must-be-at-least-32-characters-long",
"JwtExpiryMinutes": 15,
"IdleTimeoutMinutes": 30,
@@ -51,7 +51,17 @@ public static class AuthEndpoints
// system-wide flag — is carried in the mapping's opaque Scope so the
// site-scope→SiteId claims below are built exactly as before.
var roleMapping = await roleMapper.MapAsync(authResult.Groups, context.RequestAborted);
var scope = (RoleMappingResult)roleMapping.Scope!;
// The ScadaBridge mapper carries the full RoleMappingResult in the seam's
// opaque Scope (see ScadaBridgeGroupRoleMapper). Guard the unwrap (review I4):
// a future/alternate IGroupRoleMapper<string> could leave Scope null or set a
// different type. Rather than throw InvalidCastException mid-login, fall back to
// the most restrictive interpretation — not a system-wide deployment and no
// permitted sites — so no SiteId claims are stamped (deny-by-omission). The real
// ScadaBridge mapper always supplies a RoleMappingResult, so behaviour is unchanged.
var scope = roleMapping.Scope is RoleMappingResult mapped
? mapped
: new RoleMappingResult(roleMapping.Roles, [], IsSystemWideDeployment: false);
// Build claims from LDAP auth + role mapping.
// CentralUI-005: no fixed "expires_at" absolute-cap claim is stamped
@@ -116,7 +126,13 @@ public static class AuthEndpoints
}
var roleMapping = await roleMapper.MapAsync(authResult.Groups, context.RequestAborted);
var scope = (RoleMappingResult)roleMapping.Scope!;
// Guard the opaque-Scope unwrap (review I4); see the matching note on
// /auth/login. Fall back to no site-scope rather than throwing if a future
// mapper leaves Scope null or sets a different type.
var scope = roleMapping.Scope is RoleMappingResult mapped
? mapped
: new RoleMappingResult(roleMapping.Roles, [], IsSystemWideDeployment: false);
var displayName = string.IsNullOrEmpty(authResult.DisplayName) ? username : authResult.DisplayName;
var resolvedUsername = string.IsNullOrEmpty(authResult.Username) ? username : authResult.Username;
@@ -61,14 +61,20 @@ public class ConfigSecretsTests
[Fact]
public void CentralConfig_LdapServiceAccountPassword_IsNotCommitted()
{
// Task 1.4 cutover: the LDAP service-account password moved out of the flat
// Security:LdapServiceAccountPassword key into the nested Security:Ldap
// sub-section (Security:Ldap:ServiceAccountPassword), bound to the shared
// ZB.MOM.WW.Auth LdapOptions. Walk into Security:Ldap and guard the nested
// key — checking the deleted flat key would pass vacuously.
var security = ScadaBridgeSection().GetProperty("Security");
if (security.TryGetProperty("LdapServiceAccountPassword", out var pw))
var ldap = security.GetProperty("Ldap");
if (ldap.TryGetProperty("ServiceAccountPassword", out var pw))
{
var value = pw.GetString() ?? string.Empty;
Assert.True(
value.Length == 0 || value.Contains('{') || value.Contains('$'),
$"appsettings.Central.json carries a plaintext LdapServiceAccountPassword '{value}'. " +
"Move it to an environment variable.");
$"appsettings.Central.json carries a plaintext Security:Ldap:ServiceAccountPassword '{value}'. " +
"Move it to an environment variable (ScadaBridge__Security__Ldap__ServiceAccountPassword).");
}
}