From fe77229b489ab5448664df17108b3b8fac31bea7 Mon Sep 17 00:00:00 2001 From: Alejandro Rosales Date: Tue, 1 Sep 2026 16:42:25 -0600 Subject: [PATCH] Harden NTLM authentication and restore SGU profiles --- SGU-CredentialProvider.sln | 15 + docs/lab-runbook.md | 2 +- docs/windows-client-onboarding.md | 2 +- scripts/Deploy-AuthBroker.ps1 | 4 +- scripts/Enroll-SguDomainClient.ps1 | 4 +- scripts/Install-CredentialProvider.ps1 | 4 +- scripts/Install-SguEnrollmentGuard.ps1 | 4 +- scripts/Test-Broker.ps1 | 2 +- src/SGU.AuthBroker/Options/BrokerOptions.cs | 4 +- src/SGU.AuthBroker/SGU.AuthBroker.csproj | 6 + .../Services/NtlmCredentialValidator.cs | 628 ++++++++++++------ src/SGU.AuthBroker/appsettings.json | 2 +- .../ProviderSettings.cs | 4 +- .../ProviderTileIcon.cs | 5 +- .../settings.example.json | 2 +- .../NtlmCredentialValidatorTests.cs | 167 +++++ .../SGU.AuthBroker.Tests.csproj | 18 + .../BrokerClientTests.cs | 2 +- .../ProviderTileIconTests.cs | 8 +- 19 files changed, 657 insertions(+), 226 deletions(-) create mode 100644 tests/SGU.AuthBroker.Tests/NtlmCredentialValidatorTests.cs create mode 100644 tests/SGU.AuthBroker.Tests/SGU.AuthBroker.Tests.csproj diff --git a/SGU-CredentialProvider.sln b/SGU-CredentialProvider.sln index e5a887a..dc7b515 100644 --- a/SGU-CredentialProvider.sln +++ b/SGU-CredentialProvider.sln @@ -19,6 +19,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SGU.CredentialProvider.Test EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SGU.CredentialProvider.SmokeProbe", "tests\SGU.CredentialProvider.SmokeProbe\SGU.CredentialProvider.SmokeProbe.csproj", "{B5171244-2BBD-465B-BBAF-96D5C6F9A84C}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SGU.AuthBroker.Tests", "tests\SGU.AuthBroker.Tests\SGU.AuthBroker.Tests.csproj", "{8B3E5C97-5EE6-4F83-95AC-A7955B650094}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -101,6 +103,18 @@ Global {B5171244-2BBD-465B-BBAF-96D5C6F9A84C}.Release|x64.Build.0 = Release|x64 {B5171244-2BBD-465B-BBAF-96D5C6F9A84C}.Release|x86.ActiveCfg = Release|x64 {B5171244-2BBD-465B-BBAF-96D5C6F9A84C}.Release|x86.Build.0 = Release|x64 + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|Any CPU.Build.0 = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|x64.ActiveCfg = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|x64.Build.0 = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|x86.ActiveCfg = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Debug|x86.Build.0 = Debug|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|Any CPU.ActiveCfg = Release|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|Any CPU.Build.0 = Release|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|x64.ActiveCfg = Release|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|x64.Build.0 = Release|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|x86.ActiveCfg = Release|Any CPU + {8B3E5C97-5EE6-4F83-95AC-A7955B650094}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -112,5 +126,6 @@ Global {5749FA85-9760-4884-9475-C760879B1953} = {0AB3BF05-4346-4AA6-1389-037BE0695223} {72DF14BC-9050-4AF3-B311-36F2A4140366} = {0AB3BF05-4346-4AA6-1389-037BE0695223} {B5171244-2BBD-465B-BBAF-96D5C6F9A84C} = {0AB3BF05-4346-4AA6-1389-037BE0695223} + {8B3E5C97-5EE6-4F83-95AC-A7955B650094} = {0AB3BF05-4346-4AA6-1389-037BE0695223} EndGlobalSection EndGlobal diff --git a/docs/lab-runbook.md b/docs/lab-runbook.md index d46f411..6665fb2 100644 --- a/docs/lab-runbook.md +++ b/docs/lab-runbook.md @@ -124,7 +124,7 @@ On Windows 10: -BrokerEndpoint https://sgu-auth.lci.lasalle.mx:8443/v1/authenticate ` -ClientCertificateThumbprint CLIENT_CERT_THUMBPRINT ` -ServerCertificateThumbprint SERVER_CERT_THUMBPRINT ` - -TimeoutSeconds 35 ` + -TimeoutSeconds 90 ` -InstallDotNetRuntime ` -DotNetRuntimeInstallerPath C:\SGUDeploy\prerequisites\dotnet-runtime-10.0.11-win-x64.exe ``` diff --git a/docs/windows-client-onboarding.md b/docs/windows-client-onboarding.md index 31439bc..762e6fe 100644 --- a/docs/windows-client-onboarding.md +++ b/docs/windows-client-onboarding.md @@ -82,7 +82,7 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass ` -BrokerEndpoint https://sgu-auth.lci.lasalle.mx:8443/v1/authenticate ` -ClientCertificateThumbprint CLIENT_CERT_THUMBPRINT ` -ServerCertificateThumbprint SERVER_CERT_THUMBPRINT ` - -TimeoutSeconds 35 + -TimeoutSeconds 90 ``` Los binarios se guardan en un subdirectorio `versions` identificado por su diff --git a/scripts/Deploy-AuthBroker.ps1 b/scripts/Deploy-AuthBroker.ps1 index 69d9d96..86afc55 100644 --- a/scripts/Deploy-AuthBroker.ps1 +++ b/scripts/Deploy-AuthBroker.ps1 @@ -31,8 +31,8 @@ param( [string]$DefaultCompany = 'Universidad La Salle', [ValidateRange(10, 60)] [int]$NtlmTimeoutSeconds = 20, - [ValidateRange(2, 30)] - [int]$ProfileTimeoutSeconds = 10, + [ValidateRange(2, 90)] + [int]$ProfileTimeoutSeconds = 60, [switch]$CreateMissingOus, [switch]$DisableCertificateRevocationCheckForLab ) diff --git a/scripts/Enroll-SguDomainClient.ps1 b/scripts/Enroll-SguDomainClient.ps1 index 699db6d..41ec5ba 100644 --- a/scripts/Enroll-SguDomainClient.ps1 +++ b/scripts/Enroll-SguDomainClient.ps1 @@ -55,7 +55,7 @@ $installParams = @{ ClientCertificateThumbprint = $ClientCertificateThumbprint ServerCertificateThumbprint = $ServerCertificateThumbprint DomainNetbios = $DomainNetbios - TimeoutSeconds = 35 + TimeoutSeconds = 90 } if ($DotNetRuntimeInstallerPath) { $installParams.InstallDotNetRuntime = $true @@ -68,7 +68,7 @@ $guardParams = @{ ClientCertificateThumbprint = $ClientCertificateThumbprint ServerCertificateThumbprint = $ServerCertificateThumbprint DomainNetbios = $DomainNetbios - TimeoutSeconds = 35 + TimeoutSeconds = 90 RemoteDesktopPrincipal = $RemoteDesktopPrincipal DotNetRuntimeInstallerPath = $DotNetRuntimeInstallerPath } diff --git a/scripts/Install-CredentialProvider.ps1 b/scripts/Install-CredentialProvider.ps1 index 01baf69..1f1e611 100644 --- a/scripts/Install-CredentialProvider.ps1 +++ b/scripts/Install-CredentialProvider.ps1 @@ -17,8 +17,8 @@ param( [string]$DomainNetbios = 'LCI', - [ValidateRange(2, 60)] - [int]$TimeoutSeconds = 35, + [ValidateRange(2, 90)] + [int]$TimeoutSeconds = 90, [switch]$DoNotSetAsDefaultCredentialProvider, diff --git a/scripts/Install-SguEnrollmentGuard.ps1 b/scripts/Install-SguEnrollmentGuard.ps1 index 8ac9659..c24830b 100644 --- a/scripts/Install-SguEnrollmentGuard.ps1 +++ b/scripts/Install-SguEnrollmentGuard.ps1 @@ -16,8 +16,8 @@ param( [string]$ServerCertificateThumbprint, [string]$DomainNetbios = 'LCI', - [ValidateRange(2, 60)] - [int]$TimeoutSeconds = 35, + [ValidateRange(2, 90)] + [int]$TimeoutSeconds = 90, [string]$RemoteDesktopPrincipal = 'LCI\SG-Laboratorio-Usuarios-RDP', [string]$DotNetRuntimeInstallerPath ) diff --git a/scripts/Test-Broker.ps1 b/scripts/Test-Broker.ps1 index a3b5413..8444000 100644 --- a/scripts/Test-Broker.ps1 +++ b/scripts/Test-Broker.ps1 @@ -23,7 +23,7 @@ if (-not $certificate -or -not $certificate.HasPrivateKey) { try { $body = @{ clave = $clave; password = $password } | ConvertTo-Json -Compress Invoke-RestMethod -Method Post -Uri $BrokerEndpoint -Certificate $certificate ` - -ContentType 'application/json' -Body $body -TimeoutSec 20 + -ContentType 'application/json' -Body $body -TimeoutSec 90 } finally { $password = $null diff --git a/src/SGU.AuthBroker/Options/BrokerOptions.cs b/src/SGU.AuthBroker/Options/BrokerOptions.cs index 615290a..81e2e31 100644 --- a/src/SGU.AuthBroker/Options/BrokerOptions.cs +++ b/src/SGU.AuthBroker/Options/BrokerOptions.cs @@ -32,7 +32,7 @@ public sealed class BrokerOptions } if (Ntlm.TimeoutSeconds is < 2 or > 60 || - Ntlm.ProfileTimeoutSeconds is < 2 or > 30 || + Ntlm.ProfileTimeoutSeconds is < 2 or > 90 || Ntlm.MaxRedirects is < 0 or > 10) { throw new InvalidOperationException("NTLM timeout or redirect limits are outside the supported range."); @@ -122,7 +122,7 @@ public sealed class NtlmOptions public int TimeoutSeconds { get; init; } = 20; - public int ProfileTimeoutSeconds { get; init; } = 10; + public int ProfileTimeoutSeconds { get; init; } = 60; public int MaxRedirects { get; init; } = 5; diff --git a/src/SGU.AuthBroker/SGU.AuthBroker.csproj b/src/SGU.AuthBroker/SGU.AuthBroker.csproj index 5930771..acf6143 100644 --- a/src/SGU.AuthBroker/SGU.AuthBroker.csproj +++ b/src/SGU.AuthBroker/SGU.AuthBroker.csproj @@ -12,4 +12,10 @@ + + + + <_Parameter1>SGU.AuthBroker.Tests + + diff --git a/src/SGU.AuthBroker/Services/NtlmCredentialValidator.cs b/src/SGU.AuthBroker/Services/NtlmCredentialValidator.cs index 5eeb865..3ab7b95 100644 --- a/src/SGU.AuthBroker/Services/NtlmCredentialValidator.cs +++ b/src/SGU.AuthBroker/Services/NtlmCredentialValidator.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Net; using SGU.AuthBroker.Core.Authentication; using SGU.AuthBroker.Core.Identity; @@ -6,9 +7,28 @@ using SGU.AuthBroker.Options; namespace SGU.AuthBroker.Services; -public sealed class NtlmCredentialValidator(BrokerOptions options) : INtlmCredentialValidator +public sealed class NtlmCredentialValidator : INtlmCredentialValidator { - private readonly NtlmOptions options = options.Ntlm; + private readonly NtlmOptions options; + private readonly ILogger logger; + private readonly Func handlerFactory; + + public NtlmCredentialValidator( + BrokerOptions options, + ILogger logger) + : this(options, logger, CreateHandler) + { + } + + internal NtlmCredentialValidator( + BrokerOptions options, + ILogger logger, + Func handlerFactory) + { + this.options = options.Ntlm; + this.logger = logger; + this.handlerFactory = handlerFactory; + } public async Task ValidateAsync( UserIdentity identity, @@ -24,10 +44,372 @@ public sealed class NtlmCredentialValidator(BrokerOptions options) : INtlmCreden NetworkCredential credential = new(identity.UserName, password, this.options.Domain); CredentialCache credentialCache = new(); - HashSet credentialedAuthorities = new(StringComparer.OrdinalIgnoreCase); CookieContainer cookieContainer = new(); - using HttpClientHandler handler = new() + using HttpMessageHandler handler = handlerFactory(credentialCache, cookieContainer); + using HttpClient client = new(handler) + { + Timeout = Timeout.InfiniteTimeSpan, + DefaultRequestVersion = HttpVersion.Version11, + DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact + }; + client.DefaultRequestHeaders.UserAgent.ParseAdd("SGU-AuthBroker/1.0"); + + try + { + (NtlmValidationResult? authenticationFailure, Uri? continuationUri) = await ValidateCredentialsAsync( + client, + authenticationUri, + allowedHosts, + credentialCache, + credential, + cancellationToken).ConfigureAwait(false); + if (authenticationFailure is not null) + { + return authenticationFailure; + } + + InstitutionalProfile? profile = await TryFetchProfileAsync( + client, + identity, + allowedHosts, + continuationUri, + cancellationToken).ConfigureAwait(false); + return NtlmValidationResult.Valid(profile); + } + finally + { + credential.Password = string.Empty; + } + } + + private async Task<(NtlmValidationResult? Failure, Uri? ContinuationUri)> ValidateCredentialsAsync( + HttpClient client, + Uri authenticationUri, + HashSet allowedHosts, + CredentialCache credentialCache, + NetworkCredential credential, + CancellationToken cancellationToken) + { + if (!IsAllowedHttpsUri(authenticationUri, allowedHosts)) + { + return (NtlmValidationResult.Unavailable("NTLM_AUTH_ENDPOINT_REJECTED"), null); + } + + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(options.TimeoutSeconds)); + Stopwatch elapsed = Stopwatch.StartNew(); + + try + { + Uri current = authenticationUri; + for (int hop = 0; hop <= options.MaxRedirects; hop++) + { + if (!IsAllowedHttpsUri(current, allowedHosts)) + { + return (NtlmValidationResult.Unavailable("NTLM_AUTH_ENDPOINT_REJECTED"), null); + } + + using HttpRequestMessage probeRequest = new(HttpMethod.Get, current); + using HttpResponseMessage probeResponse = await client + .SendAsync(probeRequest, HttpCompletionOption.ResponseHeadersRead, timeout.Token) + .ConfigureAwait(false); + + int probeStatus = (int)probeResponse.StatusCode; + if (probeStatus is >= 300 and < 400) + { + Uri? redirect = ResolveAllowedRedirect(current, probeResponse, allowedHosts); + if (redirect is null) + { + return (NtlmValidationResult.Unavailable("NTLM_AUTH_REDIRECT_REJECTED"), null); + } + + logger.LogDebug( + "SGU authentication discovery followed redirect hop {Hop} to {Path}.", + hop + 1, + redirect.AbsolutePath); + current = redirect; + continue; + } + + if (probeResponse.StatusCode == HttpStatusCode.Unauthorized) + { + if (!OffersNtlmChallenge(probeResponse)) + { + logger.LogWarning( + "SGU returned 401 without an NTLM challenge at {Path}.", + current.AbsolutePath); + return (NtlmValidationResult.Unavailable("NTLM_CHALLENGE_MISSING"), null); + } + + await DrainResponseAsync(probeResponse, timeout.Token).ConfigureAwait(false); + AddCredential(current, credentialCache, credential); + (NtlmValidationResult? failure, Uri? continuationUri) = await AuthenticateChallengedEndpointAsync( + client, + current, + allowedHosts, + timeout.Token).ConfigureAwait(false); + if (failure is not null) + { + return (failure, null); + } + + logger.LogInformation( + "SGU accepted credentials after an explicit NTLM challenge in {ElapsedMilliseconds} ms.", + elapsed.ElapsedMilliseconds); + return (null, continuationUri); + } + + if (probeResponse.StatusCode == HttpStatusCode.Forbidden) + { + return (NtlmValidationResult.Unavailable("NTLM_CHALLENGE_REJECTED"), null); + } + + if (probeStatus is >= 200 and < 300) + { + logger.LogWarning( + "SGU authentication discovery reached {StatusCode} at {Path} without an NTLM challenge; credentials were not accepted.", + probeStatus, + current.AbsolutePath); + return (NtlmValidationResult.Unavailable("NTLM_CHALLENGE_MISSING"), null); + } + + return probeStatus is 429 or >= 500 + ? (NtlmValidationResult.Unavailable("NTLM_UPSTREAM_ERROR"), null) + : (NtlmValidationResult.Unavailable("NTLM_UNEXPECTED_RESPONSE"), null); + } + + return (NtlmValidationResult.Unavailable("NTLM_REDIRECT_LIMIT"), null); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + logger.LogWarning( + "SGU NTLM authentication timed out after {ElapsedMilliseconds} ms.", + elapsed.ElapsedMilliseconds); + return (NtlmValidationResult.Unavailable("NTLM_TIMEOUT"), null); + } + catch (HttpRequestException exception) + { + logger.LogWarning( + exception, + "SGU NTLM authentication failed after {ElapsedMilliseconds} ms.", + elapsed.ElapsedMilliseconds); + return (NtlmValidationResult.Unavailable(), null); + } + } + + private static async Task<(NtlmValidationResult? Failure, Uri? ContinuationUri)> AuthenticateChallengedEndpointAsync( + HttpClient client, + Uri challengeUri, + HashSet allowedHosts, + CancellationToken cancellationToken) + { + using HttpRequestMessage authenticationRequest = new(HttpMethod.Get, challengeUri); + using HttpResponseMessage authenticationResponse = await client + .SendAsync(authenticationRequest, HttpCompletionOption.ResponseHeadersRead, cancellationToken) + .ConfigureAwait(false); + + if (authenticationResponse.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden) + { + return (NtlmValidationResult.Invalid(), null); + } + + int statusCode = (int)authenticationResponse.StatusCode; + if (statusCode is >= 200 and < 300) + { + await DrainResponseAsync(authenticationResponse, cancellationToken).ConfigureAwait(false); + return (null, null); + } + + if (statusCode is >= 300 and < 400) + { + Uri? continuationUri = ResolveAllowedRedirect( + challengeUri, + authenticationResponse, + allowedHosts); + if (continuationUri is null) + { + return (NtlmValidationResult.Unavailable("NTLM_AUTH_REDIRECT_REJECTED"), null); + } + + await DrainResponseAsync(authenticationResponse, cancellationToken).ConfigureAwait(false); + return (null, continuationUri); + } + + return statusCode is 429 or >= 500 + ? (NtlmValidationResult.Unavailable("NTLM_UPSTREAM_ERROR"), null) + : (NtlmValidationResult.Unavailable("NTLM_UNEXPECTED_RESPONSE"), null); + } + + private async Task TryFetchProfileAsync( + HttpClient client, + UserIdentity identity, + HashSet allowedHosts, + Uri? continuationUri, + CancellationToken cancellationToken) + { + Stopwatch elapsed = Stopwatch.StartNew(); + try + { + Uri originalProfileUri = GetProfileUri(identity.Role); + Uri menuUri = GetProfileUri(InstitutionalRole.Professor); + Uri current = continuationUri ?? originalProfileUri; + bool bootstrappingSession = continuationUri is not null; + bool retriedAfterSessionBootstrap = continuationUri is not null; + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(options.ProfileTimeoutSeconds)); + + for (int hop = 0; hop <= options.MaxRedirects; hop++) + { + if (!IsAllowedHttpsUri(current, allowedHosts)) + { + return null; + } + + using HttpRequestMessage request = new(HttpMethod.Get, current); + using HttpResponseMessage response = await client + .SendAsync(request, HttpCompletionOption.ResponseHeadersRead, timeout.Token) + .ConfigureAwait(false); + + int statusCode = (int)response.StatusCode; + logger.LogDebug( + "SGU profile response for role {Role} returned HTTP {StatusCode} at hop {Hop} after {ElapsedMilliseconds} ms.", + identity.Role, + statusCode, + hop, + elapsed.ElapsedMilliseconds); + if (statusCode is >= 300 and < 400) + { + Uri? redirect = ResolveAllowedRedirect(current, response, allowedHosts); + if (redirect is null) + { + logger.LogWarning( + "SGU profile redirect was rejected for role {Role} at hop {Hop}.", + identity.Role, + hop + 1); + return null; + } + + await DrainResponseAsync(response, timeout.Token).ConfigureAwait(false); + current = redirect; + continue; + } + + if (statusCode is >= 200 and < 300) + { + if (bootstrappingSession) + { + await DrainResponseAsync(response, timeout.Token).ConfigureAwait(false); + bootstrappingSession = false; + current = originalProfileUri; + logger.LogInformation( + "SGU post-authentication session bootstrap completed for role {Role}; requesting the original profile page.", + identity.Role); + continue; + } + + if (identity.Role != InstitutionalRole.Professor && + !retriedAfterSessionBootstrap && + HasSamePath(current, menuUri)) + { + await DrainResponseAsync(response, timeout.Token).ConfigureAwait(false); + retriedAfterSessionBootstrap = true; + current = originalProfileUri; + logger.LogInformation( + "SGU ASP.NET session bootstrap completed for role {Role}; retrying the original profile page.", + identity.Role); + continue; + } + + InstitutionalProfile? profile = await TryReadProfileAsync( + response, + identity, + timeout.Token).ConfigureAwait(false); + if (profile is null) + { + logger.LogWarning( + "SGU returned a profile page for role {Role}, but no supported profile fields were found after {ElapsedMilliseconds} ms.", + identity.Role, + elapsed.ElapsedMilliseconds); + } + else + { + logger.LogInformation( + "SGU profile enrichment completed for role {Role} in {ElapsedMilliseconds} ms.", + identity.Role, + elapsed.ElapsedMilliseconds); + } + + return profile; + } + + logger.LogWarning( + "SGU profile request for role {Role} returned HTTP {StatusCode} after {ElapsedMilliseconds} ms.", + identity.Role, + statusCode, + elapsed.ElapsedMilliseconds); + return null; + } + + logger.LogWarning( + "SGU profile request for role {Role} exceeded the redirect limit after {ElapsedMilliseconds} ms.", + identity.Role, + elapsed.ElapsedMilliseconds); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (OperationCanceledException) + { + logger.LogWarning( + "SGU profile request for role {Role} timed out after {ElapsedMilliseconds} ms.", + identity.Role, + elapsed.ElapsedMilliseconds); + } + catch (Exception exception) + { + logger.LogWarning( + exception, + "SGU profile enrichment failed for role {Role} after {ElapsedMilliseconds} ms.", + identity.Role, + elapsed.ElapsedMilliseconds); + } + + return null; + } + + private static void AddCredential( + Uri uri, + CredentialCache credentialCache, + NetworkCredential credential) + { + string authority = uri.GetLeftPart(UriPartial.Authority); + credentialCache.Add(new Uri(authority + "/"), "NTLM", credential); + } + + private static bool OffersNtlmChallenge(HttpResponseMessage response) => + response.Headers.WwwAuthenticate.Any(value => + string.Equals(value.Scheme, "NTLM", StringComparison.OrdinalIgnoreCase)); + + private static Uri? ResolveAllowedRedirect( + Uri current, + HttpResponseMessage response, + HashSet allowedHosts) + { + Uri? location = response.Headers.Location; + Uri? redirect = location is null + ? null + : location.IsAbsoluteUri ? location : new Uri(current, location); + return redirect is not null && IsAllowedHttpsUri(redirect, allowedHosts) + ? redirect + : null; + } + + private static HttpMessageHandler CreateHandler( + CredentialCache credentialCache, + CookieContainer cookieContainer) => + new HttpClientHandler { AllowAutoRedirect = false, AutomaticDecompression = DecompressionMethods.All, @@ -42,190 +424,38 @@ public sealed class NtlmCredentialValidator(BrokerOptions options) : INtlmCreden UseProxy = false }; - using HttpClient client = new(handler) - { - Timeout = Timeout.InfiniteTimeSpan, - DefaultRequestVersion = HttpVersion.Version11, - DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact - }; - client.DefaultRequestHeaders.UserAgent.ParseAdd("SGU-AuthBroker/1.0"); + private static bool HasSamePath(Uri left, Uri right) => + string.Equals( + left.AbsolutePath.TrimEnd('/'), + right.AbsolutePath.TrimEnd('/'), + StringComparison.OrdinalIgnoreCase); - try - { - NtlmValidationResult? authenticationFailure = await ValidateCredentialsAsync( - client, - authenticationUri, - allowedHosts, - credentialCache, - credentialedAuthorities, - credential, - cancellationToken).ConfigureAwait(false); - if (authenticationFailure is not null) - { - return authenticationFailure; - } - - InstitutionalProfile? profile = await TryFetchProfileAsync( - client, - identity, - allowedHosts, - credentialCache, - credentialedAuthorities, - credential, - cancellationToken).ConfigureAwait(false); - return NtlmValidationResult.Valid(profile); - } - finally - { - credential.Password = string.Empty; - } - } - - private async Task ValidateCredentialsAsync( - HttpClient client, - Uri authenticationUri, - HashSet allowedHosts, - CredentialCache credentialCache, - HashSet credentialedAuthorities, - NetworkCredential credential, + private static async Task DrainResponseAsync( + HttpResponseMessage response, CancellationToken cancellationToken) { - if (!IsAllowedHttpsUri(authenticationUri, allowedHosts)) + const int maximumDrainBytes = 64 * 1024; + if (response.Content.Headers.ContentLength is > maximumDrainBytes) { - return NtlmValidationResult.Unavailable("NTLM_AUTH_ENDPOINT_REJECTED"); + return; } - AddCredential( - authenticationUri, - credentialCache, - credentialedAuthorities, - credential); - using HttpRequestMessage request = new(HttpMethod.Get, authenticationUri); - using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeout.CancelAfter(TimeSpan.FromSeconds(options.TimeoutSeconds)); - - HttpResponseMessage response; - try + await using Stream stream = await response.Content + .ReadAsStreamAsync(cancellationToken) + .ConfigureAwait(false); + byte[] buffer = new byte[8192]; + int total = 0; + while (total <= maximumDrainBytes) { - response = await client - .SendAsync(request, HttpCompletionOption.ResponseHeadersRead, timeout.Token) + int read = await stream + .ReadAsync(buffer.AsMemory(), cancellationToken) .ConfigureAwait(false); - } - catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) - { - return NtlmValidationResult.Unavailable("NTLM_TIMEOUT"); - } - catch (HttpRequestException) - { - return NtlmValidationResult.Unavailable(); - } - - using (response) - { - if (response.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden) + if (read == 0) { - return NtlmValidationResult.Invalid(); + return; } - int statusCode = (int)response.StatusCode; - if (statusCode is >= 200 and < 300) - { - return null; - } - - if (statusCode is >= 300 and < 400) - { - Uri? location = response.Headers.Location; - Uri? redirect = location is null - ? null - : location.IsAbsoluteUri ? location : new Uri(authenticationUri, location); - return redirect is not null && IsAllowedHttpsUri(redirect, allowedHosts) - ? null - : NtlmValidationResult.Unavailable("NTLM_AUTH_REDIRECT_REJECTED"); - } - - return statusCode is 429 or >= 500 - ? NtlmValidationResult.Unavailable("NTLM_UPSTREAM_ERROR") - : NtlmValidationResult.Unavailable("NTLM_UNEXPECTED_RESPONSE"); - } - } - - private async Task TryFetchProfileAsync( - HttpClient client, - UserIdentity identity, - HashSet allowedHosts, - CredentialCache credentialCache, - HashSet credentialedAuthorities, - NetworkCredential credential, - CancellationToken cancellationToken) - { - try - { - Uri current = GetProfileUri(identity.Role); - using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeout.CancelAfter(TimeSpan.FromSeconds(options.ProfileTimeoutSeconds)); - - for (int hop = 0; hop <= options.MaxRedirects; hop++) - { - if (!IsAllowedHttpsUri(current, allowedHosts)) - { - return null; - } - - AddCredential(current, credentialCache, credentialedAuthorities, credential); - using HttpRequestMessage request = new(HttpMethod.Get, current); - using HttpResponseMessage response = await client - .SendAsync(request, HttpCompletionOption.ResponseHeadersRead, timeout.Token) - .ConfigureAwait(false); - - int statusCode = (int)response.StatusCode; - if (statusCode is >= 300 and < 400) - { - Uri? location = response.Headers.Location; - if (location is null) - { - return null; - } - - current = location.IsAbsoluteUri ? location : new Uri(current, location); - continue; - } - - if (statusCode is >= 200 and < 300) - { - return await TryReadProfileAsync( - response, - identity, - timeout.Token, - cancellationToken).ConfigureAwait(false); - } - - return null; - } - } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) - { - throw; - } - catch - { - // Enrichment is optional once the lightweight NTLM endpoint has - // authoritatively accepted the credentials. - } - - return null; - } - - private static void AddCredential( - Uri uri, - CredentialCache credentialCache, - HashSet credentialedAuthorities, - NetworkCredential credential) - { - string authority = uri.GetLeftPart(UriPartial.Authority); - if (credentialedAuthorities.Add(authority)) - { - credentialCache.Add(new Uri(authority + "/"), "NTLM", credential); + total += read; } } @@ -245,37 +475,23 @@ public sealed class NtlmCredentialValidator(BrokerOptions options) : INtlmCreden private async Task TryReadProfileAsync( HttpResponseMessage response, UserIdentity identity, - CancellationToken timeoutToken, - CancellationToken requestCancellationToken) + CancellationToken timeoutToken) { - try + string html = await ReadLimitedStringAsync( + response.Content, + options.MaxProfileBytes, + timeoutToken).ConfigureAwait(false); + return identity.Role switch { - string html = await ReadLimitedStringAsync( - response.Content, - options.MaxProfileBytes, - timeoutToken).ConfigureAwait(false); - return identity.Role switch - { - InstitutionalRole.Administrative => - SguProfileParser.ParseAdministrative(html, identity.NumericId) ?? - SguProfileParser.ParseMenu(html), - InstitutionalRole.Student => - SguProfileParser.ParseStudent(html, identity.NumericId) ?? - SguProfileParser.ParseMenu(html), - InstitutionalRole.Professor => SguProfileParser.ParseMenu(html), - _ => null - }; - } - catch (OperationCanceledException) when (requestCancellationToken.IsCancellationRequested) - { - throw; - } - catch - { - // Profile enrichment is optional. A successful NTLM response must still - // synchronize the exact password even if SGU changes its presentation HTML. - return null; - } + InstitutionalRole.Administrative => + SguProfileParser.ParseAdministrative(html, identity.NumericId) ?? + SguProfileParser.ParseMenu(html), + InstitutionalRole.Student => + SguProfileParser.ParseStudent(html, identity.NumericId) ?? + SguProfileParser.ParseMenu(html), + InstitutionalRole.Professor => SguProfileParser.ParseMenu(html), + _ => null + }; } private static async Task ReadLimitedStringAsync( diff --git a/src/SGU.AuthBroker/appsettings.json b/src/SGU.AuthBroker/appsettings.json index 7a65163..1c33ba3 100644 --- a/src/SGU.AuthBroker/appsettings.json +++ b/src/SGU.AuthBroker/appsettings.json @@ -30,7 +30,7 @@ "Endpoint": "https://sgu.ulsa.edu.mx/", "Domain": "", "TimeoutSeconds": 20, - "ProfileTimeoutSeconds": 10, + "ProfileTimeoutSeconds": 60, "MaxRedirects": 5, "AuthenticationPath": "/psulsa/", "AdministrativeProfilePath": "/psulsa/gadmon/capitalhumano/controlincidencias/incidencias.aspx", diff --git a/src/SGU.CredentialProvider/ProviderSettings.cs b/src/SGU.CredentialProvider/ProviderSettings.cs index 27dc3dd..3d9854d 100644 --- a/src/SGU.CredentialProvider/ProviderSettings.cs +++ b/src/SGU.CredentialProvider/ProviderSettings.cs @@ -10,7 +10,7 @@ internal sealed class ProviderSettings public string DomainNetbios { get; init; } = "LCI"; - public int TimeoutSeconds { get; init; } = 35; + public int TimeoutSeconds { get; init; } = 90; public string ClientCertificateThumbprint { get; init; } = string.Empty; @@ -54,7 +54,7 @@ internal sealed class ProviderSettings throw new InvalidOperationException("BrokerEndpoint must target /v1/authenticate."); } - if (string.IsNullOrWhiteSpace(DomainNetbios) || TimeoutSeconds is < 2 or > 60) + if (string.IsNullOrWhiteSpace(DomainNetbios) || TimeoutSeconds is < 2 or > 90) { throw new InvalidOperationException("DomainNetbios or TimeoutSeconds is invalid."); } diff --git a/src/SGU.CredentialProvider/ProviderTileIcon.cs b/src/SGU.CredentialProvider/ProviderTileIcon.cs index 68cc1d0..bb58066 100644 --- a/src/SGU.CredentialProvider/ProviderTileIcon.cs +++ b/src/SGU.CredentialProvider/ProviderTileIcon.cs @@ -14,7 +14,10 @@ internal static class ProviderTileIcon using Graphics graphics = Graphics.FromImage(bitmap); graphics.SmoothingMode = SmoothingMode.AntiAlias; graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; - graphics.Clear(Color.FromArgb(0, 83, 155)); + graphics.Clear(Color.Transparent); + + using SolidBrush background = new(Color.FromArgb(0, 83, 155)); + graphics.FillEllipse(background, 1, 1, Size - 2, Size - 2); using Pen key = new(Color.White, 5.5f) { diff --git a/src/SGU.CredentialProvider/settings.example.json b/src/SGU.CredentialProvider/settings.example.json index 33536df..f5b82cb 100644 --- a/src/SGU.CredentialProvider/settings.example.json +++ b/src/SGU.CredentialProvider/settings.example.json @@ -1,7 +1,7 @@ { "BrokerEndpoint": "https://sgu-auth.lci.lasalle.mx:8443/v1/authenticate", "DomainNetbios": "LCI", - "TimeoutSeconds": 35, + "TimeoutSeconds": 90, "ClientCertificateThumbprint": "0000000000000000000000000000000000000000", "ServerCertificateThumbprint": "0000000000000000000000000000000000000000" } diff --git a/tests/SGU.AuthBroker.Tests/NtlmCredentialValidatorTests.cs b/tests/SGU.AuthBroker.Tests/NtlmCredentialValidatorTests.cs new file mode 100644 index 0000000..fc4b2dc --- /dev/null +++ b/tests/SGU.AuthBroker.Tests/NtlmCredentialValidatorTests.cs @@ -0,0 +1,167 @@ +using System.Net; +using System.Net.Http.Headers; +using Microsoft.Extensions.Logging.Abstractions; +using SGU.AuthBroker.Core.Authentication; +using SGU.AuthBroker.Core.Identity; +using SGU.AuthBroker.Options; +using SGU.AuthBroker.Services; +using Xunit; + +namespace SGU.AuthBroker.Tests; + +public sealed class NtlmCredentialValidatorTests +{ + private static readonly UserIdentity Student = new( + "AL123456", + "AL", + "123456", + InstitutionalRole.Student); + + [Fact] + public async Task RedirectAndSuccessWithoutChallengeNeverAuthorizes() + { + SequenceHandler handler = new( + Redirect("/psulsa/login.aspx?AspxAutoDetectCookieSupport=1"), + Response(HttpStatusCode.OK)); + NtlmCredentialValidator validator = CreateValidator(handler); + + NtlmValidationResult result = await validator.ValidateAsync( + Student, + "test-password", + TestContext.Current.CancellationToken); + + Assert.Equal(NtlmValidationStatus.Unavailable, result.Status); + Assert.Equal("NTLM_CHALLENGE_MISSING", result.ErrorCode); + Assert.Equal(2, handler.RequestPaths.Count); + } + + [Fact] + public async Task CredentialsAreAcceptedOnlyAfterExplicitNtlmChallenge() + { + SequenceHandler handler = new( + Challenge(), + Response(HttpStatusCode.OK), + Response(HttpStatusCode.OK)); + NtlmCredentialValidator validator = CreateValidator(handler); + + NtlmValidationResult result = await validator.ValidateAsync( + Student, + "test-password", + TestContext.Current.CancellationToken); + + Assert.Equal(NtlmValidationStatus.Valid, result.Status); + Assert.Equal(3, handler.RequestPaths.Count); + Assert.Equal("/psulsa/", handler.RequestPaths[0]); + Assert.Equal("/psulsa/", handler.RequestPaths[1]); + Assert.Equal( + "/psulsa/alumnos/consultainformacionalumnos/consultainformacion.aspx", + handler.RequestPaths[2]); + } + + [Fact] + public async Task RejectedCredentialsAfterChallengeAreInvalid() + { + SequenceHandler handler = new( + Challenge(), + Challenge()); + NtlmCredentialValidator validator = CreateValidator(handler); + + NtlmValidationResult result = await validator.ValidateAsync( + Student, + "wrong-password", + TestContext.Current.CancellationToken); + + Assert.Equal(NtlmValidationStatus.Invalid, result.Status); + Assert.Equal("INVALID_INSTITUTIONAL_CREDENTIALS", result.ErrorCode); + Assert.Equal(2, handler.RequestPaths.Count); + } + + [Fact] + public async Task SessionBootstrapRetriesOriginalRoleProfile() + { + SequenceHandler handler = new( + Challenge(), + Redirect("/psulsa/login.aspx?AspxAutoDetectCookieSupport=1"), + Redirect("/psulsa/menu.aspx"), + Response(HttpStatusCode.OK), + Response(HttpStatusCode.OK)); + NtlmCredentialValidator validator = CreateValidator(handler); + + NtlmValidationResult result = await validator.ValidateAsync( + Student, + "test-password", + TestContext.Current.CancellationToken); + + Assert.Equal(NtlmValidationStatus.Valid, result.Status); + Assert.Equal(5, handler.RequestPaths.Count); + Assert.Equal("/psulsa/login.aspx", handler.RequestPaths[2]); + Assert.Equal("/psulsa/menu.aspx", handler.RequestPaths[3]); + Assert.Equal( + "/psulsa/alumnos/consultainformacionalumnos/consultainformacion.aspx", + handler.RequestPaths[4]); + } + + private static NtlmCredentialValidator CreateValidator(SequenceHandler handler) + { + BrokerOptions options = new() + { + Ntlm = new NtlmOptions + { + Endpoint = "https://sgu.example/", + AuthenticationPath = "/psulsa/", + StudentProfilePath = "/psulsa/alumnos/consultainformacionalumnos/consultainformacion.aspx", + AllowedRedirectHosts = ["sgu.example"], + TimeoutSeconds = 5, + ProfileTimeoutSeconds = 5 + } + }; + return new NtlmCredentialValidator( + options, + NullLogger.Instance, + (_, _) => handler); + } + + private static HttpResponseMessage Challenge() + { + HttpResponseMessage response = Response(HttpStatusCode.Unauthorized); + response.Headers.WwwAuthenticate.Add(new AuthenticationHeaderValue("Negotiate")); + response.Headers.WwwAuthenticate.Add(new AuthenticationHeaderValue("NTLM")); + return response; + } + + private static HttpResponseMessage Redirect(string location) + { + HttpResponseMessage response = Response(HttpStatusCode.Found); + response.Headers.Location = new Uri(location, UriKind.Relative); + return response; + } + + private static HttpResponseMessage Response(HttpStatusCode statusCode) => + new(statusCode) + { + Content = new StringContent("") + }; + + private sealed class SequenceHandler(params HttpResponseMessage[] responses) : HttpMessageHandler + { + private readonly Queue responses = new(responses); + + public List RequestPaths { get; } = []; + + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + RequestPaths.Add(request.RequestUri!.AbsolutePath); + if (responses.Count == 0) + { + throw new InvalidOperationException("The validator sent more requests than expected."); + } + + HttpResponseMessage response = responses.Dequeue(); + response.RequestMessage = request; + return Task.FromResult(response); + } + } +} diff --git a/tests/SGU.AuthBroker.Tests/SGU.AuthBroker.Tests.csproj b/tests/SGU.AuthBroker.Tests/SGU.AuthBroker.Tests.csproj new file mode 100644 index 0000000..9de5070 --- /dev/null +++ b/tests/SGU.AuthBroker.Tests/SGU.AuthBroker.Tests.csproj @@ -0,0 +1,18 @@ + + + net10.0-windows + win-x64 + false + true + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + diff --git a/tests/SGU.CredentialProvider.Tests/BrokerClientTests.cs b/tests/SGU.CredentialProvider.Tests/BrokerClientTests.cs index 4b04235..48dc064 100644 --- a/tests/SGU.CredentialProvider.Tests/BrokerClientTests.cs +++ b/tests/SGU.CredentialProvider.Tests/BrokerClientTests.cs @@ -10,7 +10,7 @@ public sealed class BrokerClientTests [Fact] public void DefaultClientTimeoutLeavesMarginForThePortalAndBroker() { - Assert.Equal(35, new ProviderSettings().TimeoutSeconds); + Assert.Equal(90, new ProviderSettings().TimeoutSeconds); } [Fact] diff --git a/tests/SGU.CredentialProvider.Tests/ProviderTileIconTests.cs b/tests/SGU.CredentialProvider.Tests/ProviderTileIconTests.cs index 0bbf5b6..b5c8179 100644 --- a/tests/SGU.CredentialProvider.Tests/ProviderTileIconTests.cs +++ b/tests/SGU.CredentialProvider.Tests/ProviderTileIconTests.cs @@ -16,7 +16,13 @@ public sealed class ProviderTileIconTests Assert.Equal(ProviderTileIcon.Size, logo.Bitmap.Width); Assert.Equal(ProviderTileIcon.Size, logo.Bitmap.Height); - Assert.Equal(Color.FromArgb(0, 83, 155).ToArgb(), logo.Bitmap.GetPixel(0, 0).ToArgb()); + Assert.Equal(0, logo.Bitmap.GetPixel(0, 0).A); + Assert.Equal(0, logo.Bitmap.GetPixel(ProviderTileIcon.Size - 1, 0).A); + Assert.Equal(0, logo.Bitmap.GetPixel(0, ProviderTileIcon.Size - 1).A); + Assert.Equal(0, logo.Bitmap.GetPixel(ProviderTileIcon.Size - 1, ProviderTileIcon.Size - 1).A); + Assert.Equal( + Color.FromArgb(0, 83, 155).ToArgb(), + logo.Bitmap.GetPixel(6, ProviderTileIcon.Size / 2).ToArgb()); int lightPixels = 0; for (int x = 0; x < logo.Bitmap.Width; x++) {