diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs index 3da8e5f8..937e2da2 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs @@ -221,6 +221,35 @@ namespace OpenIddict.Server.Internal } } + // Unless permission enforcement was explicitly disabled, ensure + // the client application is allowed to use the specified scopes. + if (!options.IgnoreScopePermissions) + { + foreach (var scope in context.Request.GetScopes()) + { + // Avoid validating the "openid" and "offline_access" scopes as they represent protocol scopes. + if (string.Equals(scope, OpenIddictConstants.Scopes.OfflineAccess, StringComparison.Ordinal) || + string.Equals(scope, OpenIddictConstants.Scopes.OpenId, StringComparison.Ordinal)) + { + continue; + } + + // Reject the request if the application is not allowed to use the iterated scope. + if (!await applicationManager.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + scope)) + { + logger.LogError("The token request was rejected because the application '{ClientId}' " + + "was not allowed to use the scope {Scope}.", context.ClientId, scope); + + context.Reject( + error: OpenIddictConstants.Errors.InvalidRequest, + description: "This client application is not allowed to use the specified scope."); + + return; + } + } + } + if (await applicationManager.IsPublicAsync(application)) { // Note: public applications are not allowed to use the client credentials grant. @@ -285,35 +314,6 @@ namespace OpenIddict.Server.Internal return; } - // Unless permission enforcement was explicitly disabled, ensure - // the client application is allowed to use the specified scopes. - if (!options.IgnoreScopePermissions) - { - foreach (var scope in context.Request.GetScopes()) - { - // Avoid validating the "openid" and "offline_access" scopes as they represent protocol scopes. - if (string.Equals(scope, OpenIddictConstants.Scopes.OfflineAccess, StringComparison.Ordinal) || - string.Equals(scope, OpenIddictConstants.Scopes.OpenId, StringComparison.Ordinal)) - { - continue; - } - - // Reject the request if the application is not allowed to use the iterated scope. - if (!await applicationManager.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + scope)) - { - logger.LogError("The token request was rejected because the application '{ClientId}' " + - "was not allowed to use the scope {Scope}.", context.ClientId, scope); - - context.Reject( - error: OpenIddictConstants.Errors.InvalidRequest, - description: "This client application is not allowed to use the specified scope."); - - return; - } - } - } - context.Validate(); await GetEventService(context.HttpContext.RequestServices) diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs index 5316eb52..0bda9b7d 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs @@ -475,7 +475,7 @@ namespace OpenIddict.Server.Internal.Tests } [Fact] - public async Task ValidateTokenRequest_ClientSecretCannotBeUsedByPublicClients() + public async Task ValidateTokenRequest_RequestIsRejectedWhenScopePermissionIsNotGranted() { // Arrange var application = new OpenIddictApplication(); @@ -487,11 +487,23 @@ namespace OpenIddict.Server.Internal.Tests instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny())) .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Public)); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.Profile, It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.Email, It.IsAny())) + .ReturnsAsync(false); }); var server = CreateAuthorizationServer(builder => { builder.Services.AddSingleton(manager); + builder.RegisterScopes(OpenIddictConstants.Scopes.Email, OpenIddictConstants.Scopes.Profile); + builder.Configure(options => options.IgnoreScopePermissions = false); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -500,22 +512,32 @@ namespace OpenIddict.Server.Internal.Tests var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest { ClientId = "Fabrikam", - ClientSecret = "7Fjfp0ZBr1KtDRbnfVdmIw", GrantType = OpenIddictConstants.GrantTypes.Password, Username = "johndoe", - Password = "A3ddj3w" + Password = "A3ddj3w", + Scope = "openid offline_access profile email" }); // Assert Assert.Equal(OpenIddictConstants.Errors.InvalidRequest, response.Error); - Assert.Equal("The 'client_secret' parameter is not valid for this client application.", response.ErrorDescription); + Assert.Equal("This client application is not allowed to use the specified scope.", response.ErrorDescription); - Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny()), Times.Once()); - Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.OpenId, It.IsAny()), Times.Never()); + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.OfflineAccess, It.IsAny()), Times.Never()); + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.Profile, It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.Scope + + OpenIddictConstants.Scopes.Email, It.IsAny()), Times.Once()); } [Fact] - public async Task ValidateTokenRequest_ClientSecretIsRequiredForConfidentialClients() + public async Task ValidateTokenRequest_ClientSecretCannotBeUsedByPublicClients() { // Arrange var application = new OpenIddictApplication(); @@ -526,7 +548,7 @@ namespace OpenIddict.Server.Internal.Tests .ReturnsAsync(application); instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny())) - .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Confidential)); + .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Public)); }); var server = CreateAuthorizationServer(builder => @@ -540,22 +562,22 @@ namespace OpenIddict.Server.Internal.Tests var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest { ClientId = "Fabrikam", - ClientSecret = null, + ClientSecret = "7Fjfp0ZBr1KtDRbnfVdmIw", GrantType = OpenIddictConstants.GrantTypes.Password, Username = "johndoe", Password = "A3ddj3w" }); // Assert - Assert.Equal(OpenIddictConstants.Errors.InvalidClient, response.Error); - Assert.Equal("The 'client_secret' parameter required for this client application is missing.", response.ErrorDescription); + Assert.Equal(OpenIddictConstants.Errors.InvalidRequest, response.Error); + Assert.Equal("The 'client_secret' parameter is not valid for this client application.", response.ErrorDescription); Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny()), Times.Once()); Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny()), Times.Once()); } [Fact] - public async Task ValidateTokenRequest_ClientSecretIsRequiredForHybridClients() + public async Task ValidateTokenRequest_ClientSecretIsRequiredForConfidentialClients() { // Arrange var application = new OpenIddictApplication(); @@ -566,7 +588,7 @@ namespace OpenIddict.Server.Internal.Tests .ReturnsAsync(application); instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny())) - .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Hybrid)); + .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Confidential)); }); var server = CreateAuthorizationServer(builder => @@ -595,7 +617,7 @@ namespace OpenIddict.Server.Internal.Tests } [Fact] - public async Task ValidateTokenRequest_RequestIsRejectedWhenClientCredentialsAreInvalid() + public async Task ValidateTokenRequest_ClientSecretIsRequiredForHybridClients() { // Arrange var application = new OpenIddictApplication(); @@ -606,10 +628,7 @@ namespace OpenIddict.Server.Internal.Tests .ReturnsAsync(application); instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny())) - .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Confidential)); - - instance.Setup(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny())) - .ReturnsAsync(false); + .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Hybrid)); }); var server = CreateAuthorizationServer(builder => @@ -623,7 +642,7 @@ namespace OpenIddict.Server.Internal.Tests var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest { ClientId = "Fabrikam", - ClientSecret = "7Fjfp0ZBr1KtDRbnfVdmIw", + ClientSecret = null, GrantType = OpenIddictConstants.GrantTypes.Password, Username = "johndoe", Password = "A3ddj3w" @@ -631,15 +650,14 @@ namespace OpenIddict.Server.Internal.Tests // Assert Assert.Equal(OpenIddictConstants.Errors.InvalidClient, response.Error); - Assert.Equal("The specified client credentials are invalid.", response.ErrorDescription); + Assert.Equal("The 'client_secret' parameter required for this client application is missing.", response.ErrorDescription); Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny()), Times.Once()); Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny()), Times.Once()); - Mock.Get(manager).Verify(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny()), Times.Once()); } [Fact] - public async Task ValidateTokenRequest_RequestIsRejectedWhenScopePermissionIsNotGranted() + public async Task ValidateTokenRequest_RequestIsRejectedWhenClientCredentialsAreInvalid() { // Arrange var application = new OpenIddictApplication(); @@ -653,27 +671,12 @@ namespace OpenIddict.Server.Internal.Tests .Returns(new ValueTask(OpenIddictConstants.ClientTypes.Confidential)); instance.Setup(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny())) - .ReturnsAsync(true); - - instance.Setup(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.Profile, It.IsAny())) - .ReturnsAsync(true); - - instance.Setup(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.Email, It.IsAny())) .ReturnsAsync(false); - - instance.Setup(mock => mock.ValidateRedirectUriAsync(application, "http://www.fabrikam.com/path", It.IsAny())) - .ReturnsAsync(true); }); var server = CreateAuthorizationServer(builder => { builder.Services.AddSingleton(manager); - builder.RegisterScopes(OpenIddictConstants.Scopes.Email, OpenIddictConstants.Scopes.Profile); - builder.Configure(options => options.IgnoreScopePermissions = false); }); var client = new OpenIdConnectClient(server.CreateClient()); @@ -685,26 +688,16 @@ namespace OpenIddict.Server.Internal.Tests ClientSecret = "7Fjfp0ZBr1KtDRbnfVdmIw", GrantType = OpenIddictConstants.GrantTypes.Password, Username = "johndoe", - Password = "A3ddj3w", - Scope = "openid offline_access profile email" + Password = "A3ddj3w" }); // Assert - Assert.Equal(OpenIddictConstants.Errors.InvalidRequest, response.Error); - Assert.Equal("This client application is not allowed to use the specified scope.", response.ErrorDescription); + Assert.Equal(OpenIddictConstants.Errors.InvalidClient, response.Error); + Assert.Equal("The specified client credentials are invalid.", response.ErrorDescription); - Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.OpenId, It.IsAny()), Times.Never()); - Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.OfflineAccess, It.IsAny()), Times.Never()); - Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.Profile, It.IsAny()), Times.Once()); - Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.Scope + - OpenIddictConstants.Scopes.Email, It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny()), Times.Once()); } [Fact]