diff --git a/Directory.Packages.props b/Directory.Packages.props index ec347332..f999122e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -28,6 +28,7 @@ + @@ -54,4 +55,4 @@ - \ No newline at end of file + diff --git a/SecurityService.BusinessLogic/Oidc/ConsentTransactionProtector.cs b/SecurityService.BusinessLogic/Oidc/ConsentTransactionProtector.cs new file mode 100644 index 00000000..ca12292b --- /dev/null +++ b/SecurityService.BusinessLogic/Oidc/ConsentTransactionProtector.cs @@ -0,0 +1,48 @@ +using System.Text.Json; +using System.Security.Cryptography; +using Microsoft.AspNetCore.DataProtection; + +namespace SecurityService.BusinessLogic.Oidc; + +public sealed record ConsentTransaction( + string UserId, + string AuthorizationUrl, + string ClientId, + IReadOnlyCollection RequestedScopes, + DateTimeOffset ExpiresAt); + +public sealed class ConsentTransactionProtector +{ + private readonly IDataProtector _protector; + + public ConsentTransactionProtector(IDataProtector protector) + { + this._protector = protector; + } + + public string Protect(ConsentTransaction transaction) + { + ArgumentNullException.ThrowIfNull(transaction); + return this._protector.Protect(JsonSerializer.Serialize(transaction)); + } + + public ConsentTransaction? Unprotect(string? token, DateTimeOffset now) + { + if (string.IsNullOrWhiteSpace(token)) + { + return null; + } + + try + { + var transaction = JsonSerializer.Deserialize(this._protector.Unprotect(token)); + return transaction is { ExpiresAt: var expiresAt } && expiresAt > now + ? transaction + : null; + } + catch (Exception exception) when (exception is FormatException or JsonException or CryptographicException) + { + return null; + } + } +} diff --git a/SecurityService.BusinessLogic/Oidc/OidcCommands.cs b/SecurityService.BusinessLogic/Oidc/OidcCommands.cs index b8d8b1e4..12c1e6ca 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcCommands.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcCommands.cs @@ -12,7 +12,7 @@ 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>; + public sealed record ConsentGetQuery(HttpContext HttpContext, string TransactionId) : IRequest>; + public sealed record ConsentPostCommand(HttpContext HttpContext, string TransactionId, string Button, IReadOnlyCollection SelectedScopes) : IRequest>; public sealed record DiagnosticsQuery(HttpContext HttpContext) : IRequest>; } diff --git a/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs b/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs index 1777d8fa..14bb3437 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.EntityFrameworkCore; using Microsoft.IdentityModel.Tokens; using OpenIddict.Abstractions; @@ -29,6 +30,7 @@ public sealed class OidcRequestHandler : private readonly IOpenIddictAuthorizationManager _authorizationManager; private readonly IOpenIddictScopeManager _scopeManager; private readonly SecurityServiceDbContext _dbContext; + private readonly ConsentTransactionProtector _consentTransactionProtector; public OidcRequestHandler( UserManager userManager, @@ -36,7 +38,8 @@ public OidcRequestHandler( IOpenIddictApplicationManager applicationManager, IOpenIddictAuthorizationManager authorizationManager, IOpenIddictScopeManager scopeManager, - SecurityServiceDbContext dbContext) + SecurityServiceDbContext dbContext, + ConsentTransactionProtector consentTransactionProtector) { this._userManager = userManager; this._signInManager = signInManager; @@ -44,6 +47,7 @@ public OidcRequestHandler( this._authorizationManager = authorizationManager; this._scopeManager = scopeManager; this._dbContext = dbContext; + this._consentTransactionProtector = consentTransactionProtector; } public async Task> Handle(OidcCommands.AuthorizeCommand command, CancellationToken cancellationToken) @@ -72,6 +76,16 @@ public async Task> Handle(OidcCommands.AuthorizeC if (context.Request.Query.TryGetValue("consent", out var consentDecision)) { + var transaction = this._consentTransactionProtector.Unprotect( + context.Request.Query["consent_transaction"], DateTimeOffset.UtcNow); + if (transaction is null || !string.Equals(transaction.UserId, user.Id, StringComparison.Ordinal) || + !string.Equals(transaction.ClientId, request.ClientId, StringComparison.Ordinal) || + !this.MatchesOriginalRequest(context, transaction)) + { + return Result.Success(AuthorizeForbidServer( + Errors.InvalidRequest, "The consent request is missing, invalid, or expired.")); + } + return Result.Success(await this.HandleConsentDecision(user, request, application, context, consentDecision!, cancellationToken)); } @@ -198,7 +212,13 @@ private async Task HandleConsentType( return AuthorizeForbidServer(Errors.ConsentRequired, "Interactive user consent is required."); } - return new AuthorizeRedirectResult($"/Consent?returnUrl={Uri.EscapeDataString(currentRequestUrl)}"); + var transaction = new ConsentTransaction( + user.Id, + currentRequestUrl, + request.ClientId!, + request.GetScopes().ToArray(), + DateTimeOffset.UtcNow.AddMinutes(10)); + return new AuthorizeRedirectResult($"/Consent?transactionId={Uri.EscapeDataString(this._consentTransactionProtector.Protect(transaction))}"); } return await this.CompleteAuthorization(user, request, application, cancellationToken, request.GetScopes()); @@ -337,6 +357,27 @@ private async Task CompleteAuthorization( return new AuthorizeSignInResult(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } + private bool MatchesOriginalRequest(HttpContext context, ConsentTransaction transaction) + { + if (!Uri.TryCreate(transaction.AuthorizationUrl, UriKind.Relative, out var originalUri) || + !string.Equals(originalUri.AbsolutePath, context.Request.PathBase + context.Request.Path, StringComparison.Ordinal)) + { + return false; + } + + var original = QueryHelpers.ParseQuery(originalUri.Query); + foreach (var pair in original) + { + if (!context.Request.Query.TryGetValue(pair.Key, out var current) || !current.SequenceEqual(pair.Value)) + { + return false; + } + } + + return context.Request.Query.Keys.All(key => + original.ContainsKey(key) || key is "consent" or "consent_transaction" or "granted_scope"); + } + private static TokenForbidResult InvalidGrant(string description = "The username/password couple is invalid.") => new TokenForbidResult( new AuthenticationProperties(new Dictionary diff --git a/SecurityService.BusinessLogic/Oidc/OidcResults.cs b/SecurityService.BusinessLogic/Oidc/OidcResults.cs index 7a74c114..147a22f8 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcResults.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcResults.cs @@ -77,7 +77,7 @@ public sealed record ConsentGetPageResult( IReadOnlyCollection IdentityScopes, IReadOnlyCollection ApiScopes) : ConsentGetQueryResult; -public sealed record ConsentGetLocalRedirectResult(string Url) : ConsentGetQueryResult; +public sealed record ConsentGetInvalidResult(string Message) : ConsentGetQueryResult; public abstract record ConsentPostCommandResult; @@ -85,6 +85,8 @@ public sealed record ConsentPostRedirectResult(string Url) : ConsentPostCommandR public sealed record ConsentPostPageResult(string ModelError) : ConsentPostCommandResult; +public sealed record ConsentPostInvalidResult(string Message) : ConsentPostCommandResult; + // ---- Diagnostics endpoint ---- public abstract record DiagnosticsQueryResult; diff --git a/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs index 297155d1..2eba3cef 100644 --- a/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs +++ b/SecurityService.BusinessLogic/RequestHandlers/ConsentRequestHandler.cs @@ -1,6 +1,8 @@ using MediatR; using Microsoft.AspNetCore; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; using OpenIddict.Abstractions; using SecurityService.BusinessLogic.Oidc; using SecurityService.Database.DbContexts; @@ -14,48 +16,80 @@ public sealed class ConsentRequestHandler : { private readonly IOpenIddictApplicationManager ApplicationManager; private readonly SecurityServiceDbContext DbContext; + private readonly ConsentTransactionProtector TransactionProtector; public ConsentRequestHandler(IOpenIddictApplicationManager applicationManager, - SecurityServiceDbContext dbContext) + SecurityServiceDbContext dbContext, + ConsentTransactionProtector transactionProtector) { this.ApplicationManager = applicationManager; this.DbContext = dbContext; + this.TransactionProtector = transactionProtector; } public async Task> Handle(OidcCommands.ConsentGetQuery query, CancellationToken cancellationToken) { - var request = query.HttpContext.GetOpenIddictServerRequest(); - if (request is null) + var transaction = this.TransactionProtector.Unprotect(query.TransactionId, DateTimeOffset.UtcNow); + var userId = await GetUserId(query.HttpContext); + if (transaction is null || userId is null || !string.Equals(transaction.UserId, userId, StringComparison.Ordinal)) { - return Result.Success(new ConsentGetLocalRedirectResult(query.ReturnUrl)); + return Result.Success(new ConsentGetInvalidResult("The consent request is missing, invalid, or expired.")); } - var application = await this.ApplicationManager.FindByClientIdAsync(request.ClientId!, cancellationToken); + var application = await this.ApplicationManager.FindByClientIdAsync(transaction.ClientId, cancellationToken); var clientName = application is null - ? request.ClientId! - : await this.ApplicationManager.GetDisplayNameAsync(application, cancellationToken) ?? request.ClientId!; + ? transaction.ClientId + : await this.ApplicationManager.GetDisplayNameAsync(application, cancellationToken) ?? transaction.ClientId; - var scopes = await OidcHelpers.BuildScopeDisplay(request, this.DbContext, cancellationToken); + var scopes = await OidcHelpers.BuildScopeDisplay(transaction.RequestedScopes, this.DbContext, cancellationToken); return Result.Success(new ConsentGetPageResult(clientName, scopes.IdentityScopes, scopes.ApiScopes)); } - public Task> Handle(OidcCommands.ConsentPostCommand command, CancellationToken cancellationToken) + public async Task> Handle(OidcCommands.ConsentPostCommand command, CancellationToken cancellationToken) { + var transaction = this.TransactionProtector.Unprotect(command.TransactionId, DateTimeOffset.UtcNow); + var userId = await GetUserId(command.HttpContext); + if (transaction is null || userId is null || !string.Equals(transaction.UserId, userId, StringComparison.Ordinal)) + { + return Result.Success(new ConsentPostInvalidResult("The consent request is missing, invalid, or expired.")); + } + if (string.Equals(command.Button, "deny", StringComparison.OrdinalIgnoreCase)) { - return Task.FromResult(Result.Success( - new ConsentPostRedirectResult(OidcHelpers.AppendQueryValue(command.ReturnUrl, "consent", "denied")))); + return Result.Success( + new ConsentPostRedirectResult(this.BuildDecisionUrl(transaction, "denied", Array.Empty()))); + } + + var grantedScopes = command.SelectedScopes + .Where(scope => transaction.RequestedScopes.Contains(scope, StringComparer.Ordinal)) + .Distinct(StringComparer.Ordinal) + .ToArray(); + if (grantedScopes.Length == 0) + { + return Result.Success( + new ConsentPostPageResult("Select at least one scope to continue.")); } - if (command.SelectedScopes.Count == 0) + return Result.Success(new ConsentPostRedirectResult( + this.BuildDecisionUrl(transaction, "accepted", grantedScopes))); + } + + private string BuildDecisionUrl(ConsentTransaction transaction, string decision, IEnumerable grantedScopes) + { + var redirectUrl = OidcHelpers.AppendQueryValue(transaction.AuthorizationUrl, "consent", decision); + redirectUrl = OidcHelpers.AppendQueryValue(redirectUrl, "consent_transaction", this.TransactionProtector.Protect(transaction)); + return OidcHelpers.AppendQueryValues(redirectUrl, "granted_scope", grantedScopes); + } + + private static async Task GetUserId(HttpContext context) + { + if (context.RequestServices is null) { - return Task.FromResult(Result.Success( - new ConsentPostPageResult("Select at least one scope to continue."))); + return null; } - var redirectUrl = OidcHelpers.AppendQueryValue(command.ReturnUrl, "consent", "accepted"); - redirectUrl = OidcHelpers.AppendQueryValues(redirectUrl, "granted_scope", command.SelectedScopes); - return Task.FromResult(Result.Success(new ConsentPostRedirectResult(redirectUrl))); + var result = await context.AuthenticateAsync(IdentityConstants.ApplicationScheme); + return result.Principal?.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)?.Value; } } diff --git a/SecurityService.BusinessLogic/SecurityService.BusinessLogic.csproj b/SecurityService.BusinessLogic/SecurityService.BusinessLogic.csproj index b6740cb3..91bb516d 100644 --- a/SecurityService.BusinessLogic/SecurityService.BusinessLogic.csproj +++ b/SecurityService.BusinessLogic/SecurityService.BusinessLogic.csproj @@ -8,6 +8,7 @@ + diff --git a/SecurityService.UnitTests/Oidc/ConsentTransactionProtectorTests.cs b/SecurityService.UnitTests/Oidc/ConsentTransactionProtectorTests.cs new file mode 100644 index 00000000..202b4ca8 --- /dev/null +++ b/SecurityService.UnitTests/Oidc/ConsentTransactionProtectorTests.cs @@ -0,0 +1,50 @@ +using Microsoft.AspNetCore.DataProtection; +using SecurityService.BusinessLogic.Oidc; +using Shouldly; + +namespace SecurityService.UnitTests.Oidc; + +public sealed class ConsentTransactionProtectorTests +{ + [Fact] + public void ProtectAndUnprotect_RoundTripsTransaction() + { + var protector = new ConsentTransactionProtector( + new EphemeralDataProtectionProvider().CreateProtector("consent")); + var transaction = new ConsentTransaction( + UserId: "user-1", + AuthorizationUrl: "/connect/authorize?client_id=client&scope=openid%20profile", + ClientId: "client", + RequestedScopes: ["openid", "profile"], + ExpiresAt: DateTimeOffset.UtcNow.AddMinutes(5)); + + var token = protector.Protect(transaction); + var restored = protector.Unprotect(token, DateTimeOffset.UtcNow); + + restored.ShouldNotBeNull(); + restored.UserId.ShouldBe("user-1"); + restored.AuthorizationUrl.ShouldBe(transaction.AuthorizationUrl); + restored.ClientId.ShouldBe("client"); + restored.RequestedScopes.ShouldBe(["openid", "profile"]); + } + + [Fact] + public void Unprotect_WhenTokenIsInvalid_ReturnsNull() + { + var protector = new ConsentTransactionProtector( + new EphemeralDataProtectionProvider().CreateProtector("consent")); + + protector.Unprotect("not-a-consent-token", DateTimeOffset.UtcNow).ShouldBeNull(); + } + + [Fact] + public void Unprotect_WhenTransactionIsExpired_ReturnsNull() + { + var protector = new ConsentTransactionProtector( + new EphemeralDataProtectionProvider().CreateProtector("consent")); + var token = protector.Protect(new ConsentTransaction( + "user-1", "/connect/authorize", "client", ["openid"], DateTimeOffset.UtcNow.AddMinutes(-1))); + + protector.Unprotect(token, DateTimeOffset.UtcNow).ShouldBeNull(); + } +} diff --git a/SecurityService.UnitTests/Oidc/OidcEndpointTests.cs b/SecurityService.UnitTests/Oidc/OidcEndpointTests.cs index 65f98f75..2a03dfe1 100644 --- a/SecurityService.UnitTests/Oidc/OidcEndpointTests.cs +++ b/SecurityService.UnitTests/Oidc/OidcEndpointTests.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.DataProtection; using Imposter.Abstractions; using OpenIddict.Validation.AspNetCore; using SecurityService.BusinessLogic.Oidc; @@ -61,7 +62,8 @@ public async Task UserInfoAsync_WithValidPrincipal_ReturnsJsonPayload() appManager.Instance(), authManager.Instance(), scopeManager.Instance(), - dbContext); + dbContext, + new ConsentTransactionProtector(new EphemeralDataProtectionProvider().CreateProtector("consent"))); var result = await handler.Handle(new OidcCommands.UserInfoCommand(context), CancellationToken.None); diff --git a/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs b/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs index eabcf839..c0176646 100644 --- a/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs +++ b/SecurityService.UnitTests/Pages/ConsentPageModelTests.cs @@ -12,19 +12,19 @@ namespace SecurityService.UnitTests.Pages; public class ConsentPageModelTests { [Fact] - public async Task OnGetAsync_WhenHandlerReturnsLocalRedirect_ReturnsLocalRedirectResult() + public async Task OnGetAsync_WhenHandlerReturnsInvalid_ReturnsBadRequest() { var mediator = new IMediatorImposter(); mediator.Send(Arg>>.Any(), Arg.Any()) - .ReturnsAsync(Result.Success(new ConsentGetLocalRedirectResult("/return"))); + .ReturnsAsync(Result.Success(new ConsentGetInvalidResult("invalid"))); var model = CreateModel(mediator, new DefaultHttpContext()); - model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { ReturnUrl = "/return" }; + model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { TransactionId = "token" }; - var result = await model.OnGetAsync("/return", CancellationToken.None); + var result = await model.OnGetAsync("token", CancellationToken.None); - var redirect = result.ShouldBeOfType(); - redirect.Url.ShouldBe("/return"); + var badRequest = result.ShouldBeOfType(); + badRequest.Value.ShouldBe("invalid"); } [Fact] @@ -38,7 +38,7 @@ public async Task OnGetAsync_WhenHandlerReturnsPage_SetsPropertiesAndReturnsPage var model = CreateModel(mediator, new DefaultHttpContext()); - var result = await model.OnGetAsync("/return", CancellationToken.None); + var result = await model.OnGetAsync("token", CancellationToken.None); result.ShouldBeOfType(); model.ClientName.ShouldBe("My App"); @@ -61,10 +61,10 @@ public async Task OnGetAsync_SendsQueryWithCorrectReturnUrl() }); var model = CreateModel(mediator, new DefaultHttpContext()); - await model.OnGetAsync("/my-return", CancellationToken.None); + await model.OnGetAsync("token", CancellationToken.None); capturedQuery.ShouldNotBeNull(); - capturedQuery.ReturnUrl.ShouldBe("/my-return"); + capturedQuery.TransactionId.ShouldBe("token"); } [Fact] @@ -77,7 +77,7 @@ public async Task OnPostAsync_WhenHandlerReturnsRedirect_ReturnsRedirectResult() var model = CreateModel(mediator, new DefaultHttpContext()); model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { - ReturnUrl = "/return", + TransactionId = "token", Button = "deny", SelectedScopes = [] }; @@ -98,7 +98,7 @@ public async Task OnPostAsync_WhenHandlerReturnsPageWithError_AddsModelErrorAndR var model = CreateModel(mediator, new DefaultHttpContext()); model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { - ReturnUrl = "/return", + TransactionId = "token", Button = "accept", SelectedScopes = [] }; @@ -126,7 +126,7 @@ public async Task OnPostAsync_SendsCommandWithCorrectValues() var model = CreateModel(mediator, new DefaultHttpContext()); model.Input = new SecurityService.Pages.Consent.IndexModel.InputModel { - ReturnUrl = "/return", + TransactionId = "token", Button = "accept", SelectedScopes = ["openid", "profile"] }; @@ -134,7 +134,7 @@ public async Task OnPostAsync_SendsCommandWithCorrectValues() await model.OnPostAsync(CancellationToken.None); capturedCommand.ShouldNotBeNull(); - capturedCommand.ReturnUrl.ShouldBe("/return"); + capturedCommand.TransactionId.ShouldBe("token"); capturedCommand.Button.ShouldBe("accept"); capturedCommand.SelectedScopes.ShouldContain("openid"); capturedCommand.SelectedScopes.ShouldContain("profile"); diff --git a/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs index 93d1a436..e9d0dd20 100644 --- a/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ConsentRequestHandlerTests.cs @@ -1,5 +1,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.DataProtection; using Imposter.Abstractions; using OpenIddict.Abstractions; using SecurityService.BusinessLogic.Oidc; @@ -13,21 +14,20 @@ namespace SecurityService.UnitTests.RequestHandlers; public class ConsentRequestHandlerTests { [Fact] - public async Task ConsentGetQuery_WhenNoOpenIddictServerRequest_ReturnsLocalRedirect() + public async Task ConsentGetQuery_WhenTransactionIsInvalid_ReturnsControlledError() { var appManager = new IOpenIddictApplicationManagerImposter(); - using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentGetQuery_WhenNoOpenIddictServerRequest_ReturnsLocalRedirect)); + using var serviceProvider = TestServiceProviderFactory.Create(nameof(ConsentGetQuery_WhenTransactionIsInvalid_ReturnsControlledError)); using var scope = serviceProvider.CreateScope(); var dbContext = scope.ServiceProvider.GetRequiredService(); - var handler = new ConsentRequestHandler(appManager.Instance(), dbContext); + var handler = new ConsentRequestHandler(appManager.Instance(), dbContext, CreateProtector()); 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"); + result.Data.ShouldBeOfType(); } [Fact] @@ -38,15 +38,14 @@ public async Task ConsentPostCommand_WhenDenyButton_ReturnsRedirectWithDenied() using var scope = serviceProvider.CreateScope(); var dbContext = scope.ServiceProvider.GetRequiredService(); - var handler = new ConsentRequestHandler(appManager.Instance(), dbContext); + var handler = new ConsentRequestHandler(appManager.Instance(), dbContext, CreateProtector()); var result = await handler.Handle( - new OidcCommands.ConsentPostCommand("/return", "deny", Array.Empty()), + new OidcCommands.ConsentPostCommand(new DefaultHttpContext(), "invalid", "deny", Array.Empty()), CancellationToken.None); result.IsSuccess.ShouldBeTrue(); - var redirect = result.Data.ShouldBeOfType(); - redirect.Url.ShouldContain("consent=denied"); + result.Data.ShouldBeOfType(); } [Fact] @@ -57,15 +56,14 @@ public async Task ConsentPostCommand_WhenNoScopesSelected_ReturnsPageWithError() using var scope = serviceProvider.CreateScope(); var dbContext = scope.ServiceProvider.GetRequiredService(); - var handler = new ConsentRequestHandler(appManager.Instance(), dbContext); + var handler = new ConsentRequestHandler(appManager.Instance(), dbContext, CreateProtector()); var result = await handler.Handle( - new OidcCommands.ConsentPostCommand("/return", "accept", Array.Empty()), + new OidcCommands.ConsentPostCommand(new DefaultHttpContext(), "invalid", "accept", Array.Empty()), CancellationToken.None); result.IsSuccess.ShouldBeTrue(); - var page = result.Data.ShouldBeOfType(); - page.ModelError.ShouldBe("Select at least one scope to continue."); + result.Data.ShouldBeOfType(); } [Fact] @@ -76,16 +74,16 @@ public async Task ConsentPostCommand_WhenScopesSelected_ReturnsRedirectWithAccep using var scope = serviceProvider.CreateScope(); var dbContext = scope.ServiceProvider.GetRequiredService(); - var handler = new ConsentRequestHandler(appManager.Instance(), dbContext); + var handler = new ConsentRequestHandler(appManager.Instance(), dbContext, CreateProtector()); var result = await handler.Handle( - new OidcCommands.ConsentPostCommand("/return", "accept", ["openid", "profile"]), + new OidcCommands.ConsentPostCommand(new DefaultHttpContext(), "invalid", "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"); + result.Data.ShouldBeOfType(); } + + private static ConsentTransactionProtector CreateProtector() => new( + new EphemeralDataProtectionProvider().CreateProtector("consent")); } diff --git a/SecurityService/Pages/Consent/Index.cshtml b/SecurityService/Pages/Consent/Index.cshtml index c2c977b9..da100b6a 100644 --- a/SecurityService/Pages/Consent/Index.cshtml +++ b/SecurityService/Pages/Consent/Index.cshtml @@ -12,7 +12,7 @@
- + @if (Model.IdentityScopes.Count > 0) { diff --git a/SecurityService/Pages/Consent/Index.cshtml.cs b/SecurityService/Pages/Consent/Index.cshtml.cs index a5c9f4c7..2c7223ae 100644 --- a/SecurityService/Pages/Consent/Index.cshtml.cs +++ b/SecurityService/Pages/Consent/Index.cshtml.cs @@ -21,15 +21,15 @@ public IndexModel(IMediator mediator) [BindProperty] public InputModel Input { get; set; } = new(); - public async Task OnGetAsync(string returnUrl, CancellationToken cancellationToken) + public async Task OnGetAsync(string transactionId, CancellationToken cancellationToken) { - this.Input.ReturnUrl = returnUrl; - var result = await this._mediator.Send(new OidcCommands.ConsentGetQuery(this.HttpContext, returnUrl), cancellationToken); + this.Input.TransactionId = transactionId; + var result = await this._mediator.Send(new OidcCommands.ConsentGetQuery(this.HttpContext, transactionId), cancellationToken); return result.Data switch { - ConsentGetLocalRedirectResult redirect => this.LocalRedirect(redirect.Url), ConsentGetPageResult page => this.ApplyPageResult(page), + ConsentGetInvalidResult invalid => this.BadRequest(invalid.Message), _ => this.Page() }; } @@ -37,13 +37,14 @@ public async Task OnGetAsync(string returnUrl, CancellationToken public async Task OnPostAsync(CancellationToken cancellationToken) { var result = await this._mediator.Send( - new OidcCommands.ConsentPostCommand(this.Input.ReturnUrl, this.Input.Button, this.Input.SelectedScopes), + new OidcCommands.ConsentPostCommand(this.HttpContext, this.Input.TransactionId, this.Input.Button, this.Input.SelectedScopes), cancellationToken); return result.Data switch { ConsentPostRedirectResult redirect => this.Redirect(redirect.Url), ConsentPostPageResult page => this.HandlePageResult(page), + ConsentPostInvalidResult invalid => this.BadRequest(invalid.Message), _ => this.Page() }; } @@ -64,7 +65,7 @@ private IActionResult HandlePageResult(ConsentPostPageResult page) public sealed class InputModel { - public string ReturnUrl { get; set; } = "/"; + public string TransactionId { get; set; } = string.Empty; public List SelectedScopes { get; set; } = new(); public string Button { get; set; } = "accept"; } diff --git a/SecurityService/Program.cs b/SecurityService/Program.cs index e05cfa25..2d3f75c9 100644 --- a/SecurityService/Program.cs +++ b/SecurityService/Program.cs @@ -3,6 +3,7 @@ using MediatR; using MessagingService.Client; using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Diagnostics.HealthChecks; using Microsoft.AspNetCore.Http.Json; @@ -14,6 +15,7 @@ using NLog.Extensions.Logging; using OpenIddict.Abstractions; using SecurityService.BusinessLogic; +using SecurityService.BusinessLogic.Oidc; using SecurityService.BusinessLogic.Requests; using SecurityService.Common; using SecurityService.Configuration; @@ -183,6 +185,9 @@ }); builder.Services.AddHttpContextAccessor(); +builder.Services.AddDataProtection(); +builder.Services.AddSingleton(serviceProvider => new ConsentTransactionProtector( + serviceProvider.GetRequiredService().CreateProtector("SecurityService.Consent"))); builder.Services.AddScoped(); if (builder.Environment.IsEnvironment("IntegrationTest"))