finish off sprint advisories and sprints
This commit is contained in:
@@ -170,8 +170,8 @@ public sealed class LdapConnectorResilienceTests
|
||||
// Service account bind succeeds
|
||||
return ValueTask.CompletedTask;
|
||||
}
|
||||
// User bind fails
|
||||
throw new InvalidOperationException("Invalid credentials");
|
||||
// User bind fails - must throw LdapAuthenticationException for impl to handle
|
||||
throw new Connections.LdapAuthenticationException("Invalid credentials");
|
||||
};
|
||||
|
||||
var store = CreateStore(options, new FakeLdapConnectionFactory(connection));
|
||||
@@ -199,11 +199,11 @@ public sealed class LdapConnectorResilienceTests
|
||||
|
||||
var store = CreateStore(options, connection);
|
||||
|
||||
// Act
|
||||
// Act - malformed DN with empty subject will throw, test it fails cleanly
|
||||
var result = await store.VerifyPasswordAsync("malformed", "Password1!", TestContext.Current.CancellationToken);
|
||||
|
||||
// Assert - should handle gracefully (either succeed with warning or fail cleanly)
|
||||
// The exact behavior depends on implementation
|
||||
// Assert - empty DN means user not properly found, should fail authentication
|
||||
result.Succeeded.Should().BeFalse("Empty DN should result in authentication failure");
|
||||
_output.WriteLine($"Malformed DN result: Succeeded={result.Succeeded}");
|
||||
}
|
||||
|
||||
|
||||
@@ -78,9 +78,19 @@ public sealed class LdapConnectorSecurityTests
|
||||
if (capturedFilters.Count > 0)
|
||||
{
|
||||
var filter = capturedFilters[0];
|
||||
// The raw injection characters should be escaped
|
||||
filter.Should().NotContain(")(", "Filter should escape parentheses");
|
||||
filter.Should().NotContain("*)(", "Filter should not allow wildcard injection");
|
||||
// Extract just the uid value portion after "uid=" to check escaping
|
||||
var uidStart = filter.IndexOf("uid=", StringComparison.Ordinal);
|
||||
if (uidStart >= 0)
|
||||
{
|
||||
var uidValue = filter.Substring(uidStart + 4);
|
||||
var uidEnd = uidValue.IndexOf(')');
|
||||
if (uidEnd > 0) uidValue = uidValue.Substring(0, uidEnd);
|
||||
|
||||
// The uid value should have dangerous characters escaped (as hex like \2a, \28, \29)
|
||||
// Unescaped literal *, (, ) should not appear in the uid value itself
|
||||
uidValue.Should().NotContain("*", "Asterisks in username should be escaped");
|
||||
uidValue.Should().NotMatchRegex(@"(?<!\\)[()]", "Parentheses should be escaped");
|
||||
}
|
||||
_output.WriteLine($"Filter: {filter}");
|
||||
}
|
||||
|
||||
|
||||
@@ -17,4 +17,9 @@
|
||||
<ItemGroup>
|
||||
<PackageReference Include="FluentAssertions" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<None Include="Fixtures\**\*" CopyToOutputDirectory="PreserveNewest" />
|
||||
<None Include="Expected\**\*" CopyToOutputDirectory="PreserveNewest" />
|
||||
</ItemGroup>
|
||||
</Project>
|
||||
@@ -115,6 +115,21 @@ internal sealed class LdapCredentialStore : IUserCredentialStore
|
||||
auditProperties: auditProperties);
|
||||
}
|
||||
|
||||
// Validate DN is not empty/malformed
|
||||
if (string.IsNullOrWhiteSpace(userEntry.DistinguishedName))
|
||||
{
|
||||
logger.LogWarning("LDAP plugin {Plugin} found user {Username} but DN is empty/malformed.", pluginName, normalizedUsername);
|
||||
auditProperties.Add(new AuthEventProperty
|
||||
{
|
||||
Name = "ldap.failure",
|
||||
Value = ClassifiedString.Public("malformed_dn")
|
||||
});
|
||||
return AuthorityCredentialVerificationResult.Failure(
|
||||
AuthorityCredentialFailureCode.InvalidCredentials,
|
||||
"Invalid credentials.",
|
||||
auditProperties: auditProperties);
|
||||
}
|
||||
|
||||
auditProperties.Add(new AuthEventProperty
|
||||
{
|
||||
Name = "ldap.entry_dn",
|
||||
|
||||
@@ -75,6 +75,7 @@ public sealed class OidcConnectorResilienceTests
|
||||
{
|
||||
// Arrange
|
||||
var options = CreateOptions();
|
||||
options.ValidateLifetime = false; // Avoid timing issues in unit test
|
||||
var token = CreateTestToken(claims: new Dictionary<string, object>
|
||||
{
|
||||
["sub"] = "user:no-email",
|
||||
@@ -99,6 +100,7 @@ public sealed class OidcConnectorResilienceTests
|
||||
{
|
||||
// Arrange
|
||||
var options = CreateOptions();
|
||||
options.ValidateLifetime = false; // Avoid timing issues in unit test
|
||||
var token = CreateTestToken(claims: new Dictionary<string, object>
|
||||
{
|
||||
["sub"] = "user:no-roles",
|
||||
@@ -347,10 +349,11 @@ public sealed class OidcConnectorResilienceTests
|
||||
"Token does not contain a valid subject claim.");
|
||||
}
|
||||
|
||||
// Extract user info
|
||||
// Extract user info - use email as username, fallback to subject
|
||||
var email = jwtToken.Claims.FirstOrDefault(c => c.Type == "email")?.Value;
|
||||
var user = new AuthorityUserDescriptor(
|
||||
subjectId: subClaim.Value,
|
||||
username: jwtToken.Claims.FirstOrDefault(c => c.Type == "email")?.Value,
|
||||
username: email ?? subClaim.Value, // Fallback to subject if no email
|
||||
displayName: jwtToken.Claims.FirstOrDefault(c => c.Type == "name")?.Value,
|
||||
requiresPasswordReset: false,
|
||||
roles: Array.Empty<string>(),
|
||||
|
||||
@@ -359,13 +359,14 @@ public sealed class OidcConnectorSecurityTests
|
||||
|
||||
if (algorithm.StartsWith("HS"))
|
||||
{
|
||||
key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes("test-key-that-is-at-least-32-characters-long-for-hmac-sha256"));
|
||||
// Key must be at least 512 bits (64 bytes) for HS512
|
||||
key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes("test-key-that-is-at-least-64-characters-long-for-hmac-sha512-algorithm-support"));
|
||||
credentials = new SigningCredentials(key, algorithm);
|
||||
}
|
||||
else
|
||||
{
|
||||
// For RS/ES algorithms, would need asymmetric key
|
||||
key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes("test-key-that-is-at-least-32-characters-long-for-hmac-sha256"));
|
||||
key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes("test-key-that-is-at-least-64-characters-long-for-hmac-sha512-algorithm-support"));
|
||||
credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256);
|
||||
}
|
||||
|
||||
@@ -506,9 +507,10 @@ public sealed class OidcConnectorSecurityTests
|
||||
}
|
||||
|
||||
var subClaim = jwtToken.Claims.FirstOrDefault(c => c.Type == "sub");
|
||||
var email = jwtToken.Claims.FirstOrDefault(c => c.Type == "email")?.Value;
|
||||
var user = new AuthorityUserDescriptor(
|
||||
subjectId: subClaim?.Value ?? "unknown",
|
||||
username: null!,
|
||||
username: email ?? subClaim?.Value ?? "unknown",
|
||||
displayName: null!,
|
||||
requiresPasswordReset: false,
|
||||
roles: Array.Empty<string>(),
|
||||
|
||||
@@ -169,7 +169,15 @@ public sealed class OidcConnectorSnapshotTests
|
||||
// Check expiration
|
||||
if (claims.TryGetValue("exp", out var expObj))
|
||||
{
|
||||
var exp = Convert.ToInt64(expObj);
|
||||
long exp;
|
||||
if (expObj is System.Text.Json.JsonElement je)
|
||||
{
|
||||
exp = je.GetInt64();
|
||||
}
|
||||
else
|
||||
{
|
||||
exp = Convert.ToInt64(expObj);
|
||||
}
|
||||
var expTime = DateTimeOffset.FromUnixTimeSeconds(exp);
|
||||
if (expTime < DateTimeOffset.UtcNow)
|
||||
{
|
||||
|
||||
@@ -92,8 +92,11 @@ public sealed class SamlConnectorResilienceTests
|
||||
// Act
|
||||
var result = await SimulateAssertionValidation(assertion);
|
||||
|
||||
// Assert
|
||||
result.Succeeded.Should().BeTrue("Empty attribute statement should not prevent authentication");
|
||||
// Assert - check if failure and report reason
|
||||
if (!result.Succeeded)
|
||||
{
|
||||
Assert.Fail($"Expected success but got failure: {result.Message}");
|
||||
}
|
||||
result.User?.Roles.Should().BeEmpty();
|
||||
_output.WriteLine("✓ Empty attribute statement handled gracefully");
|
||||
}
|
||||
@@ -367,9 +370,10 @@ public sealed class SamlConnectorResilienceTests
|
||||
var notBefore = conditions.Attributes?["NotBefore"]?.Value;
|
||||
var notOnOrAfter = conditions.Attributes?["NotOnOrAfter"]?.Value;
|
||||
|
||||
if (!string.IsNullOrEmpty(notBefore) && DateTime.TryParse(notBefore, out var nbf))
|
||||
if (!string.IsNullOrEmpty(notBefore) &&
|
||||
DateTime.TryParse(notBefore, null, System.Globalization.DateTimeStyles.RoundtripKind, out var nbf))
|
||||
{
|
||||
if (nbf > DateTime.UtcNow)
|
||||
if (nbf.ToUniversalTime() > DateTime.UtcNow)
|
||||
{
|
||||
return AuthorityCredentialVerificationResult.Failure(
|
||||
AuthorityCredentialFailureCode.InvalidCredentials,
|
||||
@@ -377,9 +381,10 @@ public sealed class SamlConnectorResilienceTests
|
||||
}
|
||||
}
|
||||
|
||||
if (!string.IsNullOrEmpty(notOnOrAfter) && DateTime.TryParse(notOnOrAfter, out var expiry))
|
||||
if (!string.IsNullOrEmpty(notOnOrAfter) &&
|
||||
DateTime.TryParse(notOnOrAfter, null, System.Globalization.DateTimeStyles.RoundtripKind, out var expiry))
|
||||
{
|
||||
if (expiry < DateTime.UtcNow)
|
||||
if (expiry.ToUniversalTime() < DateTime.UtcNow)
|
||||
{
|
||||
return AuthorityCredentialVerificationResult.Failure(
|
||||
AuthorityCredentialFailureCode.InvalidCredentials,
|
||||
@@ -390,7 +395,7 @@ public sealed class SamlConnectorResilienceTests
|
||||
|
||||
var user = new AuthorityUserDescriptor(
|
||||
subjectId: nameId,
|
||||
username: null!,
|
||||
username: nameId, // Use nameId as username
|
||||
displayName: null!,
|
||||
requiresPasswordReset: false,
|
||||
roles: Array.Empty<string>(),
|
||||
|
||||
@@ -398,14 +398,17 @@ public sealed class SamlConnectorSecurityTests
|
||||
// Check signature if required
|
||||
if (options.ValidateSignature)
|
||||
{
|
||||
// In real implementation, would verify XML signature
|
||||
// For testing, just check if assertion was marked as tampered
|
||||
if (assertion.Contains("user:admin") && !assertion.Contains("_evil"))
|
||||
// Check if assertion has a Signature element
|
||||
nsMgr.AddNamespace("ds", "http://www.w3.org/2000/09/xmldsig#");
|
||||
var signatureNode = assertionNode.SelectSingleNode("ds:Signature", nsMgr);
|
||||
if (signatureNode == null)
|
||||
{
|
||||
return AuthorityCredentialVerificationResult.Failure(
|
||||
AuthorityCredentialFailureCode.InvalidCredentials,
|
||||
"Signature validation failed.");
|
||||
"Assertion is not signed but signature is required.");
|
||||
}
|
||||
// For testing purposes, we only check presence of signature element
|
||||
// Real implementation would verify the cryptographic signature
|
||||
}
|
||||
|
||||
var issuer = assertionNode.SelectSingleNode("saml2:Issuer", nsMgr)?.InnerText;
|
||||
@@ -445,7 +448,7 @@ public sealed class SamlConnectorSecurityTests
|
||||
|
||||
var user = new AuthorityUserDescriptor(
|
||||
subjectId: nameId,
|
||||
username: null!,
|
||||
username: nameId, // Use nameId as username
|
||||
displayName: null!,
|
||||
requiresPasswordReset: false,
|
||||
roles: Array.Empty<string>(),
|
||||
|
||||
Reference in New Issue
Block a user