From e7abdab60d0eba22396aa1b1f78e8f0f215bb806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 15 Jun 2018 17:55:19 +0200 Subject: [PATCH] Update the token endpoint validation logic to reject scope=offline_access requests if the client application is not allowed to use the refresh token flow --- ...OpenIddictServerProvider.Authentication.cs | 4 +- .../OpenIddictServerProvider.Exchange.cs | 37 +++++++++++---- ...ddictServerProviderTests.Authentication.cs | 46 ++++++++++++++++++ .../OpenIddictServerProviderTests.Exchange.cs | 47 +++++++++++++++++++ 4 files changed, 122 insertions(+), 12 deletions(-) diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs index fd7fcb51..c75d9eec 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs @@ -374,8 +374,8 @@ namespace OpenIddict.Server return; } - // Reject the request if the offline_access scope was request and if the - // application is not allowed to use the authorization code/implicit flows. + // Reject the request if the offline_access scope was request and if + // the application is not allowed to use the refresh token grant type. if (context.Request.HasScope(OpenIdConnectConstants.Scopes.OfflineAccess) && !await _applicationManager.HasPermissionAsync(application, OpenIddictConstants.Permissions.GrantTypes.RefreshToken)) { diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs index c8d3bbe6..7c4ce3e5 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs @@ -182,19 +182,36 @@ namespace OpenIddict.Server return; } - // Reject the request if the application is not allowed to use the specified grant type. - if (!options.IgnoreGrantTypePermissions && - !await _applicationManager.HasPermissionAsync(application, - OpenIddictConstants.Permissions.Prefixes.GrantType + context.Request.GrantType)) + if (!options.IgnoreGrantTypePermissions) { - _logger.LogError("The token request was rejected because the application '{ClientId}' was not allowed to " + - "use the specified grant type: {GrantType}.", context.ClientId, context.Request.GrantType); + // Reject the request if the application is not allowed to use the specified grant type. + if (!await _applicationManager.HasPermissionAsync(application, + OpenIddictConstants.Permissions.Prefixes.GrantType + context.Request.GrantType)) + { + _logger.LogError("The token request was rejected because the application '{ClientId}' was not allowed to " + + "use the specified grant type: {GrantType}.", context.ClientId, context.Request.GrantType); - context.Reject( - error: OpenIdConnectConstants.Errors.UnauthorizedClient, - description: "This client application is not allowed to use the specified grant type."); + context.Reject( + error: OpenIdConnectConstants.Errors.UnauthorizedClient, + description: "This client application is not allowed to use the specified grant type."); - return; + return; + } + + // Reject the request if the offline_access scope was request and if + // the application is not allowed to use the refresh token grant type. + if (context.Request.HasScope(OpenIdConnectConstants.Scopes.OfflineAccess) && + !await _applicationManager.HasPermissionAsync(application, OpenIddictConstants.Permissions.GrantTypes.RefreshToken)) + { + _logger.LogError("The token request was rejected because the application '{ClientId}' " + + "was not allowed to request the 'offline_access' scope.", context.ClientId); + + context.Reject( + error: OpenIdConnectConstants.Errors.InvalidRequest, + description: "The client application is not allowed to use the 'offline_access' scope."); + + return; + } } if (await _applicationManager.IsPublicAsync(application)) diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs index bf929c4c..b09a00d1 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs @@ -655,6 +655,52 @@ namespace OpenIddict.Server.Tests Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, permissions[0], It.IsAny()), Times.Once()); } + [Fact] + public async Task ValidateAuthorizationRequest_RequestWithOfflineAccessScopeIsRejectedWhenRefreshTokenPermissionIsNotGranted() + { + // Arrange + var application = new OpenIddictApplication(); + + var manager = CreateApplicationManager(instance => + { + instance.Setup(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode, It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.RefreshToken, It.IsAny())) + .ReturnsAsync(false); + }); + + var server = CreateAuthorizationServer(builder => + { + builder.Services.AddSingleton(manager); + + builder.Configure(options => options.IgnoreGrantTypePermissions = false); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act + var response = await client.PostAsync(AuthorizationEndpoint, new OpenIdConnectRequest + { + ClientId = "Fabrikam", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = OpenIdConnectConstants.ResponseTypes.Code, + Scope = OpenIdConnectConstants.Scopes.OfflineAccess + }); + + // Assert + Assert.Equal(OpenIdConnectConstants.Errors.InvalidRequest, response.Error); + Assert.Equal("The client application is not allowed to use the 'offline_access' scope.", response.ErrorDescription); + + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.RefreshToken, It.IsAny()), Times.Once()); + } + [Fact] public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenRedirectUriIsInvalid() { diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs index 8e8ce206..897b196c 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs @@ -387,6 +387,53 @@ namespace OpenIddict.Server.Tests OpenIddictConstants.Permissions.GrantTypes.Password, It.IsAny()), Times.Once()); } + [Fact] + public async Task ValidateTokenRequest_RequestWithOfflineAccessScopeIsRejectedWhenRefreshTokenPermissionIsNotGranted() + { + // Arrange + var application = new OpenIddictApplication(); + + var manager = CreateApplicationManager(instance => + { + instance.Setup(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.Password, It.IsAny())) + .ReturnsAsync(true); + + instance.Setup(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.RefreshToken, It.IsAny())) + .ReturnsAsync(false); + }); + + var server = CreateAuthorizationServer(builder => + { + builder.Services.AddSingleton(manager); + + builder.Configure(options => options.IgnoreGrantTypePermissions = false); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act + var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest + { + ClientId = "Fabrikam", + GrantType = OpenIdConnectConstants.GrantTypes.Password, + Username = "johndoe", + Password = "A3ddj3w", + Scope = OpenIdConnectConstants.Scopes.OfflineAccess + }); + + // Assert + Assert.Equal(OpenIdConnectConstants.Errors.InvalidRequest, response.Error); + Assert.Equal("The client application is not allowed to use the 'offline_access' scope.", response.ErrorDescription); + + Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application, + OpenIddictConstants.Permissions.GrantTypes.RefreshToken, It.IsAny()), Times.Once()); + } + [Fact] public async Task ValidateTokenRequest_ClientCredentialsRequestFromPublicClientIsRejected() {