tsai
/
openiddict-core
mirror of https://github.com/openiddict/openiddict-core.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Backport the scope permissions security fix to OpenIddict 1.x

pull/2236/head
Kévin Chalet 8 years ago
parent
28a67b7711 e7c4942777
commit
87eb5b5cd9
2 changed files with 75 additions and 82 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  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
View File

@ -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)

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

@ -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<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());
@ -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<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();
@ -526,7 +548,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 =>
@ -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<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();
@ -566,7 +588,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 =>
@ -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<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 =>
@ -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<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();
@ -653,27 +671,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());
@ -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<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]

Write Preview
Loading…
Cancel
Save