From 6dce31567ca4059a6e75f3f676737d18756b0417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Thu, 2 Jul 2020 04:01:22 +0200 Subject: [PATCH] Update ValidateIdentityModelToken and ValidateDataProtectionToken to immediately return an error when the token can be read but is invalid --- .../OpenIddictServerAspNetCoreHandlers.cs | 2 +- .../OpenIddictServerDataProtectionHandlers.cs | 11 +++++-- .../OpenIddictServerOwinHandlers.cs | 2 +- .../OpenIddictServerHandlers.cs | 29 +++++++++++++++++-- .../OpenIddictValidationAspNetCoreHandlers.cs | 2 +- ...nIddictValidationDataProtectionHandlers.cs | 6 ++-- .../OpenIddictValidationOwinHandlers.cs | 2 +- .../OpenIddictValidationHandlers.cs | 19 ++++++++++-- 8 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs index e3757a97..2d50d1e1 100644 --- a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs +++ b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs @@ -1111,7 +1111,7 @@ namespace OpenIddict.Server.AspNetCore return default; } - context.Logger.LogInformation("The response was successfully returned as an empty challenge response."); + context.Logger.LogInformation("The response was successfully returned as a challenge response: {Response}.", context.Response); context.HandleRequest(); return default; diff --git a/src/OpenIddict.Server.DataProtection/OpenIddictServerDataProtectionHandlers.cs b/src/OpenIddict.Server.DataProtection/OpenIddictServerDataProtectionHandlers.cs index 8bd6cb0a..e0dece81 100644 --- a/src/OpenIddict.Server.DataProtection/OpenIddictServerDataProtectionHandlers.cs +++ b/src/OpenIddict.Server.DataProtection/OpenIddictServerDataProtectionHandlers.cs @@ -87,13 +87,11 @@ namespace OpenIddict.Server.DataProtection // Note: ASP.NET Core Data Protection tokens always start with "CfDJ8", that corresponds // to the base64 representation of the magic "09 F0 C9 F0" header identifying DP payloads. - // As an optimization, always ignore tokens that don't start with the "CfDJ8" string. if (string.IsNullOrEmpty(context.Token) || !context.Token.StartsWith("CfDJ8", StringComparison.Ordinal)) { return default; } - // If the token cannot be validated, don't return an error to allow another handle to validate it. var principal = !string.IsNullOrEmpty(context.TokenType) ? ValidateToken(context.Token, context.TokenType) : ValidateToken(context.Token, TokenTypeHints.AccessToken) ?? @@ -101,8 +99,17 @@ namespace OpenIddict.Server.DataProtection ValidateToken(context.Token, TokenTypeHints.AuthorizationCode) ?? ValidateToken(context.Token, TokenTypeHints.DeviceCode) ?? ValidateToken(context.Token, TokenTypeHints.UserCode); + if (principal == null) { + context.Reject( + error: context.EndpointType switch + { + OpenIddictServerEndpointType.Token => Errors.InvalidGrant, + _ => Errors.InvalidToken + }, + description: "The specified token is not valid."); + return default; } diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs index 89bac6e3..679fe9d3 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs @@ -1041,7 +1041,7 @@ namespace OpenIddict.Server.Owin return default; } - context.Logger.LogInformation("The response was successfully returned as an empty challenge response."); + context.Logger.LogInformation("The response was successfully returned as a challenge response: {Response}.", context.Response); context.HandleRequest(); return default; diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.cs index c91a0ae3..55a321a3 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.cs @@ -438,7 +438,7 @@ namespace OpenIddict.Server return default; } - // If the token cannot be validated, don't return an error to allow another handler to validate it. + // If the token cannot be read, don't return an error to allow another handler to validate it. if (!context.Options.JsonWebTokenHandler.CanReadToken(context.Token)) { return default; @@ -481,12 +481,37 @@ namespace OpenIddict.Server _ => throw new InvalidOperationException("The token type is not supported.") }; - // If the token cannot be validated, don't return an error to allow another handle to validate it. var result = context.Options.JsonWebTokenHandler.ValidateToken(context.Token, parameters); if (!result.IsValid) { context.Logger.LogTrace(result.Exception, "An error occurred while validating the token '{Token}'.", context.Token); + context.Reject( + error: context.EndpointType switch + { + OpenIddictServerEndpointType.Token => Errors.InvalidGrant, + _ => Errors.InvalidToken + }, + description: (result.Exception, context.EndpointType) switch + { + (SecurityTokenInvalidTypeException _, OpenIddictServerEndpointType.Token) + when context.Request.IsAuthorizationCodeGrantType() + => "The specified token is not an authorization code.", + + (SecurityTokenInvalidTypeException _, OpenIddictServerEndpointType.Token) + when context.Request.IsDeviceCodeGrantType() + => "The specified token is not an device code.", + + (SecurityTokenInvalidTypeException _, OpenIddictServerEndpointType.Token) + when context.Request.IsRefreshTokenGrantType() + => "The specified token is not a refresh token.", + + (SecurityTokenInvalidTypeException _, OpenIddictServerEndpointType.Userinfo) + => "The specified token is not an access token.", + + _ => "The specified token is not valid." + }); + return default; } diff --git a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs index b778534b..bc7b2a2a 100644 --- a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs +++ b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs @@ -547,7 +547,7 @@ namespace OpenIddict.Validation.AspNetCore return default; } - context.Logger.LogInformation("The response was successfully returned as an empty challenge response."); + context.Logger.LogInformation("The response was successfully returned as a challenge response: {Response}.", context.Response); context.HandleRequest(); return default; diff --git a/src/OpenIddict.Validation.DataProtection/OpenIddictValidationDataProtectionHandlers.cs b/src/OpenIddict.Validation.DataProtection/OpenIddictValidationDataProtectionHandlers.cs index 16dd8f68..c97a550b 100644 --- a/src/OpenIddict.Validation.DataProtection/OpenIddictValidationDataProtectionHandlers.cs +++ b/src/OpenIddict.Validation.DataProtection/OpenIddictValidationDataProtectionHandlers.cs @@ -75,7 +75,6 @@ namespace OpenIddict.Validation.DataProtection // Note: ASP.NET Core Data Protection tokens always start with "CfDJ8", that corresponds // to the base64 representation of the magic "09 F0 C9 F0" header identifying DP payloads. - // As an optimization, always ignore tokens that don't start with the "CfDJ8" string. if (string.IsNullOrEmpty(context.Token) || !context.Token.StartsWith("CfDJ8", StringComparison.Ordinal)) { return default; @@ -109,9 +108,12 @@ namespace OpenIddict.Validation.DataProtection context.Logger.LogTrace(exception, "An exception occured while deserializing the token '{Token}'.", context.Token); } - // If the token cannot be validated, don't return an error to allow another handle to validate it. if (context.Principal == null) { + context.Reject( + error: Errors.InvalidToken, + description: "The specified token is not valid."); + return default; } diff --git a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs index e888fb08..a1738790 100644 --- a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs +++ b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs @@ -553,7 +553,7 @@ namespace OpenIddict.Validation.Owin return default; } - context.Logger.LogInformation("The response was successfully returned as an empty challenge response."); + context.Logger.LogInformation("The response was successfully returned as a challenge response: {Response}.", context.Response); context.HandleRequest(); return default; diff --git a/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs b/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs index cef58b16..a62610eb 100644 --- a/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs +++ b/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs @@ -210,7 +210,7 @@ namespace OpenIddict.Validation return; } - // If the token cannot be validated, don't return an error to allow another handler to validate it. + // If the token cannot be read, don't return an error to allow another handler to validate it. if (!context.Options.JsonWebTokenHandler.CanReadToken(context.Token)) { return; @@ -245,7 +245,6 @@ namespace OpenIddict.Validation _ => throw new InvalidOperationException("The token type is not supported.") }; - // If the token cannot be validated, don't return an error to allow another handle to validate it. var result = context.Options.JsonWebTokenHandler.ValidateToken(context.Token, parameters); if (!result.IsValid) { @@ -258,6 +257,22 @@ namespace OpenIddict.Validation context.Logger.LogTrace(result.Exception, "An error occurred while validating the token '{Token}'.", context.Token); + context.Reject( + error: Errors.InvalidToken, + description: result.Exception switch + { + SecurityTokenInvalidIssuerException _ + => "The issuer associated to the specified token is not valid.", + SecurityTokenInvalidTypeException _ + => "The specified token is not of the expected type.", + SecurityTokenSignatureKeyNotFoundException _ + => "The signing key associated to the specified token was not found.", + SecurityTokenInvalidSignatureException _ + => "The signature associated to the specified token is not valid.", + + _ => "The specified token is not valid." + }); + return; }