From e0909c87a8a51a292f0c78bc47902b47fb098376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Sat, 4 Jul 2020 18:10:08 +0200 Subject: [PATCH] Update the logout endpoint logic to not trigger a sign-out response by default and reword some of the exception messages --- .../OpenIddictServerEvents.Authentication.cs | 6 ++ .../OpenIddictServerEvents.Exchange.cs | 6 ++ .../OpenIddictServerEvents.Session.cs | 12 ++-- ...OpenIddictServerHandlers.Authentication.cs | 10 +-- .../OpenIddictServerHandlers.Device.cs | 33 ++++------ .../OpenIddictServerHandlers.Exchange.cs | 10 +-- .../OpenIddictServerHandlers.Session.cs | 55 ++++++++-------- ...nIddictServerAspNetCoreIntegrationTests.cs | 16 +++++ ...penIddictServerIntegrationTests.Session.cs | 64 +++++++++++++++++++ .../OpenIddictServerIntegrationTests.cs | 16 +++++ .../OpenIddictServerOwinIntegrationTests.cs | 16 +++++ 11 files changed, 187 insertions(+), 57 deletions(-) diff --git a/src/OpenIddict.Server/OpenIddictServerEvents.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerEvents.Authentication.cs index fda22c03..8904a789 100644 --- a/src/OpenIddict.Server/OpenIddictServerEvents.Authentication.cs +++ b/src/OpenIddict.Server/OpenIddictServerEvents.Authentication.cs @@ -91,6 +91,12 @@ namespace OpenIddict.Server : base(transaction) { } + + /// + /// Allows OpenIddict to return a sign-in response using the specified principal. + /// + /// The claims principal. + public void SignIn(ClaimsPrincipal principal) => Principal = principal; } /// diff --git a/src/OpenIddict.Server/OpenIddictServerEvents.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerEvents.Exchange.cs index 8c4460d1..25dde753 100644 --- a/src/OpenIddict.Server/OpenIddictServerEvents.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerEvents.Exchange.cs @@ -60,6 +60,12 @@ namespace OpenIddict.Server : base(transaction) { } + + /// + /// Allows OpenIddict to return a sign-in response using the specified principal. + /// + /// The claims principal. + public void SignIn(ClaimsPrincipal principal) => Principal = principal; } /// diff --git a/src/OpenIddict.Server/OpenIddictServerEvents.Session.cs b/src/OpenIddict.Server/OpenIddictServerEvents.Session.cs index 4bebd336..b5b6aa1d 100644 --- a/src/OpenIddict.Server/OpenIddictServerEvents.Session.cs +++ b/src/OpenIddict.Server/OpenIddictServerEvents.Session.cs @@ -5,7 +5,6 @@ */ using System; -using System.Security.Claims; using JetBrains.Annotations; namespace OpenIddict.Server @@ -86,11 +85,14 @@ namespace OpenIddict.Server } /// - /// Gets or sets the security principal extracted from the id_token_hint, if available. - /// Note: the principal may not represent the user currently logged in, - /// so additional validation is strongly encouraged when using this property. + /// Gets a boolean indicating whether a sign-out should be triggered. /// - public ClaimsPrincipal IdentityTokenHintPrincipal { get; set; } + public bool IsSignOutTriggered { get; private set; } + + /// + /// Allows OpenIddict to return a sign-out response. + /// + public void SignOut() => IsSignOutTriggered = true; } /// diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs index 57566db8..392c118e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs @@ -300,10 +300,12 @@ namespace OpenIddict.Server } throw new InvalidOperationException(new StringBuilder() - .Append("The authorization request was not handled. To handle authorization requests, ") - .Append("create a class implementing 'IOpenIddictServerHandler' ") - .AppendLine("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") - .Append("Alternatively, enable the pass-through mode to handle them at a later stage.") + .Append("The authorization request was not handled. To handle authorization requests in a controller, ") + .Append("create a custom controller action with the same route as the authorization endpoint ") + .Append("and enable the pass-through mode in the server ASP.NET Core or OWIN options using ") + .AppendLine("'services.AddOpenIddict().AddServer().UseAspNetCore().EnableAuthorizationEndpointPassthrough()'.") + .Append("Alternatively, create a class implementing 'IOpenIddictServerHandler' ") + .Append("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") .ToString()); } } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Device.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Device.cs index c54dd088..7f347ab8 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Device.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Device.cs @@ -256,21 +256,21 @@ namespace OpenIddict.Server return; } - var @event = new ProcessSignInContext(context.Transaction) - { - Principal = notification.Principal, - Response = new OpenIddictResponse() - }; - - if (@event.Principal == null) + if (notification.Principal == null) { // Note: no authentication type is deliberately specified to represent an unauthenticated identity. var principal = new ClaimsPrincipal(new ClaimsIdentity()); principal.SetScopes(context.Request.GetScopes()); - @event.Principal = principal; + notification.Principal = principal; } + var @event = new ProcessSignInContext(context.Transaction) + { + Principal = notification.Principal, + Response = new OpenIddictResponse() + }; + await _dispatcher.DispatchAsync(@event); if (@event.IsRequestHandled) @@ -293,13 +293,6 @@ namespace OpenIddict.Server uri: @event.ErrorUri); return; } - - throw new InvalidOperationException(new StringBuilder() - .Append("The device request was not handled. To handle device requests, ") - .Append("create a class implementing 'IOpenIddictServerHandler' ") - .AppendLine("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") - .Append("Alternatively, enable the pass-through mode to handle them at a later stage.") - .ToString()); } } @@ -1066,10 +1059,12 @@ namespace OpenIddict.Server } throw new InvalidOperationException(new StringBuilder() - .Append("The verification request was not handled. To handle verification requests, ") - .Append("create a class implementing 'IOpenIddictServerHandler' ") - .AppendLine("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") - .Append("Alternatively, enable the pass-through mode to handle them at a later stage.") + .Append("The verification request was not handled. To handle verification requests in a controller, ") + .Append("create a custom controller action with the same route as the verification endpoint ") + .Append("and enable the pass-through mode in the server ASP.NET Core or OWIN options using ") + .AppendLine("'services.AddOpenIddict().AddServer().UseAspNetCore().EnableVerificationEndpointPassthrough()'.") + .Append("Alternatively, create a class implementing 'IOpenIddictServerHandler' ") + .Append("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") .ToString()); } } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs index 0d51e45f..da7949a8 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs @@ -300,10 +300,12 @@ namespace OpenIddict.Server } throw new InvalidOperationException(new StringBuilder() - .Append("The token request was not handled. To handle token requests, ") - .Append("create a class implementing 'IOpenIddictServerHandler' ") - .AppendLine("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") - .Append("Alternatively, enable the pass-through mode to handle them at a later stage.") + .Append("The token request was not handled. To handle token requests in a controller, ") + .Append("create a custom controller action with the same route as the token endpoint ") + .Append("and enable the pass-through mode in the server ASP.NET Core or OWIN options using ") + .AppendLine("'services.AddOpenIddict().AddServer().UseAspNetCore().EnableTokenEndpointPassthrough()'.") + .Append("Alternatively, create a class implementing 'IOpenIddictServerHandler' ") + .Append("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") .ToString()); } } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Session.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Session.cs index f2d0749f..68e52857 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Session.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Session.cs @@ -242,39 +242,44 @@ namespace OpenIddict.Server return; } - var @event = new ProcessSignOutContext(context.Transaction) + if (notification.IsSignOutTriggered) { - Response = new OpenIddictResponse() - }; + var @event = new ProcessSignOutContext(context.Transaction) + { + Response = new OpenIddictResponse() + }; - await _dispatcher.DispatchAsync(@event); + await _dispatcher.DispatchAsync(@event); - if (@event.IsRequestHandled) - { - context.HandleRequest(); - return; - } + if (@event.IsRequestHandled) + { + context.HandleRequest(); + return; + } - else if (@event.IsRequestSkipped) - { - context.SkipRequest(); - return; - } + else if (@event.IsRequestSkipped) + { + context.SkipRequest(); + return; + } - else if (@event.IsRejected) - { - context.Reject( - error: @event.Error ?? Errors.InvalidRequest, - description: @event.ErrorDescription, - uri: @event.ErrorUri); - return; + else if (@event.IsRejected) + { + context.Reject( + error: @event.Error ?? Errors.InvalidRequest, + description: @event.ErrorDescription, + uri: @event.ErrorUri); + return; + } } throw new InvalidOperationException(new StringBuilder() - .Append("The logout request was not handled. To handle logout requests, ") - .Append("create a class implementing 'IOpenIddictServerHandler' ") - .AppendLine("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") - .Append("Alternatively, enable the pass-through mode to handle them at a later stage.") + .Append("The logout request was not handled. To handle logout requests in a controller, ") + .Append("create a custom controller action with the same route as the logout endpoint ") + .Append("and enable the pass-through mode in the server ASP.NET Core or OWIN options using ") + .AppendLine("'services.AddOpenIddict().AddServer().UseAspNetCore().EnableLogoutEndpointPassthrough()'.") + .Append("Alternatively, create a class implementing 'IOpenIddictServerHandler' ") + .Append("and register it using 'services.AddOpenIddict().AddServer().AddEventHandler()'.") .ToString()); } } diff --git a/test/OpenIddict.Server.AspNetCore.IntegrationTests/OpenIddictServerAspNetCoreIntegrationTests.cs b/test/OpenIddict.Server.AspNetCore.IntegrationTests/OpenIddictServerAspNetCoreIntegrationTests.cs index 3c90059f..81089e19 100644 --- a/test/OpenIddict.Server.AspNetCore.IntegrationTests/OpenIddictServerAspNetCoreIntegrationTests.cs +++ b/test/OpenIddict.Server.AspNetCore.IntegrationTests/OpenIddictServerAspNetCoreIntegrationTests.cs @@ -188,6 +188,14 @@ namespace OpenIddict.Server.AspNetCore.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -221,6 +229,14 @@ namespace OpenIddict.Server.AspNetCore.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => { builder.UseInlineHandler(context => diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Session.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Session.cs index c8b1606e..1d59d716 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Session.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Session.cs @@ -261,6 +261,14 @@ namespace OpenIddict.Server.FunctionalTests options.SetLogoutEndpointUris("/signout"); options.Configure(options => options.IgnoreEndpointPermissions = false); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); }); await using var client = await server.CreateClientAsync(); @@ -473,6 +481,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -504,6 +520,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -539,6 +563,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -568,6 +600,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -594,6 +634,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); options.SetLogoutEndpointUris("/signout"); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); }); await using var client = await server.CreateClientAsync(); @@ -616,6 +664,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); options.SetLogoutEndpointUris("/signout"); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); }); await using var client = await server.CreateClientAsync(); @@ -640,6 +696,14 @@ namespace OpenIddict.Server.FunctionalTests options.EnableDegradedMode(); options.SetLogoutEndpointUris("/signout"); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs index 6b2e9e66..bb5776d1 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs @@ -4147,6 +4147,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -4175,6 +4183,14 @@ namespace OpenIddict.Server.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { diff --git a/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs b/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs index ae62406a..ace54c2f 100644 --- a/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs +++ b/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs @@ -144,6 +144,14 @@ namespace OpenIddict.Server.Owin.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => builder.UseInlineHandler(context => { @@ -177,6 +185,14 @@ namespace OpenIddict.Server.Owin.FunctionalTests { options.EnableDegradedMode(); + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.SignOut(); + + return default; + })); + options.AddEventHandler(builder => { builder.UseInlineHandler(context =>