From 96a03a9d21e8a8e975e5aa9a9af2aa410f4bc208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Tue, 22 Oct 2019 18:13:34 +0200 Subject: [PATCH] Add a ValidateClientType handler for introspection and revocation to reject requests made by public clients and containing a client secret --- .../Managers/IOpenIddictApplicationManager.cs | 2 +- .../Managers/OpenIddictApplicationManager.cs | 9 +- .../OpenIddictServerHandlers.Introspection.cs | 87 ++++++++++++++++++- .../OpenIddictServerHandlers.Revocation.cs | 87 ++++++++++++++++++- 4 files changed, 180 insertions(+), 5 deletions(-) diff --git a/src/OpenIddict.Abstractions/Managers/IOpenIddictApplicationManager.cs b/src/OpenIddict.Abstractions/Managers/IOpenIddictApplicationManager.cs index b5d45d2b..2a898bc9 100644 --- a/src/OpenIddict.Abstractions/Managers/IOpenIddictApplicationManager.cs +++ b/src/OpenIddict.Abstractions/Managers/IOpenIddictApplicationManager.cs @@ -383,7 +383,7 @@ namespace OpenIddict.Abstractions /// A that can be used to monitor the asynchronous operation, /// whose result returns a boolean indicating whether the client secret was valid. /// - ValueTask ValidateClientSecretAsync([NotNull] object application, string secret, CancellationToken cancellationToken = default); + ValueTask ValidateClientSecretAsync([NotNull] object application, [NotNull] string secret, CancellationToken cancellationToken = default); /// /// Validates the redirect_uri to ensure it's associated with an application. diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index 4ef72521..d6ee2f1d 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs @@ -1049,12 +1049,16 @@ namespace OpenIddict.Core /// whose result returns a boolean indicating whether the client secret was valid. /// public virtual async ValueTask ValidateClientSecretAsync( - [NotNull] TApplication application, string secret, CancellationToken cancellationToken = default) + [NotNull] TApplication application, [NotNull] string secret, CancellationToken cancellationToken = default) { if (application == null) { throw new ArgumentNullException(nameof(application)); } + if (string.IsNullOrEmpty(secret)) + { + throw new ArgumentException("The secret cannot be null or empty.", nameof(secret)); + } if (await IsPublicAsync(application, cancellationToken)) { @@ -1067,7 +1071,8 @@ namespace OpenIddict.Core if (string.IsNullOrEmpty(value)) { Logger.LogError("Client authentication failed for {Client} because " + - "no client secret was associated with the application."); + "no client secret was associated with the application.", + await GetClientIdAsync(application, cancellationToken)); return false; } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Introspection.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Introspection.cs index e31f4df4..cf818037 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Introspection.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Introspection.cs @@ -44,6 +44,7 @@ namespace OpenIddict.Server ValidateTokenParameter.Descriptor, ValidateClientIdParameter.Descriptor, ValidateClientId.Descriptor, + ValidateClientType.Descriptor, ValidateClientSecret.Descriptor, ValidateEndpointPermissions.Descriptor, ValidateToken.Descriptor, @@ -527,6 +528,90 @@ namespace OpenIddict.Server } } + /// + /// Contains the logic responsible of rejecting introspection requests made by applications + /// whose client type is not compatible with the presence or absence of a client secret. + /// Note: this handler is not used when the degraded mode is enabled. + /// + public class ValidateClientType : IOpenIddictServerHandler + { + private readonly IOpenIddictApplicationManager _applicationManager; + + public ValidateClientType() => throw new InvalidOperationException(new StringBuilder() + .AppendLine("The core services must be registered when enabling the OpenIddict server feature.") + .Append("To register the OpenIddict core services, reference the 'OpenIddict.Core' package ") + .AppendLine("and call 'services.AddOpenIddict().AddCore()' from 'ConfigureServices'.") + .Append("Alternatively, you can disable the built-in database-based server features by enabling ") + .Append("the degraded mode with 'services.AddOpenIddict().AddServer().EnableDegradedMode()'.") + .ToString()); + + public ValidateClientType([NotNull] IOpenIddictApplicationManager applicationManager) + => _applicationManager = applicationManager; + + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .AddFilter() + .AddFilter() + .UseScopedHandler() + .SetOrder(ValidateClientId.Descriptor.Order + 1_000) + .Build(); + + /// + /// Processes the event. + /// + /// The context associated with the event to process. + /// + /// A that can be used to monitor the asynchronous operation. + /// + public async ValueTask HandleAsync([NotNull] ValidateIntrospectionRequestContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var application = await _applicationManager.FindByClientIdAsync(context.ClientId); + if (application == null) + { + throw new InvalidOperationException("The client application details cannot be found in the database."); + } + + if (await _applicationManager.IsPublicAsync(application)) + { + // Reject introspection requests containing a client_secret when the client is a public application. + if (!string.IsNullOrEmpty(context.ClientSecret)) + { + context.Logger.LogError("The introspection request was rejected because the public application '{ClientId}' " + + "was not allowed to send a client secret.", context.ClientId); + + context.Reject( + error: Errors.InvalidRequest, + description: "The 'client_secret' parameter is not valid for this client application."); + + return; + } + + return; + } + + // Confidential and hybrid applications MUST authenticate to protect them from impersonation attacks. + if (string.IsNullOrEmpty(context.ClientSecret)) + { + context.Logger.LogError("The introspection request was rejected because the confidential or hybrid application " + + "'{ClientId}' didn't specify a client secret.", context.ClientId); + + context.Reject( + error: Errors.InvalidClient, + description: "The 'client_secret' parameter required for this client application is missing."); + + return; + } + } + } + /// /// Contains the logic responsible of rejecting introspection requests specifying an invalid client secret. /// Note: this handler is not used when the degraded mode is enabled. @@ -554,7 +639,7 @@ namespace OpenIddict.Server .AddFilter() .AddFilter() .UseScopedHandler() - .SetOrder(ValidateClientId.Descriptor.Order + 1_000) + .SetOrder(ValidateClientType.Descriptor.Order + 1_000) .Build(); /// diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Revocation.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Revocation.cs index b51b07ab..5b8fcf2e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Revocation.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Revocation.cs @@ -39,6 +39,7 @@ namespace OpenIddict.Server ValidateTokenParameter.Descriptor, ValidateClientIdParameter.Descriptor, ValidateClientId.Descriptor, + ValidateClientType.Descriptor, ValidateClientSecret.Descriptor, ValidateEndpointPermissions.Descriptor, ValidateToken.Descriptor, @@ -475,6 +476,90 @@ namespace OpenIddict.Server } } + /// + /// Contains the logic responsible of rejecting revocation requests made by applications + /// whose client type is not compatible with the presence or absence of a client secret. + /// Note: this handler is not used when the degraded mode is enabled. + /// + public class ValidateClientType : IOpenIddictServerHandler + { + private readonly IOpenIddictApplicationManager _applicationManager; + + public ValidateClientType() => throw new InvalidOperationException(new StringBuilder() + .AppendLine("The core services must be registered when enabling the OpenIddict server feature.") + .Append("To register the OpenIddict core services, reference the 'OpenIddict.Core' package ") + .AppendLine("and call 'services.AddOpenIddict().AddCore()' from 'ConfigureServices'.") + .Append("Alternatively, you can disable the built-in database-based server features by enabling ") + .Append("the degraded mode with 'services.AddOpenIddict().AddServer().EnableDegradedMode()'.") + .ToString()); + + public ValidateClientType([NotNull] IOpenIddictApplicationManager applicationManager) + => _applicationManager = applicationManager; + + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .AddFilter() + .AddFilter() + .UseScopedHandler() + .SetOrder(ValidateClientId.Descriptor.Order + 1_000) + .Build(); + + /// + /// Processes the event. + /// + /// The context associated with the event to process. + /// + /// A that can be used to monitor the asynchronous operation. + /// + public async ValueTask HandleAsync([NotNull] ValidateRevocationRequestContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var application = await _applicationManager.FindByClientIdAsync(context.ClientId); + if (application == null) + { + throw new InvalidOperationException("The client application details cannot be found in the database."); + } + + if (await _applicationManager.IsPublicAsync(application)) + { + // Reject revocation requests containing a client_secret when the client is a public application. + if (!string.IsNullOrEmpty(context.ClientSecret)) + { + context.Logger.LogError("The revocation request was rejected because the public application '{ClientId}' " + + "was not allowed to send a client secret.", context.ClientId); + + context.Reject( + error: Errors.InvalidRequest, + description: "The 'client_secret' parameter is not valid for this client application."); + + return; + } + + return; + } + + // Confidential and hybrid applications MUST authenticate to protect them from impersonation attacks. + if (string.IsNullOrEmpty(context.ClientSecret)) + { + context.Logger.LogError("The revocation request was rejected because the confidential or hybrid application " + + "'{ClientId}' didn't specify a client secret.", context.ClientId); + + context.Reject( + error: Errors.InvalidClient, + description: "The 'client_secret' parameter required for this client application is missing."); + + return; + } + } + } + /// /// Contains the logic responsible of rejecting revocation requests specifying an invalid client secret. /// Note: this handler is not used when the degraded mode is enabled. @@ -502,7 +587,7 @@ namespace OpenIddict.Server .AddFilter() .AddFilter() .UseScopedHandler() - .SetOrder(ValidateClientId.Descriptor.Order + 1_000) + .SetOrder(ValidateClientType.Descriptor.Order + 1_000) .Build(); ///