diff --git a/samples/Mvc.Server/Controllers/AuthorizationController.cs b/samples/Mvc.Server/Controllers/AuthorizationController.cs index 05339f8e..4ec3de0f 100644 --- a/samples/Mvc.Server/Controllers/AuthorizationController.cs +++ b/samples/Mvc.Server/Controllers/AuthorizationController.cs @@ -5,18 +5,17 @@ */ using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics; -using System.Linq; +using System.Security.Claims; using System.Threading.Tasks; using AspNet.Security.OpenIdConnect.Extensions; using AspNet.Security.OpenIdConnect.Primitives; using AspNet.Security.OpenIdConnect.Server; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; -using Microsoft.Extensions.Options; using Mvc.Server.Helpers; using Mvc.Server.Models; using Mvc.Server.ViewModels.Authorization; @@ -29,18 +28,15 @@ namespace Mvc.Server public class AuthorizationController : Controller { private readonly OpenIddictApplicationManager _applicationManager; - private readonly IOptions _identityOptions; private readonly SignInManager _signInManager; private readonly UserManager _userManager; public AuthorizationController( OpenIddictApplicationManager applicationManager, - IOptions identityOptions, SignInManager signInManager, UserManager userManager) { _applicationManager = applicationManager; - _identityOptions = identityOptions; _signInManager = signInManager; _userManager = userManager; } @@ -242,51 +238,60 @@ namespace Mvc.Server if (!request.IsAuthorizationCodeGrantType() && !request.IsRefreshTokenGrantType()) { - // Set the list of scopes granted to the client application. - // Note: the offline_access scope must be granted - // to allow OpenIddict to return a refresh token. - ticket.SetScopes(new[] - { - OpenIdConnectConstants.Scopes.OpenId, - OpenIdConnectConstants.Scopes.Email, - OpenIdConnectConstants.Scopes.Profile, - OpenIdConnectConstants.Scopes.OfflineAccess, - OpenIddictConstants.Scopes.Roles - }.Intersect(request.GetScopes())); + // Note: in this sample, the granted scopes match the requested scope + // but you may want to allow the user to uncheck specific scopes. + // For that, simply restrict the list of scopes before calling SetScopes. + ticket.SetScopes(request.GetScopes()); + ticket.SetResources("resource_server"); } - ticket.SetResources("resource_server"); + foreach (var claim in ticket.Principal.Claims) + { + claim.SetDestinations(GetDestinations(claim, ticket)); + } + + return ticket; + } + private IEnumerable GetDestinations(Claim claim, AuthenticationTicket ticket) + { // Note: by default, claims are NOT automatically included in the access and identity tokens. // To allow OpenIddict to serialize them, you must attach them a destination, that specifies // whether they should be included in access tokens, in identity tokens or in both. - foreach (var claim in ticket.Principal.Claims) + switch (claim.Type) { - // Never include the security stamp in the access and identity tokens, as it's a secret value. - if (claim.Type == _identityOptions.Value.ClaimsIdentity.SecurityStampClaimType) - { - continue; - } + case OpenIdConnectConstants.Claims.Name: + yield return OpenIdConnectConstants.Destinations.AccessToken; - var destinations = new List - { - OpenIdConnectConstants.Destinations.AccessToken - }; - - // Only add the iterated claim to the id_token if the corresponding scope was granted to the client application. - // The other claims will only be added to the access_token, which is encrypted when using the default format. - if ((claim.Type == OpenIdConnectConstants.Claims.Name && ticket.HasScope(OpenIdConnectConstants.Scopes.Profile)) || - (claim.Type == OpenIdConnectConstants.Claims.Email && ticket.HasScope(OpenIdConnectConstants.Scopes.Email)) || - (claim.Type == OpenIdConnectConstants.Claims.Role && ticket.HasScope(OpenIddictConstants.Claims.Roles))) - { - destinations.Add(OpenIdConnectConstants.Destinations.IdentityToken); - } + if (ticket.HasScope(OpenIdConnectConstants.Scopes.Profile)) + yield return OpenIdConnectConstants.Destinations.IdentityToken; - claim.SetDestinations(destinations); - } + yield break; - return ticket; + case OpenIdConnectConstants.Claims.Email: + yield return OpenIdConnectConstants.Destinations.AccessToken; + + if (ticket.HasScope(OpenIdConnectConstants.Scopes.Email)) + yield return OpenIdConnectConstants.Destinations.IdentityToken; + + yield break; + + case OpenIdConnectConstants.Claims.Role: + yield return OpenIdConnectConstants.Destinations.AccessToken; + + if (ticket.HasScope(OpenIddictConstants.Claims.Roles)) + yield return OpenIdConnectConstants.Destinations.IdentityToken; + + yield break; + + // Never include the security stamp in the access and identity tokens, as it's a secret value. + case "AspNet.Identity.SecurityStamp": yield break; + + default: + yield return OpenIdConnectConstants.Destinations.AccessToken; + yield break; + } } } } \ No newline at end of file diff --git a/samples/Mvc.Server/Mvc.Server.csproj b/samples/Mvc.Server/Mvc.Server.csproj index 5809a301..7dc4f783 100644 --- a/samples/Mvc.Server/Mvc.Server.csproj +++ b/samples/Mvc.Server/Mvc.Server.csproj @@ -12,11 +12,6 @@ - - - PreserveNewest - PreserveNewest - diff --git a/samples/Mvc.Server/Startup.cs b/samples/Mvc.Server/Startup.cs index bc1af39c..12b4fdfb 100644 --- a/samples/Mvc.Server/Startup.cs +++ b/samples/Mvc.Server/Startup.cs @@ -15,19 +15,21 @@ namespace Mvc.Server { public class Startup { - public void ConfigureServices(IServiceCollection services) + public Startup(IConfiguration configuration) { - var configuration = new ConfigurationBuilder() - .AddJsonFile("config.json") - .AddEnvironmentVariables() - .Build(); + Configuration = configuration; + } + + public IConfiguration Configuration { get; } + public void ConfigureServices(IServiceCollection services) + { services.AddMvc(); services.AddDbContext(options => { // Configure the context to use Microsoft SQL Server. - options.UseSqlServer(configuration["Data:DefaultConnection:ConnectionString"]); + options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection")); // Register the entity sets needed by OpenIddict. // Note: use the generic overload if you need @@ -88,8 +90,9 @@ namespace Mvc.Server .AllowPasswordFlow() .AllowRefreshTokenFlow(); - // Mark the "profile" scope as a supported scope in the discovery document. - options.RegisterScopes(OpenIdConnectConstants.Scopes.Profile); + // Mark the "email" and "profile" scopes as supported scopes. + options.RegisterScopes(OpenIdConnectConstants.Scopes.Email, + OpenIdConnectConstants.Scopes.Profile); // Make the "client_id" parameter mandatory when sending a token request. options.RequireClientIdentification(); @@ -101,6 +104,10 @@ namespace Mvc.Server // an external authentication provider like Google, Facebook or Twitter. options.EnableRequestCaching(); + // Enable scope validation, so that authorization and token requests + // that specify unregistered scopes are automatically rejected. + options.EnableScopeValidation(); + // During development, you can disable the HTTPS requirement. options.DisableHttpsRequirement(); diff --git a/samples/Mvc.Server/appsettings.json b/samples/Mvc.Server/appsettings.json new file mode 100644 index 00000000..a4d0b394 --- /dev/null +++ b/samples/Mvc.Server/appsettings.json @@ -0,0 +1,5 @@ +{ + "ConnectionStrings": { + "DefaultConnection": "Server=(localdb)\\mssqllocaldb;Database=aspnet5-openiddict-sample-12340be6-0442-4622-b782-a7412bb7d045;Trusted_Connection=True;MultipleActiveResultSets=true" + } +} diff --git a/samples/Mvc.Server/config.json b/samples/Mvc.Server/config.json deleted file mode 100644 index 9b4d6e3b..00000000 --- a/samples/Mvc.Server/config.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "Data": { - "DefaultConnection": { - "ConnectionString": "Server=(localdb)\\mssqllocaldb;Database=aspnet5-openiddict-sample-12340be6-0442-4622-b782-a7412bb7d045;Trusted_Connection=True;MultipleActiveResultSets=true" - } - } -} diff --git a/src/OpenIddict.Core/Descriptors/OpenIddictScopeDescriptor.cs b/src/OpenIddict.Core/Descriptors/OpenIddictScopeDescriptor.cs index 9900a704..82f4418d 100644 --- a/src/OpenIddict.Core/Descriptors/OpenIddictScopeDescriptor.cs +++ b/src/OpenIddict.Core/Descriptors/OpenIddictScopeDescriptor.cs @@ -1,4 +1,7 @@ -namespace OpenIddict.Core +using System; +using System.Collections.Generic; + +namespace OpenIddict.Core { /// /// Represents an OpenIddict scope descriptor. @@ -16,5 +19,11 @@ /// associated with the scope. /// public virtual string Name { get; set; } + + /// + /// Gets the resources associated with the scope. + /// + public virtual ISet Resources { get; } + = new HashSet(StringComparer.Ordinal); } } diff --git a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs index cb996ea8..ca3b536d 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs @@ -5,6 +5,7 @@ */ using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -295,7 +296,27 @@ namespace OpenIddict.Core throw new ArgumentNullException(nameof(scope)); } - return Store.GetIdAsync(scope, cancellationToken); + return Store.GetNameAsync(scope, cancellationToken); + } + + /// + /// Retrieves the resources associated with a scope. + /// + /// The scope. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, + /// whose result returns all the resources associated with the scope. + /// + public virtual Task> GetResourcesAsync( + [NotNull] TScope scope, CancellationToken cancellationToken = default) + { + if (scope == null) + { + throw new ArgumentNullException(nameof(scope)); + } + + return Store.GetResourcesAsync(scope, cancellationToken); } /// @@ -354,6 +375,39 @@ namespace OpenIddict.Core return Store.ListAsync(query, state, cancellationToken); } + /// + /// Lists all the resources associated with the specified scopes. + /// + /// The scopes. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, + /// whose result returns all the resources associated with the specified scopes. + /// + public virtual async Task> ListResourcesAsync( + ImmutableArray scopes, CancellationToken cancellationToken = default) + { + if (scopes.IsDefaultOrEmpty) + { + return ImmutableArray.Create(); + } + + var set = new HashSet(StringComparer.Ordinal); + + foreach (var scope in await FindByNamesAsync(scopes, cancellationToken)) + { + var resources = await GetResourcesAsync(scope, cancellationToken); + if (resources.IsDefaultOrEmpty) + { + continue; + } + + set.UnionWith(resources); + } + + return set.ToImmutableArray(); + } + /// /// Updates an existing scope. /// @@ -401,6 +455,8 @@ namespace OpenIddict.Core Name = await Store.GetNameAsync(scope, cancellationToken) }; + descriptor.Resources.UnionWith(await Store.GetResourcesAsync(scope, cancellationToken)); + await operation(descriptor); await PopulateAsync(scope, descriptor, cancellationToken); await UpdateAsync(scope, cancellationToken); @@ -433,34 +489,6 @@ namespace OpenIddict.Core return results.ToImmutable(); } - /// - /// Validates the list of scopes to ensure they correspond to existing elements in the database. - /// - /// The scopes. - /// The that can be used to abort the operation. - /// true if the list of scopes is valid, false otherwise. - public virtual async Task ValidateScopesAsync(ImmutableArray scopes, CancellationToken cancellationToken = default) - { - if (scopes.Length == 0) - { - return true; - } - - async Task> GetScopesAsync() - { - var names = ImmutableHashSet.CreateBuilder(StringComparer.Ordinal); - - foreach (var scope in await FindByNamesAsync(scopes, cancellationToken)) - { - names.Add(await GetNameAsync(scope, cancellationToken)); - } - - return names.ToImmutable(); - } - - return (await GetScopesAsync()).IsSupersetOf(scopes); - } - /// /// Populates the scope using the specified descriptor. /// @@ -484,7 +512,8 @@ namespace OpenIddict.Core } await Store.SetDescriptionAsync(scope, descriptor.Description, cancellationToken); - await Store.SetNameAsync(scope, descriptor.Description, cancellationToken); + await Store.SetNameAsync(scope, descriptor.Name, cancellationToken); + await Store.SetResourcesAsync(scope, descriptor.Resources.ToImmutableArray(), cancellationToken); } } } \ No newline at end of file diff --git a/src/OpenIddict.Core/Stores/IOpenIddictScopeStore.cs b/src/OpenIddict.Core/Stores/IOpenIddictScopeStore.cs index 7cc9b41c..69b662ab 100644 --- a/src/OpenIddict.Core/Stores/IOpenIddictScopeStore.cs +++ b/src/OpenIddict.Core/Stores/IOpenIddictScopeStore.cs @@ -155,6 +155,17 @@ namespace OpenIddict.Core /// Task GetPropertiesAsync([NotNull] TScope scope, CancellationToken cancellationToken); + /// + /// Retrieves the resources associated with a scope. + /// + /// The scope. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, + /// whose result returns all the resources associated with the scope. + /// + Task> GetResourcesAsync([NotNull] TScope scope, CancellationToken cancellationToken); + /// /// Instantiates a new scope. /// @@ -226,6 +237,17 @@ namespace OpenIddict.Core /// Task SetPropertiesAsync([NotNull] TScope scope, [CanBeNull] JObject properties, CancellationToken cancellationToken); + /// + /// Sets the resources associated with a scope. + /// + /// The scope. + /// The resources associated with the scope. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation. + /// + Task SetResourcesAsync([NotNull] TScope scope, ImmutableArray resources, CancellationToken cancellationToken); + /// /// Updates an existing scope. /// diff --git a/src/OpenIddict.Core/Stores/OpenIddictApplicationStore.cs b/src/OpenIddict.Core/Stores/OpenIddictApplicationStore.cs index 715bf9a7..d183a01e 100644 --- a/src/OpenIddict.Core/Stores/OpenIddictApplicationStore.cs +++ b/src/OpenIddict.Core/Stores/OpenIddictApplicationStore.cs @@ -758,7 +758,7 @@ namespace OpenIddict.Core { if (string.IsNullOrEmpty(identifier)) { - return default(TKey); + return default; } return (TKey) TypeDescriptor.GetConverter(typeof(TKey)).ConvertFromInvariantString(identifier); diff --git a/src/OpenIddict.Core/Stores/OpenIddictAuthorizationStore.cs b/src/OpenIddict.Core/Stores/OpenIddictAuthorizationStore.cs index 147be8f1..e6b468eb 100644 --- a/src/OpenIddict.Core/Stores/OpenIddictAuthorizationStore.cs +++ b/src/OpenIddict.Core/Stores/OpenIddictAuthorizationStore.cs @@ -568,7 +568,7 @@ namespace OpenIddict.Core { if (string.IsNullOrEmpty(identifier)) { - return default(TKey); + return default; } return (TKey) TypeDescriptor.GetConverter(typeof(TKey)).ConvertFromInvariantString(identifier); diff --git a/src/OpenIddict.Core/Stores/OpenIddictScopeStore.cs b/src/OpenIddict.Core/Stores/OpenIddictScopeStore.cs index 31c1c1a2..0ed07b04 100644 --- a/src/OpenIddict.Core/Stores/OpenIddictScopeStore.cs +++ b/src/OpenIddict.Core/Stores/OpenIddictScopeStore.cs @@ -258,6 +258,42 @@ namespace OpenIddict.Core return Task.FromResult(JObject.Parse(scope.Properties)); } + /// + /// Retrieves the resources associated with a scope. + /// + /// The scope. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, + /// whose result returns all the resources associated with the scope. + /// + public virtual Task> GetResourcesAsync([NotNull] TScope scope, CancellationToken cancellationToken) + { + if (scope == null) + { + throw new ArgumentNullException(nameof(scope)); + } + + if (string.IsNullOrEmpty(scope.Resources)) + { + return Task.FromResult(ImmutableArray.Create()); + } + + // Note: parsing the stringified resources is an expensive operation. + // To mitigate that, the resulting array is stored in the memory cache. + var key = string.Concat(nameof(GetResourcesAsync), "\x1e", scope.Resources); + + var resources = Cache.Get(key) as ImmutableArray?; + if (resources == null) + { + resources = Cache.Set(key, JArray.Parse(scope.Resources) + .Select(element => (string) element) + .ToImmutableArray()); + } + + return Task.FromResult(resources.GetValueOrDefault()); + } + /// /// Instantiates a new scope. /// @@ -386,6 +422,34 @@ namespace OpenIddict.Core return Task.CompletedTask; } + /// + /// Sets the resources associated with a scope. + /// + /// The scope. + /// The resources associated with the scope. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation. + /// + public virtual Task SetResourcesAsync([NotNull] TScope scope, ImmutableArray resources, CancellationToken cancellationToken) + { + if (scope == null) + { + throw new ArgumentNullException(nameof(scope)); + } + + if (resources.IsDefaultOrEmpty) + { + scope.Resources = null; + + return Task.CompletedTask; + } + + scope.Resources = new JArray(resources.ToArray()).ToString(Formatting.None); + + return Task.CompletedTask; + } + /// /// Updates an existing scope. /// @@ -405,7 +469,7 @@ namespace OpenIddict.Core { if (string.IsNullOrEmpty(identifier)) { - return default(TKey); + return default; } return (TKey) TypeDescriptor.GetConverter(typeof(TKey)).ConvertFromInvariantString(identifier); diff --git a/src/OpenIddict.Core/Stores/OpenIddictTokenStore.cs b/src/OpenIddict.Core/Stores/OpenIddictTokenStore.cs index 30db9297..73142d9c 100644 --- a/src/OpenIddict.Core/Stores/OpenIddictTokenStore.cs +++ b/src/OpenIddict.Core/Stores/OpenIddictTokenStore.cs @@ -797,7 +797,7 @@ namespace OpenIddict.Core { if (string.IsNullOrEmpty(identifier)) { - return default(TKey); + return default; } return (TKey) TypeDescriptor.GetConverter(typeof(TKey)).ConvertFromInvariantString(identifier); diff --git a/src/OpenIddict.EntityFramework/OpenIddictExtensions.cs b/src/OpenIddict.EntityFramework/OpenIddictExtensions.cs index f195e345..431e5c7c 100644 --- a/src/OpenIddict.EntityFramework/OpenIddictExtensions.cs +++ b/src/OpenIddict.EntityFramework/OpenIddictExtensions.cs @@ -251,7 +251,9 @@ namespace Microsoft.Extensions.DependencyInjection builder.Entity() .Property(scope => scope.Name) - .IsRequired(); + .IsRequired() + .HasMaxLength(450) + .HasColumnAnnotation(IndexAnnotation.AnnotationName, new IndexAnnotation(new IndexAttribute())); builder.Entity() .ToTable("OpenIddictScopes"); diff --git a/src/OpenIddict.EntityFrameworkCore/OpenIddictExtensions.cs b/src/OpenIddict.EntityFrameworkCore/OpenIddictExtensions.cs index 31cb2804..a18074a7 100644 --- a/src/OpenIddict.EntityFrameworkCore/OpenIddictExtensions.cs +++ b/src/OpenIddict.EntityFrameworkCore/OpenIddictExtensions.cs @@ -274,6 +274,9 @@ namespace Microsoft.Extensions.DependencyInjection { entity.HasKey(scope => scope.Id); + entity.HasIndex(scope => scope.Name) + .IsUnique(); + entity.Property(scope => scope.ConcurrencyToken) .IsConcurrencyToken(); diff --git a/src/OpenIddict.Models/OpenIddictScope.cs b/src/OpenIddict.Models/OpenIddictScope.cs index 2b726c31..3d07847a 100644 --- a/src/OpenIddict.Models/OpenIddictScope.cs +++ b/src/OpenIddict.Models/OpenIddictScope.cs @@ -53,5 +53,11 @@ namespace OpenIddict.Models /// or null if no bag was associated with the current scope. /// public virtual string Properties { get; set; } + + /// + /// Gets or sets the resources associated with the + /// current scope, serialized as a JSON array. + /// + public virtual string Resources { get; set; } } } diff --git a/src/OpenIddict/OpenIddictExtensions.cs b/src/OpenIddict/OpenIddictExtensions.cs index 996d7c6d..021800f3 100644 --- a/src/OpenIddict/OpenIddictExtensions.cs +++ b/src/OpenIddict/OpenIddictExtensions.cs @@ -11,6 +11,7 @@ using System.IO; using System.Linq; using System.Reflection; using System.Security.Cryptography.X509Certificates; +using System.Threading; using AspNet.Security.OpenIdConnect.Primitives; using AspNet.Security.OpenIdConnect.Server; using JetBrains.Annotations; @@ -687,6 +688,22 @@ namespace Microsoft.Extensions.DependencyInjection return builder.Configure(options => options.RevocationEndpointPath = path); } + /// + /// Rejects authorization and token requests that specify scopes that have not been + /// registered in the database using + /// or . + /// + /// The services builder used by OpenIddict to register new services. + public static OpenIddictBuilder EnableScopeValidation([NotNull] this OpenIddictBuilder builder) + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + return builder.Configure(options => options.EnableScopeValidation = true); + } + /// /// Enables the token endpoint. /// @@ -731,21 +748,6 @@ namespace Microsoft.Extensions.DependencyInjection return builder.Configure(options => options.UserinfoEndpointPath = path); } - /// - /// Rejects authorization and token requests that specify scopes that have not been - /// registered in the database using . - /// - /// The services builder used by OpenIddict to register new services. - public static OpenIddictBuilder ValidateScopes([NotNull] this OpenIddictBuilder builder) - { - if (builder == null) - { - throw new ArgumentNullException(nameof(builder)); - } - - return builder.Configure(options => options.ValidateScopes = true); - } - /// /// Makes client identification mandatory so that token and revocation /// requests that don't specify a client_id are automatically rejected. diff --git a/src/OpenIddict/OpenIddictOptions.cs b/src/OpenIddict/OpenIddictOptions.cs index 8a8734c7..9492197b 100644 --- a/src/OpenIddict/OpenIddictOptions.cs +++ b/src/OpenIddict/OpenIddictOptions.cs @@ -47,6 +47,11 @@ namespace OpenIddict OpenIdConnectConstants.Claims.Subject }; + /// + /// Gets or sets a boolean indicating whether scope validation is enabled. + /// + public bool EnableScopeValidation { get; set; } + /// /// Gets or sets a boolean indicating whether token revocation should be disabled. /// When disabled, authorization code and refresh tokens are not stored @@ -73,12 +78,6 @@ namespace OpenIddict /// public RandomNumberGenerator RandomNumberGenerator { get; set; } = RandomNumberGenerator.Create(); - /// - /// Gets or sets a boolean indicating whether scopes that are not explicitly registered - /// in the database are automatically rejected. This option is not enabled by default. - /// - public bool ValidateScopes { get; set; } - /// /// Gets or sets a boolean determining whether client identification is required. /// Enabling this option requires registering a client application and sending a diff --git a/src/OpenIddict/OpenIddictProvider.Authentication.cs b/src/OpenIddict/OpenIddictProvider.Authentication.cs index 5f1691ab..1c25d163 100644 --- a/src/OpenIddict/OpenIddictProvider.Authentication.cs +++ b/src/OpenIddict/OpenIddictProvider.Authentication.cs @@ -166,22 +166,6 @@ namespace OpenIddict return; } - // If the corresponding option was enabled, reject the request if scopes can't be validated. - if (options.ValidateScopes && !await Scopes.ValidateScopesAsync( - context.Request.GetScopes() - .ToImmutableArray() - .Remove(OpenIdConnectConstants.Scopes.OfflineAccess) - .Remove(OpenIdConnectConstants.Scopes.OpenId))) - { - Logger.LogError("The authorization request was rejected because an unregistered scope was specified."); - - context.Reject( - error: OpenIdConnectConstants.Errors.InvalidRequest, - description: "The specified 'scope' parameter is not valid."); - - return; - } - // Reject authorization requests that specify scope=offline_access if the refresh token flow is not enabled. if (context.Request.HasScope(OpenIdConnectConstants.Scopes.OfflineAccess) && !options.GrantTypes.Contains(OpenIdConnectConstants.GrantTypes.RefreshToken)) @@ -193,6 +177,23 @@ namespace OpenIddict return; } + // Validates scopes, unless scope validation was explicitly disabled. + foreach (var scope in context.Request.GetScopes()) + { + if (options.EnableScopeValidation && !options.Scopes.Contains(scope) && + await Scopes.FindByNameAsync(scope) == null) + { + Logger.LogError("The authorization request was rejected because an " + + "unregistered scope was specified: {Scope}.", scope); + + context.Reject( + error: OpenIdConnectConstants.Errors.InvalidRequest, + description: "The specified 'scope' parameter is not valid."); + + return; + } + } + // Note: the OpenID Connect server middleware supports the query, form_post and fragment response modes // and doesn't reject unknown/custom modes until the ApplyAuthorizationResponse event is invoked. // To ensure authorization requests are rejected early enough, an additional check is made by OpenIddict. diff --git a/src/OpenIddict/OpenIddictProvider.Exchange.cs b/src/OpenIddict/OpenIddictProvider.Exchange.cs index 34b10159..040874c2 100644 --- a/src/OpenIddict/OpenIddictProvider.Exchange.cs +++ b/src/OpenIddict/OpenIddictProvider.Exchange.cs @@ -62,22 +62,6 @@ namespace OpenIddict return; } - // If the corresponding option was enabled, reject the request if scopes can't be validated. - if (options.ValidateScopes && !await Scopes.ValidateScopesAsync( - context.Request.GetScopes() - .ToImmutableArray() - .Remove(OpenIdConnectConstants.Scopes.OfflineAccess) - .Remove(OpenIdConnectConstants.Scopes.OpenId))) - { - Logger.LogError("The token request was rejected because an unregistered scope was specified."); - - context.Reject( - error: OpenIdConnectConstants.Errors.InvalidRequest, - description: "The specified 'scope' parameter is not valid."); - - return; - } - // Note: the OpenID Connect server middleware allows returning a refresh token with grant_type=client_credentials, // though it's usually not recommended by the OAuth2 specification. To encourage developers to make a new // grant_type=client_credentials request instead of using refresh tokens, OpenIddict uses a stricter policy @@ -93,6 +77,23 @@ namespace OpenIddict return; } + // Validates scopes, unless scope validation was explicitly disabled. + foreach (var scope in context.Request.GetScopes()) + { + if (options.EnableScopeValidation && !options.Scopes.Contains(scope) && + await Scopes.FindByNameAsync(scope) == null) + { + Logger.LogError("The token request was rejected because an " + + "unregistered scope was specified: {Scope}.", scope); + + context.Reject( + error: OpenIdConnectConstants.Errors.InvalidRequest, + description: "The specified 'scope' parameter is not valid."); + + return; + } + } + // Optimization: the OpenID Connect server middleware automatically rejects grant_type=client_credentials // requests when validation is skipped but an earlier check is made here to avoid making unnecessary // database roundtrips to retrieve the client application corresponding to the client_id. diff --git a/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs b/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs index 77c3e11c..1da25a46 100644 --- a/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs +++ b/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs @@ -460,6 +460,22 @@ namespace OpenIddict.Tests Assert.Equal("/endpoint-path", options.RevocationEndpointPath); } + [Fact] + public void EnableScopeValidation_ScopeValidationIsDisabled() + { + // Arrange + var services = CreateServices(); + var builder = new OpenIddictBuilder(services); + + // Act + builder.EnableScopeValidation(); + + var options = GetOptions(services); + + // Assert + Assert.True(options.EnableScopeValidation); + } + [Fact] public void EnableTokenEndpoint_TokenEndpointIsEnabled() { diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs index 3115cc14..d279ea50 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs @@ -197,19 +197,9 @@ namespace OpenIddict.Tests public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenUnregisteredScopeIsSpecified() { // Arrange - var manager = CreateScopeManager(instance => - { - instance.Setup(mock => mock.ValidateScopesAsync( - It.Is>(scopes => scopes[0] == "unregistered_scope"), - It.IsAny())) - .ReturnsAsync(false); - }); - var server = CreateAuthorizationServer(builder => { - builder.Services.AddSingleton(manager); - - builder.ValidateScopes(); + builder.EnableScopeValidation(); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -228,16 +218,69 @@ namespace OpenIddict.Tests Assert.Equal("The specified 'scope' parameter is not valid.", response.ErrorDescription); } + [Fact] + public async Task ValidateAuthorizationRequest_RequestIsValidatedWhenScopeRegisteredInOptionsIsSpecified() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.Services.AddSingleton(CreateApplicationManager(instance => + { + var application = new OpenIddictApplication(); + + instance.Setup(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + instance.Setup(mock => mock.ValidateRedirectUriAsync(application, "http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny())) + .ReturnsAsync(OpenIddictConstants.ClientTypes.Public); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Endpoints.Authorization, It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.Implicit, It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + "registered_scope", It.IsAny())) + .ReturnsAsync(true); + })); + + builder.EnableScopeValidation(); + builder.RegisterScopes("registered_scope"); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act + var response = await client.PostAsync(AuthorizationEndpoint, new OpenIdConnectRequest + { + ClientId = "Fabrikam", + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = OpenIdConnectConstants.ResponseTypes.Token, + Scope = "registered_scope" + }); + + // Assert + Assert.Null(response.Error); + Assert.Null(response.ErrorDescription); + Assert.Null(response.ErrorUri); + Assert.NotNull(response.AccessToken); + } + [Fact] public async Task ValidateAuthorizationRequest_RequestIsValidatedWhenRegisteredScopeIsSpecified() { // Arrange var manager = CreateScopeManager(instance => { - instance.Setup(mock => mock.ValidateScopesAsync( - It.Is>(scopes => scopes[0] == "registered_scope"), - It.IsAny())) - .ReturnsAsync(true); + instance.Setup(mock => mock.FindByNameAsync("registered_scope", It.IsAny())) + .ReturnsAsync(new OpenIddictScope()); }); var server = CreateAuthorizationServer(builder => @@ -268,9 +311,8 @@ namespace OpenIddict.Tests .ReturnsAsync(true); })); + builder.EnableScopeValidation(); builder.Services.AddSingleton(manager); - - builder.ValidateScopes(); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -718,6 +760,7 @@ namespace OpenIddict.Tests var server = CreateAuthorizationServer(builder => { builder.Services.AddSingleton(manager); + builder.RegisterScopes(OpenIdConnectConstants.Scopes.Email, OpenIdConnectConstants.Scopes.Profile); }); var client = new OpenIdConnectClient(server.CreateClient()); diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs index 61d095b1..172b7de5 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs @@ -83,7 +83,10 @@ namespace OpenIddict.Tests public async Task ValidateTokenRequest_AuthorizationCodeRequestIsRejectedWhenRedirectUriIsMissing() { // Arrange - var server = CreateAuthorizationServer(); + var server = CreateAuthorizationServer(builder => + { + builder.EnableScopeValidation(); + }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -105,9 +108,9 @@ namespace OpenIddict.Tests public async Task ValidateTokenRequest_RequestIsRejectedWhenUnregisteredScopeIsSpecified() { // Arrange - var server = CreateAuthorizationServer(options => + var server = CreateAuthorizationServer(builder => { - options.ValidateScopes(); + builder.EnableScopeValidation(); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -126,23 +129,47 @@ namespace OpenIddict.Tests Assert.Equal("The specified 'scope' parameter is not valid.", response.ErrorDescription); } + [Fact] + public async Task ValidateTokenRequest_RequestIsValidatedWhenScopeRegisteredInOptionsIsSpecified() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.EnableScopeValidation(); + builder.RegisterScopes("registered_scope"); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act + var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest + { + GrantType = OpenIdConnectConstants.GrantTypes.Password, + Username = "johndoe", + Password = "A3ddj3w", + Scope = "registered_scope" + }); + + // Assert + Assert.Null(response.Error); + Assert.Null(response.ErrorDescription); + Assert.Null(response.ErrorUri); + Assert.NotNull(response.AccessToken); + } + [Fact] public async Task ValidateTokenRequest_RequestIsValidatedWhenRegisteredScopeIsSpecified() { // Arrange var manager = CreateScopeManager(instance => { - instance.Setup(mock => mock.ValidateScopesAsync( - It.Is>(scopes => scopes[0] == "registered_scope"), - It.IsAny())) - .ReturnsAsync(true); + instance.Setup(mock => mock.FindByNameAsync("registered_scope", It.IsAny())) + .ReturnsAsync(new OpenIddictScope()); }); var server = CreateAuthorizationServer(builder => { builder.Services.AddSingleton(manager); - builder.ValidateScopes(); - builder.RegisterScopes("registered_scope"); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -658,6 +685,7 @@ namespace OpenIddict.Tests var server = CreateAuthorizationServer(builder => { builder.Services.AddSingleton(manager); + builder.RegisterScopes(OpenIdConnectConstants.Scopes.Email, OpenIdConnectConstants.Scopes.Profile); }); var client = new OpenIdConnectClient(server.CreateClient());