From 2fd207b59d262f8c68f9431e1fe6c56d7cfbf89a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Sun, 16 Jul 2023 17:57:14 +0200 Subject: [PATCH] Update the ValidateAsync() methods to throw ArgumentNullException synchronously --- .../Managers/OpenIddictApplicationManager.cs | 143 +++++++++--------- .../OpenIddictAuthorizationManager.cs | 55 ++++--- .../Managers/OpenIddictScopeManager.cs | 50 +++--- .../Managers/OpenIddictTokenManager.cs | 52 ++++--- 4 files changed, 159 insertions(+), 141 deletions(-) diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index 880dc297..3860e6a1 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs @@ -1133,106 +1133,111 @@ public class OpenIddictApplicationManager : IOpenIddictApplication /// The application. /// The that can be used to abort the operation. /// The validation error encountered when validating the application. - public virtual async IAsyncEnumerable ValidateAsync( - TApplication application, [EnumeratorCancellation] CancellationToken cancellationToken = default) + public virtual IAsyncEnumerable ValidateAsync( + TApplication application, CancellationToken cancellationToken = default) { if (application is null) { throw new ArgumentNullException(nameof(application)); } - // Ensure the client_id is not null or empty and is not already used for a different application. - var identifier = await Store.GetClientIdAsync(application, cancellationToken); - if (string.IsNullOrEmpty(identifier)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2036)); - } + return ExecuteAsync(cancellationToken); - else + async IAsyncEnumerable ExecuteAsync([EnumeratorCancellation] CancellationToken cancellationToken) { - // Note: depending on the database/table/query collation used by the store, an application - // whose client_id doesn't exactly match the specified value may be returned (e.g because - // the casing is different). To avoid issues when the client identifier is part of an index - // using the same collation, an error is added even if the two identifiers don't exactly match. - var other = await Store.FindByClientIdAsync(identifier, cancellationToken); - if (other is not null && !string.Equals( - await Store.GetIdAsync(other, cancellationToken), - await Store.GetIdAsync(application, cancellationToken), StringComparison.Ordinal)) + // Ensure the client_id is not null or empty and is not already used for a different application. + var identifier = await Store.GetClientIdAsync(application, cancellationToken); + if (string.IsNullOrEmpty(identifier)) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2111)); + yield return new ValidationResult(SR.GetResourceString(SR.ID2036)); } - } - - var type = await Store.GetClientTypeAsync(application, cancellationToken); - if (string.IsNullOrEmpty(type)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2050)); - } - else - { - // Ensure the application type is supported by the manager. - if (!string.Equals(type, ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase) && - !string.Equals(type, ClientTypes.Public, StringComparison.OrdinalIgnoreCase)) + else { - yield return new ValidationResult(SR.GetResourceString(SR.ID2112)); + // Note: depending on the database/table/query collation used by the store, an application + // whose client_id doesn't exactly match the specified value may be returned (e.g because + // the casing is different). To avoid issues when the client identifier is part of an index + // using the same collation, an error is added even if the two identifiers don't exactly match. + var other = await Store.FindByClientIdAsync(identifier, cancellationToken); + if (other is not null && !string.Equals( + await Store.GetIdAsync(other, cancellationToken), + await Store.GetIdAsync(application, cancellationToken), StringComparison.Ordinal)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2111)); + } } - // Ensure a client secret was specified if the client is a confidential application. - var secret = await Store.GetClientSecretAsync(application, cancellationToken); - if (string.IsNullOrEmpty(secret) && string.Equals(type, ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase)) + var type = await Store.GetClientTypeAsync(application, cancellationToken); + if (string.IsNullOrEmpty(type)) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2113)); + yield return new ValidationResult(SR.GetResourceString(SR.ID2050)); } - // Ensure no client secret was specified if the client is a public application. - else if (!string.IsNullOrEmpty(secret) && string.Equals(type, ClientTypes.Public, StringComparison.OrdinalIgnoreCase)) + else { - yield return new ValidationResult(SR.GetResourceString(SR.ID2114)); - } - } + // Ensure the application type is supported by the manager. + if (!string.Equals(type, ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase) && + !string.Equals(type, ClientTypes.Public, StringComparison.OrdinalIgnoreCase)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2112)); + } - // When callback URIs are specified, ensure they are valid and spec-compliant. - // See https://tools.ietf.org/html/rfc6749#section-3.1 for more information. - foreach (var uri in ImmutableArray.Create() - .AddRange(await Store.GetPostLogoutRedirectUrisAsync(application, cancellationToken)) - .AddRange(await Store.GetRedirectUrisAsync(application, cancellationToken))) - { - // Ensure the URI is not null or empty. - if (string.IsNullOrEmpty(uri)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2061)); + // Ensure a client secret was specified if the client is a confidential application. + var secret = await Store.GetClientSecretAsync(application, cancellationToken); + if (string.IsNullOrEmpty(secret) && string.Equals(type, ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2113)); + } - break; + // Ensure no client secret was specified if the client is a public application. + else if (!string.IsNullOrEmpty(secret) && string.Equals(type, ClientTypes.Public, StringComparison.OrdinalIgnoreCase)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2114)); + } } - // Ensure the URI is a valid absolute URI. - if (!Uri.TryCreate(uri, UriKind.Absolute, out Uri? value) || !value.IsWellFormedOriginalString()) + // When callback URIs are specified, ensure they are valid and spec-compliant. + // See https://tools.ietf.org/html/rfc6749#section-3.1 for more information. + foreach (var uri in ImmutableArray.Create() + .AddRange(await Store.GetPostLogoutRedirectUrisAsync(application, cancellationToken)) + .AddRange(await Store.GetRedirectUrisAsync(application, cancellationToken))) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2062)); + // Ensure the URI is not null or empty. + if (string.IsNullOrEmpty(uri)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2061)); - break; - } + break; + } - // Ensure the URI doesn't contain a fragment. - if (!string.IsNullOrEmpty(value.Fragment)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2115)); + // Ensure the URI is a valid absolute URI. + if (!Uri.TryCreate(uri, UriKind.Absolute, out Uri? value) || !value.IsWellFormedOriginalString()) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2062)); - break; - } + break; + } - // To prevent issuer fixation attacks where a malicious client would specify an "iss" parameter - // in the callback URI, ensure the query - if present - doesn't include an "iss" parameter. - if (!string.IsNullOrEmpty(value.Query)) - { - var parameters = OpenIddictHelpers.ParseQuery(value.Query); - if (parameters.ContainsKey(Parameters.Iss)) + // Ensure the URI doesn't contain a fragment. + if (!string.IsNullOrEmpty(value.Fragment)) { - yield return new ValidationResult(SR.FormatID2134(Parameters.Iss)); + yield return new ValidationResult(SR.GetResourceString(SR.ID2115)); break; } + + // To prevent issuer fixation attacks where a malicious client would specify an "iss" parameter + // in the callback URI, ensure the query - if present - doesn't include an "iss" parameter. + if (!string.IsNullOrEmpty(value.Query)) + { + var parameters = OpenIddictHelpers.ParseQuery(value.Query); + if (parameters.ContainsKey(Parameters.Iss)) + { + yield return new ValidationResult(SR.FormatID2134(Parameters.Iss)); + + break; + } + } } } } diff --git a/src/OpenIddict.Core/Managers/OpenIddictAuthorizationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictAuthorizationManager.cs index d2e8719c..7cd65946 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictAuthorizationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictAuthorizationManager.cs @@ -1155,46 +1155,51 @@ public class OpenIddictAuthorizationManager : IOpenIddictAuthori /// The authorization. /// The that can be used to abort the operation. /// The validation error encountered when validating the authorization. - public virtual async IAsyncEnumerable ValidateAsync( - TAuthorization authorization, [EnumeratorCancellation] CancellationToken cancellationToken = default) + public virtual IAsyncEnumerable ValidateAsync( + TAuthorization authorization, CancellationToken cancellationToken = default) { if (authorization is null) { throw new ArgumentNullException(nameof(authorization)); } - var type = await Store.GetTypeAsync(authorization, cancellationToken); - if (string.IsNullOrEmpty(type)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2116)); - } - - else if (!string.Equals(type, AuthorizationTypes.AdHoc, StringComparison.OrdinalIgnoreCase) && - !string.Equals(type, AuthorizationTypes.Permanent, StringComparison.OrdinalIgnoreCase)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2117)); - } + return ExecuteAsync(cancellationToken); - if (string.IsNullOrEmpty(await Store.GetStatusAsync(authorization, cancellationToken))) + async IAsyncEnumerable ExecuteAsync([EnumeratorCancellation] CancellationToken cancellationToken) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2038)); - } + var type = await Store.GetTypeAsync(authorization, cancellationToken); + if (string.IsNullOrEmpty(type)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2116)); + } - // Ensure that the scopes are not null or empty and do not contain spaces. - foreach (var scope in await Store.GetScopesAsync(authorization, cancellationToken)) - { - if (string.IsNullOrEmpty(scope)) + else if (!string.Equals(type, AuthorizationTypes.AdHoc, StringComparison.OrdinalIgnoreCase) && + !string.Equals(type, AuthorizationTypes.Permanent, StringComparison.OrdinalIgnoreCase)) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2039)); + yield return new ValidationResult(SR.GetResourceString(SR.ID2117)); + } - break; + if (string.IsNullOrEmpty(await Store.GetStatusAsync(authorization, cancellationToken))) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2038)); } - if (scope.Contains(Separators.Space[0])) + // Ensure that the scopes are not null or empty and do not contain spaces. + foreach (var scope in await Store.GetScopesAsync(authorization, cancellationToken)) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2042)); + if (string.IsNullOrEmpty(scope)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2039)); - break; + break; + } + + if (scope.Contains(Separators.Space[0])) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2042)); + + break; + } } } } diff --git a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs index 9741b785..072fb5e2 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs @@ -918,39 +918,43 @@ public class OpenIddictScopeManager : IOpenIddictScopeManager where TSco /// The scope. /// The that can be used to abort the operation. /// The validation error encountered when validating the scope. - public virtual async IAsyncEnumerable ValidateAsync( - TScope scope, [EnumeratorCancellation] CancellationToken cancellationToken = default) + public virtual IAsyncEnumerable ValidateAsync(TScope scope, CancellationToken cancellationToken = default) { if (scope is null) { throw new ArgumentNullException(nameof(scope)); } - // Ensure the name is not null or empty, does not contain a - // space and is not already used for a different scope entity. - var name = await Store.GetNameAsync(scope, cancellationToken); - if (string.IsNullOrEmpty(name)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2044)); - } + return ExecuteAsync(cancellationToken); - else if (name!.Contains(Separators.Space[0])) + async IAsyncEnumerable ExecuteAsync([EnumeratorCancellation] CancellationToken cancellationToken) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2045)); - } + // Ensure the name is not null or empty, does not contain a + // space and is not already used for a different scope entity. + var name = await Store.GetNameAsync(scope, cancellationToken); + if (string.IsNullOrEmpty(name)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2044)); + } - else - { - // Note: depending on the database/table/query collation used by the store, a scope - // whose name doesn't exactly match the specified value may be returned (e.g because - // the casing is different). To avoid issues when the scope name is part of an index - // using the same collation, an error is added even if the two names don't exactly match. - var other = await Store.FindByNameAsync(name, cancellationToken); - if (other is not null && !string.Equals( - await Store.GetIdAsync(other, cancellationToken), - await Store.GetIdAsync(scope, cancellationToken), StringComparison.Ordinal)) + else if (name!.Contains(Separators.Space[0])) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2045)); + } + + else { - yield return new ValidationResult(SR.GetResourceString(SR.ID2060)); + // Note: depending on the database/table/query collation used by the store, a scope + // whose name doesn't exactly match the specified value may be returned (e.g because + // the casing is different). To avoid issues when the scope name is part of an index + // using the same collation, an error is added even if the two names don't exactly match. + var other = await Store.FindByNameAsync(name, cancellationToken); + if (other is not null && !string.Equals( + await Store.GetIdAsync(other, cancellationToken), + await Store.GetIdAsync(scope, cancellationToken), StringComparison.Ordinal)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2060)); + } } } } diff --git a/src/OpenIddict.Core/Managers/OpenIddictTokenManager.cs b/src/OpenIddict.Core/Managers/OpenIddictTokenManager.cs index 3c4b3b27..a0ebd3cd 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictTokenManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictTokenManager.cs @@ -1275,41 +1275,45 @@ public class OpenIddictTokenManager : IOpenIddictTokenManager where TTok /// The token. /// The that can be used to abort the operation. /// The validation error encountered when validating the token. - public virtual async IAsyncEnumerable ValidateAsync( - TToken token, [EnumeratorCancellation] CancellationToken cancellationToken = default) + public virtual IAsyncEnumerable ValidateAsync(TToken token, CancellationToken cancellationToken = default) { if (token is null) { throw new ArgumentNullException(nameof(token)); } - // If a reference identifier was associated with the token, - // ensure it's not already used for a different token. - var identifier = await Store.GetReferenceIdAsync(token, cancellationToken); - if (!string.IsNullOrEmpty(identifier)) + return ExecuteAsync(cancellationToken); + + async IAsyncEnumerable ExecuteAsync([EnumeratorCancellation] CancellationToken cancellationToken) { - // Note: depending on the database/table/query collation used by the store, a reference token - // whose identifier doesn't exactly match the specified value may be returned (e.g because - // the casing is different). To avoid issues when the reference identifier is part of an index - // using the same collation, an error is added even if the two identifiers don't exactly match. - var other = await Store.FindByReferenceIdAsync(identifier, cancellationToken); - if (other is not null && !string.Equals( - await Store.GetIdAsync(other, cancellationToken), - await Store.GetIdAsync(token, cancellationToken), StringComparison.Ordinal)) + // If a reference identifier was associated with the token, + // ensure it's not already used for a different token. + var identifier = await Store.GetReferenceIdAsync(token, cancellationToken); + if (!string.IsNullOrEmpty(identifier)) { - yield return new ValidationResult(SR.GetResourceString(SR.ID2085)); + // Note: depending on the database/table/query collation used by the store, a reference token + // whose identifier doesn't exactly match the specified value may be returned (e.g because + // the casing is different). To avoid issues when the reference identifier is part of an index + // using the same collation, an error is added even if the two identifiers don't exactly match. + var other = await Store.FindByReferenceIdAsync(identifier, cancellationToken); + if (other is not null && !string.Equals( + await Store.GetIdAsync(other, cancellationToken), + await Store.GetIdAsync(token, cancellationToken), StringComparison.Ordinal)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2085)); + } } - } - var type = await Store.GetTypeAsync(token, cancellationToken); - if (string.IsNullOrEmpty(type)) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2086)); - } + var type = await Store.GetTypeAsync(token, cancellationToken); + if (string.IsNullOrEmpty(type)) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2086)); + } - if (string.IsNullOrEmpty(await Store.GetStatusAsync(token, cancellationToken))) - { - yield return new ValidationResult(SR.GetResourceString(SR.ID2038)); + if (string.IsNullOrEmpty(await Store.GetStatusAsync(token, cancellationToken))) + { + yield return new ValidationResult(SR.GetResourceString(SR.ID2038)); + } } }