From 967dc6431ed1dd18a1d62bbe978f9a1ec01ab224 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 17:06:18 +0000 Subject: [PATCH 1/2] Initial plan From 505745ce40d265a4c8c51170f3e1ffbda1945364 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 17:36:06 +0000 Subject: [PATCH 2/2] Refactor Consent Index to use MediatR commands/queries pattern Agent-Logs-Url: https://github.com/TransactionProcessing/SecurityService/sessions/1d5763a2-23e4-4d57-9ef6-af64f093204a Co-authored-by: StuartFerguson <16325469+StuartFerguson@users.noreply.github.com> --- .../Oidc/OidcCommands.cs | 2 + .../Oidc/OidcResults.cs | 17 ++ .../RequestHandlers/ConsentRequestHandler.cs | 62 +++++++ .../Pages/ConsentPageModelTests.cs | 151 ++++++++++++++++++ .../ConsentRequestHandlerTests.cs | 91 +++++++++++ SecurityService/Pages/Consent/Index.cshtml.cs | 65 ++++---- 6 files changed, 357 insertions(+), 31 deletions(-) create mode 100644 SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs create mode 100644 SecurityService.UnitTests/Pages/ConsentPageModelTests.cs create mode 100644 SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs diff --git a/SecurityService.BusinessLogic/Oidc/OidcCommands.cs b/SecurityService.BusinessLogic/Oidc/OidcCommands.cs index 4a879e90..ff4112ee 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcCommands.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcCommands.cs @@ -12,4 +12,6 @@ public sealed record LogoutCommand(HttpContext HttpContext) : IRequest>; public sealed record VerifyGetQuery(HttpContext HttpContext) : IRequest>; public sealed record VerifyPostCommand(HttpContext HttpContext, string Action, string UserCode) : IRequest>; + public sealed record ConsentGetQuery(HttpContext HttpContext, string ReturnUrl) : IRequest>; + public sealed record ConsentPostCommand(string ReturnUrl, string Button, IReadOnlyCollection SelectedScopes) : IRequest>; } diff --git a/SecurityService.BusinessLogic/Oidc/OidcResults.cs b/SecurityService.BusinessLogic/Oidc/OidcResults.cs index b1313c2b..90941599 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcResults.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcResults.cs @@ -67,3 +67,20 @@ public sealed record VerifyDisplayData( IReadOnlyCollection IdentityScopes, IReadOnlyCollection ApiScopes, string UserCode); + +// ---- Consent endpoint ---- + +public abstract record ConsentGetQueryResult; + +public sealed record ConsentGetPageResult( + string ClientName, + IReadOnlyCollection IdentityScopes, + IReadOnlyCollection ApiScopes) : ConsentGetQueryResult; + +public sealed record ConsentGetLocalRedirectResult(string Url) : ConsentGetQueryResult; + +public abstract record ConsentPostCommandResult; + +public sealed record ConsentPostRedirectResult(string Url) : ConsentPostCommandResult; + +public sealed record ConsentPostPageResult(string ModelError) : ConsentPostCommandResult; diff --git a/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs new file mode 100644 index 00000000..5af6d55e --- /dev/null +++ b/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs @@ -0,0 +1,62 @@ +using MediatR; +using Microsoft.AspNetCore; +using Microsoft.AspNetCore.Http; +using OpenIddict.Abstractions; +using SecurityService.BusinessLogic.Oidc; +using SecurityService.Database.DbContexts; +using SimpleResults; + +namespace SecurityService.BusinessLogic.RequestHandlers; + +public sealed class ConsentRequestHandler : + IRequestHandler>, + IRequestHandler> +{ + private readonly IOpenIddictApplicationManager _applicationManager; + private readonly SecurityServiceDbContext _dbContext; + + public ConsentRequestHandler( + IOpenIddictApplicationManager applicationManager, + SecurityServiceDbContext dbContext) + { + this._applicationManager = applicationManager; + this._dbContext = dbContext; + } + + public async Task> Handle(OidcCommands.ConsentGetQuery query, CancellationToken cancellationToken) + { + var request = query.HttpContext.GetOpenIddictServerRequest(); + if (request is null) + { + return Result.Success(new ConsentGetLocalRedirectResult(query.ReturnUrl)); + } + + var application = await this._applicationManager.FindByClientIdAsync(request.ClientId!, cancellationToken); + var clientName = application is null + ? request.ClientId! + : await this._applicationManager.GetDisplayNameAsync(application, cancellationToken) ?? request.ClientId!; + + var scopes = await OidcHelpers.BuildScopeDisplay(request, this._dbContext, cancellationToken); + + return Result.Success(new ConsentGetPageResult(clientName, scopes.IdentityScopes, scopes.ApiScopes)); + } + + public Task> Handle(OidcCommands.ConsentPostCommand command, CancellationToken cancellationToken) + { + if (string.Equals(command.Button, "deny", StringComparison.OrdinalIgnoreCase)) + { + return Task.FromResult(Result.Success( + new ConsentPostRedirectResult(OidcHelpers.AppendQueryValue(command.ReturnUrl, "consent", "denied")))); + } + + if (command.SelectedScopes.Count == 0) + { + return Task.FromResult(Result.Success( + new ConsentPostPageResult("Select at least one scope to continue."))); + } + + var redirectUrl = OidcHelpers.AppendQueryValue(command.ReturnUrl, "consent", "accepted"); + redirectUrl = OidcHelpers.AppendQueryValues(redirectUrl, "granted_scope", command.SelectedScopes); + return Task.FromResult(Result.Success(new ConsentPostRedirectResult(redirectUrl))); + } +} diff --git a/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs b/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs new file mode 100644 index 00000000..a6ddfcd7 --- /dev/null +++ b/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs @@ -0,0 +1,151 @@ +using MediatR; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Moq; +using SecurityService.BusinessLogic.Oidc; +using Shouldly; +using SimpleResults; + +namespace SecurityService.UnitTests.Pages; + +public class ConsentPageModelTests +{ + [Fact] + public async Task OnGetAsync_WhenHandlerReturnsLocalRedirect_ReturnsLocalRedirectResult() + { + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync(Result.Success(new ConsentGetLocalRedirectResult("/return"))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { ReturnUrl = "/return" }; + + var result = await model.OnGetAsync("/return", CancellationToken.None); + + var redirect = result.ShouldBeOfType(); + redirect.Url.ShouldBe("/return"); + } + + [Fact] + public async Task OnGetAsync_WhenHandlerReturnsPage_SetsPropertiesAndReturnsPage() + { + var identityScopes = new[] { new ScopeDisplayItem("openid", "OpenID", null, true, false) }; + var apiScopes = new[] { new ScopeDisplayItem("api", "API", null, false, false) }; + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync(Result.Success(new ConsentGetPageResult("My App", identityScopes, apiScopes))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + + var result = await model.OnGetAsync("/return", CancellationToken.None); + + result.ShouldBeOfType(); + model.ClientName.ShouldBe("My App"); + model.IdentityScopes.Count.ShouldBe(1); + model.ApiScopes.Count.ShouldBe(1); + } + + [Fact] + public async Task OnGetAsync_SendsQueryWithCorrectReturnUrl() + { + OidcCommands.ConsentGetQuery? capturedQuery = null; + + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .Callback>, CancellationToken>((req, _) => + { + capturedQuery = req.ShouldBeOfType(); + }) + .ReturnsAsync(Result.Success(new ConsentGetPageResult(string.Empty, [], []))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + await model.OnGetAsync("/my-return", CancellationToken.None); + + capturedQuery.ShouldNotBeNull(); + capturedQuery.ReturnUrl.ShouldBe("/my-return"); + } + + [Fact] + public async Task OnPostAsync_WhenHandlerReturnsRedirect_ReturnsRedirectResult() + { + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync(Result.Success(new ConsentPostRedirectResult("/return?consent=denied"))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel + { + ReturnUrl = "/return", + Button = "deny", + SelectedScopes = [] + }; + + var result = await model.OnPostAsync(CancellationToken.None); + + var redirect = result.ShouldBeOfType(); + redirect.Url.ShouldBe("/return?consent=denied"); + } + + [Fact] + public async Task OnPostAsync_WhenHandlerReturnsPageWithError_AddsModelErrorAndReturnsPage() + { + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync(Result.Success(new ConsentPostPageResult("Select at least one scope to continue."))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel + { + ReturnUrl = "/return", + Button = "accept", + SelectedScopes = [] + }; + + var result = await model.OnPostAsync(CancellationToken.None); + + result.ShouldBeOfType(); + model.ModelState[string.Empty]!.Errors.ShouldContain(e => e.ErrorMessage == "Select at least one scope to continue."); + } + + [Fact] + public async Task OnPostAsync_SendsCommandWithCorrectValues() + { + OidcCommands.ConsentPostCommand? capturedCommand = null; + + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .Callback>, CancellationToken>((req, _) => + { + capturedCommand = req.ShouldBeOfType(); + }) + .ReturnsAsync(Result.Success(new ConsentPostRedirectResult("/return?consent=accepted"))); + + var model = CreateModel(mediator, new DefaultHttpContext()); + model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel + { + ReturnUrl = "/return", + Button = "accept", + SelectedScopes = ["openid", "profile"] + }; + + await model.OnPostAsync(CancellationToken.None); + + capturedCommand.ShouldNotBeNull(); + capturedCommand.ReturnUrl.ShouldBe("/return"); + capturedCommand.Button.ShouldBe("accept"); + capturedCommand.SelectedScopes.ShouldContain("openid"); + capturedCommand.SelectedScopes.ShouldContain("profile"); + } + + private static SecurityService.Pages.Consent.IndexModel CreateModel(Mock mediator, HttpContext httpContext) + { + return new SecurityService.Pages.Consent.IndexModel(mediator.Object) + { + PageContext = new PageContext + { + HttpContext = httpContext + } + }; + } +} diff --git a/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs new file mode 100644 index 00000000..7dcc7a1a --- /dev/null +++ b/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs @@ -0,0 +1,91 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using OpenIddict.Abstractions; +using SecurityService.BusinessLogic.Oidc; +using SecurityService.BusinessLogic.RequestHandlers; +using SecurityService.Database.DbContexts; +using SecurityService.UnitTests.Infrastructure; +using Shouldly; + +namespace SecurityService.UnitTests.RequestHandlers; + +public class ConsentRequestHandlerTests +{ + [Fact] + public async Task ConsentGetQuery_WhenNoOpenIddictServerRequest_ReturnsLocalRedirect() + { + var appManager = new Mock(); + using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentGetQuery_WhenNoOpenIddictServerRequest_ReturnsLocalRedirect)); + using var scope = serviceProvider.CreateScope(); + var dbContext = scope.ServiceProvider.GetRequiredService(); + + var handler = new ConsentRequestHandler(appManager.Object, dbContext); + var context = new DefaultHttpContext(); + + var result = await handler.Handle(new OidcCommands.ConsentGetQuery(context, "/return"), CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + var redirect = result.Data.ShouldBeOfType(); + redirect.Url.ShouldBe("/return"); + } + + [Fact] + public async Task ConsentPostCommand_WhenDenyButton_ReturnsRedirectWithDenied() + { + var appManager = new Mock(); + using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentPostCommand_WhenDenyButton_ReturnsRedirectWithDenied)); + using var scope = serviceProvider.CreateScope(); + var dbContext = scope.ServiceProvider.GetRequiredService(); + + var handler = new ConsentRequestHandler(appManager.Object, dbContext); + + var result = await handler.Handle( + new OidcCommands.ConsentPostCommand("/return", "deny", Array.Empty()), + CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + var redirect = result.Data.ShouldBeOfType(); + redirect.Url.ShouldContain("consent=denied"); + } + + [Fact] + public async Task ConsentPostCommand_WhenNoScopesSelected_ReturnsPageWithError() + { + var appManager = new Mock(); + using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentPostCommand_WhenNoScopesSelected_ReturnsPageWithError)); + using var scope = serviceProvider.CreateScope(); + var dbContext = scope.ServiceProvider.GetRequiredService(); + + var handler = new ConsentRequestHandler(appManager.Object, dbContext); + + var result = await handler.Handle( + new OidcCommands.ConsentPostCommand("/return", "accept", Array.Empty()), + CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + var page = result.Data.ShouldBeOfType(); + page.ModelError.ShouldBe("Select at least one scope to continue."); + } + + [Fact] + public async Task ConsentPostCommand_WhenScopesSelected_ReturnsRedirectWithAccepted() + { + var appManager = new Mock(); + using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentPostCommand_WhenScopesSelected_ReturnsRedirectWithAccepted)); + using var scope = serviceProvider.CreateScope(); + var dbContext = scope.ServiceProvider.GetRequiredService(); + + var handler = new ConsentRequestHandler(appManager.Object, dbContext); + + var result = await handler.Handle( + new OidcCommands.ConsentPostCommand("/return", "accept", ["openid", "profile"]), + CancellationToken.None); + + result.IsSuccess.ShouldBeTrue(); + var redirect = result.Data.ShouldBeOfType(); + redirect.Url.ShouldContain("consent=accepted"); + redirect.Url.ShouldContain("granted_scope=openid"); + redirect.Url.ShouldContain("granted_scope=profile"); + } +} diff --git a/SecurityService/Pages/Consent/Index.cshtml.cs b/SecurityService/Pages/Consent/Index.cshtml.cs index 48bee498..a5c9f4c7 100644 --- a/SecurityService/Pages/Consent/Index.cshtml.cs +++ b/SecurityService/Pages/Consent/Index.cshtml.cs @@ -1,21 +1,17 @@ -using Microsoft.AspNetCore; +using MediatR; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; -using OpenIddict.Abstractions; -using SecurityService.Database.DbContexts; using SecurityService.BusinessLogic.Oidc; namespace SecurityService.Pages.Consent; public sealed class IndexModel : PageModel { - private readonly SecurityServiceDbContext _dbContext; - private readonly IOpenIddictApplicationManager _applicationManager; + private readonly IMediator _mediator; - public IndexModel(SecurityServiceDbContext dbContext, IOpenIddictApplicationManager applicationManager) + public IndexModel(IMediator mediator) { - this._dbContext = dbContext; - this._applicationManager = applicationManager; + this._mediator = mediator; } public string ClientName { get; private set; } = string.Empty; @@ -28,36 +24,42 @@ public IndexModel(SecurityServiceDbContext dbContext, IOpenIddictApplicationMana public async Task OnGetAsync(string returnUrl, CancellationToken cancellationToken) { this.Input.ReturnUrl = returnUrl; - var request = this.HttpContext.GetOpenIddictServerRequest(); - if (request is null) - { - return this.LocalRedirect(returnUrl); - } + var result = await this._mediator.Send(new OidcCommands.ConsentGetQuery(this.HttpContext, returnUrl), cancellationToken); - var application = await this._applicationManager.FindByClientIdAsync(request.ClientId!, cancellationToken); - this.ClientName = application is null ? request.ClientId! : await this._applicationManager.GetDisplayNameAsync(application, cancellationToken) ?? request.ClientId!; - var scopes = await OidcHelpers.BuildScopeDisplay(request, this._dbContext, cancellationToken); - this.IdentityScopes = scopes.IdentityScopes; - this.ApiScopes = scopes.ApiScopes; - return this.Page(); + return result.Data switch + { + ConsentGetLocalRedirectResult redirect => this.LocalRedirect(redirect.Url), + ConsentGetPageResult page => this.ApplyPageResult(page), + _ => this.Page() + }; } - public IActionResult OnPost() + public async Task OnPostAsync(CancellationToken cancellationToken) { - if (string.Equals(this.Input.Button, "deny", StringComparison.OrdinalIgnoreCase)) - { - return this.Redirect(OidcHelpers.AppendQueryValue(this.Input.ReturnUrl, "consent", "denied")); - } + var result = await this._mediator.Send( + new OidcCommands.ConsentPostCommand(this.Input.ReturnUrl, this.Input.Button, this.Input.SelectedScopes), + cancellationToken); - if (this.Input.SelectedScopes.Count == 0) + return result.Data switch { - this.ModelState.AddModelError(string.Empty, "Select at least one scope to continue."); - return this.Page(); - } + ConsentPostRedirectResult redirect => this.Redirect(redirect.Url), + ConsentPostPageResult page => this.HandlePageResult(page), + _ => this.Page() + }; + } + + private IActionResult ApplyPageResult(ConsentGetPageResult page) + { + this.ClientName = page.ClientName; + this.IdentityScopes = page.IdentityScopes; + this.ApiScopes = page.ApiScopes; + return this.Page(); + } - var redirectUrl = OidcHelpers.AppendQueryValue(this.Input.ReturnUrl, "consent", "accepted"); - redirectUrl = OidcHelpers.AppendQueryValues(redirectUrl, "granted_scope", this.Input.SelectedScopes); - return this.Redirect(redirectUrl); + private IActionResult HandlePageResult(ConsentPostPageResult page) + { + this.ModelState.AddModelError(string.Empty, page.ModelError); + return this.Page(); } public sealed class InputModel @@ -67,3 +69,4 @@ public sealed class InputModel public string Button { get; set; } = "accept"; } } +