Skip to content

Commit

Permalink
Merge pull request #1796 from DuendeSoftware/beh/no-clientcredentials…
Browse files Browse the repository at this point in the history 
…-without-required-secret

Ensure Client Secret is Required for Clients with ClientCredentials Grant
  • Loading branch information
@bhazen
bhazen authored Feb 13, 2025
2 parents 4f309d6 + fb0fef6 commit dab1717
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 74 deletions.
6 changes: 6 additions & 0 deletions identity-server/src/IdentityServer/Validation/Default/DefaultClientConfigurationValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ protected virtual Task ValidateSecretsAsync(ClientConfigurationValidationContext
return Task.CompletedTask;
}
}

if (string.Equals(grantType, GrantType.ClientCredentials) && !context.Client.RequireClientSecret)
{
context.SetError("RequireClientSecret is false, but client is using client credentials grant type.");
return Task.CompletedTask;
}
}
}

Expand Down
24 changes: 3 additions & 21 deletions identity-server/test/IdentityServer.IntegrationTests/Clients/ClientCredentialsClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,14 @@
// See LICENSE in the project root for license information.


using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text;
using System.Text.Json;
using System.Threading.Tasks;
using Shouldly;
using Duende.IdentityModel;
using Duende.IdentityModel.Client;
using IntegrationTests.Clients.Setup;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.TestHost;
using Xunit;

namespace IntegrationTests.Clients;

Expand Down Expand Up @@ -265,7 +259,7 @@ public async Task Request_posting_client_secret_in_body_should_succeed()


[Fact]
public async Task Request_For_client_with_no_secret_and_basic_authentication_should_succeed()
public async Task Request_For_client_with_no_secret_and_basic_authentication_should_fail()
{
var response = await _client.RequestClientCredentialsTokenAsync(new ClientCredentialsTokenRequest
{
Expand All @@ -274,20 +268,8 @@ public async Task Request_For_client_with_no_secret_and_basic_authentication_sho
Scope = "api1"
});

response.IsError.ShouldBe(false);
response.ExpiresIn.ShouldBe(3600);
response.TokenType.ShouldBe("Bearer");
response.IdentityToken.ShouldBeNull();
response.RefreshToken.ShouldBeNull();

var payload = GetPayload(response);

payload["iss"].GetString().ShouldBe("https://idsvr4");
payload["aud"].GetString().ShouldBe("api");
payload["client_id"].GetString().ShouldBe("client.no_secret");

var scopes = payload["scope"].EnumerateArray();
scopes.First().ToString().ShouldBe("api1");
response.IsError.ShouldBeTrue();
response.Error.ShouldBe("invalid_client");
}

[Fact]
Expand Down
108 changes: 59 additions & 49 deletions ...ity-server/test/IdentityServer.IntegrationTests/Endpoints/Token/DPoPTokenEndpointTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,16 @@
// See LICENSE in the project root for license information.


using Shouldly;
using System.Collections.Generic;
using System.Security.Claims;
using System.Text.Json;
using System.Threading.Tasks;
using Duende.IdentityServer.Models;
using Duende.IdentityServer.Test;
using IntegrationTests.Common;
using Xunit;
using Duende.IdentityModel.Client;
using System;
using Microsoft.AspNetCore.Authentication;
using System.IdentityModel.Tokens.Jwt;
using Microsoft.IdentityModel.JsonWebTokens;
using Microsoft.IdentityModel.Tokens;
using System.Linq;
using System.Net;
using Duende.IdentityModel;
using Duende.IdentityServer.Configuration;
Expand All @@ -26,7 +20,6 @@
using Microsoft.Extensions.Logging;
using Duende.IdentityServer.Extensions;
using Microsoft.Extensions.DependencyInjection;
using System.Text;
using Duende.IdentityServer;

namespace IntegrationTests.Endpoints.Token;
Expand All @@ -37,7 +30,9 @@ public class DPoPTokenEndpointTests

IdentityServerPipeline _mockPipeline = new IdentityServerPipeline();

Client _dpopClient;
Client _dpopConfidentialClient;

Client _dpopPublicClient;

private DateTime _now = new DateTime(2020, 3, 10, 9, 0, 0, DateTimeKind.Utc);
public DateTime UtcNow
Expand All @@ -62,7 +57,7 @@ public DPoPTokenEndpointTests()
};

_mockPipeline.Clients.AddRange(new Client[] {
_dpopClient = new Client
_dpopConfidentialClient = new Client
{
ClientId = "client1",
AllowedGrantTypes = GrantTypes.CodeAndClientCredentials,
Expand All @@ -75,6 +70,17 @@ public DPoPTokenEndpointTests()
AllowOfflineAccess = true,
RefreshTokenUsage = TokenUsage.ReUse,
AllowedScopes = new List<string> { "openid", "profile", "scope1" },
},
_dpopPublicClient = new Client
{
ClientId = "client2",
AllowedGrantTypes = GrantTypes.Code,
RequireClientSecret = false,
RequirePkce = false,
RedirectUris = { "https://client2/callback" },
AllowOfflineAccess = true,
RefreshTokenUsage = TokenUsage.ReUse,
AllowedScopes = new List<string> { "openid", "profile", "scope2" },
}
});

Expand All @@ -96,19 +102,34 @@ public DPoPTokenEndpointTests()
new IdentityResources.Profile(),
new IdentityResources.Email()
});
_mockPipeline.ApiResources.Add(new ApiResource("api1")
_mockPipeline.ApiResources.AddRange(new[]
{
Scopes = { "scope1" },
ApiSecrets =
new ApiResource("api1")
{
new Secret("secret".Sha256())
Scopes = { "scope1" },
ApiSecrets =
{
new Secret("secret".Sha256())
}
},
new ApiResource("api2")
{
Scopes = { "scope2" },
ApiSecrets =
{
new Secret("secret".Sha256())
}
}
});
_mockPipeline.ApiScopes.AddRange(new ApiScope[] {
new ApiScope
{
Name = "scope1"
},
new ApiScope
{
Name = "scope2"
}
});

_mockPipeline.Initialize();
Expand Down Expand Up @@ -359,7 +380,7 @@ public async Task invalid_dpop_request_should_fail()
[Trait("Category", Category)]
public async Task missing_dpop_token_when_required_should_fail()
{
_dpopClient.RequireDPoP = true;
_dpopConfidentialClient.RequireDPoP = true;

var request = new ClientCredentialsTokenRequest
{
Expand Down Expand Up @@ -501,33 +522,31 @@ public async Task confidential_client_dpop_proof_should_be_required_on_renewal()
[Trait("Category", Category)]
public async Task public_client_dpop_proof_should_be_required_on_renewal()
{
_dpopClient.RequireClientSecret = false;

await _mockPipeline.LoginAsync("bob");

_mockPipeline.BrowserClient.AllowAutoRedirect = false;

var url = _mockPipeline.CreateAuthorizeUrl(
clientId: "client1",
clientId: "client2",
responseType: "code",
responseMode: "query",
scope: "openid scope1 offline_access",
redirectUri: "https://client1/callback");
scope: "openid scope2 offline_access",
redirectUri: "https://client2/callback");
var response = await _mockPipeline.BrowserClient.GetAsync(url);

response.StatusCode.ShouldBe(HttpStatusCode.Redirect);
response.Headers.Location.ToString().ShouldStartWith("https://client1/callback");
response.Headers.Location.ToString().ShouldStartWith("https://client2/callback");

var authorization = new AuthorizeResponse(response.Headers.Location.ToString());
authorization.IsError.ShouldBeFalse();

var codeRequest = new AuthorizationCodeTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientId = "client2",
ClientSecret = "secret",
Code = authorization.Code,
RedirectUri = "https://client1/callback",
RedirectUri = "https://client2/callback",
};
codeRequest.Headers.Add("DPoP", CreateDPoPProofToken());

Expand All @@ -539,7 +558,7 @@ public async Task public_client_dpop_proof_should_be_required_on_renewal()
var rtRequest = new RefreshTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientId = "client2",
ClientSecret = "secret",
RefreshToken = codeResponse.RefreshToken
};
Expand Down Expand Up @@ -660,33 +679,30 @@ public async Task confidential_client_should_be_able_to_use_different_dpop_key_f
[Trait("Category", Category)]
public async Task public_client_should_not_be_able_to_use_different_dpop_key_for_refresh_token_request()
{
_dpopClient.RequireClientSecret = false;

await _mockPipeline.LoginAsync("bob");

_mockPipeline.BrowserClient.AllowAutoRedirect = false;

var url = _mockPipeline.CreateAuthorizeUrl(
clientId: "client1",
clientId: "client2",
responseType: "code",
responseMode: "query",
scope: "openid scope1 offline_access",
redirectUri: "https://client1/callback");
scope: "openid scope2 offline_access",
redirectUri: "https://client2/callback");
var response = await _mockPipeline.BrowserClient.GetAsync(url);

response.StatusCode.ShouldBe(HttpStatusCode.Redirect);
response.Headers.Location.ToString().ShouldStartWith("https://client1/callback");
response.Headers.Location.ToString().ShouldStartWith("https://client2/callback");

var authorization = new AuthorizeResponse(response.Headers.Location.ToString());
authorization.IsError.ShouldBeFalse();

var codeRequest = new AuthorizationCodeTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientSecret = "secret",
ClientId = "client2",
Code = authorization.Code,
RedirectUri = "https://client1/callback",
RedirectUri = "https://client2/callback",
};
codeRequest.Headers.Add("DPoP", CreateDPoPProofToken());

Expand All @@ -698,8 +714,7 @@ public async Task public_client_should_not_be_able_to_use_different_dpop_key_for
var rtRequest = new RefreshTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientSecret = "secret",
ClientId = "client2",
RefreshToken = codeResponse.RefreshToken
};

Expand All @@ -719,33 +734,30 @@ public async Task public_client_should_not_be_able_to_use_different_dpop_key_for
[Trait("Category", Category)]
public async Task public_client_using_same_dpop_key_for_refresh_token_request_should_succeed()
{
_dpopClient.RequireClientSecret = false;

await _mockPipeline.LoginAsync("bob");

_mockPipeline.BrowserClient.AllowAutoRedirect = false;

var url = _mockPipeline.CreateAuthorizeUrl(
clientId: "client1",
clientId: "client2",
responseType: "code",
responseMode: "query",
scope: "openid scope1 offline_access",
redirectUri: "https://client1/callback");
scope: "openid scope2 offline_access",
redirectUri: "https://client2/callback");
var response = await _mockPipeline.BrowserClient.GetAsync(url);

response.StatusCode.ShouldBe(HttpStatusCode.Redirect);
response.Headers.Location.ToString().ShouldStartWith("https://client1/callback");
response.Headers.Location.ToString().ShouldStartWith("https://client2/callback");

var authorization = new AuthorizeResponse(response.Headers.Location.ToString());
authorization.IsError.ShouldBeFalse();

var codeRequest = new AuthorizationCodeTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientSecret = "secret",
ClientId = "client2",
Code = authorization.Code,
RedirectUri = "https://client1/callback",
RedirectUri = "https://client2/callback",
};
codeRequest.Headers.Add("DPoP", CreateDPoPProofToken());

Expand All @@ -757,8 +769,7 @@ public async Task public_client_using_same_dpop_key_for_refresh_token_request_sh
var firstRefreshRequest = new RefreshTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientSecret = "secret",
ClientId = "client2",
RefreshToken = codeResponse.RefreshToken
};
firstRefreshRequest.Headers.Add("DPoP", CreateDPoPProofToken());
Expand All @@ -771,8 +782,7 @@ public async Task public_client_using_same_dpop_key_for_refresh_token_request_sh
var secondRefreshRequest = new RefreshTokenRequest
{
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client1",
ClientSecret = "secret",
ClientId = "client2",
RefreshToken = codeResponse.RefreshToken
};
secondRefreshRequest.Headers.Add("DPoP", CreateDPoPProofToken());
Expand All @@ -791,7 +801,7 @@ public async Task public_client_using_same_dpop_key_for_refresh_token_request_sh
[Trait("Category", Category)]
public async Task missing_proof_token_when_required_on_refresh_token_request_should_fail()
{
_dpopClient.RequireDPoP = true;
_dpopConfidentialClient.RequireDPoP = true;

await _mockPipeline.LoginAsync("bob");

Expand Down Expand Up @@ -843,7 +853,7 @@ public async Task missing_proof_token_when_required_on_refresh_token_request_sho
[Trait("Category", Category)]
public async Task valid_dpop_request_using_reference_token_at_introspection_should_return_binding_information()
{
_dpopClient.AccessTokenType = AccessTokenType.Reference;
_dpopConfidentialClient.AccessTokenType = AccessTokenType.Reference;

await _mockPipeline.LoginAsync("bob");

Expand Down
Loading

0 comments on commit dab1717

Please sign in to comment.