From c60ca71f741d7f59a1929cf25098513d13488997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Sun, 8 May 2016 05:00:53 +0200 Subject: [PATCH] Fix OpenIddictProvider.HandleLogoutRequest to prevent throwing an ArgumentNullException when the authentication cookie is no longer valid --- .../OpenIddictProvider.Authentication.cs | 12 +++++- .../OpenIddictProvider.Session.cs | 42 ++++++++++++------- src/OpenIddict.Mvc/OpenIddictController.cs | 2 +- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/OpenIddict.Core/OpenIddictProvider.Authentication.cs b/src/OpenIddict.Core/OpenIddictProvider.Authentication.cs index d188b1ad..18e9101f 100644 --- a/src/OpenIddict.Core/OpenIddictProvider.Authentication.cs +++ b/src/OpenIddict.Core/OpenIddictProvider.Authentication.cs @@ -107,6 +107,16 @@ namespace OpenIddict { return; } + // Ensure that the authentication cookie contains the required NameIdentifier claim. + var identifier = context.HttpContext.User.GetClaim(ClaimTypes.NameIdentifier); + if (string.IsNullOrEmpty(identifier)) { + context.Reject( + error: OpenIdConnectConstants.Errors.ServerError, + description: "The authorization request cannot be processed."); + + return; + } + // Extract the principal contained in the id_token_hint parameter. // If no principal can be extracted, an error is returned to the client application. var principal = await context.HttpContext.Authentication.AuthenticateAsync(context.Options.AuthenticationScheme); @@ -121,7 +131,7 @@ namespace OpenIddict { // Ensure the client application is listed as a valid audience in the identity token // and that the identity token corresponds to the authenticated user. if (!principal.HasClaim(OpenIdConnectConstants.Claims.Audience, context.Request.ClientId) || - !principal.HasClaim(ClaimTypes.NameIdentifier, context.HttpContext.User.GetClaim(ClaimTypes.NameIdentifier))) { + !principal.HasClaim(ClaimTypes.NameIdentifier, identifier)) { context.Reject( error: OpenIdConnectConstants.Errors.InvalidRequest, description: "The id_token_hint parameter is invalid."); diff --git a/src/OpenIddict.Core/OpenIddictProvider.Session.cs b/src/OpenIddict.Core/OpenIddictProvider.Session.cs index a90f0420..9251cf5f 100644 --- a/src/OpenIddict.Core/OpenIddictProvider.Session.cs +++ b/src/OpenIddict.Core/OpenIddictProvider.Session.cs @@ -4,6 +4,7 @@ * the license and the contributors participating to this project. */ +using System.Linq; using System.Security.Claims; using System.Threading.Tasks; using AspNet.Security.OpenIdConnect.Extensions; @@ -39,23 +40,36 @@ namespace OpenIddict { public override async Task HandleLogoutRequest([NotNull] HandleLogoutRequestContext context) { var services = context.HttpContext.RequestServices.GetRequiredService>(); - // When the client application sends an id_token_hint parameter, the corresponding identity can be retrieved using - // AuthenticateAsync and used as a way to determine whether the logout request has been sent by a legit caller. - // If the token cannot be extracted, don't handle the logout request at this stage and continue to the next middleware. - var principal = await context.HttpContext.Authentication.AuthenticateAsync(context.Options.AuthenticationScheme); - if (principal == null) { - return; - } + // Only validate the id_token_hint if the user is still logged in. + // If the authentication cookie doesn't exist or is no longer valid, + // the user agent is immediately redirected to the client application. + if (context.HttpContext.User.Identities.Any(identity => identity.IsAuthenticated)) { + // Ensure that the authentication cookie contains the required NameIdentifier claim. + // If it cannot be found, ignore the logout request and continue to the next middleware. + var identifier = context.HttpContext.User.GetClaim(ClaimTypes.NameIdentifier); + if (string.IsNullOrEmpty(identifier)) { + return; + } - // Ensure that the identity token corresponds to the authenticated user. If the token cannot be - // validated, don't handle the logout request at this stage and continue to the next middleware. - if (!principal.HasClaim(ClaimTypes.NameIdentifier, context.HttpContext.User.GetClaim(ClaimTypes.NameIdentifier))) { - return; + // When the client application sends an id_token_hint parameter, the corresponding identity can be retrieved using + // AuthenticateAsync and used as a way to determine whether the logout request has been sent by a legit caller. + // If the token cannot be extracted, don't handle the logout request at this stage and continue to the next middleware. + var principal = await context.HttpContext.Authentication.AuthenticateAsync(context.Options.AuthenticationScheme); + if (principal == null) { + return; + } + + // Ensure that the identity token corresponds to the authenticated user. If the token cannot be + // validated, don't handle the logout request at this stage and continue to the next middleware. + if (!principal.HasClaim(ClaimTypes.NameIdentifier, identifier)) { + return; + } + + // Delete the ASP.NET Core Identity cookies. + await services.SignIn.SignOutAsync(); } - // Immediately sign out the user and redirect him - // to the post_logout_redirect_uri if provided. - await services.SignIn.SignOutAsync(); + // Redirect the user agent back to the client application. await context.HttpContext.Authentication.SignOutAsync(context.Options.AuthenticationScheme); // Mark the response as handled diff --git a/src/OpenIddict.Mvc/OpenIddictController.cs b/src/OpenIddict.Mvc/OpenIddictController.cs index 7fcb0464..1abbaca0 100644 --- a/src/OpenIddict.Mvc/OpenIddictController.cs +++ b/src/OpenIddict.Mvc/OpenIddictController.cs @@ -166,7 +166,7 @@ namespace OpenIddict.Mvc { return Task.FromResult(View("Logout", request)); } - [HttpPost, ValidateAntiForgeryToken] + [ActionName(nameof(Logout)), HttpPost, ValidateAntiForgeryToken] public virtual async Task Signout() { // Instruct the cookies middleware to delete the local cookie created // when the user agent is redirected from the external identity provider