Browse Source

Move the scope permissions validation logic to ensure it also applies to public clients

pull/711/head
Kévin Chalet 7 years ago
parent
commit
e7c4942777
  1. 58
      src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs
  2. 99
      test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs

58
src/OpenIddict.Server/Internal/OpenIddictServerProvider.Exchange.cs

@ -215,6 +215,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.
@ -279,35 +308,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 _eventService.PublishAsync(new OpenIddictServerEvents.ValidateTokenRequest(context));

99
test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Exchange.cs

@ -473,7 +473,7 @@ namespace OpenIddict.Server.Internal.Tests
}
[Fact]
public async Task ValidateTokenRequest_ClientSecretCannotBeUsedByPublicClients()
public async Task ValidateTokenRequest_RequestIsRejectedWhenScopePermissionIsNotGranted()
{
// Arrange
var application = new OpenIddictApplication();
@ -485,11 +485,23 @@ namespace OpenIddict.Server.Internal.Tests
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Public));
instance.Setup(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Profile, It.IsAny<CancellationToken>()))
.ReturnsAsync(true);
instance.Setup(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Email, It.IsAny<CancellationToken>()))
.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());
@ -498,22 +510,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<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.OpenId, It.IsAny<CancellationToken>()), Times.Never());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.OfflineAccess, It.IsAny<CancellationToken>()), Times.Never());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Profile, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Email, It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]
public async Task ValidateTokenRequest_ClientSecretIsRequiredForConfidentialClients()
public async Task ValidateTokenRequest_ClientSecretCannotBeUsedByPublicClients()
{
// Arrange
var application = new OpenIddictApplication();
@ -524,7 +546,7 @@ namespace OpenIddict.Server.Internal.Tests
.ReturnsAsync(application);
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Confidential));
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Public));
});
var server = CreateAuthorizationServer(builder =>
@ -538,22 +560,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<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]
public async Task ValidateTokenRequest_ClientSecretIsRequiredForHybridClients()
public async Task ValidateTokenRequest_ClientSecretIsRequiredForConfidentialClients()
{
// Arrange
var application = new OpenIddictApplication();
@ -564,7 +586,7 @@ namespace OpenIddict.Server.Internal.Tests
.ReturnsAsync(application);
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Hybrid));
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Confidential));
});
var server = CreateAuthorizationServer(builder =>
@ -593,7 +615,7 @@ namespace OpenIddict.Server.Internal.Tests
}
[Fact]
public async Task ValidateTokenRequest_RequestIsRejectedWhenClientCredentialsAreInvalid()
public async Task ValidateTokenRequest_ClientSecretIsRequiredForHybridClients()
{
// Arrange
var application = new OpenIddictApplication();
@ -604,10 +626,7 @@ namespace OpenIddict.Server.Internal.Tests
.ReturnsAsync(application);
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Confidential));
instance.Setup(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny<CancellationToken>()))
.ReturnsAsync(false);
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Hybrid));
});
var server = CreateAuthorizationServer(builder =>
@ -621,7 +640,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"
@ -629,15 +648,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<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]
public async Task ValidateTokenRequest_RequestIsRejectedWhenScopePermissionIsNotGranted()
public async Task ValidateTokenRequest_RequestIsRejectedWhenClientCredentialsAreInvalid()
{
// Arrange
var application = new OpenIddictApplication();
@ -651,27 +669,12 @@ namespace OpenIddict.Server.Internal.Tests
.Returns(new ValueTask<string>(OpenIddictConstants.ClientTypes.Confidential));
instance.Setup(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny<CancellationToken>()))
.ReturnsAsync(true);
instance.Setup(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Profile, It.IsAny<CancellationToken>()))
.ReturnsAsync(true);
instance.Setup(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Email, It.IsAny<CancellationToken>()))
.ReturnsAsync(false);
instance.Setup(mock => mock.ValidateRedirectUriAsync(application, "http://www.fabrikam.com/path", It.IsAny<CancellationToken>()))
.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());
@ -683,26 +686,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<CancellationToken>()), Times.Never());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.OfflineAccess, It.IsAny<CancellationToken>()), Times.Never());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Profile, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.HasPermissionAsync(application,
OpenIddictConstants.Permissions.Prefixes.Scope +
OpenIddictConstants.Scopes.Email, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.ValidateClientSecretAsync(application, "7Fjfp0ZBr1KtDRbnfVdmIw", It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]

Loading…
Cancel
Save