feat(auth): cut ScadaBridge over to ZB.MOM.WW.Auth.Ldap; nest+rename Ldap config; roles+sitescope via IGroupRoleMapper (Task 1.2/1.4)
This commit is contained in:
@@ -43,11 +43,15 @@ public class CentralActorPathTests : IAsyncLifetime
|
||||
["ScadaBridge:Cluster:MinNrOfMembers"] = "1",
|
||||
["ScadaBridge:Database:SkipMigrations"] = "true",
|
||||
["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!",
|
||||
["ScadaBridge:Security:LdapServer"] = "localhost",
|
||||
["ScadaBridge:Security:LdapPort"] = "3893",
|
||||
["ScadaBridge:Security:LdapUseTls"] = "false",
|
||||
["ScadaBridge:Security:AllowInsecureLdap"] = "true",
|
||||
["ScadaBridge:Security:LdapSearchBase"] = "dc=scadabridge,dc=local",
|
||||
// Task 1.4: LDAP settings nest under Security:Ldap (shared LdapOptions).
|
||||
// ServiceAccountDn is now required by the library's LdapOptionsValidator
|
||||
// (ValidateOnStart), so it must be present for the host to start.
|
||||
["ScadaBridge:Security:Ldap:Server"] = "localhost",
|
||||
["ScadaBridge:Security:Ldap:Port"] = "3893",
|
||||
["ScadaBridge:Security:Ldap:Transport"] = "None",
|
||||
["ScadaBridge:Security:Ldap:AllowInsecure"] = "true",
|
||||
["ScadaBridge:Security:Ldap:SearchBase"] = "dc=scadabridge,dc=local",
|
||||
["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=scadabridge,dc=local",
|
||||
});
|
||||
});
|
||||
builder.UseSetting("ScadaBridge:Node:Role", "Central");
|
||||
|
||||
@@ -108,11 +108,15 @@ public class CentralAuditWiringTests : IDisposable
|
||||
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
|
||||
["ScadaBridge:Database:SkipMigrations"] = "true",
|
||||
["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!",
|
||||
["ScadaBridge:Security:LdapServer"] = "localhost",
|
||||
["ScadaBridge:Security:LdapPort"] = "3893",
|
||||
["ScadaBridge:Security:LdapUseTls"] = "false",
|
||||
["ScadaBridge:Security:AllowInsecureLdap"] = "true",
|
||||
["ScadaBridge:Security:LdapSearchBase"] = "dc=scadabridge,dc=local",
|
||||
// Task 1.4: LDAP settings nest under Security:Ldap (shared LdapOptions).
|
||||
// ServiceAccountDn is now required by the library's LdapOptionsValidator
|
||||
// (ValidateOnStart), so it must be present for the host to start.
|
||||
["ScadaBridge:Security:Ldap:Server"] = "localhost",
|
||||
["ScadaBridge:Security:Ldap:Port"] = "3893",
|
||||
["ScadaBridge:Security:Ldap:Transport"] = "None",
|
||||
["ScadaBridge:Security:Ldap:AllowInsecure"] = "true",
|
||||
["ScadaBridge:Security:Ldap:SearchBase"] = "dc=scadabridge,dc=local",
|
||||
["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=scadabridge,dc=local",
|
||||
["ScadaBridge:InboundApi:ApiKeyPepper"] = "test-inbound-api-key-pepper-at-least-32-chars!",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,6 +21,7 @@ using ZB.MOM.WW.ScadaBridge.Host.Health;
|
||||
using ZB.MOM.WW.ScadaBridge.InboundAPI;
|
||||
using ZB.MOM.WW.ScadaBridge.ManagementService;
|
||||
using ZB.MOM.WW.ScadaBridge.NotificationService;
|
||||
using ZB.MOM.WW.Auth.Abstractions.Ldap;
|
||||
using ZB.MOM.WW.ScadaBridge.Security;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteEventLogging;
|
||||
using ZB.MOM.WW.ScadaBridge.Communication.Grpc;
|
||||
@@ -102,11 +103,15 @@ public class CentralCompositionRootTests : IDisposable
|
||||
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
|
||||
["ScadaBridge:Database:SkipMigrations"] = "true",
|
||||
["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!",
|
||||
["ScadaBridge:Security:LdapServer"] = "localhost",
|
||||
["ScadaBridge:Security:LdapPort"] = "3893",
|
||||
["ScadaBridge:Security:LdapUseTls"] = "false",
|
||||
["ScadaBridge:Security:AllowInsecureLdap"] = "true",
|
||||
["ScadaBridge:Security:LdapSearchBase"] = "dc=scadabridge,dc=local",
|
||||
// Task 1.4: LDAP settings nest under Security:Ldap (shared LdapOptions).
|
||||
// ServiceAccountDn is now required by the library's LdapOptionsValidator
|
||||
// (ValidateOnStart), so it must be present for the host to start.
|
||||
["ScadaBridge:Security:Ldap:Server"] = "localhost",
|
||||
["ScadaBridge:Security:Ldap:Port"] = "3893",
|
||||
["ScadaBridge:Security:Ldap:Transport"] = "None",
|
||||
["ScadaBridge:Security:Ldap:AllowInsecure"] = "true",
|
||||
["ScadaBridge:Security:Ldap:SearchBase"] = "dc=scadabridge,dc=local",
|
||||
["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=scadabridge,dc=local",
|
||||
// ConfigurationDatabase-012: inbound-API keys are hashed
|
||||
// with a server-side HMAC pepper; ApiKeyHasher fails fast
|
||||
// if it is missing or weak, so resolving ApiKeyValidator
|
||||
@@ -167,6 +172,9 @@ public class CentralCompositionRootTests : IDisposable
|
||||
new object[] { typeof(OperationLockManager) },
|
||||
new object[] { typeof(OAuth2TokenService) },
|
||||
new object[] { typeof(InboundScriptExecutor) },
|
||||
// Security: the shared ZB.MOM.WW.Auth.Ldap service is registered as a stateless
|
||||
// singleton by AddZbLdapAuth (Task 1.2), replacing the old scoped LdapAuthService.
|
||||
new object[] { typeof(ILdapAuthService) },
|
||||
};
|
||||
|
||||
// --- Scoped services ---
|
||||
@@ -193,8 +201,7 @@ public class CentralCompositionRootTests : IDisposable
|
||||
new object[] { typeof(IFlatteningPipeline) },
|
||||
new object[] { typeof(DeploymentService) },
|
||||
new object[] { typeof(ArtifactDeploymentService) },
|
||||
// Security
|
||||
new object[] { typeof(LdapAuthService) },
|
||||
// Security (ILdapAuthService is now a singleton — see CentralSingletonServices)
|
||||
new object[] { typeof(JwtTokenService) },
|
||||
new object[] { typeof(RoleMapper) },
|
||||
// InboundAPI
|
||||
|
||||
@@ -20,7 +20,7 @@ public class StartupValidatorTests
|
||||
["ScadaBridge:Node:NodeHostname"] = "central-node1",
|
||||
["ScadaBridge:Node:RemotingPort"] = "8081",
|
||||
["ScadaBridge:Database:ConfigurationDb"] = "Server=localhost;Database=Config;",
|
||||
["ScadaBridge:Security:LdapServer"] = "ldap.example.com",
|
||||
["ScadaBridge:Security:Ldap:Server"] = "ldap.example.com",
|
||||
["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-at-least-32-chars-long",
|
||||
["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@central-node1:8081",
|
||||
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@central-node2:8081",
|
||||
@@ -166,12 +166,14 @@ public class StartupValidatorTests
|
||||
[Fact]
|
||||
public void Central_MissingLdapServer_FailsValidation()
|
||||
{
|
||||
// Task 1.4: the LDAP server key nests under Security:Ldap now. The pre-host
|
||||
// preflight validates the nested key and still fails fast for Central.
|
||||
var values = ValidCentralConfig();
|
||||
values.Remove("ScadaBridge:Security:LdapServer");
|
||||
values.Remove("ScadaBridge:Security:Ldap:Server");
|
||||
var config = BuildConfig(values);
|
||||
|
||||
var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
|
||||
Assert.Contains("LdapServer required for Central", ex.Message);
|
||||
Assert.Contains("Ldap:Server required for Central", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -38,17 +38,19 @@ public class ScadaBridgeWebApplicationFactory : WebApplicationFactory<Program>
|
||||
["ScadaBridge__Database__MachineDataDb"] = "Server=localhost;Database=ScadaBridge_MachineData_Test;TrustServerCertificate=True",
|
||||
["ScadaBridge__Database__SkipMigrations"] = "true",
|
||||
["ScadaBridge__Security__JwtSigningKey"] = "integration-test-signing-key-must-be-at-least-32-chars-long",
|
||||
["ScadaBridge__Security__LdapServer"] = "localhost",
|
||||
["ScadaBridge__Security__LdapPort"] = "3893",
|
||||
["ScadaBridge__Security__LdapUseTls"] = "false",
|
||||
["ScadaBridge__Security__AllowInsecureLdap"] = "true",
|
||||
["ScadaBridge__Security__LdapSearchBase"] = "dc=scadabridge,dc=local",
|
||||
// GLAuth places users at cn=<name>,ou=<group>,ou=users,dc=... — the
|
||||
// no-service-account fallback DN (uid=<name>,dc=...) does not match,
|
||||
// so a service account is configured to enable search-then-bind:
|
||||
// resolve the user's real DN by (uid=<name>) lookup, then bind it.
|
||||
["ScadaBridge__Security__LdapServiceAccountDn"] = "cn=admin,ou=SCADA-Admins,ou=users,dc=scadabridge,dc=local",
|
||||
["ScadaBridge__Security__LdapServiceAccountPassword"] = "password",
|
||||
// Task 1.4: LDAP settings nest under Security:Ldap (shared LdapOptions) and use
|
||||
// the renamed keys (Transport replaces LdapUseTls; None == plaintext for the
|
||||
// GLAuth dev directory, paired with AllowInsecure=true).
|
||||
["ScadaBridge__Security__Ldap__Server"] = "localhost",
|
||||
["ScadaBridge__Security__Ldap__Port"] = "3893",
|
||||
["ScadaBridge__Security__Ldap__Transport"] = "None",
|
||||
["ScadaBridge__Security__Ldap__AllowInsecure"] = "true",
|
||||
["ScadaBridge__Security__Ldap__SearchBase"] = "dc=scadabridge,dc=local",
|
||||
// GLAuth places users at cn=<name>,ou=<group>,ou=users,dc=... — a service
|
||||
// account is configured to enable the shared service's search-then-bind:
|
||||
// resolve the user's real DN by (UserNameAttribute=<name>) lookup, then bind it.
|
||||
["ScadaBridge__Security__Ldap__ServiceAccountDn"] = "cn=admin,ou=SCADA-Admins,ou=users,dc=scadabridge,dc=local",
|
||||
["ScadaBridge__Security__Ldap__ServiceAccountPassword"] = "password",
|
||||
};
|
||||
|
||||
foreach (var (key, value) in envVars)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ZB.MOM.WW.Auth.Abstractions.Ldap;
|
||||
using ZB.MOM.WW.ScadaBridge.Security;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.IntegrationTests;
|
||||
@@ -23,18 +24,19 @@ public class SecurityHardeningTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SecurityOptions_LdapUseTls_DefaultsToTrue()
|
||||
public void LdapOptions_Transport_DefaultsToLdaps()
|
||||
{
|
||||
// Production requires LDAPS. The default must be true.
|
||||
var options = new SecurityOptions();
|
||||
Assert.True(options.LdapUseTls);
|
||||
// Production requires encrypted transport. The shared LdapOptions defaults to
|
||||
// LDAPS (secure-by-default), preserving the donor's LdapUseTls=true default.
|
||||
var options = new LdapOptions();
|
||||
Assert.Equal(LdapTransport.Ldaps, options.Transport);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SecurityOptions_AllowInsecureLdap_DefaultsToFalse()
|
||||
public void LdapOptions_AllowInsecure_DefaultsToFalse()
|
||||
{
|
||||
var options = new SecurityOptions();
|
||||
Assert.False(options.AllowInsecureLdap);
|
||||
var options = new LdapOptions();
|
||||
Assert.False(options.AllowInsecure);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -172,10 +174,21 @@ public class SecurityHardeningTests
|
||||
[Fact]
|
||||
public void StartupValidator_RejectsInsecureLdapInProduction()
|
||||
{
|
||||
// The SecurityOptions.AllowInsecureLdap defaults to false.
|
||||
// Only when explicitly set to true (for dev/test) is insecure LDAP allowed.
|
||||
var prodOptions = new SecurityOptions { LdapUseTls = true, AllowInsecureLdap = false };
|
||||
Assert.True(prodOptions.LdapUseTls);
|
||||
Assert.False(prodOptions.AllowInsecureLdap);
|
||||
// The shared LdapOptionsValidator (registered with ValidateOnStart by AddZbLdapAuth)
|
||||
// rejects plaintext transport (Transport=None) unless AllowInsecure is explicitly set,
|
||||
// preserving the donor's production LDAPS-enforcement guarantee.
|
||||
var insecure = new LdapOptions
|
||||
{
|
||||
Server = "ldap.example.com",
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
Transport = LdapTransport.None,
|
||||
AllowInsecure = false,
|
||||
};
|
||||
|
||||
var result = new ZB.MOM.WW.Auth.Ldap.LdapOptionsValidator().Validate(name: null, insecure);
|
||||
|
||||
Assert.True(result.Failed);
|
||||
Assert.Contains(nameof(LdapOptions.AllowInsecure), result.FailureMessage);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,7 +47,7 @@ public class StartupValidationTests
|
||||
["ScadaBridge__Cluster__SeedNodes__1"] = "akka.tcp://scadabridge@localhost:8082",
|
||||
["ScadaBridge__Database__ConfigurationDb"] = "Server=x;Database=x",
|
||||
["ScadaBridge__Database__MachineDataDb"] = "Server=x;Database=x",
|
||||
["ScadaBridge__Security__LdapServer"] = "localhost",
|
||||
["ScadaBridge__Security__Ldap__Server"] = "localhost",
|
||||
// Deliberately missing JwtSigningKey
|
||||
});
|
||||
|
||||
@@ -92,11 +92,13 @@ public class StartupValidationTests
|
||||
"ScadaBridge__Database__MachineDataDb",
|
||||
"ScadaBridge__Database__SkipMigrations",
|
||||
"ScadaBridge__Security__JwtSigningKey",
|
||||
"ScadaBridge__Security__LdapServer",
|
||||
"ScadaBridge__Security__LdapPort",
|
||||
"ScadaBridge__Security__LdapUseTls",
|
||||
"ScadaBridge__Security__AllowInsecureLdap",
|
||||
"ScadaBridge__Security__LdapSearchBase",
|
||||
"ScadaBridge__Security__Ldap__Server",
|
||||
"ScadaBridge__Security__Ldap__Port",
|
||||
"ScadaBridge__Security__Ldap__Transport",
|
||||
"ScadaBridge__Security__Ldap__AllowInsecure",
|
||||
"ScadaBridge__Security__Ldap__SearchBase",
|
||||
"ScadaBridge__Security__Ldap__ServiceAccountDn",
|
||||
"ScadaBridge__Security__Ldap__ServiceAccountPassword",
|
||||
};
|
||||
|
||||
public TempEnvironment(Dictionary<string, string> varsToSet)
|
||||
|
||||
@@ -14,6 +14,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||
using ZB.MOM.WW.Auth.Abstractions.Ldap;
|
||||
using ZB.MOM.WW.ScadaBridge.ManagementService;
|
||||
using ZB.MOM.WW.ScadaBridge.Security;
|
||||
|
||||
@@ -71,14 +72,12 @@ public class AuditEndpointsTests
|
||||
.Returns(Task.FromResult<IReadOnlyList<AuditEvent>>(Array.Empty<AuditEvent>()));
|
||||
}
|
||||
|
||||
// Substituted LDAP bind — AuthenticateAsync is virtual (test seam).
|
||||
var ldap = Substitute.For<LdapAuthService>(
|
||||
Options.Create(new SecurityOptions()),
|
||||
Substitute.For<ILogger<LdapAuthService>>());
|
||||
// Substituted LDAP bind — the shared ILdapAuthService is the seam now (Task 1.2).
|
||||
var ldap = Substitute.For<ILdapAuthService>();
|
||||
ldap.AuthenticateAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
|
||||
.Returns(ldapSucceeds
|
||||
? new LdapAuthResult(true, "Auditor", "auditor", new[] { "cn=audit" }, null)
|
||||
: new LdapAuthResult(false, null, null, null, "Bad credentials."));
|
||||
? LdapAuthResult.Success("auditor", "Auditor", new[] { "audit" })
|
||||
: LdapAuthResult.Fail(LdapAuthFailure.BadCredentials));
|
||||
|
||||
// Substituted role mapper — MapGroupsToRolesAsync is virtual (test seam).
|
||||
var roleMapper = Substitute.For<RoleMapper>(Substitute.For<ISecurityRepository>());
|
||||
|
||||
@@ -122,6 +122,49 @@ public class ScadaBridgeGroupRoleMapperTests
|
||||
Assert.True(scope.IsSystemWideDeployment);
|
||||
Assert.Empty(scope.PermittedSiteIds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task MapAsync_EmptyGroups_ReturnsNoRoles_AndNonNullScope()
|
||||
{
|
||||
// Reviewer-requested empty-groups case (Task 1.2): a user that resolves to NO
|
||||
// groups must map to NO roles and a non-null, not-system-wide Scope with no
|
||||
// permitted sites — so the login endpoint can build a roleless principal (which
|
||||
// every authorization policy then denies) without NRE-ing on Scope. This is the
|
||||
// post-cutover home for the donor's "no mapped groups" outcome, now that the
|
||||
// shared LDAP service fail-closes a zero-GROUP LDAP result before it ever reaches
|
||||
// the mapper.
|
||||
var repo = new FakeSecurityRepository(
|
||||
new List<LdapGroupMapping> { Mapping(1, "SCADA-Admins", Roles.Admin) },
|
||||
new Dictionary<int, IReadOnlyList<SiteScopeRule>>());
|
||||
var sut = new ScadaBridgeGroupRoleMapper(new RoleMapper(repo));
|
||||
|
||||
var mapping = await sut.MapAsync(Array.Empty<string>(), CancellationToken.None);
|
||||
|
||||
Assert.Empty(mapping.Roles);
|
||||
var scope = Assert.IsType<RoleMappingResult>(mapping.Scope);
|
||||
Assert.Empty(scope.Roles);
|
||||
Assert.False(scope.IsSystemWideDeployment);
|
||||
Assert.Empty(scope.PermittedSiteIds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task MapAsync_GroupsMatchNoMapping_ReturnsNoRoles()
|
||||
{
|
||||
// A user WITH groups that match no configured LDAP-group→role mapping likewise
|
||||
// yields zero roles (not an error) — the mapper is the authoritative empty-roles
|
||||
// boundary now that the LDAP service no longer admits zero-group successes.
|
||||
var repo = new FakeSecurityRepository(
|
||||
new List<LdapGroupMapping> { Mapping(1, "SCADA-Admins", Roles.Admin) },
|
||||
new Dictionary<int, IReadOnlyList<SiteScopeRule>>());
|
||||
var sut = new ScadaBridgeGroupRoleMapper(new RoleMapper(repo));
|
||||
|
||||
var mapping = await sut.MapAsync(new[] { "some-unmapped-group" }, CancellationToken.None);
|
||||
|
||||
Assert.Empty(mapping.Roles);
|
||||
var scope = Assert.IsType<RoleMappingResult>(mapping.Scope);
|
||||
Assert.Empty(scope.PermittedSiteIds);
|
||||
Assert.False(scope.IsSystemWideDeployment);
|
||||
}
|
||||
}
|
||||
|
||||
#endregion
|
||||
|
||||
@@ -5,6 +5,8 @@ using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ZB.MOM.WW.Auth.Abstractions.Ldap;
|
||||
using ZB.MOM.WW.Auth.Ldap;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Security;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
|
||||
@@ -13,70 +15,76 @@ using ZB.MOM.WW.ScadaBridge.Security;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Security.Tests;
|
||||
|
||||
#region WP-6: LdapAuthService Tests
|
||||
#region WP-6: LdapAuthService Tests (re-aimed at the shared ZB.MOM.WW.Auth.Ldap service)
|
||||
|
||||
// Task 1.2 cutover: ScadaBridge's bespoke LdapAuthService was replaced by the shared
|
||||
// ZB.MOM.WW.Auth.Ldap.LdapAuthService (ScadaBridge was the donor for its hardened
|
||||
// bind-then-search / RFC-4514+4515 escaping / fail-closed / service-account-bind-
|
||||
// distinction / per-op-timeout semantics). The deep hygiene parity now lives in the
|
||||
// LIBRARY's own test suite (ZB.MOM.WW.Auth.Ldap.Tests: LdapAuthServiceTests,
|
||||
// LdapAuthServiceFailureTests, LdapEscapingTests), which exercises those paths through
|
||||
// the library's internal ILdapConnection seam. Re-aiming them here would require that
|
||||
// internal seam, which ScadaBridge cannot reach. The tests below instead pin the
|
||||
// LIBRARY service's public, network-free surface and ScadaBridge's adopter wiring.
|
||||
public class LdapAuthServiceTests
|
||||
{
|
||||
private static SecurityOptions CreateOptions(bool useTls = true, bool allowInsecure = false) => new()
|
||||
private static LdapOptions CreateOptions(LdapTransport transport = LdapTransport.Ldaps, bool allowInsecure = false) => new()
|
||||
{
|
||||
LdapServer = "ldap.example.com",
|
||||
LdapPort = 636,
|
||||
LdapUseTls = useTls,
|
||||
AllowInsecureLdap = allowInsecure,
|
||||
LdapSearchBase = "dc=example,dc=com",
|
||||
JwtSigningKey = "test-key-that-is-long-enough-for-hmac-sha256-minimum"
|
||||
Enabled = true,
|
||||
Server = "ldap.example.com",
|
||||
Port = 636,
|
||||
Transport = transport,
|
||||
AllowInsecure = allowInsecure,
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_EmptyUsername_ReturnsFailed()
|
||||
public async Task AuthenticateAsync_Disabled_FailsWithDisabled_NoNetwork()
|
||||
{
|
||||
var service = new LdapAuthService(
|
||||
Options.Create(CreateOptions()),
|
||||
NullLogger<LdapAuthService>.Instance);
|
||||
var options = CreateOptions() with { Enabled = false };
|
||||
var service = new LdapAuthService(options);
|
||||
|
||||
var result = await service.AuthenticateAsync("", "password");
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Username is required", result.ErrorMessage);
|
||||
var result = await service.AuthenticateAsync("user", "password", CancellationToken.None);
|
||||
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Equal(LdapAuthFailure.Disabled, result.Failure);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
public async Task AuthenticateAsync_EmptyUsername_FailsBadCredentials_NoNetwork(string username)
|
||||
{
|
||||
// An empty/whitespace username is rejected before any connection is attempted,
|
||||
// mapped to the enumeration-safe BadCredentials bucket (no "user is required"
|
||||
// string leak). This is the donor's empty-username guard, preserved.
|
||||
var service = new LdapAuthService(CreateOptions());
|
||||
|
||||
var result = await service.AuthenticateAsync(username, "password", CancellationToken.None);
|
||||
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Equal(LdapAuthFailure.BadCredentials, result.Failure);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_EmptyPassword_ReturnsFailed()
|
||||
public async Task AuthenticateAsync_ConnectionFailure_FailsClosed_NeverThrows()
|
||||
{
|
||||
var service = new LdapAuthService(
|
||||
Options.Create(CreateOptions()),
|
||||
NullLogger<LdapAuthService>.Instance);
|
||||
// Point at a non-existent server: the library fails closed (never throws) and
|
||||
// maps the unreachable directory to the system-side ServiceAccountBindFailed
|
||||
// bucket — preserving the donor's "directory unavailable ⇒ login fails" rule.
|
||||
var options = CreateOptions(LdapTransport.None, allowInsecure: true) with
|
||||
{
|
||||
Server = "nonexistent.invalid",
|
||||
Port = 9999,
|
||||
ConnectionTimeoutMs = 2_000,
|
||||
};
|
||||
var service = new LdapAuthService(options);
|
||||
|
||||
var result = await service.AuthenticateAsync("user", "");
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Password is required", result.ErrorMessage);
|
||||
}
|
||||
var result = await service.AuthenticateAsync("user", "password", CancellationToken.None);
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_InsecureLdapNotAllowed_ReturnsFailed()
|
||||
{
|
||||
var service = new LdapAuthService(
|
||||
Options.Create(CreateOptions(useTls: false, allowInsecure: false)),
|
||||
NullLogger<LdapAuthService>.Instance);
|
||||
|
||||
var result = await service.AuthenticateAsync("user", "password");
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Insecure LDAP", result.ErrorMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_ConnectionFailure_ReturnsFailed()
|
||||
{
|
||||
// Point to a non-existent server — connection should fail
|
||||
var options = CreateOptions();
|
||||
options.LdapServer = "nonexistent.invalid";
|
||||
options.LdapPort = 9999;
|
||||
|
||||
var service = new LdapAuthService(
|
||||
Options.Create(options),
|
||||
NullLogger<LdapAuthService>.Instance);
|
||||
|
||||
var result = await service.AuthenticateAsync("user", "password");
|
||||
Assert.False(result.Success);
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Equal(LdapAuthFailure.ServiceAccountBindFailed, result.Failure);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -433,11 +441,7 @@ public class SecurityReviewRegressionTests
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
// Security-020: the cookie PostConfigure reads SecurityOptions.Value,
|
||||
// which triggers SecurityOptionsValidator — supply the required LDAP
|
||||
// fields so the cookie wiring under test can be resolved.
|
||||
ConfigureValidLdapDefaults(services);
|
||||
services.AddSecurity(EmptyConfig());
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var cookieOptions = provider
|
||||
@@ -451,18 +455,14 @@ public class SecurityReviewRegressionTests
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Security-020: supplies the minimum-valid LDAP fields so the cookie /
|
||||
/// JWT wiring under test can be resolved without hitting
|
||||
/// <see cref="SecurityOptionsValidator"/>. Used by the cookie-policy
|
||||
/// integration tests in this class, which only care about the cookie
|
||||
/// options shape — not the LDAP fields.
|
||||
/// Task 1.2: an empty <see cref="Microsoft.Extensions.Configuration.IConfiguration"/>
|
||||
/// for <c>AddSecurity(config)</c>. The cookie PostConfigure under test reads only the
|
||||
/// non-LDAP <see cref="SecurityOptions"/> fields (idle timeout / HTTPS-cookie policy),
|
||||
/// and the library's <c>LdapOptions</c> ValidateOnStart only fires at host start (not on
|
||||
/// <c>BuildServiceProvider</c>), so no LDAP config is needed to resolve the cookie wiring.
|
||||
/// </summary>
|
||||
private static void ConfigureValidLdapDefaults(IServiceCollection services) =>
|
||||
services.Configure<SecurityOptions>(o =>
|
||||
{
|
||||
o.LdapServer = "ldap.example.com";
|
||||
o.LdapSearchBase = "dc=example,dc=com";
|
||||
});
|
||||
private static Microsoft.Extensions.Configuration.IConfiguration EmptyConfig() =>
|
||||
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build();
|
||||
|
||||
// --- CentralUI-005: cookie auth must use a sliding session window ---
|
||||
// Documented policy (CLAUDE.md Security & Auth): sliding refresh with a
|
||||
@@ -475,8 +475,7 @@ public class SecurityReviewRegressionTests
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
ConfigureValidLdapDefaults(services);
|
||||
services.AddSecurity(EmptyConfig());
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var cookieOptions = provider
|
||||
@@ -492,8 +491,7 @@ public class SecurityReviewRegressionTests
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
ConfigureValidLdapDefaults(services);
|
||||
services.AddSecurity(EmptyConfig());
|
||||
// The idle timeout drives the cookie's expiry window.
|
||||
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 30);
|
||||
|
||||
@@ -511,8 +509,7 @@ public class SecurityReviewRegressionTests
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
ConfigureValidLdapDefaults(services);
|
||||
services.AddSecurity(EmptyConfig());
|
||||
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 45);
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
@@ -524,53 +521,62 @@ public class SecurityReviewRegressionTests
|
||||
Assert.True(cookieOptions.SlidingExpiration);
|
||||
}
|
||||
|
||||
// --- Security-001: StartTLS transport must be reachable ---
|
||||
// --- Security-001: transport security (now owned by the shared LdapOptions) ---
|
||||
|
||||
[Fact]
|
||||
public void SecurityOptions_LdapTransport_DefaultsToLdaps()
|
||||
public void LdapOptions_Transport_DefaultsToLdaps()
|
||||
{
|
||||
var options = new SecurityOptions();
|
||||
Assert.Equal(LdapTransport.Ldaps, options.LdapTransport);
|
||||
// The shared LdapOptions keeps the donor's secure-by-default transport.
|
||||
var options = new LdapOptions();
|
||||
Assert.Equal(LdapTransport.Ldaps, options.Transport);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_StartTlsTransport_AttemptsConnection()
|
||||
public async Task AuthenticateAsync_StartTlsTransport_NotBlockedByInsecureGuard()
|
||||
{
|
||||
// With StartTLS selected the service must not be blocked by the insecure-LDAP
|
||||
// guard and must reach the connection stage (which fails against a dead host).
|
||||
// This proves the StartTLS path is reachable rather than dead code.
|
||||
var options = new SecurityOptions
|
||||
// With StartTLS selected the service is NOT rejected as insecure: it reaches the
|
||||
// connection stage (which fails against a dead host) and fails closed to the
|
||||
// system-side bucket — proving the StartTLS path is reachable, not dead code.
|
||||
var options = new LdapOptions
|
||||
{
|
||||
LdapServer = "nonexistent.invalid",
|
||||
LdapPort = 389,
|
||||
LdapTransport = LdapTransport.StartTls,
|
||||
LdapSearchBase = "dc=example,dc=com"
|
||||
Enabled = true,
|
||||
Server = "nonexistent.invalid",
|
||||
Port = 389,
|
||||
Transport = LdapTransport.StartTls,
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
ConnectionTimeoutMs = 2_000,
|
||||
};
|
||||
var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
|
||||
var service = new LdapAuthService(options);
|
||||
|
||||
var result = await service.AuthenticateAsync("user", "password");
|
||||
var result = await service.AuthenticateAsync("user", "password", CancellationToken.None);
|
||||
|
||||
// Connection fails (host invalid) — but crucially NOT with the insecure-LDAP message.
|
||||
Assert.False(result.Success);
|
||||
Assert.DoesNotContain("Insecure LDAP", result.ErrorMessage ?? string.Empty);
|
||||
// Connection fails (host invalid) — fail-closed to ServiceAccountBindFailed, NOT Disabled.
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Equal(LdapAuthFailure.ServiceAccountBindFailed, result.Failure);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_NoTlsTransport_RejectedWithoutAllowInsecure()
|
||||
public void LdapOptionsValidator_NoTlsTransport_RejectedWithoutAllowInsecure()
|
||||
{
|
||||
var options = new SecurityOptions
|
||||
// The donor blocked plaintext LDAP (Transport=None) unless AllowInsecure was set.
|
||||
// That guard moved to the shared LdapOptionsValidator (registered with
|
||||
// ValidateOnStart by AddZbLdapAuth), which now fails fast at boot.
|
||||
var options = new LdapOptions
|
||||
{
|
||||
LdapServer = "ldap.example.com",
|
||||
LdapPort = 389,
|
||||
LdapTransport = LdapTransport.None,
|
||||
AllowInsecureLdap = false
|
||||
Server = "ldap.example.com",
|
||||
Port = 389,
|
||||
Transport = LdapTransport.None,
|
||||
AllowInsecure = false,
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
};
|
||||
var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
|
||||
var validator = new LdapOptionsValidator();
|
||||
|
||||
var result = await service.AuthenticateAsync("user", "password");
|
||||
var result = validator.Validate(name: null, options);
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Insecure LDAP", result.ErrorMessage);
|
||||
Assert.True(result.Failed);
|
||||
Assert.Contains("AllowInsecure", result.FailureMessage);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -579,11 +585,18 @@ public class SecurityReviewRegressionTests
|
||||
#region Code Review Regression Tests — Security-004/005/006/007
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for Security-004 (uid/cn attribute mismatch between search filter
|
||||
/// and fallback DN), Security-005 (DN injection in the no-service-account fallback),
|
||||
/// Security-006 (JWT issuer/audience checks disabled), and Security-007 (idle-timeout
|
||||
/// claim reset on every token refresh).
|
||||
/// Regression tests for Security-006 (JWT issuer/audience checks disabled) and
|
||||
/// Security-007 (idle-timeout claim reset on every token refresh).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Task 1.2 cutover: the former Security-004 (uid/cn attribute parity) and
|
||||
/// Security-005 (no-service-account fallback-DN injection) cases tested ScadaBridge's
|
||||
/// bespoke <c>LdapAuthService.BuildFallbackUserDn</c> / <c>EscapeLdapDn</c> static
|
||||
/// helpers. The shared <c>ZB.MOM.WW.Auth.Ldap</c> service is search-then-bind only (no
|
||||
/// no-service-account fallback DN) and its RFC-4514/4515 escaping parity is covered by
|
||||
/// the library's own <c>LdapEscapingTests</c> / <c>LdapAuthServiceTests</c>. Those cases
|
||||
/// were therefore removed here rather than re-aimed.
|
||||
/// </remarks>
|
||||
public class SecurityReviewRegressionTests2
|
||||
{
|
||||
private static SecurityOptions JwtOptions() => new()
|
||||
@@ -597,58 +610,6 @@ public class SecurityReviewRegressionTests2
|
||||
private static JwtTokenService CreateJwtService(SecurityOptions? options = null) =>
|
||||
new(Options.Create(options ?? JwtOptions()), NullLogger<JwtTokenService>.Instance);
|
||||
|
||||
// --- Security-004: search filter and fallback DN must use the same attribute ---
|
||||
|
||||
[Fact]
|
||||
public void BuildFallbackUserDn_UsesConfiguredUserIdAttribute()
|
||||
{
|
||||
// The default user-id attribute is "uid"; the fallback DN must use it,
|
||||
// not a hard-coded "cn", so search-then-bind and direct-bind are interchangeable.
|
||||
var dn = LdapAuthService.BuildFallbackUserDn("alice", "dc=example,dc=com", "uid");
|
||||
Assert.Equal("uid=alice,dc=example,dc=com", dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildFallbackUserDn_HonoursNonDefaultUserIdAttribute()
|
||||
{
|
||||
var dn = LdapAuthService.BuildFallbackUserDn("alice", "dc=example,dc=com", "sAMAccountName");
|
||||
Assert.Equal("sAMAccountName=alice,dc=example,dc=com", dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SecurityOptions_LdapUserIdAttribute_DefaultsToUid()
|
||||
{
|
||||
Assert.Equal("uid", new SecurityOptions().LdapUserIdAttribute);
|
||||
}
|
||||
|
||||
// --- Security-005: DN-component escaping must be applied to the username ---
|
||||
|
||||
[Fact]
|
||||
public void BuildFallbackUserDn_EscapesDnMetacharacters()
|
||||
{
|
||||
// A hostile username must not be able to alter the DN structure: the comma
|
||||
// that would otherwise start a new RDN ("ou=admins") must be escaped so the
|
||||
// whole string remains a single RDN value.
|
||||
var dn = LdapAuthService.BuildFallbackUserDn("victim,ou=admins", "dc=example,dc=com", "uid");
|
||||
Assert.Equal(@"uid=victim\,ou=admins,dc=example,dc=com", dn);
|
||||
// The comma from the username is backslash-escaped, so it does not act as an
|
||||
// RDN separator: the only unescaped comma is the one joining RDN and base DN.
|
||||
Assert.Contains(@"victim\,ou=admins", dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EscapeLdapDn_EscapesAllRfc4514Specials()
|
||||
{
|
||||
var escaped = LdapAuthService.EscapeLdapDn("a,b+c\"d\\e<f>g;h");
|
||||
Assert.Equal(@"a\,b\+c\""d\\e\<f\>g\;h", escaped);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EscapeLdapDn_EscapesLeadingAndTrailingSpaces()
|
||||
{
|
||||
Assert.Equal(@"\ x \ ", LdapAuthService.EscapeLdapDn(" x "));
|
||||
}
|
||||
|
||||
// --- Security-006: JWT issuer/audience must be bound and validated ---
|
||||
|
||||
[Fact]
|
||||
@@ -785,12 +746,13 @@ public class SecurityReviewRegressionTests3
|
||||
// --- Security-009: LDAP connection timeout must be configurable and bounded ---
|
||||
|
||||
[Fact]
|
||||
public void SecurityOptions_LdapConnectionTimeout_HasSaneDefault()
|
||||
public void LdapOptions_ConnectionTimeout_HasSaneDefault()
|
||||
{
|
||||
var options = new SecurityOptions();
|
||||
// A positive, finite default so a hung LDAP server cannot pin a thread forever.
|
||||
Assert.True(options.LdapConnectionTimeoutMs > 0);
|
||||
Assert.True(options.LdapConnectionTimeoutMs <= 60_000,
|
||||
var options = new LdapOptions();
|
||||
// A positive, finite default so a hung LDAP server cannot pin a thread forever
|
||||
// (the donor's Security-009 safeguard, preserved on the shared LdapOptions).
|
||||
Assert.True(options.ConnectionTimeoutMs > 0);
|
||||
Assert.True(options.ConnectionTimeoutMs <= 60_000,
|
||||
"Default LDAP connection timeout should be bounded (<= 60s).");
|
||||
}
|
||||
|
||||
@@ -798,52 +760,37 @@ public class SecurityReviewRegressionTests3
|
||||
public async Task AuthenticateAsync_UnreachableHost_FailsWithinConfiguredTimeout()
|
||||
{
|
||||
// A routable-but-non-responsive address would otherwise hang for the OS default
|
||||
// (often minutes). With LdapConnectionTimeoutMs applied to the LdapConnection the
|
||||
// call must give up promptly. 198.51.100.0/24 (TEST-NET-2, RFC 5737) is reserved
|
||||
// and not routed, so the connect attempt stalls until the timeout fires.
|
||||
var options = new SecurityOptions
|
||||
// (often minutes). With ConnectionTimeoutMs applied to the LdapConnection the call
|
||||
// must give up promptly. 198.51.100.0/24 (TEST-NET-2, RFC 5737) is reserved and not
|
||||
// routed, so the connect attempt stalls until the timeout fires.
|
||||
var options = new LdapOptions
|
||||
{
|
||||
LdapServer = "198.51.100.1",
|
||||
LdapPort = 389,
|
||||
LdapTransport = LdapTransport.None,
|
||||
AllowInsecureLdap = true,
|
||||
LdapSearchBase = "dc=example,dc=com",
|
||||
LdapConnectionTimeoutMs = 2_000
|
||||
Enabled = true,
|
||||
Server = "198.51.100.1",
|
||||
Port = 389,
|
||||
Transport = LdapTransport.None,
|
||||
AllowInsecure = true,
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
ConnectionTimeoutMs = 2_000,
|
||||
};
|
||||
var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
|
||||
var service = new LdapAuthService(options);
|
||||
|
||||
var sw = System.Diagnostics.Stopwatch.StartNew();
|
||||
var result = await service.AuthenticateAsync("user", "password");
|
||||
var result = await service.AuthenticateAsync("user", "password", CancellationToken.None);
|
||||
sw.Stop();
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.False(result.Succeeded);
|
||||
// Generous ceiling: the 2s timeout plus scheduling/CI overhead, far below the
|
||||
// multi-minute OS default that an unconfigured connection would incur.
|
||||
Assert.True(sw.Elapsed < TimeSpan.FromSeconds(30),
|
||||
$"Authentication did not honour the LDAP connection timeout: took {sw.Elapsed}.");
|
||||
}
|
||||
|
||||
// --- Security-011: additional coverage for the no-service-account / DN paths ---
|
||||
|
||||
[Fact]
|
||||
public void BuildFallbackUserDn_NoSearchBase_ReturnsBareRdn()
|
||||
{
|
||||
var dn = LdapAuthService.BuildFallbackUserDn("alice", "", "uid");
|
||||
Assert.Equal("uid=alice", dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EscapeLdapDn_LeadingHash_IsEscaped()
|
||||
{
|
||||
Assert.Equal(@"\#admin", LdapAuthService.EscapeLdapDn("#admin"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EscapeLdapDn_NullOrEmpty_ReturnedUnchanged()
|
||||
{
|
||||
Assert.Equal("", LdapAuthService.EscapeLdapDn(""));
|
||||
Assert.Null(LdapAuthService.EscapeLdapDn(null!));
|
||||
}
|
||||
// Note: the former Security-011 no-service-account fallback-DN / DN-escape static-helper
|
||||
// cases (BuildFallbackUserDn / EscapeLdapDn) were removed: the shared service is
|
||||
// search-then-bind only (no fallback DN), and its escaping parity is covered by the
|
||||
// library's own LdapEscapingTests.
|
||||
}
|
||||
|
||||
#endregion
|
||||
@@ -851,13 +798,18 @@ public class SecurityReviewRegressionTests3
|
||||
#region Code Review Regression Tests — Security-012/013/014/015
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for Security-012 (a partial LDAP outage during login — bind OK but
|
||||
/// group search failing — silently yields a roleless authenticated session),
|
||||
/// Security-013 (<c>ExtractFirstRdnValue</c> mis-parses group DNs containing an escaped
|
||||
/// comma), Security-014 (<c>RefreshToken</c> re-issues a token without checking the idle
|
||||
/// timeout, so an idle-expired session can be renewed indefinitely), and Security-015
|
||||
/// (a username with leading/trailing whitespace is not trimmed before use).
|
||||
/// Regression tests for Security-014 (<c>RefreshToken</c> re-issues a token without
|
||||
/// checking the idle timeout, so an idle-expired session can be renewed indefinitely).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Task 1.2 cutover: the former Security-013 (<c>ExtractFirstRdnValue</c> escaped-comma
|
||||
/// parsing) and Security-015 (username trim before use) cases tested ScadaBridge's
|
||||
/// bespoke <c>LdapAuthService.ExtractFirstRdnValue</c> / <c>NormalizeUsername</c> /
|
||||
/// <c>BuildFallbackUserDn</c> statics. Those behaviours are now owned by the shared
|
||||
/// library and covered by its own tests — <c>LdapEscapingTests.FirstRdnValue_IsEscapeAware</c>
|
||||
/// (escaped-comma RDN extraction) and <c>LdapAuthServiceFailureTests</c>
|
||||
/// (whitespace-username → BadCredentials, no connection) — so they were removed here.
|
||||
/// </remarks>
|
||||
public class SecurityReviewRegressionTests4
|
||||
{
|
||||
private static SecurityOptions JwtOptions() => new()
|
||||
@@ -928,85 +880,6 @@ public class SecurityReviewRegressionTests4
|
||||
|
||||
Assert.Null(refreshed);
|
||||
}
|
||||
|
||||
// --- Security-013: ExtractFirstRdnValue must honour RFC 4514 escaped commas ---
|
||||
|
||||
[Fact]
|
||||
public void ExtractFirstRdnValue_EscapedComma_KeepsWholeGroupName()
|
||||
{
|
||||
// A CN that legitimately contains a comma is RFC 4514 backslash-escaped in the
|
||||
// memberOf DN. The extracted group name must be the full unescaped CN value,
|
||||
// not the fragment before the escaped comma.
|
||||
var name = LdapAuthService.ExtractFirstRdnValue(
|
||||
@"cn=Acme\, Inc Operators,ou=groups,dc=example,dc=com");
|
||||
Assert.Equal("Acme, Inc Operators", name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExtractFirstRdnValue_PlainDn_ReturnsFirstRdnValue()
|
||||
{
|
||||
var name = LdapAuthService.ExtractFirstRdnValue("ou=SCADA-Admins,ou=groups,dc=example,dc=com");
|
||||
Assert.Equal("SCADA-Admins", name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExtractFirstRdnValue_SingleRdn_ReturnsValue()
|
||||
{
|
||||
var name = LdapAuthService.ExtractFirstRdnValue("cn=Operators");
|
||||
Assert.Equal("Operators", name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExtractFirstRdnValue_EscapedSpecials_AreUnescaped()
|
||||
{
|
||||
// RFC 4514 escape sequences (escaped '+', '"', '\\') must be unescaped in the
|
||||
// returned value so it matches the configured LdapGroupName verbatim.
|
||||
var name = LdapAuthService.ExtractFirstRdnValue(
|
||||
@"cn=A\+B\\C,ou=groups,dc=example,dc=com");
|
||||
Assert.Equal(@"A+B\C", name);
|
||||
}
|
||||
|
||||
// --- Security-015: username must be trimmed before use ---
|
||||
|
||||
[Fact]
|
||||
public void BuildFallbackUserDn_TrimmedUsername_NoLeadingTrailingSpace()
|
||||
{
|
||||
// The whitespace-edge escaping in EscapeLdapDn only fires when whitespace is NOT
|
||||
// trimmed. AuthenticateAsync trims first; this asserts the trimmed value yields
|
||||
// a clean DN with no escaped edge spaces.
|
||||
var dn = LdapAuthService.BuildFallbackUserDn("alice".Trim(), "dc=example,dc=com", "uid");
|
||||
Assert.Equal("uid=alice,dc=example,dc=com", dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AuthenticateAsync_UsernameWithSurroundingWhitespace_StillRejectedForInsecure()
|
||||
{
|
||||
// Sanity guard: a padded but otherwise-valid username is not rejected by the
|
||||
// IsNullOrWhiteSpace guard — it passes through to the (here, insecure-LDAP) path.
|
||||
var options = new SecurityOptions
|
||||
{
|
||||
LdapServer = "ldap.example.com",
|
||||
LdapPort = 389,
|
||||
LdapTransport = LdapTransport.None,
|
||||
AllowInsecureLdap = false
|
||||
};
|
||||
var service = new LdapAuthService(Options.Create(options), NullLogger<LdapAuthService>.Instance);
|
||||
|
||||
var result = await service.AuthenticateAsync(" alice ", "password");
|
||||
|
||||
// Reaches the insecure-LDAP guard (not the empty-username guard) — proves the
|
||||
// padded username is treated as a real, non-empty username.
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Insecure LDAP", result.ErrorMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void NormalizeUsername_TrimsLeadingAndTrailingWhitespace()
|
||||
{
|
||||
Assert.Equal("alice", LdapAuthService.NormalizeUsername(" alice "));
|
||||
Assert.Equal("alice", LdapAuthService.NormalizeUsername("alice"));
|
||||
Assert.Equal("alice", LdapAuthService.NormalizeUsername("\talice\n"));
|
||||
}
|
||||
}
|
||||
|
||||
#endregion
|
||||
@@ -1014,79 +887,94 @@ public class SecurityReviewRegressionTests4
|
||||
#region Code Review Regression Tests — Security-012 (partial LDAP outage)
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for Security-012: a partial LDAP outage during login (the user bind
|
||||
/// succeeds but the subsequent group/attribute search fails) must fail the login per the
|
||||
/// design's LDAP-failure rule, rather than returning an authenticated session with zero
|
||||
/// roles. These exercise the seam through a stubbed group-lookup so the bind itself can
|
||||
/// be treated as successful.
|
||||
/// Security-012 (fail-closed group lookup) + the ScadaBridge-side
|
||||
/// <see cref="LdapAuthFailureMessages"/> failure-code→message adapter.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Task 1.2 cutover: the donor's fail-closed group-lookup rule (a post-bind group
|
||||
/// search failure FAILS the login rather than admitting a roleless session) is now
|
||||
/// owned by the shared <c>ZB.MOM.WW.Auth.Ldap</c> service and asserted by its own
|
||||
/// <c>LdapAuthServiceFailureTests.GroupLookupFailed_WhenUserHasNoGroups</c> /
|
||||
/// <c>ServiceAccountBindFailed_Distinctly_WhenServiceBindThrows</c>. The donor's
|
||||
/// bespoke <c>BuildAuthResultFromGroupLookup</c> / <c>ServiceAccountBindException</c>
|
||||
/// statics no longer exist, so those cases were re-aimed at the shared
|
||||
/// <see cref="LdapAuthResult"/> contract here.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// DEVIATION (documented): the donor admitted an "authenticated with zero groups"
|
||||
/// login when the group lookup SUCCEEDED but returned no groups
|
||||
/// (<c>BuildAuthResultFromGroupLookup_LookupSucceededNoGroups_IsAuthenticated</c>).
|
||||
/// The shared library is STRICTER and fail-closes a zero-group result to
|
||||
/// <see cref="LdapAuthFailure.GroupLookupFailed"/> — see
|
||||
/// <c>EmptyGroups_IsTreatedAsGroupLookupFailed_LibraryIsStricter</c> below. The
|
||||
/// downstream effect (a user with no mapped groups cannot obtain a useful session) is
|
||||
/// unchanged: the donor's empty-group session mapped to zero roles and was denied by
|
||||
/// every authorization policy anyway. The empty-GROUP-set-at-the-mapper case (a user
|
||||
/// whose groups map to no roles) is covered by
|
||||
/// <c>ScadaBridgeGroupRoleMapperTests.MapAsync_EmptyGroups_*</c>.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public class Security012GroupLookupFailureTests
|
||||
{
|
||||
private static SecurityOptions Options() => new()
|
||||
{
|
||||
LdapServer = "ldap.example.com",
|
||||
LdapPort = 636,
|
||||
LdapTransport = LdapTransport.Ldaps,
|
||||
LdapSearchBase = "dc=example,dc=com"
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public void BuildAuthResultFromGroupLookup_LookupFailed_FailsTheLogin()
|
||||
public void LdapAuthResult_Fail_GroupLookupFailed_IsFailedLoginWithNoGroups()
|
||||
{
|
||||
// When the group lookup failed (directory partially unavailable mid-login) the
|
||||
// result must be a FAILED login — not a success with an empty group list.
|
||||
var result = LdapAuthService.BuildAuthResultFromGroupLookup(
|
||||
username: "alice",
|
||||
displayName: "Alice",
|
||||
groups: new List<string>(),
|
||||
groupLookupSucceeded: false);
|
||||
// The fail-closed outcome: a failed group lookup surfaces as a FAILED result with
|
||||
// the GroupLookupFailed code and an empty group set (never a roleless success).
|
||||
var result = LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed);
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.Null(result.Groups);
|
||||
Assert.False(string.IsNullOrEmpty(result.ErrorMessage));
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Equal(LdapAuthFailure.GroupLookupFailed, result.Failure);
|
||||
Assert.Empty(result.Groups);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildAuthResultFromGroupLookup_LookupSucceededNoGroups_IsAuthenticated()
|
||||
public void LdapAuthResult_Success_CarriesGroups()
|
||||
{
|
||||
// A genuine "user belongs to no mapped groups" outcome must remain a successful
|
||||
// login — it must be distinguishable from a failed lookup.
|
||||
var result = LdapAuthService.BuildAuthResultFromGroupLookup(
|
||||
username: "alice",
|
||||
displayName: "Alice",
|
||||
groups: new List<string>(),
|
||||
groupLookupSucceeded: true);
|
||||
var result = LdapAuthResult.Success("alice", "Alice", new[] { "SCADA-Admins" });
|
||||
|
||||
Assert.True(result.Success);
|
||||
Assert.NotNull(result.Groups);
|
||||
Assert.Empty(result.Groups!);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildAuthResultFromGroupLookup_LookupSucceededWithGroups_CarriesGroups()
|
||||
{
|
||||
var result = LdapAuthService.BuildAuthResultFromGroupLookup(
|
||||
username: "alice",
|
||||
displayName: "Alice",
|
||||
groups: new List<string> { "SCADA-Admins" },
|
||||
groupLookupSucceeded: true);
|
||||
|
||||
Assert.True(result.Success);
|
||||
Assert.True(result.Succeeded);
|
||||
Assert.Equal(new[] { "SCADA-Admins" }, result.Groups);
|
||||
Assert.Null(result.Failure);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ServiceAccountBindException_DoesNotInheritLdapException_SoCatchOrderIsCorrect()
|
||||
public void EmptyGroups_IsTreatedAsGroupLookupFailed_LibraryIsStricter()
|
||||
{
|
||||
// Security-019: the LdapAuthService catch chain matches
|
||||
// ServiceAccountBindException *before* the generic LdapException catch. That only
|
||||
// produces the distinct "Authentication service is misconfigured" message if the
|
||||
// exception type is NOT an LdapException subtype (otherwise it would be caught
|
||||
// by the broader handler first regardless of ordering).
|
||||
var ex = new ServiceAccountBindException(new InvalidOperationException("boom"));
|
||||
// Documents the deviation: the shared library NEVER admits a zero-group success.
|
||||
// The success factory requires >=1 group at the service level; a fabricated
|
||||
// zero-group "success" is not a state the service produces. We pin the contract
|
||||
// the cutover relies on: a zero-group outcome is the GroupLookupFailed failure.
|
||||
var failClosed = LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed);
|
||||
Assert.False(failClosed.Succeeded);
|
||||
|
||||
Assert.IsNotType<Novell.Directory.Ldap.LdapException>(ex);
|
||||
Assert.IsType<InvalidOperationException>(ex.InnerException);
|
||||
// And the donor's "Security-012 directory-unavailable" message is preserved for it.
|
||||
Assert.Equal(
|
||||
LdapAuthFailureMessages.DirectoryUnavailable,
|
||||
LdapAuthFailureMessages.ToMessage(LdapAuthFailure.GroupLookupFailed));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(LdapAuthFailure.BadCredentials, LdapAuthFailureMessages.InvalidCredentials)]
|
||||
[InlineData(LdapAuthFailure.UserNotFound, LdapAuthFailureMessages.InvalidCredentials)]
|
||||
[InlineData(LdapAuthFailure.AmbiguousUser, LdapAuthFailureMessages.Misconfigured)]
|
||||
[InlineData(LdapAuthFailure.ServiceAccountBindFailed, LdapAuthFailureMessages.Misconfigured)]
|
||||
[InlineData(LdapAuthFailure.GroupLookupFailed, LdapAuthFailureMessages.DirectoryUnavailable)]
|
||||
[InlineData(LdapAuthFailure.Disabled, LdapAuthFailureMessages.Disabled)]
|
||||
public void LdapAuthFailureMessages_MapsEachFailure_ToDonorEquivalentMessage(
|
||||
LdapAuthFailure failure, string expected)
|
||||
{
|
||||
// Security-019 framing preserved: user-credential failures are enumeration-safe
|
||||
// ("Invalid username or password."), system-side faults point the OPERATOR at the
|
||||
// cause ("...misconfigured...") rather than blaming user input.
|
||||
Assert.Equal(expected, LdapAuthFailureMessages.ToMessage(failure));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LdapAuthFailureMessages_NullFailure_FallsBackToGeneric()
|
||||
{
|
||||
Assert.Equal(LdapAuthFailureMessages.Generic, LdapAuthFailureMessages.ToMessage(null));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1218,98 +1106,94 @@ public class AuthorizationPolicyTests
|
||||
|
||||
#endregion
|
||||
|
||||
#region Code Review Regression Tests — Security-020
|
||||
#region Code Review Regression Tests — Security-020 (LDAP fail-fast validation)
|
||||
|
||||
/// <summary>
|
||||
/// Security-020: <see cref="SecurityOptionsValidator"/> must reject empty
|
||||
/// <see cref="SecurityOptions.LdapServer"/> / <see cref="SecurityOptions.LdapSearchBase"/>
|
||||
/// at startup with a clear, key-naming message, so a typo'd appsettings section
|
||||
/// fails fast at boot instead of surfacing minutes/hours later as a generic
|
||||
/// LDAP error on the first real login.
|
||||
/// Security-020: a missing/empty required LDAP field must fail fast at startup with a
|
||||
/// clear, field-naming message, so a typo'd appsettings section fails at boot instead
|
||||
/// of surfacing minutes/hours later as a generic LDAP error on the first real login.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Task 1.2/1.4 cutover: the donor's <c>SecurityOptionsValidator</c> (which checked
|
||||
/// <c>Security:LdapServer</c> + <c>Security:LdapSearchBase</c> non-empty) was replaced by
|
||||
/// the shared <c>ZB.MOM.WW.Auth.Ldap.LdapOptionsValidator</c>, registered with
|
||||
/// <c>ValidateOnStart()</c> by <c>AddZbLdapAuth</c> inside <c>AddSecurity</c>. The shared
|
||||
/// validator is STRICTER — it also requires <c>ServiceAccountDn</c> and rejects plaintext
|
||||
/// transport without <c>AllowInsecure</c> — so the boot-time fail-fast guarantee is
|
||||
/// preserved and broadened. These tests pin the shared validator's behaviour and the
|
||||
/// ScadaBridge wiring that registers it.
|
||||
/// </remarks>
|
||||
public class SecurityOptionsValidatorTests
|
||||
{
|
||||
private static SecurityOptions ValidOptions() => new()
|
||||
private static LdapOptions ValidOptions() => new()
|
||||
{
|
||||
LdapServer = "ldap.example.com",
|
||||
LdapSearchBase = "dc=example,dc=com",
|
||||
Server = "ldap.example.com",
|
||||
SearchBase = "dc=example,dc=com",
|
||||
ServiceAccountDn = "cn=admin,dc=example,dc=com",
|
||||
Transport = LdapTransport.Ldaps,
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public void Validate_AllRequiredFieldsSet_Succeeds()
|
||||
{
|
||||
var validator = new SecurityOptionsValidator();
|
||||
|
||||
var result = validator.Validate(name: null, ValidOptions());
|
||||
|
||||
var result = new LdapOptionsValidator().Validate(name: null, ValidOptions());
|
||||
Assert.True(result.Succeeded);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
public void Validate_EmptyOrWhitespaceLdapServer_Fails(string ldapServer)
|
||||
public void Validate_EmptyOrWhitespaceServer_Fails(string server)
|
||||
{
|
||||
var options = ValidOptions();
|
||||
options.LdapServer = ldapServer;
|
||||
var validator = new SecurityOptionsValidator();
|
||||
var options = ValidOptions() with { Server = server };
|
||||
|
||||
var result = validator.Validate(name: null, options);
|
||||
var result = new LdapOptionsValidator().Validate(name: null, options);
|
||||
|
||||
Assert.True(result.Failed);
|
||||
// Must name the full Section:Field key so the operator can find it.
|
||||
Assert.Contains("Security:LdapServer", result.FailureMessage);
|
||||
Assert.Contains(nameof(LdapOptions.Server), result.FailureMessage);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
public void Validate_EmptyOrWhitespaceLdapSearchBase_Fails(string ldapSearchBase)
|
||||
public void Validate_EmptyOrWhitespaceSearchBase_Fails(string searchBase)
|
||||
{
|
||||
var options = ValidOptions();
|
||||
options.LdapSearchBase = ldapSearchBase;
|
||||
var validator = new SecurityOptionsValidator();
|
||||
var options = ValidOptions() with { SearchBase = searchBase };
|
||||
|
||||
var result = validator.Validate(name: null, options);
|
||||
var result = new LdapOptionsValidator().Validate(name: null, options);
|
||||
|
||||
Assert.True(result.Failed);
|
||||
Assert.Contains("Security:LdapSearchBase", result.FailureMessage);
|
||||
Assert.Contains(nameof(LdapOptions.SearchBase), result.FailureMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_BothRequiredFieldsEmpty_ReportsBoth()
|
||||
public void Validate_EmptyServiceAccountDn_Fails()
|
||||
{
|
||||
var options = new SecurityOptions
|
||||
{
|
||||
LdapServer = string.Empty,
|
||||
LdapSearchBase = string.Empty,
|
||||
};
|
||||
var validator = new SecurityOptionsValidator();
|
||||
// Stricter than the donor: an empty ServiceAccountDn would bind anonymously and
|
||||
// defeat search-then-bind, so the shared validator rejects it at boot.
|
||||
var options = ValidOptions() with { ServiceAccountDn = string.Empty };
|
||||
|
||||
var result = validator.Validate(name: null, options);
|
||||
var result = new LdapOptionsValidator().Validate(name: null, options);
|
||||
|
||||
Assert.True(result.Failed);
|
||||
// Both keys named — the operator should not need to re-run after
|
||||
// fixing the first one to discover the second is also missing.
|
||||
Assert.Contains("Security:LdapServer", result.FailureMessage);
|
||||
Assert.Contains("Security:LdapSearchBase", result.FailureMessage);
|
||||
Assert.Contains(nameof(LdapOptions.ServiceAccountDn), result.FailureMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AddSecurity_RegistersSecurityOptionsValidator()
|
||||
public void AddSecurity_RegistersLdapOptionsValidator_WithValidateOnStart()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
services.AddSecurity(
|
||||
new Microsoft.Extensions.Configuration.ConfigurationBuilder().Build());
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
|
||||
// The validator participates in IValidateOptions<SecurityOptions> —
|
||||
// registration is the load-bearing wiring that makes Security-020
|
||||
// ValidateOnStart() actually fire.
|
||||
var validators = provider.GetServices<IValidateOptions<SecurityOptions>>().ToList();
|
||||
Assert.Contains(validators, v => v is SecurityOptionsValidator);
|
||||
// The shared validator participates in IValidateOptions<LdapOptions> —
|
||||
// registration is the load-bearing wiring that makes ValidateOnStart() fire.
|
||||
var validators = provider.GetServices<IValidateOptions<LdapOptions>>().ToList();
|
||||
Assert.Contains(validators, v => v is LdapOptionsValidator);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,9 @@
|
||||
<PackageReference Include="Microsoft.NET.Test.Sdk" />
|
||||
<PackageReference Include="xunit" />
|
||||
<PackageReference Include="xunit.runner.visualstudio" />
|
||||
<!-- Task 1.2: the LDAP tests construct the shared library service / options directly. -->
|
||||
<PackageReference Include="ZB.MOM.WW.Auth.Abstractions" />
|
||||
<PackageReference Include="ZB.MOM.WW.Auth.Ldap" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
|
||||
Reference in New Issue
Block a user