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 client types changes to OpenIddict 1.x

pull/553/head
Kévin Chalet 8 years ago
parent
dff9c69205 f95a17acae
commit
f2b55463dc
9 changed files with 144 additions and 44 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. 51
      src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs
  2. 1
      src/OpenIddict.Core/OpenIddictConstants.cs
  3. 10
      src/OpenIddict/OpenIddictProvider.Authentication.cs
  4. 13
      src/OpenIddict/OpenIddictProvider.Exchange.cs
  5. 6
      src/OpenIddict/OpenIddictProvider.Introspection.cs
  6. 24
      src/OpenIddict/OpenIddictProvider.Revocation.cs
  7. 4
      test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs
  8. 40
      test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs
  9. 39
      test/OpenIddict.Tests/OpenIddictProviderTests.Revocation.cs

51
src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs
View File

@ -5,7 +5,6 @@
*/
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
@ -88,12 +87,12 @@ namespace OpenIddict.Core
OpenIddictConstants.ClientTypes.Confidential, cancellationToken);
}
// If the client is a confidential application, throw an
// If the client is not a public application, throw an
// exception as the client secret is required in this case.
if (string.IsNullOrEmpty(secret) &&
string.Equals(type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase))
if (string.IsNullOrEmpty(secret) && !await IsPublicAsync(application, cancellationToken))
{
throw new InvalidOperationException("A client secret must be provided when creating a confidential application.");
throw new InvalidOperationException("A client secret must be provided when creating " +
"a confidential or hybrid application.");
}
if (!string.IsNullOrEmpty(secret))
@ -133,12 +132,13 @@ namespace OpenIddict.Core
OpenIddictConstants.ClientTypes.Confidential;
}
// If the client is a confidential application, throw an
// If the client is not a public application, throw an
// exception as the client secret is required in this case.
if (string.IsNullOrEmpty(descriptor.ClientSecret) &&
string.Equals(descriptor.Type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase))
!string.Equals(descriptor.Type, OpenIddictConstants.ClientTypes.Public, StringComparison.OrdinalIgnoreCase))
{
throw new InvalidOperationException("A client secret must be provided when creating a confidential application.");
throw new InvalidOperationException("A client secret must be provided when creating " +
"a confidential or hybrid application.");
}
// Obfuscate the provided client secret.
@ -265,9 +265,10 @@ namespace OpenIddict.Core
// Ensure the application type returned by the store is supported by the manager.
if (!string.Equals(type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase) &&
!string.Equals(type, OpenIddictConstants.ClientTypes.Hybrid, StringComparison.OrdinalIgnoreCase) &&
!string.Equals(type, OpenIddictConstants.ClientTypes.Public, StringComparison.OrdinalIgnoreCase))
{
throw new InvalidOperationException("Only 'confidential' or 'public' applications are " +
throw new InvalidOperationException("Only 'confidential', 'hybrid' or 'public' applications are " +
"supported by the default application manager.");
}
@ -351,31 +352,34 @@ namespace OpenIddict.Core
}
/// <summary>
/// Determines whether the specified application has a client secret.
/// Determines whether an application is a confidential client.
/// </summary>
/// <param name="application">The application.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> that can be used to abort the operation.</param>
/// <returns>
/// A <see cref="Task"/> that can be used to monitor the asynchronous operation,
/// whose result returns a boolean indicating whether a client secret is registered.
/// </returns>
public virtual async Task<bool> HasClientSecretAsync([NotNull] TApplication application, CancellationToken cancellationToken)
/// <returns><c>true</c> if the application is a confidential client, <c>false</c> otherwise.</returns>
public async Task<bool> IsConfidentialAsync([NotNull] TApplication application, CancellationToken cancellationToken)
{
if (application == null)
{
throw new ArgumentNullException(nameof(application));
}
return !string.IsNullOrEmpty(await Store.GetClientSecretAsync(application, cancellationToken));
var type = await GetClientTypeAsync(application, cancellationToken);
if (string.IsNullOrEmpty(type))
{
return false;
}
return string.Equals(type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase);
}
/// <summary>
/// Determines whether an application is a confidential client.
/// Determines whether an application is a hybrid client.
/// </summary>
/// <param name="application">The application.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> that can be used to abort the operation.</param>
/// <returns><c>true</c> if the application is a confidential client, <c>false</c> otherwise.</returns>
public async Task<bool> IsConfidentialAsync([NotNull] TApplication application, CancellationToken cancellationToken)
/// <returns><c>true</c> if the application is a hybrid client, <c>false</c> otherwise.</returns>
public async Task<bool> IsHybridAsync([NotNull] TApplication application, CancellationToken cancellationToken)
{
if (application == null)
{
@ -388,7 +392,7 @@ namespace OpenIddict.Core
return false;
}
return string.Equals(type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase);
return string.Equals(type, OpenIddictConstants.ClientTypes.Hybrid, StringComparison.OrdinalIgnoreCase);
}
/// <summary>
@ -505,9 +509,9 @@ namespace OpenIddict.Core
throw new ArgumentNullException(nameof(application));
}
if (!await IsConfidentialAsync(application, cancellationToken))
if (await IsPublicAsync(application, cancellationToken))
{
Logger.LogWarning("Client authentication cannot be enforced for non-confidential applications.");
Logger.LogWarning("Client authentication cannot be enforced for public applications.");
return false;
}
@ -640,9 +644,10 @@ namespace OpenIddict.Core
// Ensure the application type is supported by the manager.
if (!string.Equals(descriptor.Type, OpenIddictConstants.ClientTypes.Confidential, StringComparison.OrdinalIgnoreCase) &&
!string.Equals(descriptor.Type, OpenIddictConstants.ClientTypes.Hybrid, StringComparison.OrdinalIgnoreCase) &&
!string.Equals(descriptor.Type, OpenIddictConstants.ClientTypes.Public, StringComparison.OrdinalIgnoreCase))
{
throw new ArgumentException("Only 'confidential' or 'public' applications are " +
throw new ArgumentException("Only 'confidential', 'hybrid' or 'public' applications are " +
"supported by the default application manager.", nameof(descriptor));
}

1
src/OpenIddict.Core/OpenIddictConstants.cs
View File

@ -16,6 +16,7 @@ namespace OpenIddict.Core
public static class ClientTypes
{
public const string Confidential = "confidential";
public const string Hybrid = "hybrid";
public const string Public = "public";
}

10
src/OpenIddict/OpenIddictProvider.Authentication.cs
View File

@ -298,17 +298,17 @@ namespace OpenIddict
return;
}
// To prevent downgrade attacks, ensure that authorization requests returning an access token directly
// from the authorization endpoint are rejected if the client_id corresponds to a confidential application.
// To prevent downgrade attacks, ensure that authorization requests returning a token directly from
// the authorization endpoint are rejected if the client_id corresponds to a confidential application.
// Note: when using the authorization code grant, ValidateTokenRequest is responsible of rejecting
// the token request if the client_id corresponds to an unauthenticated confidential client.
if (await applications.IsConfidentialAsync(application, context.HttpContext.RequestAborted) &&
context.Request.HasResponseType(OpenIdConnectConstants.ResponseTypes.Token))
(context.Request.HasResponseType(OpenIdConnectConstants.ResponseTypes.IdToken) ||
context.Request.HasResponseType(OpenIdConnectConstants.ResponseTypes.Token)))
{
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidRequest,
description: "Confidential clients are not allowed to retrieve " +
"an access token from the authorization endpoint.");
description: "Confidential clients are not allowed to retrieve a token from the authorization endpoint.");
return;
}

13
src/OpenIddict/OpenIddictProvider.Exchange.cs
View File

@ -102,6 +102,9 @@ namespace OpenIddict
// Reject the request if client identification is mandatory.
if (options.RequireClientIdentification)
{
logger.LogError("The token request was rejected becaused the " +
"mandatory client_id parameter was missing or empty.");
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidRequest,
description: "The mandatory 'client_id' parameter was missing.");
@ -159,8 +162,8 @@ namespace OpenIddict
return;
}
logger.LogInformation("The token request validation process was not fully validated because " +
"the client '{ClientId}' was a public application.", context.ClientId);
logger.LogDebug("The token request validation process was not fully validated because " +
"the client '{ClientId}' was a public application.", context.ClientId);
// If client authentication cannot be enforced, call context.Skip() to inform
// the OpenID Connect server middleware that the caller cannot be fully trusted.
@ -169,11 +172,11 @@ namespace OpenIddict
return;
}
// Confidential applications MUST authenticate
// Confidential and hybrid applications MUST authenticate
// to protect them from impersonation attacks.
if (string.IsNullOrEmpty(context.ClientSecret))
{
logger.LogError("The token request was rejected because the confidential application " +
logger.LogError("The token request was rejected because the confidential or hybrid application " +
"'{ClientId}' didn't specify a client secret.", context.ClientId);
context.Reject(
@ -185,7 +188,7 @@ namespace OpenIddict
if (!await applications.ValidateClientSecretAsync(application, context.ClientSecret, context.HttpContext.RequestAborted))
{
logger.LogError("The token request was rejected because the confidential application " +
logger.LogError("The token request was rejected because the confidential or hybrid application " +
"'{ClientId}' didn't specify valid client credentials.", context.ClientId);
context.Reject(

6
src/OpenIddict/OpenIddictProvider.Introspection.cs
View File

@ -68,8 +68,8 @@ namespace OpenIddict
return;
}
// Reject non-confidential applications.
if (!await applications.IsConfidentialAsync(application, context.HttpContext.RequestAborted))
// Reject introspection requests sent by public applications.
if (await applications.IsPublicAsync(application, context.HttpContext.RequestAborted))
{
logger.LogError("The introspection request was rejected because the public application " +
"'{ClientId}' was not allowed to use this endpoint.", context.ClientId);
@ -84,7 +84,7 @@ namespace OpenIddict
// Validate the client credentials.
if (!await applications.ValidateClientSecretAsync(application, context.ClientSecret, context.HttpContext.RequestAborted))
{
logger.LogError("The introspection request was rejected because the confidential application " +
logger.LogError("The introspection request was rejected because the confidential or hybrid application " +
"'{ClientId}' didn't specify valid client credentials.", context.ClientId);
context.Reject(

24
src/OpenIddict/OpenIddictProvider.Revocation.cs
View File

@ -73,8 +73,8 @@ namespace OpenIddict
return;
}
logger.LogInformation("The revocation request validation process was skipped " +
"because the client_id parameter was missing or empty.");
logger.LogDebug("The revocation request validation process was skipped " +
"because the client_id parameter was missing or empty.");
context.Skip();
@ -85,6 +85,9 @@ namespace OpenIddict
var application = await applications.FindByClientIdAsync(context.ClientId, context.HttpContext.RequestAborted);
if (application == null)
{
logger.LogError("The revocation request was rejected because the client " +
"application was not found: '{ClientId}'.", context.ClientId);
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidClient,
description: "Application not found in the database: ensure that your client_id is correct.");
@ -92,12 +95,15 @@ namespace OpenIddict
return;
}
// Reject revocation requests containing a client_secret if the client application is not confidential.
// Reject revocation requests containing a client_secret if the application is a public client.
if (await applications.IsPublicAsync(application, context.HttpContext.RequestAborted))
{
// Reject tokens requests containing a client_secret when the client is a public application.
if (!string.IsNullOrEmpty(context.ClientSecret))
{
logger.LogError("The revocation request was rejected because the public application " +
"'{ClientId}' was not allowed to use this endpoint.", context.ClientId);
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidRequest,
description: "Public clients are not allowed to send a client_secret.");
@ -105,8 +111,8 @@ namespace OpenIddict
return;
}
logger.LogInformation("The revocation request validation process was not fully validated because " +
"the client '{ClientId}' was a public application.", context.ClientId);
logger.LogDebug("The revocation request validation process was not fully validated because " +
"the client '{ClientId}' was a public application.", context.ClientId);
// If client authentication cannot be enforced, call context.Skip() to inform
// the OpenID Connect server middleware that the caller cannot be fully trusted.
@ -115,10 +121,13 @@ namespace OpenIddict
return;
}
// Confidential applications MUST authenticate
// Confidential and hybrid applications MUST authenticate
// to protect them from impersonation attacks.
if (string.IsNullOrEmpty(context.ClientSecret))
{
logger.LogError("The revocation request was rejected because the confidential or hybrid application " +
"'{ClientId}' didn't specify a client secret.", context.ClientId);
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidClient,
description: "Missing credentials: ensure that you specified a client_secret.");
@ -128,6 +137,9 @@ namespace OpenIddict
if (!await applications.ValidateClientSecretAsync(application, context.ClientSecret, context.HttpContext.RequestAborted))
{
logger.LogError("The revocation request was rejected because the confidential or hybrid application " +
"'{ClientId}' didn't specify valid client credentials.", context.ClientId);
context.Reject(
error: OpenIdConnectConstants.Errors.InvalidClient,
description: "Invalid credentials: ensure that you specified a correct client_secret.");

4
test/OpenIddict.Tests/OpenIddictProviderTests.Authentication.cs
View File

@ -423,6 +423,7 @@ namespace OpenIddict.Tests
[Theory]
[InlineData("code id_token token")]
[InlineData("code token")]
[InlineData("id_token")]
[InlineData("id_token token")]
[InlineData("token")]
public async Task ValidateAuthorizationRequest_ImplicitOrHybridRequestIsRejectedWhenClientIsConfidential(string type)
@ -464,8 +465,7 @@ namespace OpenIddict.Tests
// Assert
Assert.Equal(OpenIdConnectConstants.Errors.InvalidRequest, response.Error);
Assert.Equal("Confidential clients are not allowed to retrieve " +
"an access token from the authorization endpoint.", response.ErrorDescription);
Assert.Equal("Confidential clients are not allowed to retrieve a token from the authorization endpoint.", response.ErrorDescription);
Mock.Get(manager).Verify(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny<CancellationToken>()), Times.Once());
Mock.Get(manager).Verify(mock => mock.HasRedirectUriAsync(application, It.IsAny<CancellationToken>()), Times.Once());

40
test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs
View File

@ -313,6 +313,46 @@ namespace OpenIddict.Tests
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]
public async Task ValidateTokenRequest_ClientSecretIsRequiredForHybridClients()
{
// Arrange
var application = new OpenIddictApplication();
var manager = CreateApplicationManager(instance =>
{
instance.Setup(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny<CancellationToken>()))
.ReturnsAsync(application);
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.ReturnsAsync(OpenIddictConstants.ClientTypes.Hybrid);
});
var server = CreateAuthorizationServer(builder =>
{
builder.Services.AddSingleton(manager);
});
var client = new OpenIdConnectClient(server.CreateClient());
// Act
var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest
{
ClientId = "Fabrikam",
ClientSecret = null,
GrantType = OpenIdConnectConstants.GrantTypes.Password,
Username = "johndoe",
Password = "A3ddj3w"
});
// Assert
Assert.Equal(OpenIdConnectConstants.Errors.InvalidClient, response.Error);
Assert.Equal("Missing credentials: ensure that you specified a client_secret.", 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_RequestIsRejectedWhenClientCredentialsAreInvalid()
{

39
test/OpenIddict.Tests/OpenIddictProviderTests.Revocation.cs
View File

@ -195,6 +195,45 @@ namespace OpenIddict.Tests
Mock.Get(manager).Verify(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()), Times.Once());
}
[Fact]
public async Task ValidateRevocationRequest_ClientSecretIsRequiredForHybridClients()
{
// Arrange
var application = new OpenIddictApplication();
var manager = CreateApplicationManager(instance =>
{
instance.Setup(mock => mock.FindByClientIdAsync("Fabrikam", It.IsAny<CancellationToken>()))
.ReturnsAsync(application);
instance.Setup(mock => mock.GetClientTypeAsync(application, It.IsAny<CancellationToken>()))
.ReturnsAsync(OpenIddictConstants.ClientTypes.Hybrid);
});
var server = CreateAuthorizationServer(builder =>
{
builder.Services.AddSingleton(manager);
});
var client = new OpenIdConnectClient(server.CreateClient());
// Act
var response = await client.PostAsync(RevocationEndpoint, new OpenIdConnectRequest
{
ClientId = "Fabrikam",
ClientSecret = null,
Token = "SlAV32hkKG",
TokenTypeHint = OpenIdConnectConstants.TokenTypeHints.RefreshToken
});
// Assert
Assert.Equal(OpenIdConnectConstants.Errors.InvalidClient, response.Error);
Assert.Equal("Missing credentials: ensure that you specified a client_secret.", 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 ValidateRevocationRequest_RequestIsRejectedWhenClientCredentialsAreInvalid()
{

Write Preview
Loading…
Cancel
Save