diff --git a/test/Classes/HomeControllerCasUrlTests.cs b/test/Classes/HomeControllerCasUrlTests.cs new file mode 100644 index 000000000..bcb823d39 --- /dev/null +++ b/test/Classes/HomeControllerCasUrlTests.cs @@ -0,0 +1,148 @@ +using System.Net; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Options; +using NSubstitute; +using Viper.Classes; +using Viper.Classes.SQLContext; +using Viper.Controllers; +using Web.Authorization; + +namespace Viper.test.Classes; + +/// +/// CAS service callbacks must be built from the configured canonical origin, never from the +/// request Host. Login covers the shared BuildRedirectUri helper that CasLogin's ticket +/// validation also uses. +/// +public class HomeControllerCasUrlTests +{ + private const string CasBaseUrl = "https://ssodev.ucdavis.edu/cas/"; + private const string PublicBaseUrl = "https://secure-test.vetmed.ucdavis.edu/2"; + private const string ForgedHost = "attacker.example"; + + [Fact] + public void Login_BuildsServiceFromConfiguredOrigin_NotHostHeader() + { + var controller = CreateController(ForgedHost, pathBase: "/2"); + + var result = Assert.IsType(controller.Login()); + + Assert.DoesNotContain(ForgedHost, result.Url, StringComparison.OrdinalIgnoreCase); + Assert.StartsWith($"{PublicBaseUrl}/CasLogin?", ServiceParameter(result.Url), StringComparison.Ordinal); + } + + [Fact] + public void Login_DefaultReturnUrl_PreservesPathBase() + { + var controller = CreateController(ForgedHost, pathBase: "/2"); + + var result = Assert.IsType(controller.Login()); + + // ReturnUrl is encoded inside the service value, which is then encoded again for CAS, + // so one decode leaves the inner encoding intact. + Assert.Equal($"{PublicBaseUrl}/CasLogin?ReturnUrl={WebUtility.UrlEncode("/2")}", ServiceParameter(result.Url)); + } + + [Fact] + public void Login_NoPathBase_DefaultsToEmptyReturnUrl() + { + var controller = CreateController("localhost:7157", pathBase: string.Empty); + + var result = Assert.IsType(controller.Login()); + + Assert.Equal($"{PublicBaseUrl}/CasLogin?ReturnUrl=", ServiceParameter(result.Url)); + } + + [Fact] + public void Login_ExplicitReturnUrl_IsPreserved() + { + var controller = CreateController(ForgedHost, pathBase: "/2"); + + var result = Assert.IsType(controller.Login("/2/Students/StudentClassYear")); + + Assert.Equal( + $"{PublicBaseUrl}/CasLogin?ReturnUrl={WebUtility.UrlEncode("/2/Students/StudentClassYear")}", + ServiceParameter(result.Url)); + } + + [Fact] + public void Login_ApiReturnUrlUnderPathBase_ReturnsUnauthorized() + { + // The SPAs send ReturnUrl already prefixed with the deployed PathBase, so without + // stripping it the API guard never fired on TEST/PROD and an API caller got a CAS + // HTML redirect instead of a 401. + var controller = CreateController("secure-test.vetmed.ucdavis.edu", pathBase: "/2"); + + Assert.IsType(controller.Login("/2/api/students/dvm")); + } + + [Fact] + public void Login_ApiReturnUrlWithoutPathBase_ReturnsUnauthorized() + { + var controller = CreateController("localhost:7157", pathBase: string.Empty); + + Assert.IsType(controller.Login("/api/students/dvm")); + } + + [Fact] + public async Task Logout_BuildsServiceFromConfiguredOrigin_NotHostHeader() + { + var controller = CreateController(ForgedHost, pathBase: "/2"); + + var result = Assert.IsType(await controller.Logout()); + + Assert.DoesNotContain(ForgedHost, result.Url, StringComparison.OrdinalIgnoreCase); + Assert.Equal($"{CasBaseUrl}logout?service={WebUtility.UrlEncode(PublicBaseUrl)}", result.Url); + } + + /// + /// Pulls the decoded CAS service parameter out of the redirect so assertions read as URLs + /// rather than percent-encoded soup. + /// + private static string ServiceParameter(string redirectUrl) + { + const string marker = "service="; + int start = redirectUrl.IndexOf(marker, StringComparison.Ordinal); + Assert.True(start >= 0, $"No service parameter in '{redirectUrl}'."); + + return WebUtility.UrlDecode(redirectUrl[(start + marker.Length)..]); + } + + private static HomeController CreateController(string host, string pathBase) + { + var publicUrl = new PublicUrlService( + Options.Create(new PublicUrlOptions { PublicBaseUrl = PublicBaseUrl }), + Substitute.For()); + + var controller = new HomeController( + Substitute.For(), + Options.Create(new CasSettings { CasBaseUrl = CasBaseUrl }), + publicUrl, + Substitute.For(), + Substitute.For(), + Substitute.For()); + + var httpContext = new DefaultHttpContext + { + RequestServices = AuthenticationServices() + }; + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString(host); + httpContext.Request.PathBase = new PathString(pathBase); + httpContext.Request.Path = new PathString("/Login"); + + controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + return controller; + } + + // Logout signs the cookie out, which resolves IAuthenticationService from the request. + private static IServiceProvider AuthenticationServices() + { + var authentication = Substitute.For(); + var services = Substitute.For(); + services.GetService(typeof(IAuthenticationService)).Returns(authentication); + return services; + } +} diff --git a/test/Classes/PublicUrlServiceTests.cs b/test/Classes/PublicUrlServiceTests.cs new file mode 100644 index 000000000..c964244ac --- /dev/null +++ b/test/Classes/PublicUrlServiceTests.cs @@ -0,0 +1,179 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Viper.Classes; + +namespace Viper.test.Classes; + +/// +/// The canonical public origin must come from configuration in deployed environments so a +/// forged Host header cannot influence a CAS callback. Development keeps the request-derived +/// fallback because the local port is dynamic. +/// +public class PublicUrlServiceTests +{ + private const string TestBaseUrl = "https://secure-test.vetmed.ucdavis.edu/2"; + private const string ProductionBaseUrl = "https://viper.vetmed.ucdavis.edu/2"; + + [Fact] + public void BaseUrl_ConfiguredOriginWins_OverForgedHostHeader() + { + var service = CreateService(TestBaseUrl, host: "attacker.example", pathBase: "/2"); + + Assert.Equal(TestBaseUrl, service.BaseUrl); + } + + [Fact] + public void BuildUrl_ConfiguredOriginWins_OverForgedHostHeader() + { + var service = CreateService(ProductionBaseUrl, host: "attacker.example", pathBase: "/2"); + + Assert.Equal($"{ProductionBaseUrl}/CasLogin", service.BuildUrl("/CasLogin")); + Assert.DoesNotContain("attacker.example", service.BuildUrl("/CasLogin"), StringComparison.OrdinalIgnoreCase); + } + + [Theory] + [InlineData("https://viper.vetmed.ucdavis.edu/2/", "https://viper.vetmed.ucdavis.edu/2")] + [InlineData(" https://viper.vetmed.ucdavis.edu/2 ", "https://viper.vetmed.ucdavis.edu/2")] + [InlineData("https://viper.vetmed.ucdavis.edu/", "https://viper.vetmed.ucdavis.edu")] + public void NormalizeBaseUrl_TrimsWhitespaceAndTrailingSlash(string configured, string expected) + { + Assert.Equal(expected, PublicUrlService.NormalizeBaseUrl(configured)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void NormalizeBaseUrl_BlankIsNull(string? configured) + { + Assert.Null(PublicUrlService.NormalizeBaseUrl(configured)); + } + + [Fact] + public void BuildUrl_AddsSeparator_WhenPathHasNoLeadingSlash() + { + var service = CreateService(TestBaseUrl, host: "secure-test.vetmed.ucdavis.edu", pathBase: "/2"); + + Assert.Equal($"{TestBaseUrl}/CasLogin", service.BuildUrl("CasLogin")); + } + + [Fact] + public void BuildUrl_EmptyPath_ReturnsBaseUrl() + { + var service = CreateService(TestBaseUrl, host: "secure-test.vetmed.ucdavis.edu", pathBase: "/2"); + + Assert.Equal(TestBaseUrl, service.BuildUrl(string.Empty)); + } + + [Fact] + public void BaseUrl_Unconfigured_FallsBackToRequestIncludingPathBase() + { + // Development only: no PublicBaseUrl set, so the origin comes from the request. + var service = CreateService(configured: null, host: "localhost:7157", pathBase: "/2"); + + Assert.Equal("https://localhost:7157/2", service.BaseUrl); + } + + [Fact] + public void BaseUrl_Unconfigured_NoPathBase_ReturnsOriginOnly() + { + var service = CreateService(configured: null, host: "localhost:7157", pathBase: string.Empty); + + Assert.Equal("https://localhost:7157", service.BaseUrl); + } + + [Fact] + public void BaseUrl_Unconfigured_NoRequest_FallsBackToLocalDevelopmentOrigin() + { + // Development background work (Hangfire email) has no request to derive from. Deployed + // environments never reach this because startup validation requires the configured value. + var accessor = Substitute.For(); + accessor.HttpContext.ReturnsNull(); + var service = new PublicUrlService(Options.Create(new PublicUrlOptions()), accessor); + + string expectedPort = Environment.GetEnvironmentVariable("ASPNETCORE_HTTPS_PORT") ?? "7157"; + + Assert.Equal($"https://localhost:{expectedPort}", service.BaseUrl); + } + + [Fact] + public void BaseUrl_Configured_NoRequest_StillUsesTheCanonicalOrigin() + { + // The email path must not pick up the local development origin in a deployed environment. + var accessor = Substitute.For(); + accessor.HttpContext.ReturnsNull(); + var service = new PublicUrlService(Options.Create(new PublicUrlOptions { PublicBaseUrl = ProductionBaseUrl }), accessor); + + Assert.Equal(ProductionBaseUrl, service.BaseUrl); + } + + #region Startup validation + + [Theory] + [InlineData(TestBaseUrl)] + [InlineData(ProductionBaseUrl)] + [InlineData("https://viper.vetmed.ucdavis.edu")] + public void Validate_AcceptsCanonicalDeployedUrls(string configured) + { + Assert.True(PublicUrlOptionsValidator.ValidateBaseUrl(configured, isDevelopment: false).Succeeded); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void Validate_MissingOutsideDevelopment_FailsStartup(string? configured) + { + var result = PublicUrlOptionsValidator.ValidateBaseUrl(configured, isDevelopment: false); + + Assert.True(result.Failed); + Assert.Contains("Application:PublicBaseUrl", result.FailureMessage, StringComparison.Ordinal); + } + + [Fact] + public void Validate_MissingInDevelopment_Succeeds() + { + // Development derives the origin from the request so dynamic local ports keep working. + Assert.True(PublicUrlOptionsValidator.ValidateBaseUrl(null, isDevelopment: true).Succeeded); + } + + [Fact] + public void Validate_HttpOutsideDevelopment_Fails() + { + Assert.True(PublicUrlOptionsValidator.ValidateBaseUrl("http://viper.vetmed.ucdavis.edu/2", isDevelopment: false).Failed); + } + + [Fact] + public void Validate_HttpInDevelopment_Succeeds() + { + Assert.True(PublicUrlOptionsValidator.ValidateBaseUrl("http://localhost:5000", isDevelopment: true).Succeeded); + } + + [Theory] + [InlineData("/2")] + [InlineData("viper.vetmed.ucdavis.edu/2")] + [InlineData("https://user:pass@viper.vetmed.ucdavis.edu/2")] + [InlineData("https://viper.vetmed.ucdavis.edu/2?next=x")] + [InlineData("https://viper.vetmed.ucdavis.edu/2#frag")] + public void Validate_RejectsMalformedOrUnsafeValues(string configured) + { + Assert.True(PublicUrlOptionsValidator.ValidateBaseUrl(configured, isDevelopment: false).Failed); + } + + #endregion + + private static PublicUrlService CreateService(string? configured, string host, string pathBase) + { + var context = new DefaultHttpContext(); + context.Request.Scheme = "https"; + context.Request.Host = new HostString(host); + context.Request.PathBase = new PathString(pathBase); + context.Request.Path = new PathString("/CasLogin"); + + var accessor = Substitute.For(); + accessor.HttpContext.Returns(context); + + return new PublicUrlService(Options.Create(new PublicUrlOptions { PublicBaseUrl = configured }), accessor); + } +} diff --git a/test/ClinicalScheduler/EmailNotificationTest.cs b/test/ClinicalScheduler/EmailNotificationTest.cs index 7dd97c420..06baafbe4 100644 --- a/test/ClinicalScheduler/EmailNotificationTest.cs +++ b/test/ClinicalScheduler/EmailNotificationTest.cs @@ -6,6 +6,7 @@ using NSubstitute.ExceptionExtensions; using Viper.Areas.ClinicalScheduler.EmailTemplates.Models; using Viper.Areas.ClinicalScheduler.Services; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.EmailTemplates.Services; using Viper.Models.ClinicalScheduler; @@ -79,8 +80,8 @@ public EmailNotificationTest() .Returns(currentYear); // Setup email settings - var mockEmailSettingsOptions = Substitute.For>(); - mockEmailSettingsOptions.Value.Returns(new EmailSettings { BaseUrl = "https://test.example.com" }); + var mockPublicUrl = Substitute.For(); + mockPublicUrl.BaseUrl.Returns("https://test.example.com"); // Setup audit service _mockAuditService.LogInstructorRemovedAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) @@ -92,7 +93,7 @@ public EmailNotificationTest() _mockLogger, _mockEmailService, _mockEmailNotificationOptions, - mockEmailSettingsOptions, + mockPublicUrl, _mockGradYearService, _mockPermissionValidator, _mockEmailTemplateRenderer); @@ -560,8 +561,8 @@ public async Task RemoveInstructorScheduleAsync_MultipleEmailRecipients_SendsToA } }; _mockEmailNotificationOptions.Value.Returns(emailNotificationSettings); - var mockEmailSettingsOptions = Substitute.For>(); - mockEmailSettingsOptions.Value.Returns(new EmailSettings { BaseUrl = "https://test.example.com" }); + var mockPublicUrl = Substitute.For(); + mockPublicUrl.BaseUrl.Returns("https://test.example.com"); // Create a new service instance with the updated configuration var serviceWithMultipleRecipients = new TestableScheduleEditService( @@ -570,7 +571,7 @@ public async Task RemoveInstructorScheduleAsync_MultipleEmailRecipients_SendsToA _mockLogger, _mockEmailService, _mockEmailNotificationOptions, - mockEmailSettingsOptions, + mockPublicUrl, _mockGradYearService, _mockPermissionValidator, _mockEmailTemplateRenderer); diff --git a/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs b/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs index 9bb83e89c..d7bf6692d 100644 --- a/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs +++ b/test/ClinicalScheduler/Integration/ControllerServiceIntegrationTest.cs @@ -14,6 +14,8 @@ using Viper.Services; using CS = Viper.Models.ClinicalScheduler; +using Viper.Classes; + namespace Viper.test.ClinicalScheduler.Integration { /// @@ -55,8 +57,8 @@ public ControllerServiceIntegrationTest() var mockEmailService = Substitute.For(); var mockEmailNotificationSettings = Substitute.For>(); mockEmailNotificationSettings.Value.Returns(new EmailNotificationSettings()); - var mockEmailSettings = Substitute.For>(); - mockEmailSettings.Value.Returns(new EmailSettings()); + var mockPublicUrl = Substitute.For(); + mockPublicUrl.BaseUrl.Returns("https://test.example.com"); var mockGradYearService = Substitute.For(); var mockPermissionValidator = Substitute.For(); var mockEmailTemplateRenderer = Substitute.For(); @@ -67,7 +69,7 @@ public ControllerServiceIntegrationTest() scheduleEditLogger, mockEmailService, mockEmailNotificationSettings, - mockEmailSettings, + mockPublicUrl, mockGradYearService, mockPermissionValidator, mockEmailTemplateRenderer); diff --git a/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs b/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs index 7d08884f6..dd3a09fc5 100644 --- a/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs +++ b/test/ClinicalScheduler/ScheduleEditServiceRollbackTest.cs @@ -5,6 +5,7 @@ using NSubstitute; using NSubstitute.ExceptionExtensions; using Viper.Areas.ClinicalScheduler.Services; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.EmailTemplates.Services; using Viper.Services; @@ -56,8 +57,8 @@ public ScheduleEditServiceRollbackTest() var emailNotificationOptions = Substitute.For>(); emailNotificationOptions.Value.Returns(new EmailNotificationSettings()); - var emailSettingsOptions = Substitute.For>(); - emailSettingsOptions.Value.Returns(new EmailSettings()); + var publicUrl = Substitute.For(); + publicUrl.BaseUrl.Returns("https://test.example.com"); _service = new ScheduleEditService( _context, @@ -65,7 +66,7 @@ public ScheduleEditServiceRollbackTest() Substitute.For>(), Substitute.For(), emailNotificationOptions, - emailSettingsOptions, + publicUrl, gradYearService, permissionValidator, Substitute.For()); diff --git a/test/ClinicalScheduler/ScheduleEditServiceTest.cs b/test/ClinicalScheduler/ScheduleEditServiceTest.cs index a05887886..08488427d 100644 --- a/test/ClinicalScheduler/ScheduleEditServiceTest.cs +++ b/test/ClinicalScheduler/ScheduleEditServiceTest.cs @@ -6,6 +6,7 @@ using NSubstitute.ExceptionExtensions; using Viper.Areas.ClinicalScheduler.EmailTemplates.Models; using Viper.Areas.ClinicalScheduler.Services; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.EmailTemplates.Services; using Viper.Models.ClinicalScheduler; @@ -96,8 +97,8 @@ public ScheduleEditServiceTest() SeedTestData(); // Setup email settings - var mockEmailSettingsOptions = Substitute.For>(); - mockEmailSettingsOptions.Value.Returns(new EmailSettings { BaseUrl = "https://test.example.com" }); + var mockPublicUrl = Substitute.For(); + mockPublicUrl.BaseUrl.Returns("https://test.example.com"); _service = new TestableScheduleEditService( _context, @@ -105,7 +106,7 @@ public ScheduleEditServiceTest() _mockLogger, _mockEmailService, _mockEmailNotificationOptions, - mockEmailSettingsOptions, + mockPublicUrl, _mockGradYearService, _mockPermissionValidator, _mockEmailTemplateRenderer); diff --git a/test/ClinicalScheduler/TestableScheduleEditService.cs b/test/ClinicalScheduler/TestableScheduleEditService.cs index 51a2443fc..b79b65152 100644 --- a/test/ClinicalScheduler/TestableScheduleEditService.cs +++ b/test/ClinicalScheduler/TestableScheduleEditService.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Viper.Areas.ClinicalScheduler.Services; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.EmailTemplates.Services; using Viper.Services; @@ -19,11 +20,11 @@ public TestableScheduleEditService( ILogger logger, IEmailService emailService, IOptions emailNotificationOptions, - IOptions emailSettingsOptions, + IPublicUrlService publicUrl, IGradYearService gradYearService, IPermissionValidator permissionValidator, IEmailTemplateRenderer emailTemplateRenderer) - : base(context, auditService, logger, emailService, emailNotificationOptions, emailSettingsOptions, gradYearService, permissionValidator, emailTemplateRenderer) + : base(context, auditService, logger, emailService, emailNotificationOptions, publicUrl, gradYearService, permissionValidator, emailTemplateRenderer) { } diff --git a/test/Effort/VerificationServiceTests.cs b/test/Effort/VerificationServiceTests.cs index 3d6273af7..18b79ea58 100644 --- a/test/Effort/VerificationServiceTests.cs +++ b/test/Effort/VerificationServiceTests.cs @@ -10,6 +10,7 @@ using Viper.Areas.Effort.Models.DTOs.Responses; using Viper.Areas.Effort.Models.Entities; using Viper.Areas.Effort.Services; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.EmailTemplates.Services; using Viper.Models.VIPER; @@ -66,11 +67,8 @@ public VerificationServiceTests() }; var settingsOptions = Options.Create(_settings); - var emailSettings = new EmailSettings - { - BaseUrl = "https://test.example.com" - }; - var emailSettingsOptions = Options.Create(emailSettings); + var publicUrl = Substitute.For(); + publicUrl.BaseUrl.Returns("https://test.example.com"); _emailTemplateRendererMock = Substitute.For(); _emailTemplateRendererMock @@ -102,7 +100,7 @@ public VerificationServiceTests() _classificationServiceMock, _loggerMock, settingsOptions, - emailSettingsOptions, + publicUrl, _emailTemplateRendererMock); SeedTestData(); @@ -670,10 +668,9 @@ public async Task SendVerificationEmailAsync_ReturnsError_WhenBaseUrlNotConfigur VerificationEmailSubject = "Please Verify Your Effort", VerificationReplyDays = 7 }; - var badEmailSettings = new EmailSettings - { - BaseUrl = "" // Missing/empty BaseUrl - }; + // Missing canonical origin: Application:PublicBaseUrl unset + var badPublicUrl = Substitute.For(); + badPublicUrl.BaseUrl.Returns(string.Empty); var serviceWithBadConfig = new VerificationService( _context, @@ -685,7 +682,7 @@ public async Task SendVerificationEmailAsync_ReturnsError_WhenBaseUrlNotConfigur _classificationServiceMock, _loggerMock, Options.Create(badSettings), - Options.Create(badEmailSettings), + badPublicUrl, _emailTemplateRendererMock); _permissionServiceMock.GetCurrentUserEmail().Returns("sender@ucdavis.edu"); diff --git a/web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs b/web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs index 9f9643b3c..b7d8ea606 100644 --- a/web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs +++ b/web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs @@ -3,6 +3,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using Viper.Areas.ClinicalScheduler.EmailTemplates.Models; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.Classes.Utilities; using Viper.EmailTemplates.Services; @@ -21,7 +22,7 @@ public class ScheduleEditService : IScheduleEditService private readonly ILogger _logger; private readonly IEmailService _emailService; private readonly EmailNotificationSettings _emailNotificationSettings; - private readonly EmailSettings _emailSettings; + private readonly IPublicUrlService _publicUrl; private readonly IGradYearService _gradYearService; private readonly IPermissionValidator _permissionValidator; private readonly IEmailTemplateRenderer _emailTemplateRenderer; @@ -32,7 +33,7 @@ public ScheduleEditService( ILogger logger, IEmailService emailService, IOptions emailNotificationOptions, - IOptions emailSettingsOptions, + IPublicUrlService publicUrl, IGradYearService gradYearService, IPermissionValidator permissionValidator, IEmailTemplateRenderer emailTemplateRenderer) @@ -42,7 +43,7 @@ public ScheduleEditService( _logger = logger; _emailService = emailService; _emailNotificationSettings = emailNotificationOptions.Value; - _emailSettings = emailSettingsOptions.Value; + _publicUrl = publicUrl; _gradYearService = gradYearService; _permissionValidator = permissionValidator; _emailTemplateRenderer = emailTemplateRenderer; @@ -656,7 +657,7 @@ private async Task SendPrimaryEvaluatorRemovedNotificationAsync(InstructorSchedu return; } // Get base URL for links - var baseUrl = string.IsNullOrWhiteSpace(_emailSettings.BaseUrl) ? null : _emailSettings.BaseUrl; + var baseUrl = string.IsNullOrWhiteSpace(_publicUrl.BaseUrl) ? null : _publicUrl.BaseUrl; // Get instructor information var instructorName = "Unknown Instructor"; diff --git a/web/Areas/Effort/Services/VerificationService.cs b/web/Areas/Effort/Services/VerificationService.cs index 5ab438658..12ad6aba0 100644 --- a/web/Areas/Effort/Services/VerificationService.cs +++ b/web/Areas/Effort/Services/VerificationService.cs @@ -7,6 +7,7 @@ using Viper.Areas.Effort.Models; using Viper.Areas.Effort.Models.DTOs.Responses; using Viper.Areas.Effort.Models.Entities; +using Viper.Classes; using Viper.Classes.SQLContext; using Viper.Classes.Utilities; using Viper.EmailTemplates.Services; @@ -29,7 +30,7 @@ public class VerificationService : IVerificationService private readonly ICourseClassificationService _classificationService; private readonly ILogger _logger; private readonly EffortSettings _settings; - private readonly EmailSettings _emailSettings; + private readonly IPublicUrlService _publicUrl; private readonly IEmailTemplateRenderer _emailTemplateRenderer; public VerificationService( @@ -42,7 +43,7 @@ public VerificationService( ICourseClassificationService classificationService, ILogger logger, IOptions settings, - IOptions emailSettings, + IPublicUrlService publicUrl, IEmailTemplateRenderer emailTemplateRenderer) { _context = context; @@ -54,7 +55,7 @@ public VerificationService( _classificationService = classificationService; _logger = logger; _settings = settings.Value; - _emailSettings = emailSettings.Value; + _publicUrl = publicUrl; _emailTemplateRenderer = emailTemplateRenderer; } @@ -719,15 +720,15 @@ private async Task> GetCourseRelationshipsAsync( private string BuildVerificationUrl(int termCode) { // Require configured base URL to avoid Host header injection - if (string.IsNullOrWhiteSpace(_emailSettings.BaseUrl)) + if (string.IsNullOrWhiteSpace(_publicUrl.BaseUrl)) { - throw new InvalidOperationException("EmailSettings:BaseUrl must be configured for verification emails."); + throw new InvalidOperationException("Application:PublicBaseUrl must be configured for verification emails."); } - var baseUrlNormalized = _emailSettings.BaseUrl.TrimEnd('/') + "/"; + var baseUrlNormalized = _publicUrl.BaseUrl.TrimEnd('/') + "/"; if (!Uri.TryCreate(baseUrlNormalized, UriKind.Absolute, out var baseUri)) { - throw new InvalidOperationException($"EmailSettings:BaseUrl value '{_emailSettings.BaseUrl}' is not a valid absolute URL."); + throw new InvalidOperationException($"Configured public base URL '{_publicUrl.BaseUrl}' is not a valid absolute URL."); } return new Uri(baseUri, $"Effort/{termCode}/my-effort").ToString(); @@ -846,7 +847,7 @@ private VerificationReminderViewModel BuildVerificationEmailViewModel( return new VerificationReminderViewModel { - BaseUrl = _emailSettings.BaseUrl ?? "", + BaseUrl = _publicUrl.BaseUrl, TermDescription = termDescription, TermStartDate = termStartDate, TermEndDate = termEndDate, diff --git a/web/Classes/HealthChecks/HealthCheckExtensions.cs b/web/Classes/HealthChecks/HealthCheckExtensions.cs index d11df6c7d..1ed3767b7 100644 --- a/web/Classes/HealthChecks/HealthCheckExtensions.cs +++ b/web/Classes/HealthChecks/HealthCheckExtensions.cs @@ -232,8 +232,8 @@ public static IServiceCollection AddViperHealthChecks( // UseApiEndpointDelegatingHandler below) so the endpoint filter // can recognize the self-call without widening the IP allowlist // to cover whatever NAT'd source IP the loop-out produces. - // Dev has no BaseUrl configured, so fall back to a relative URL. - var baseUrl = configuration["EmailSettings:BaseUrl"]?.TrimEnd('/'); + // Dev leaves the canonical origin unset, so fall back to a relative URL. + var baseUrl = configuration["Application:PublicBaseUrl"]?.TrimEnd('/'); var healthEndpointUrl = string.IsNullOrWhiteSpace(baseUrl) ? "/health/detail" : $"{baseUrl}/health/detail"; diff --git a/web/Classes/HttpHelper.cs b/web/Classes/HttpHelper.cs index ffde52032..987f9d618 100644 --- a/web/Classes/HttpHelper.cs +++ b/web/Classes/HttpHelper.cs @@ -1,9 +1,9 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; -using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Caching.Memory; using NLog; +using Viper.Classes; namespace Viper { @@ -16,18 +16,20 @@ public static class HttpHelper private static IHttpContextAccessor? httpContextAccessor; private static IAuthorizationService? authorizationService; private static IDataProtectionProvider? dataProtectionProvider; + private static IPublicUrlService? publicUrlService; /// - /// Configures the helper with system-wide services (memory cache, configuration, environment, context accessor, authorization, data protection) + /// Configures the helper with system-wide services (memory cache, configuration, environment, context accessor, authorization, data protection, public URL) /// - public static void Configure(IMemoryCache? memoryCache, IConfiguration? configurationSettings, IWebHostEnvironment env, IHttpContextAccessor? httpContextAccessor, IAuthorizationService? authorizationService, IDataProtectionProvider? dataProtectionProvider) + public static void Configure(IMemoryCache? memoryCache, IConfiguration? configurationSettings, IWebHostEnvironment env, IHttpContextAccessor? contextAccessor, IAuthorizationService? authService, IDataProtectionProvider? dataProtection, IPublicUrlService? publicUrl = null) { Cache = memoryCache; Settings = configurationSettings; Environment = env; - HttpHelper.httpContextAccessor = httpContextAccessor; - HttpHelper.authorizationService = authorizationService; - HttpHelper.dataProtectionProvider = dataProtectionProvider; + httpContextAccessor = contextAccessor; + authorizationService = authService; + dataProtectionProvider = dataProtection; + publicUrlService = publicUrl; } /// @@ -77,27 +79,15 @@ public static HttpContext? HttpContext public static IDataProtectionProvider? DataProtectionProvider { get { return dataProtectionProvider; } } /// - /// Gets the root URL including protocol and port for Viper.Net + /// Gets the root URL including protocol and port for Viper.Net. Deployed environments + /// return the configured canonical origin (Application:PublicBaseUrl); Development + /// derives it from the request. See . /// public static string GetRootURL() { - string rootURL = String.Empty; - - HttpRequest? thisRequest = httpContextAccessor?.HttpContext?.Request; - - if (thisRequest != null) - { - Uri url = new(thisRequest.GetDisplayUrl()); - rootURL = url.GetLeftPart(UriPartial.Authority); - - if (url.AbsolutePath.StartsWith("/2/")) - { - rootURL += "/2"; - } - - } - - return rootURL ?? String.Empty; + return publicUrlService != null + ? publicUrlService.BaseUrl + : PublicUrlService.FromRequest(httpContextAccessor?.HttpContext?.Request); } /// /// Gets the root URL for ColdFusion Viper based off the enviroment diff --git a/web/Classes/PublicUrlService.cs b/web/Classes/PublicUrlService.cs new file mode 100644 index 000000000..7c2af55eb --- /dev/null +++ b/web/Classes/PublicUrlService.cs @@ -0,0 +1,176 @@ +using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.Extensions.Options; + +namespace Viper.Classes +{ + /// + /// Canonical public origin for this deployment, bound from the "Application" configuration + /// section. Deployed environments must set it; Development derives the origin from the + /// request so the dynamic local port keeps working. + /// + public class PublicUrlOptions + { + public const string SectionName = "Application"; + + /// + /// Absolute base URL including scheme, host, optional port and PathBase, e.g. + /// "https://viper.vetmed.ucdavis.edu/2". + /// + public string? PublicBaseUrl { get; set; } + } + + /// + /// Supplies the origin for URLs that leave the application (CAS service callbacks, sitemap + /// entries, emulation links). Deployed environments read it from configuration so a forged + /// Host header cannot influence a security callback. + /// + public interface IPublicUrlService + { + /// + /// Canonical base URL with no trailing slash, e.g. "https://viper.vetmed.ucdavis.edu/2". + /// + string BaseUrl { get; } + + /// + /// Canonical base URL plus an application-relative path, e.g. BuildUrl("/CasLogin"). + /// + string BuildUrl(string relativePath); + } + + /// + public class PublicUrlService : IPublicUrlService + { + private readonly string? _configuredBaseUrl; + private readonly IHttpContextAccessor _httpContextAccessor; + + public PublicUrlService(IOptions options, IHttpContextAccessor httpContextAccessor) + { + _configuredBaseUrl = NormalizeBaseUrl(options.Value.PublicBaseUrl); + _httpContextAccessor = httpContextAccessor; + } + + public string BaseUrl + { + get + { + if (_configuredBaseUrl != null) + { + return _configuredBaseUrl; + } + + string fromRequest = FromRequest(_httpContextAccessor.HttpContext?.Request); + return fromRequest.Length > 0 ? fromRequest : LocalDevelopmentOrigin(); + } + } + + public string BuildUrl(string relativePath) + { + if (string.IsNullOrEmpty(relativePath)) + { + return BaseUrl; + } + + return BaseUrl + (relativePath.StartsWith('/') ? relativePath : "/" + relativePath); + } + + /// + /// Trims whitespace and any trailing slash so callers can append "/Path" unconditionally. + /// Returns null when nothing is configured. + /// + public static string? NormalizeBaseUrl(string? configured) + { + return string.IsNullOrWhiteSpace(configured) ? null : configured.Trim().TrimEnd('/'); + } + + /// + /// Last resort for Development work that has no request to derive from, such as email + /// sent from a background job. Deployed environments never reach it because + /// PublicUrlOptionsValidator fails startup when the canonical origin is missing. + /// + private static string LocalDevelopmentOrigin() + { + string httpsPort = System.Environment.GetEnvironmentVariable("ASPNETCORE_HTTPS_PORT") ?? "7157"; + return int.TryParse(httpsPort, out int port) && port > 0 && port < 65536 + ? $"https://localhost:{port}" + : string.Empty; + } + + /// + /// Development fallback: derive the origin from the current request, preserving the + /// PathBase. Deployed environments never reach this because PublicUrlOptionsValidator + /// fails startup when the setting is missing. + /// + public static string FromRequest(HttpRequest? request) + { + if (request == null) + { + return string.Empty; + } + + string origin = new Uri(request.GetDisplayUrl()).GetLeftPart(UriPartial.Authority); + return origin + request.PathBase.Value?.TrimEnd('/'); + } + } + + /// + /// Fails startup when a deployed environment has no usable canonical origin, so the app + /// cannot silently fall back to request-derived URLs for CAS callbacks. + /// + public class PublicUrlOptionsValidator : IValidateOptions + { + private readonly IWebHostEnvironment _environment; + + public PublicUrlOptionsValidator(IWebHostEnvironment environment) + { + _environment = environment; + } + + public ValidateOptionsResult Validate(string? name, PublicUrlOptions options) + { + return ValidateBaseUrl(options.PublicBaseUrl, _environment.IsDevelopment()); + } + + /// + /// Exposed for tests: applies the same rules the startup validator uses. + /// + public static ValidateOptionsResult ValidateBaseUrl(string? configured, bool isDevelopment) + { + const string setting = "Application:PublicBaseUrl"; + string? normalized = PublicUrlService.NormalizeBaseUrl(configured); + + if (normalized == null) + { + return isDevelopment + ? ValidateOptionsResult.Success + : ValidateOptionsResult.Fail($"{setting} is required outside Development. Set it to the canonical public URL, for example https://viper.vetmed.ucdavis.edu/2."); + } + + if (!Uri.TryCreate(normalized, UriKind.Absolute, out Uri? uri)) + { + return ValidateOptionsResult.Fail($"{setting} must be an absolute URL."); + } + + if (uri.Scheme != Uri.UriSchemeHttps && !(isDevelopment && uri.Scheme == Uri.UriSchemeHttp)) + { + return ValidateOptionsResult.Fail($"{setting} must use https outside Development."); + } + + if (!string.IsNullOrEmpty(uri.UserInfo)) + { + return ValidateOptionsResult.Fail($"{setting} must not contain user information."); + } + + if (!string.IsNullOrEmpty(uri.Query)) + { + return ValidateOptionsResult.Fail($"{setting} must not contain a query string."); + } + + if (!string.IsNullOrEmpty(uri.Fragment)) + { + return ValidateOptionsResult.Fail($"{setting} must not contain a fragment."); + } + + return ValidateOptionsResult.Success; + } + } +} diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index 1f7d613b7..8fd4053c5 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; -using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Caching.Memory; @@ -36,13 +35,15 @@ public class HomeController : AreaController #pragma warning restore S5332 private readonly IHttpClientFactory _clientFactory; private readonly CasSettings _settings; + private readonly IPublicUrlService _publicUrl; private readonly List _casAttributesToCapture = new() { "authenticationDate", "credentialType" }; private readonly IUserHelper _userHelper; - public HomeController(IHttpClientFactory clientFactory, IOptions settingsOptions, AAUDContext aAUDContext, RAPSContext rapsContext, VIPERContext viperContext) + public HomeController(IHttpClientFactory clientFactory, IOptions settingsOptions, IPublicUrlService publicUrl, AAUDContext aAUDContext, RAPSContext rapsContext, VIPERContext viperContext) { this._clientFactory = clientFactory; this._settings = settingsOptions.Value; + this._publicUrl = publicUrl; this._aAUDContext = aAUDContext; this._rapsContext = rapsContext; this._viperContext = viperContext; @@ -94,16 +95,15 @@ private NavMenu Nav() [SearchExclude] public IActionResult Login([FromQuery] string? ReturnUrl = null) { - Uri url = new(Request.GetDisplayUrl()); - string baseURl = url.GetLeftPart(UriPartial.Authority); - string returnURL = HttpHelper.GetRootURL().Replace(baseURl, ""); + // Default to the application root under the deployed PathBase ("" locally, "/2" on TEST/PROD). + string returnURL = Request.PathBase.Value ?? string.Empty; if (!string.IsNullOrEmpty(ReturnUrl)) { returnURL = ReturnUrl; } - if (returnURL.StartsWith("/api")) + if (IsApiPath(returnURL)) { return Unauthorized(); } @@ -113,6 +113,24 @@ public IActionResult Login([FromQuery] string? ReturnUrl = null) return new RedirectResult(authorizationEndpoint); } + /// + /// The SPAs send ReturnUrl already prefixed with the deployed PathBase ("/2/api/..."), + /// so the base has to come off before testing for an API path or the guard never fires + /// on TEST/PROD and an API caller gets a CAS HTML redirect instead of a 401. + /// + private bool IsApiPath(string returnUrl) + { + string path = returnUrl; + string? basePath = Request.PathBase.Value; + + if (!string.IsNullOrEmpty(basePath) && path.StartsWith(basePath, StringComparison.OrdinalIgnoreCase)) + { + path = path[basePath.Length..]; + } + + return path.StartsWith("/api", StringComparison.OrdinalIgnoreCase); + } + [Route("/[action]")] [SearchExclude] public IActionResult RefreshSession() @@ -260,7 +278,7 @@ public async Task Logout() await HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme); // Send homepage link after CAS logout - var returnUrl = WebUtility.UrlEncode(HttpHelper.GetRootURL()); + var returnUrl = WebUtility.UrlEncode(_publicUrl.BaseUrl); return new RedirectResult(_settings.CasBaseUrl + "logout?service=" + returnUrl); } @@ -287,13 +305,14 @@ public IActionResult MyPermissions() /// - /// Utility function for creating redirect URLs + /// Utility function for creating redirect URLs. Built from the configured canonical + /// origin, never the request Host, so a forged Host cannot poison a CAS callback. /// /// /// Compiled URL - private static string BuildRedirectUri(string targetPath) + private string BuildRedirectUri(string targetPath) { - return HttpHelper.GetRootURL() + targetPath; + return _publicUrl.BuildUrl(targetPath); } /// diff --git a/web/Program.cs b/web/Program.cs index 0b80fcd7c..8a6673221 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -149,6 +149,14 @@ // Add CAS settings from appSettings configuration builder.Services.Configure(builder.Configuration.GetSection("Cas")); + // Canonical public origin for CAS callbacks and other outward-facing links. Validated on + // start so a deployed environment fails fast instead of falling back to the request Host. + builder.Services.AddOptions() + .Bind(builder.Configuration.GetSection(PublicUrlOptions.SectionName)) + .ValidateOnStart(); + builder.Services.AddSingleton, PublicUrlOptionsValidator>(); + builder.Services.AddSingleton(); + // Define authorization policies builder.Services.AddAuthorization(options => { @@ -232,22 +240,6 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db builder.Services.Configure(builder.Configuration.GetSection("EffortSettings")); - // In development, derive BaseUrl from ASPNETCORE_HTTPS_PORT if not explicitly configured - if (builder.Environment.IsDevelopment()) - { - builder.Services.PostConfigure(settings => - { - if (string.IsNullOrWhiteSpace(settings.BaseUrl)) - { - var httpsPort = Environment.GetEnvironmentVariable("ASPNETCORE_HTTPS_PORT") ?? "7157"; - if (int.TryParse(httpsPort, out var port) && port > 0 && port < 65536) - { - settings.BaseUrl = $"https://localhost:{port}"; - } - } - }); - } - // Harvest phases (order matters for DI resolution, but phases self-order via Order property) builder.Services.AddScoped(); builder.Services.AddScoped(); @@ -522,7 +514,7 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db pattern: "{controller=Home}/{action=Index}").RequireAuthorization(); // Setup the memory cache so we can use it via a simple static method - HttpHelper.Configure(app.Services.GetService(), app.Services.GetService(), app.Environment, app.Services.GetService(), app.Services.GetService(), app.Services.GetService()); + HttpHelper.Configure(app.Services.GetService(), app.Services.GetService(), app.Environment, app.Services.GetService(), app.Services.GetService(), app.Services.GetService(), app.Services.GetRequiredService()); #pragma warning disable S6966 // app.Run() is appropriate for main entry point, not app.RunAsync() app.Run(); diff --git a/web/Services/EmailService.cs b/web/Services/EmailService.cs index 8c91b75d1..4b1e4e308 100644 --- a/web/Services/EmailService.cs +++ b/web/Services/EmailService.cs @@ -311,12 +311,6 @@ public class EmailSettings public string DefaultFromAddress { get; set; } = "noreply@example.com"; public bool UseMailpit { get; set; } = false; - /// - /// Base URL for links in emails (e.g., "https://viper.vetmed.ucdavis.edu/2"). - /// Used to construct absolute URLs for email content. - /// - public string? BaseUrl { get; set; } - /// /// When true, all emails are redirected to the logged-in user's email address. /// Use for non-production environments to allow testers to see emails their actions generate. diff --git a/web/appsettings.Production.json b/web/appsettings.Production.json index 8b96ae2a0..8277341d6 100644 --- a/web/appsettings.Production.json +++ b/web/appsettings.Production.json @@ -7,6 +7,14 @@ } }, "LoggingPath": "s:\\nlog", + // Only the hostnames this environment is actually reached by. Defence in depth behind + // Cloudflare/F5/IIS: CAS callbacks come from Application:PublicBaseUrl, not the Host header. + // localhost is kept so on-server probes and IIS itself are not rejected. + "AllowedHosts": "viper.vetmed.ucdavis.edu;localhost", + "Application": { + // Canonical public origin, including the /2 PathBase of the IIS sub-app. + "PublicBaseUrl": "https://viper.vetmed.ucdavis.edu/2" + }, "ConnectionStrings": { "AAUD": "", "Courses": "", @@ -29,8 +37,7 @@ "SmtpPort": 25, "EnableSsl": true, "DefaultFromAddress": "svmithelp@ucdavis.edu", - "UseMailpit": false, - "BaseUrl": "https://viper.vetmed.ucdavis.edu/2" + "UseMailpit": false }, "Hangfire": { "DashboardAppPath": "/2/Computing" diff --git a/web/appsettings.Test.json b/web/appsettings.Test.json index c8de916ae..d365db58e 100644 --- a/web/appsettings.Test.json +++ b/web/appsettings.Test.json @@ -7,6 +7,14 @@ } }, "LoggingPath": "s:\\nlog", + // Only the hostnames this environment is actually reached by. Defence in depth behind + // Cloudflare/F5/IIS: CAS callbacks come from Application:PublicBaseUrl, not the Host header. + // localhost is kept so on-server probes and IIS itself are not rejected. + "AllowedHosts": "secure-test.vetmed.ucdavis.edu;localhost", + "Application": { + // Canonical public origin, including the /2 PathBase of the IIS sub-app. + "PublicBaseUrl": "https://secure-test.vetmed.ucdavis.edu/2" + }, "ConnectionStrings": { "AAUD": "", "Courses": "", @@ -33,7 +41,6 @@ "EnableSsl": true, "DefaultFromAddress": "svmithelp@ucdavis.edu", "UseMailpit": false, - "BaseUrl": "https://secure-test.vetmed.ucdavis.edu/2", "RedirectToCurrentUser": true }, "AWS": {