From 6b97606d0f691d9654adf635e55e1920505f81e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Thu, 14 Sep 2017 15:28:45 +0200 Subject: [PATCH] Introduce a new option allowing to configure the registered scopes --- samples/Mvc.Server/Startup.cs | 3 + .../Managers/OpenIddictApplicationManager.cs | 90 +++++++++---------- src/OpenIddict/OpenIddictExtensions.cs | 29 ++++++ src/OpenIddict/OpenIddictInitializer.cs | 6 ++ src/OpenIddict/OpenIddictOptions.cs | 9 ++ .../OpenIddictProvider.Discovery.cs | 24 +++-- src/OpenIddict/OpenIddictProvider.cs | 7 ++ .../OpenIddictExtensionsTests.cs | 17 ++++ .../OpenIddictProviderTests.Discovery.cs | 29 ++++-- .../OpenIddictProviderTests.cs | 12 +++ 10 files changed, 162 insertions(+), 64 deletions(-) diff --git a/samples/Mvc.Server/Startup.cs b/samples/Mvc.Server/Startup.cs index 37b6f37a..df5a8cb7 100644 --- a/samples/Mvc.Server/Startup.cs +++ b/samples/Mvc.Server/Startup.cs @@ -89,6 +89,9 @@ namespace Mvc.Server .AllowPasswordFlow() .AllowRefreshTokenFlow(); + // Mark the "profile" scope as a supported scope in the discovery document. + options.RegisterScopes(OpenIdConnectConstants.Scopes.Profile); + // Make the "client_id" parameter mandatory when sending a token request. options.RequireClientIdentification(); diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index 41eba574..25b4fd06 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs @@ -412,6 +412,51 @@ namespace OpenIddict.Core await UpdateAsync(application, cancellationToken); } + /// + /// Validates the client_secret associated with an application. + /// + /// The application. + /// The secret that should be compared to the client_secret stored in the database. + /// The that can be used to abort the operation. + /// A that can be used to monitor the asynchronous operation. + /// + /// A that can be used to monitor the asynchronous operation, + /// whose result returns a boolean indicating whether the client secret was valid. + /// + public virtual async Task ValidateClientSecretAsync([NotNull] TApplication application, string secret, CancellationToken cancellationToken) + { + if (application == null) + { + throw new ArgumentNullException(nameof(application)); + } + + if (!await IsConfidentialAsync(application, cancellationToken)) + { + Logger.LogWarning("Client authentication cannot be enforced for non-confidential applications."); + + return false; + } + + var value = await Store.GetClientSecretAsync(application, cancellationToken); + if (string.IsNullOrEmpty(value)) + { + Logger.LogError("Client authentication failed for {Client} because " + + "no client secret was associated with the application."); + + return false; + } + + if (!await ValidateClientSecretAsync(secret, value, cancellationToken)) + { + Logger.LogWarning("Client authentication failed for {Client}.", + await GetDisplayNameAsync(application, cancellationToken)); + + return false; + } + + return true; + } + /// /// Validates the specified post_logout_redirect_uri. /// @@ -470,51 +515,6 @@ namespace OpenIddict.Core return false; } - /// - /// Validates the client_secret associated with an application. - /// - /// The application. - /// The secret that should be compared to the client_secret stored in the database. - /// The that can be used to abort the operation. - /// A that can be used to monitor the asynchronous operation. - /// - /// A that can be used to monitor the asynchronous operation, - /// whose result returns a boolean indicating whether the client secret was valid. - /// - public virtual async Task ValidateClientSecretAsync([NotNull] TApplication application, string secret, CancellationToken cancellationToken) - { - if (application == null) - { - throw new ArgumentNullException(nameof(application)); - } - - if (!await IsConfidentialAsync(application, cancellationToken)) - { - Logger.LogWarning("Client authentication cannot be enforced for non-confidential applications."); - - return false; - } - - var value = await Store.GetClientSecretAsync(application, cancellationToken); - if (string.IsNullOrEmpty(value)) - { - Logger.LogError("Client authentication failed for {Client} because " + - "no client secret was associated with the application."); - - return false; - } - - if (!await ValidateClientSecretAsync(secret, value, cancellationToken)) - { - Logger.LogWarning("Client authentication failed for {Client}.", - await GetDisplayNameAsync(application, cancellationToken)); - - return false; - } - - return true; - } - /// /// Validates the application to ensure it's in a consistent state. /// diff --git a/src/OpenIddict/OpenIddictExtensions.cs b/src/OpenIddict/OpenIddictExtensions.cs index d33d5b91..91746777 100644 --- a/src/OpenIddict/OpenIddictExtensions.cs +++ b/src/OpenIddict/OpenIddictExtensions.cs @@ -8,6 +8,7 @@ using System; using System.Collections.Generic; using System.IdentityModel.Tokens.Jwt; using System.IO; +using System.Linq; using System.Reflection; using System.Security.Cryptography.X509Certificates; using AspNet.Security.OpenIdConnect.Primitives; @@ -846,6 +847,34 @@ namespace Microsoft.AspNetCore.Builder return builder.Configure(options => options.Issuer = address); } + /// + /// Registers the specified scopes as supported scopes so + /// they can be returned as part of the discovery document. + /// + /// The services builder used by OpenIddict to register new services. + /// The supported scopes. + /// The . + public static OpenIddictBuilder RegisterScopes( + [NotNull] this OpenIddictBuilder builder, [NotNull] params string[] scopes) + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + + if (scopes == null) + { + throw new ArgumentNullException(nameof(scopes)); + } + + if (scopes.Any(scope => string.IsNullOrEmpty(scope))) + { + throw new ArgumentException("Scopes cannot be null or empty.", nameof(scopes)); + } + + return builder.Configure(options => options.Scopes.UnionWith(scopes)); + } + /// /// Configures OpenIddict to use a specific data protection provider /// instead of relying on the default instance provided by the DI container. diff --git a/src/OpenIddict/OpenIddictInitializer.cs b/src/OpenIddict/OpenIddictInitializer.cs index f01db70f..fe7e5744 100644 --- a/src/OpenIddict/OpenIddictInitializer.cs +++ b/src/OpenIddict/OpenIddictInitializer.cs @@ -177,6 +177,12 @@ namespace OpenIddict "or 'services.AddOpenIddict().AddDevelopmentSigningCertificate()' or call " + "'services.AddOpenIddict().AddEphemeralSigningKey()' to use an ephemeral key."); } + + // Automatically add the offline_access scope if the refresh token grant has been enabled. + if (options.GrantTypes.Contains(OpenIdConnectConstants.GrantTypes.RefreshToken)) + { + options.Scopes.Add(OpenIdConnectConstants.Scopes.OfflineAccess); + } } } } diff --git a/src/OpenIddict/OpenIddictOptions.cs b/src/OpenIddict/OpenIddictOptions.cs index 96231b51..f6b00f5e 100644 --- a/src/OpenIddict/OpenIddictOptions.cs +++ b/src/OpenIddict/OpenIddictOptions.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; using System.Security.Cryptography; +using AspNet.Security.OpenIdConnect.Primitives; using AspNet.Security.OpenIdConnect.Server; using Microsoft.Extensions.Caching.Distributed; @@ -67,6 +68,14 @@ namespace OpenIddict /// public bool RequireClientIdentification { get; set; } + /// + /// Gets the OAuth2/OpenID Connect scopes enabled for this application. + /// + public ISet Scopes { get; } = new HashSet(StringComparer.Ordinal) + { + OpenIdConnectConstants.Scopes.OpenId + }; + /// /// Gets or sets a boolean indicating whether reference tokens should be used. /// When set to true, authorization codes, access tokens and refresh tokens diff --git a/src/OpenIddict/OpenIddictProvider.Discovery.cs b/src/OpenIddict/OpenIddictProvider.Discovery.cs index af037899..c88fe805 100644 --- a/src/OpenIddict/OpenIddictProvider.Discovery.cs +++ b/src/OpenIddict/OpenIddictProvider.Discovery.cs @@ -32,20 +32,16 @@ namespace OpenIddict // Note: the OpenID Connect server middleware automatically populates grant_types_supported // by determining whether the authorization and token endpoints are enabled or not but // OpenIddict uses a different approach and relies on a configurable "grants list". - context.GrantTypes.IntersectWith(options.GrantTypes); - - // Note: the "openid" scope is automatically - // added by the OpenID Connect server middleware. - context.Scopes.Add(OpenIdConnectConstants.Scopes.Profile); - context.Scopes.Add(OpenIdConnectConstants.Scopes.Email); - context.Scopes.Add(OpenIdConnectConstants.Scopes.Phone); - context.Scopes.Add(OpenIddictConstants.Scopes.Roles); - - // Only add the "offline_access" scope if the refresh token grant is enabled. - if (context.GrantTypes.Contains(OpenIdConnectConstants.GrantTypes.RefreshToken)) - { - context.Scopes.Add(OpenIdConnectConstants.Scopes.OfflineAccess); - } + context.GrantTypes.Clear(); + context.GrantTypes.UnionWith(options.GrantTypes); + + // Only return the scopes configured by the developer. + context.Scopes.Clear(); + context.Scopes.UnionWith(options.Scopes); + + // Note: the optional "claims" parameter is not supported by OpenIddict, + // so a "false" flag is returned to encourage clients not to use it. + context.Metadata[OpenIdConnectConstants.Metadata.ClaimsSupported] = false; var schemes = context.HttpContext.RequestServices.GetRequiredService(); diff --git a/src/OpenIddict/OpenIddictProvider.cs b/src/OpenIddict/OpenIddictProvider.cs index 7e942429..3e17d5ab 100644 --- a/src/OpenIddict/OpenIddictProvider.cs +++ b/src/OpenIddict/OpenIddictProvider.cs @@ -26,11 +26,13 @@ namespace OpenIddict [NotNull] ILogger> logger, [NotNull] OpenIddictApplicationManager applications, [NotNull] OpenIddictAuthorizationManager authorizations, + [NotNull] OpenIddictScopeManager scopes, [NotNull] OpenIddictTokenManager tokens) { Applications = applications; Authorizations = authorizations; Logger = logger; + Scopes = scopes; Tokens = tokens; } @@ -49,6 +51,11 @@ namespace OpenIddict /// public ILogger Logger { get; } + /// + /// Gets the scopes manager. + /// + public OpenIddictScopeManager Scopes { get; } + /// /// Gets the tokens manager. /// diff --git a/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs b/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs index 769cfcdc..ee4da70e 100644 --- a/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs +++ b/test/OpenIddict.Tests/OpenIddictExtensionsTests.cs @@ -582,6 +582,23 @@ namespace OpenIddict.Tests Assert.Equal(new Uri("http://www.fabrikam.com/"), options.Issuer); } + [Fact] + public void RegisterScopes_ScopeIsAdded() + { + // Arrange + var services = CreateServices(); + var builder = new OpenIddictBuilder(services); + + // Act + builder.RegisterScopes("custom_scope_1", "custom_scope_2"); + + var options = GetOptions(services); + + // Assert + Assert.Contains("custom_scope_1", options.Scopes); + Assert.Contains("custom_scope_2", options.Scopes); + } + [Fact] public void UseDataProtectionProvider_DefaultProviderIsReplaced() { diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.Discovery.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.Discovery.cs index 79740b26..b8e4b691 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.Discovery.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.Discovery.cs @@ -61,11 +61,8 @@ namespace OpenIddict.Tests } [Theory] - [InlineData(OpenIdConnectConstants.Scopes.Profile)] - [InlineData(OpenIdConnectConstants.Scopes.Email)] - [InlineData(OpenIdConnectConstants.Scopes.Phone)] - [InlineData(OpenIddictConstants.Scopes.Roles)] - public async Task HandleConfigurationRequest_StandardScopesAreExposed(string scope) + [InlineData(OpenIdConnectConstants.Scopes.OpenId)] + public async Task HandleConfigurationRequest_DefaultScopesAreAutomaticallyReturned(string scope) { // Arrange var server = CreateAuthorizationServer(); @@ -79,6 +76,28 @@ namespace OpenIddict.Tests Assert.Contains(scope, ((JArray) response[OpenIdConnectConstants.Metadata.ScopesSupported]).Values()); } + [Fact] + public async Task HandleConfigurationRequest_CustomScopeIsReturned() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.Configure(options => + { + options.Scopes.Clear(); + options.Scopes.Add("custom_scope"); + }); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act + var response = await client.GetAsync(ConfigurationEndpoint); + + // Assert + Assert.Contains("custom_scope", ((JArray) response[OpenIdConnectConstants.Metadata.ScopesSupported]).Values()); + } + [Fact] public async Task HandleConfigurationRequest_OfflineAccessScopeIsReturnedWhenRefreshTokenFlowIsEnabled() { diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.cs index e678f17c..301554d8 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.cs @@ -66,6 +66,7 @@ namespace OpenIddict.Tests // Replace the default OpenIddict managers. services.AddSingleton(CreateApplicationManager()); services.AddSingleton(CreateAuthorizationManager()); + services.AddSingleton(CreateScopeManager()); services.AddSingleton(CreateTokenManager()); services.AddOpenIddict(options => @@ -199,6 +200,17 @@ namespace OpenIddict.Tests return manager.Object; } + private static OpenIddictScopeManager CreateScopeManager(Action>> setup = null) + { + var manager = new Mock>( + Mock.Of>(), + Mock.Of>>()); + + setup?.Invoke(manager); + + return manager.Object; + } + private static OpenIddictTokenManager CreateTokenManager(Action>> setup = null) { var manager = new Mock>(