mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 20:53:06 +00:00
test: extract pure seams for MDM cert selection + JWT validation, then cover them
Two security-critical paths had zero unit coverage because the logic was inlined behind infrastructure (a real X509Store / a live OIDC ConfigurationManager + HttpContext). Extract the decision logic into pure, testable seams — no behavioural change — and pin it. CertificateHelper (Agent): - Extract SelectMdmCertificate(IEnumerable<X509Certificate2>): the issuer discrimination (CN=Microsoft Intune MDM Device CA only, never the MMP-C CA), ClientAuth-EKU filter and NotAfter tiebreaker. FindMdmCertificate now calls it. - 13 tests incl. the dual-stack regression (issuer filter beats the NotAfter tiebreaker so a longer-lived MMP-C cert can't win over a genuine MDM cert). AuthenticationMiddleware (Backend): - Extract BuildTokenValidationParameters(signingKeys, clientId), IsValidTenantId(tid) and BuildTenantAuthority(issuer, tid). Invoke() now composes them. - 16 tests validating locally-minted tokens against a test key: alg:none and HS256 rejected (algorithm-confusion defence), expired / wrong-audience / foreign-issuer / unknown-signing-key rejected, valid RS256 + api:// audience + v1 issuer accepted, plus the tid GUID gate and v1/v2 authority mapping. Backend 2746/2746 and Agent 1615/1615 green; full solution builds clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+111
-25
@@ -1,45 +1,51 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Security.Cryptography;
|
||||
using System.Security.Cryptography.X509Certificates;
|
||||
using AutopilotMonitor.Agent.V2.Core.Security;
|
||||
using Xunit;
|
||||
|
||||
// NOTE: FindMdmCertificate (CertificateHelper.cs:20-101) is NOT unit-testable as written — it
|
||||
// opens LocalMachine\My and CurrentUser\My X509Store instances directly and performs the
|
||||
// issuer-match ("CN=Microsoft Intune MDM Device CA", explicitly NOT the MMP-C
|
||||
// "CN=Microsoft Intune Device Management Device CA"), the ClientAuth EKU (1.3.6.1.5.5.7.3.2)
|
||||
// filter, and the NotAfter tiebreaker inline inside that store loop. There is no pure seam
|
||||
// (e.g. an internal selection method over an IEnumerable<X509Certificate2> or an issuer
|
||||
// predicate) to drive those rules without a real machine cert store. Per the project rule we do
|
||||
// NOT fake the store. Refactor needed to make the security-critical selection testable: extract a
|
||||
// `SelectMdmCertificate(IEnumerable<X509Certificate2>)` pure method and unit-test the issuer
|
||||
// discrimination + NotAfter tiebreaker against it. Until then this file covers only the pure
|
||||
// helper actually exposed: IsCertificateValid.
|
||||
namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
|
||||
{
|
||||
/// <summary>
|
||||
/// Covers the pure, store-independent portion of <see cref="CertificateHelper"/> —
|
||||
/// <see cref="CertificateHelper.IsCertificateValid"/> (CertificateHelper.cs:106-113), the
|
||||
/// NotBefore/NotAfter validity-window check. See the file-top NOTE for why FindMdmCertificate's
|
||||
/// selection logic is out of unit-test reach.
|
||||
/// Covers the pure, store-independent portions of <see cref="CertificateHelper"/>:
|
||||
/// <list type="bullet">
|
||||
/// <item><see cref="CertificateHelper.IsCertificateValid"/> — the NotBefore/NotAfter window check.</item>
|
||||
/// <item><see cref="CertificateHelper.SelectMdmCertificate"/> — the security-critical MDM cert
|
||||
/// selection (issuer discrimination + ClientAuth EKU filter + NotAfter tiebreaker) that
|
||||
/// <see cref="CertificateHelper.FindMdmCertificate"/> drives over the LocalMachine\My /
|
||||
/// CurrentUser\My store contents. Exercised here against in-memory self-signed certs so the store
|
||||
/// is never touched.</item>
|
||||
/// </list>
|
||||
/// The remaining store-iteration/thumbprint-search shell of FindMdmCertificate (opening the two
|
||||
/// X509Store instances) is not unit-tested — it needs a real machine cert store and per the project
|
||||
/// rule we do not fake it — but the selection rules it depends on are now fully pinned below.
|
||||
/// </summary>
|
||||
public sealed class CertificateHelperFindMdmCertTests
|
||||
{
|
||||
// The Intune MDM device CA (the one the backend pins) vs. the MMP-C stack's CA, which is
|
||||
// co-installed on dual-enrolled devices and must NEVER be selected.
|
||||
private const string MdmIssuerCn = "Microsoft Intune MDM Device CA";
|
||||
private const string MmpcIssuerCn = "Microsoft Intune Device Management Device CA";
|
||||
private const string ClientAuthEku = "1.3.6.1.5.5.7.3.2";
|
||||
|
||||
/// <summary>
|
||||
/// In-memory self-signed cert with an explicit validity window (days relative to now),
|
||||
/// so the store is never touched. Mirrors the CreateSelfSignedCert pattern used by
|
||||
/// MtlsHttpClientFactoryTests. Windows are kept >= 1 day away from "now" so the
|
||||
/// helper's local-vs-UTC comparison skew cannot flip the expected result.
|
||||
/// In-memory self-signed cert (so a self-signed cert's Issuer == its Subject == "CN={cn}").
|
||||
/// Optionally advertises the Client Authentication EKU and an explicit validity window.
|
||||
/// </summary>
|
||||
private static X509Certificate2 CreateCert(int notBeforeDays, int notAfterDays)
|
||||
private static X509Certificate2 CreateCert(
|
||||
string subjectCn, bool clientAuthEku = true, int notBeforeDays = -1, int notAfterDays = 365)
|
||||
{
|
||||
using (var rsa = RSA.Create(2048))
|
||||
{
|
||||
var req = new CertificateRequest(
|
||||
"CN=apmon-v2-test",
|
||||
rsa,
|
||||
HashAlgorithmName.SHA256,
|
||||
RSASignaturePadding.Pkcs1);
|
||||
$"CN={subjectCn}", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
|
||||
|
||||
if (clientAuthEku)
|
||||
{
|
||||
req.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(
|
||||
new OidCollection { new Oid(ClientAuthEku) }, critical: false));
|
||||
}
|
||||
|
||||
return req.CreateSelfSigned(
|
||||
DateTimeOffset.UtcNow.AddDays(notBeforeDays),
|
||||
@@ -47,6 +53,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
|
||||
}
|
||||
}
|
||||
|
||||
// ── IsCertificateValid: NotBefore/NotAfter window ────────────────────
|
||||
|
||||
[Theory]
|
||||
// Currently within the window → valid.
|
||||
[InlineData(-1, 1, true)]
|
||||
@@ -57,7 +65,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
|
||||
[InlineData(1, 10, false)]
|
||||
public void IsCertificateValid_reflects_validity_window(int notBeforeDays, int notAfterDays, bool expected)
|
||||
{
|
||||
using var cert = CreateCert(notBeforeDays, notAfterDays);
|
||||
using var cert = CreateCert("apmon-v2-test", notBeforeDays: notBeforeDays, notAfterDays: notAfterDays);
|
||||
|
||||
Assert.Equal(expected, CertificateHelper.IsCertificateValid(cert));
|
||||
}
|
||||
@@ -67,5 +75,83 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
|
||||
{
|
||||
Assert.False(CertificateHelper.IsCertificateValid(null));
|
||||
}
|
||||
|
||||
// ── SelectMdmCertificate: issuer + EKU + NotAfter tiebreaker ─────────
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_picks_the_mdm_cert_with_client_auth()
|
||||
{
|
||||
using var mdm = CreateCert(MdmIssuerCn, clientAuthEku: true);
|
||||
|
||||
var selected = CertificateHelper.SelectMdmCertificate(new[] { mdm });
|
||||
|
||||
Assert.Same(mdm, selected);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_rejects_mmpc_issuer_even_with_client_auth()
|
||||
{
|
||||
// The MMP-C CA carries ClientAuth too, but chains to an intermediate the backend does not
|
||||
// pin — selecting it produces ChainFailed. The issuer discrimination must exclude it.
|
||||
using var mmpc = CreateCert(MmpcIssuerCn, clientAuthEku: true);
|
||||
|
||||
Assert.Null(CertificateHelper.SelectMdmCertificate(new[] { mmpc }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_rejects_mdm_issuer_without_client_auth_eku()
|
||||
{
|
||||
// Right issuer, but no Client Authentication EKU → not an mTLS client cert.
|
||||
using var noEku = CreateCert(MdmIssuerCn, clientAuthEku: false);
|
||||
|
||||
Assert.Null(CertificateHelper.SelectMdmCertificate(new[] { noEku }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_prefers_the_longest_lived_qualifying_cert()
|
||||
{
|
||||
using var shortLived = CreateCert(MdmIssuerCn, clientAuthEku: true, notAfterDays: 30);
|
||||
using var longLived = CreateCert(MdmIssuerCn, clientAuthEku: true, notAfterDays: 365);
|
||||
|
||||
// Order in the input must not matter — the NotAfter tiebreaker decides.
|
||||
Assert.Same(longLived, CertificateHelper.SelectMdmCertificate(new[] { shortLived, longLived }));
|
||||
Assert.Same(longLived, CertificateHelper.SelectMdmCertificate(new[] { longLived, shortLived }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_issuer_filter_beats_notafter_tiebreaker()
|
||||
{
|
||||
// The dual-stack regression: an MMP-C cert with a LATER NotAfter must not win over an
|
||||
// earlier-expiring genuine MDM cert. Issuer discrimination runs before the tiebreaker.
|
||||
using var mmpcLonger = CreateCert(MmpcIssuerCn, clientAuthEku: true, notAfterDays: 3650);
|
||||
using var mdmShorter = CreateCert(MdmIssuerCn, clientAuthEku: true, notAfterDays: 90);
|
||||
|
||||
Assert.Same(mdmShorter, CertificateHelper.SelectMdmCertificate(new[] { mmpcLonger, mdmShorter }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_matches_issuer_case_insensitively()
|
||||
{
|
||||
using var upper = CreateCert(MdmIssuerCn.ToUpperInvariant(), clientAuthEku: true);
|
||||
|
||||
Assert.Same(upper, CertificateHelper.SelectMdmCertificate(new[] { upper }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_returns_null_for_empty_or_null_input()
|
||||
{
|
||||
Assert.Null(CertificateHelper.SelectMdmCertificate(new X509Certificate2[0]));
|
||||
Assert.Null(CertificateHelper.SelectMdmCertificate(null));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SelectMdmCertificate_ignores_null_entries_in_the_candidate_set()
|
||||
{
|
||||
using var mdm = CreateCert(MdmIssuerCn, clientAuthEku: true);
|
||||
|
||||
var selected = CertificateHelper.SelectMdmCertificate(new X509Certificate2?[] { null, mdm, null }!);
|
||||
|
||||
Assert.Same(mdm, selected);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Security.Cryptography;
|
||||
using System.Security.Cryptography.X509Certificates;
|
||||
@@ -52,37 +53,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
|
||||
}
|
||||
else
|
||||
{
|
||||
// Auto-detect MDM certificate by issuer and EKU.
|
||||
// Match ONLY "CN=Microsoft Intune MDM Device CA" — NOT
|
||||
// "CN=Microsoft Intune Device Management Device CA" (MMP-C stack),
|
||||
// which is co-installed on dual-enrolled devices and chains to a
|
||||
// different intermediate the backend does not pin. A broader
|
||||
// substring match on "Microsoft Intune" would let the MMP-C cert
|
||||
// win the NotAfter tiebreaker and produce ChainFailed at the backend.
|
||||
var mdmCerts = store.Certificates
|
||||
.OfType<X509Certificate2>()
|
||||
.Where(c =>
|
||||
{
|
||||
var issuerIsMdm = string.Equals(
|
||||
c.Issuer,
|
||||
"CN=Microsoft Intune MDM Device CA",
|
||||
StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
// Check Enhanced Key Usage for Client Authentication (1.3.6.1.5.5.7.3.2)
|
||||
var hasClientAuth = c.Extensions
|
||||
.OfType<X509EnhancedKeyUsageExtension>()
|
||||
.Any(ext => ext.EnhancedKeyUsages
|
||||
.OfType<Oid>()
|
||||
.Any(oid => oid.Value == "1.3.6.1.5.5.7.3.2"));
|
||||
|
||||
return issuerIsMdm && hasClientAuth;
|
||||
})
|
||||
.OrderByDescending(c => c.NotAfter) // Prefer certs with longer validity
|
||||
.ToList();
|
||||
|
||||
if (mdmCerts.Any())
|
||||
// Auto-detect the MDM certificate by issuer + EKU + NotAfter tiebreaker.
|
||||
// The selection rules live in the pure SelectMdmCertificate seam so they
|
||||
// can be unit-tested without a real store.
|
||||
var cert = SelectMdmCertificate(store.Certificates.OfType<X509Certificate2>());
|
||||
if (cert != null)
|
||||
{
|
||||
var cert = mdmCerts.First();
|
||||
logger?.Info($"Found MDM certificate: Issuer={cert.Issuer}, Subject={cert.Subject}, Thumbprint={cert.Thumbprint}");
|
||||
return cert;
|
||||
}
|
||||
@@ -100,6 +76,48 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
|
||||
}
|
||||
}
|
||||
|
||||
// Client Authentication EKU (used by the MDM device cert for mTLS to the backend).
|
||||
private const string ClientAuthEku = "1.3.6.1.5.5.7.3.2";
|
||||
|
||||
// The exact issuer of the Intune MDM device certificate. Deliberately NOT a substring match
|
||||
// on "Microsoft Intune": the MMP-C stack issues "CN=Microsoft Intune Device Management Device
|
||||
// CA", co-installed on dual-enrolled devices, which chains to a different intermediate the
|
||||
// backend does not pin. Selecting it would produce ChainFailed at the backend.
|
||||
private const string MdmIssuer = "CN=Microsoft Intune MDM Device CA";
|
||||
|
||||
/// <summary>
|
||||
/// Pure selection of the MDM device certificate from a candidate set (typically the
|
||||
/// LocalMachine\My / CurrentUser\My store contents). Applies the issuer discrimination,
|
||||
/// ClientAuth-EKU filter and NotAfter tiebreaker that <see cref="FindMdmCertificate"/> relies
|
||||
/// on — extracted so the security-critical selection is unit-testable without a real store.
|
||||
/// Among qualifying certs the longest-lived (highest NotAfter) wins; returns null if none
|
||||
/// qualify (or the input is null).
|
||||
/// </summary>
|
||||
public static X509Certificate2 SelectMdmCertificate(IEnumerable<X509Certificate2> candidates)
|
||||
{
|
||||
if (candidates == null)
|
||||
return null;
|
||||
|
||||
return candidates
|
||||
.Where(c => c != null && IsMdmCertificate(c))
|
||||
.OrderByDescending(c => c.NotAfter) // prefer the cert with the longest validity
|
||||
.FirstOrDefault();
|
||||
}
|
||||
|
||||
private static bool IsMdmCertificate(X509Certificate2 c)
|
||||
{
|
||||
var issuerIsMdm = string.Equals(c.Issuer, MdmIssuer, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
// Enhanced Key Usage must advertise Client Authentication (1.3.6.1.5.5.7.3.2).
|
||||
var hasClientAuth = c.Extensions
|
||||
.OfType<X509EnhancedKeyUsageExtension>()
|
||||
.Any(ext => ext.EnhancedKeyUsages
|
||||
.OfType<Oid>()
|
||||
.Any(oid => oid.Value == ClientAuthEku));
|
||||
|
||||
return issuerIsMdm && hasClientAuth;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates that a certificate is still valid
|
||||
/// </summary>
|
||||
|
||||
+169
@@ -0,0 +1,169 @@
|
||||
using System.IdentityModel.Tokens.Jwt;
|
||||
using System.Security.Claims;
|
||||
using System.Security.Cryptography;
|
||||
using AutopilotMonitor.Functions.Middleware;
|
||||
using Microsoft.IdentityModel.Tokens;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Exercises the security-critical JWT-validation seam of <see cref="AuthenticationMiddleware"/> —
|
||||
/// <see cref="AuthenticationMiddleware.BuildTokenValidationParameters"/> — end to end against
|
||||
/// locally-minted tokens and a test signing key. This pins the rejects the middleware relies on that
|
||||
/// were previously untestable (they lived inline behind a live OIDC ConfigurationManager + HttpContext):
|
||||
/// the RS256/PS256 whitelist that hard-blocks <c>alg:none</c> and the HS family (algorithm-confusion
|
||||
/// defence), lifetime, audience and issuer validation, plus real signature verification.
|
||||
///
|
||||
/// The complementary pre-signature gates <see cref="AuthenticationMiddleware.IsValidTenantId"/> and
|
||||
/// <see cref="AuthenticationMiddleware.BuildTenantAuthority"/> are pinned below too.
|
||||
/// </summary>
|
||||
public class AuthenticationMiddlewareTokenValidationTests
|
||||
{
|
||||
private const string ClientId = "11112222-3333-4444-5555-666677778888";
|
||||
private const string Tid = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee";
|
||||
private static string V2Issuer => $"https://login.microsoftonline.com/{Tid}/v2.0";
|
||||
|
||||
// A stable RSA key used both to sign tokens and (its public half) as the TVP signing key.
|
||||
private static readonly RsaSecurityKey RsaKey = new(RSA.Create(2048)) { KeyId = "test-rsa" };
|
||||
private static SigningCredentials RsaCreds => new(RsaKey, SecurityAlgorithms.RsaSha256);
|
||||
|
||||
private static string Mint(
|
||||
SigningCredentials? creds,
|
||||
string issuer = null!,
|
||||
string audience = ClientId,
|
||||
string tid = Tid,
|
||||
DateTime? notBefore = null,
|
||||
DateTime? expires = null)
|
||||
{
|
||||
var token = new JwtSecurityToken(
|
||||
issuer: issuer ?? V2Issuer,
|
||||
audience: audience,
|
||||
claims: new[] { new Claim("tid", tid) },
|
||||
notBefore: notBefore ?? DateTime.UtcNow.AddMinutes(-5),
|
||||
expires: expires ?? DateTime.UtcNow.AddMinutes(30),
|
||||
signingCredentials: creds); // null creds → alg "none", unsigned
|
||||
return new JwtSecurityTokenHandler().WriteToken(token);
|
||||
}
|
||||
|
||||
private static ClaimsPrincipal Validate(string jwt, SecurityKey? tvpKey = null)
|
||||
{
|
||||
var tvp = AuthenticationMiddleware.BuildTokenValidationParameters(
|
||||
new[] { tvpKey ?? RsaKey }, ClientId);
|
||||
return new JwtSecurityTokenHandler().ValidateToken(jwt, tvp, out _);
|
||||
}
|
||||
|
||||
// ── Happy path ───────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void ValidRs256Token_validates_and_exposes_tid()
|
||||
{
|
||||
var principal = Validate(Mint(RsaCreds));
|
||||
|
||||
Assert.NotNull(principal);
|
||||
Assert.True(principal.Identity?.IsAuthenticated);
|
||||
// JwtSecurityTokenHandler remaps "tid" to the tenantid schema URI, so match by value.
|
||||
Assert.Contains(principal.Claims, c => c.Value == Tid);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ApiPrefixedAudience_is_accepted()
|
||||
{
|
||||
// The middleware accepts both {clientId} and api://{clientId}.
|
||||
var principal = Validate(Mint(RsaCreds, audience: $"api://{ClientId}"));
|
||||
|
||||
Assert.NotNull(principal);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void V1IssuerToken_is_accepted()
|
||||
{
|
||||
var principal = Validate(Mint(RsaCreds, issuer: $"https://sts.windows.net/{Tid}/"));
|
||||
|
||||
Assert.NotNull(principal);
|
||||
}
|
||||
|
||||
// ── Algorithm-confusion defence ──────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void UnsignedNoneAlgToken_is_rejected()
|
||||
{
|
||||
// alg:none / unsigned — RequireSignedTokens + the RS256/PS256 whitelist must reject it.
|
||||
Assert.ThrowsAny<SecurityTokenException>(() => Validate(Mint(creds: null)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Hs256Token_is_rejected()
|
||||
{
|
||||
// HS-family (symmetric) is outside the whitelist — the classic RS256→HS256 confusion attack.
|
||||
var hmac = new SigningCredentials(
|
||||
new SymmetricSecurityKey(new byte[32]), SecurityAlgorithms.HmacSha256);
|
||||
|
||||
Assert.ThrowsAny<SecurityTokenException>(() => Validate(Mint(hmac)));
|
||||
}
|
||||
|
||||
// ── Lifetime / audience / issuer / signature ─────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void ExpiredToken_is_rejected()
|
||||
{
|
||||
// Past the default 5-minute clock skew.
|
||||
Assert.Throws<SecurityTokenExpiredException>(() => Validate(
|
||||
Mint(RsaCreds, notBefore: DateTime.UtcNow.AddMinutes(-20), expires: DateTime.UtcNow.AddMinutes(-10))));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WrongAudienceToken_is_rejected()
|
||||
{
|
||||
Assert.Throws<SecurityTokenInvalidAudienceException>(() => Validate(
|
||||
Mint(RsaCreds, audience: "api://some-other-app")));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ForeignIssuerToken_is_rejected()
|
||||
{
|
||||
Assert.Throws<SecurityTokenInvalidIssuerException>(() => Validate(
|
||||
Mint(RsaCreds, issuer: "https://evil.example/")));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TokenSignedByUnknownKey_is_rejected()
|
||||
{
|
||||
// Signed by a DIFFERENT RSA key than the one the TVP trusts → signature verification fails.
|
||||
var otherKey = new RsaSecurityKey(RSA.Create(2048)) { KeyId = "attacker" };
|
||||
var forged = Mint(new SigningCredentials(otherKey, SecurityAlgorithms.RsaSha256));
|
||||
|
||||
Assert.ThrowsAny<SecurityTokenException>(() => Validate(forged));
|
||||
}
|
||||
|
||||
// ── IsValidTenantId: pre-signature tid GUID gate ─────────────────────
|
||||
|
||||
[Theory]
|
||||
[InlineData("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", true)]
|
||||
[InlineData(null, false)]
|
||||
[InlineData("", false)]
|
||||
[InlineData("not-a-guid", false)]
|
||||
[InlineData(@"DESKTOP-ABC\defaultuser0", false)]
|
||||
public void IsValidTenantId_requires_a_guid(string? tid, bool expected)
|
||||
{
|
||||
Assert.Equal(expected, AuthenticationMiddleware.IsValidTenantId(tid));
|
||||
}
|
||||
|
||||
// ── BuildTenantAuthority: v1 vs v2 authority selection ───────────────
|
||||
|
||||
[Fact]
|
||||
public void BuildTenantAuthority_uses_bare_authority_for_v1_sts_issuer()
|
||||
{
|
||||
Assert.Equal(
|
||||
$"https://login.microsoftonline.com/{Tid}",
|
||||
AuthenticationMiddleware.BuildTenantAuthority($"https://sts.windows.net/{Tid}/", Tid));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildTenantAuthority_uses_v2_authority_for_v2_issuer()
|
||||
{
|
||||
Assert.Equal(
|
||||
$"https://login.microsoftonline.com/{Tid}/v2.0",
|
||||
AuthenticationMiddleware.BuildTenantAuthority(V2Issuer, Tid));
|
||||
}
|
||||
}
|
||||
@@ -94,17 +94,14 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
// otherwise drive an outbound OIDC-metadata fetch to a bogus authority — a DoS/cost
|
||||
// surface on malformed-token floods. Every legitimate Entra token (including consumer
|
||||
// MSA, tid 9188040d-…) carries a GUID tid, so this reject is loss-free.
|
||||
if (!Guid.TryParse(tenantId, out _))
|
||||
if (!IsValidTenantId(tenantId))
|
||||
{
|
||||
_logger.LogWarning("[Auth Middleware] Rejected token with missing/non-GUID tid for {Path}", requestPath);
|
||||
throw new SecurityTokenValidationException("Token tid claim is missing or not a valid GUID");
|
||||
}
|
||||
|
||||
// Determine which endpoint to use based on the issuer (v1.0 vs v2.0)
|
||||
var isV1Token = issuer.Contains("sts.windows.net");
|
||||
var tenantSpecificAuthority = isV1Token
|
||||
? $"https://login.microsoftonline.com/{tenantId}" // v1.0
|
||||
: $"https://login.microsoftonline.com/{tenantId}/v2.0"; // v2.0
|
||||
var tenantSpecificAuthority = BuildTenantAuthority(issuer, tenantId);
|
||||
|
||||
// Get or create cached configuration manager for this tenant
|
||||
IConfigurationManager<OpenIdConnectConfiguration>? tenantConfigManager = null;
|
||||
@@ -157,45 +154,12 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
|
||||
var openIdConfig = await tenantConfigManager.GetConfigurationAsync(context.CancellationToken);
|
||||
|
||||
// Set up token validation parameters with OIDC signing keys
|
||||
var validationParameters = new TokenValidationParameters
|
||||
{
|
||||
// For multi-tenant, validate issuer format but accept any tenant
|
||||
ValidateIssuer = true,
|
||||
IssuerValidator = (issuer, token, parameters) =>
|
||||
{
|
||||
// Accept issuers from any Azure AD tenant
|
||||
if (issuer.StartsWith("https://login.microsoftonline.com/") ||
|
||||
issuer.StartsWith("https://sts.windows.net/"))
|
||||
{
|
||||
return issuer;
|
||||
}
|
||||
throw new SecurityTokenInvalidIssuerException($"Invalid issuer: {issuer}");
|
||||
},
|
||||
ValidateAudience = true,
|
||||
ValidAudiences = new[]
|
||||
{
|
||||
_configuration["EntraId:ClientId"],
|
||||
$"api://{_configuration["EntraId:ClientId"]}"
|
||||
},
|
||||
ValidateLifetime = true,
|
||||
|
||||
// Signature validation enabled
|
||||
// Token is requested for backend API (api://<clientId>/access_as_user)
|
||||
ValidateIssuerSigningKey = true,
|
||||
RequireSignedTokens = true,
|
||||
|
||||
// Use key resolver for dynamic key resolution
|
||||
IssuerSigningKeyResolver = (token, securityToken, kid, validationParameters) =>
|
||||
{
|
||||
var matchedKeys = openIdConfig.SigningKeys.Where(k => k.KeyId == kid).ToList();
|
||||
return matchedKeys.Any() ? matchedKeys : openIdConfig.SigningKeys;
|
||||
},
|
||||
|
||||
// Algorithm whitelist: RS256 today, PS256 for Entra's announced rollout.
|
||||
// Hard-blocks HS-family and "none" (algorithm-confusion defence).
|
||||
ValidAlgorithms = new[] { "RS256", "PS256" }
|
||||
};
|
||||
// Build the security-critical validation parameters (issuer/audience/lifetime + the
|
||||
// RS256/PS256 algorithm whitelist that hard-blocks alg:none and HS-family confusion).
|
||||
// Extracted to a pure seam so those rejects are unit-testable against locally-minted
|
||||
// tokens without a live OIDC ConfigurationManager.
|
||||
var validationParameters = BuildTokenValidationParameters(
|
||||
openIdConfig.SigningKeys, _configuration["EntraId:ClientId"]);
|
||||
|
||||
var principal = _tokenHandler.ValidateToken(token, validationParameters, out var validatedToken);
|
||||
|
||||
@@ -264,4 +228,68 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
|
||||
var entry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, path);
|
||||
return entry is { Policy: EndpointPolicy.PublicAnonymous or EndpointPolicy.DeviceOrBootstrapAuth };
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pre-signature gate on the (unverified) <c>tid</c> claim: it must be a GUID. Rejecting a
|
||||
/// missing/garbage tid here prevents an outbound OIDC-metadata fetch to a bogus authority on
|
||||
/// malformed-token floods. Loss-free — every legitimate Entra token carries a GUID tid.
|
||||
/// </summary>
|
||||
internal static bool IsValidTenantId(string? tid) => Guid.TryParse(tid, out _);
|
||||
|
||||
/// <summary>
|
||||
/// Maps a token issuer + tenant id to the OIDC authority to fetch signing keys from. Entra v1.0
|
||||
/// tokens carry an <c>sts.windows.net</c> issuer and use the bare authority; v2.0 tokens use the
|
||||
/// <c>/v2.0</c> authority. The tenant id is assumed already validated by <see cref="IsValidTenantId"/>.
|
||||
/// </summary>
|
||||
internal static string BuildTenantAuthority(string issuer, string tenantId)
|
||||
{
|
||||
var isV1Token = issuer != null && issuer.Contains("sts.windows.net");
|
||||
return isV1Token
|
||||
? $"https://login.microsoftonline.com/{tenantId}" // v1.0
|
||||
: $"https://login.microsoftonline.com/{tenantId}/v2.0"; // v2.0
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the token validation parameters the middleware validates against: multi-tenant issuer
|
||||
/// format check, audience = <c>{clientId}</c> / <c>api://{clientId}</c>, lifetime, and — the
|
||||
/// algorithm-confusion defence — <c>RequireSignedTokens</c> plus the RS256/PS256 whitelist that
|
||||
/// hard-blocks alg:none and the HS family. Signing keys are supplied by the caller (the tenant's
|
||||
/// OIDC config at runtime; a test key under unit test).
|
||||
/// </summary>
|
||||
internal static TokenValidationParameters BuildTokenValidationParameters(
|
||||
IEnumerable<SecurityKey> signingKeys, string? clientId)
|
||||
{
|
||||
var keys = signingKeys as ICollection<SecurityKey> ?? signingKeys?.ToList() ?? new List<SecurityKey>();
|
||||
|
||||
return new TokenValidationParameters
|
||||
{
|
||||
// For multi-tenant, validate issuer format but accept any tenant.
|
||||
ValidateIssuer = true,
|
||||
IssuerValidator = (iss, token, parameters) =>
|
||||
{
|
||||
if (iss.StartsWith("https://login.microsoftonline.com/") ||
|
||||
iss.StartsWith("https://sts.windows.net/"))
|
||||
{
|
||||
return iss;
|
||||
}
|
||||
throw new SecurityTokenInvalidIssuerException($"Invalid issuer: {iss}");
|
||||
},
|
||||
ValidateAudience = true,
|
||||
ValidAudiences = new[] { clientId, $"api://{clientId}" },
|
||||
ValidateLifetime = true,
|
||||
|
||||
// Signature validation enabled (token minted for api://<clientId>/access_as_user).
|
||||
ValidateIssuerSigningKey = true,
|
||||
RequireSignedTokens = true,
|
||||
IssuerSigningKeyResolver = (token, securityToken, kid, parameters) =>
|
||||
{
|
||||
var matched = keys.Where(k => k.KeyId == kid).ToList();
|
||||
return matched.Any() ? matched : keys;
|
||||
},
|
||||
|
||||
// Algorithm whitelist: RS256 today, PS256 for Entra's announced rollout.
|
||||
// Hard-blocks HS-family and "none" (algorithm-confusion defence).
|
||||
ValidAlgorithms = new[] { "RS256", "PS256" }
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user