From fd059944ee5afa3c5cad5cfef21e340c2f7d6853 Mon Sep 17 00:00:00 2001 From: Jonathan Kallay Date: Wed, 13 Mar 2024 17:33:47 -0700 Subject: [PATCH 1/4] Adds a specific exception when the clientId is empty. --- .../ClientCredentialsTokenEndpointService.cs | 2 +- ...ClientCredentialsTokenManagementBuilder.cs | 7 +++++ test/Tests/ClientTokenManagementTests.cs | 29 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs b/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs index 1adaf15..e59a4e6 100755 --- a/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs +++ b/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs @@ -58,7 +58,7 @@ public virtual async Task RequestToken( { var client = _options.Get(clientName); - if (string.IsNullOrWhiteSpace(client.TokenEndpoint) || string.IsNullOrEmpty(client.ClientId)) + if (string.IsNullOrWhiteSpace(client.TokenEndpoint)) { throw new InvalidOperationException("unknown client"); } diff --git a/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs b/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs index d7d69e2..e931e96 100644 --- a/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs +++ b/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs @@ -31,6 +31,13 @@ public ClientCredentialsTokenManagementBuilder(IServiceCollection services) public ClientCredentialsTokenManagementBuilder AddClient(string name, Action configureOptions) { _services.Configure(name, configureOptions); + _services.PostConfigure(name, client => + { + if (string.IsNullOrEmpty(client.ClientId)) + { + throw new InvalidOperationException("ClientId must not be empty"); + } + }); return this; } } \ No newline at end of file diff --git a/test/Tests/ClientTokenManagementTests.cs b/test/Tests/ClientTokenManagementTests.cs index 1d6240d..515c816 100644 --- a/test/Tests/ClientTokenManagementTests.cs +++ b/test/Tests/ClientTokenManagementTests.cs @@ -30,6 +30,35 @@ async Task action() } await Should.ThrowAsync(action); + } + + [Fact] + public async Task Missing_client_id_throw_exception() + { + var services = new ServiceCollection(); + + services.AddDistributedMemoryCache(); + services.AddClientCredentialsTokenManagement() + .AddClient("test", client => + { + client.TokenEndpoint = "https://as/connect/token"; + client.ClientId = null; + client.ClientSecret = "client_secret"; + + client.Scope = "scope"; + client.Resource = "resource"; + client.Parameters.Add("audience", "audience"); + }); + + var provider = services.BuildServiceProvider(); + var sut = provider.GetRequiredService(); + + async Task action() + { + var token = await sut.GetAccessTokenAsync("test"); + } + + await Should.ThrowAsync(action, "ClientId must not be empty"); } [Theory] From a458d7b3547185eebd73c4fd0e8abc8c3ccbc6c9 Mon Sep 17 00:00:00 2001 From: Jonathan Kallay Date: Mon, 22 Apr 2024 09:38:39 -0700 Subject: [PATCH 2/4] Fixes error message assertions. --- test/Tests/ClientTokenManagementTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/Tests/ClientTokenManagementTests.cs b/test/Tests/ClientTokenManagementTests.cs index 515c816..5048b0c 100644 --- a/test/Tests/ClientTokenManagementTests.cs +++ b/test/Tests/ClientTokenManagementTests.cs @@ -29,7 +29,8 @@ async Task action() var token = await sut.GetAccessTokenAsync("unknown"); } - await Should.ThrowAsync(action); + var exception = await Should.ThrowAsync(action); + Should.Equals("unknown client", exception.Message); } [Fact] @@ -58,7 +59,8 @@ async Task action() var token = await sut.GetAccessTokenAsync("test"); } - await Should.ThrowAsync(action, "ClientId must not be empty"); + var exception = await Should.ThrowAsync(action); + Should.Equals("ClientId must not be empty", exception.Message); } [Theory] From 1a6bfbc6268e953aa34497fb1ee8b01cbdaeea60 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 2 May 2024 14:00:45 -0500 Subject: [PATCH 3/4] Minor stylistic change --- test/Tests/ClientTokenManagementTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Tests/ClientTokenManagementTests.cs b/test/Tests/ClientTokenManagementTests.cs index 5048b0c..20b87f7 100644 --- a/test/Tests/ClientTokenManagementTests.cs +++ b/test/Tests/ClientTokenManagementTests.cs @@ -29,10 +29,10 @@ async Task action() var token = await sut.GetAccessTokenAsync("unknown"); } - var exception = await Should.ThrowAsync(action); - Should.Equals("unknown client", exception.Message); - } - + (await Should.ThrowAsync(action)) + .Message.ShouldBe("unknown client"); + } + [Fact] public async Task Missing_client_id_throw_exception() { @@ -59,8 +59,8 @@ async Task action() var token = await sut.GetAccessTokenAsync("test"); } - var exception = await Should.ThrowAsync(action); - Should.Equals("ClientId must not be empty", exception.Message); + (await Should.ThrowAsync(action)) + .Message.ShouldBe("ClientId must not be empty"); } [Theory] From 4a963764f0a5bb9b31e06ae30dff7eedf6145e66 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Sun, 5 May 2024 21:49:32 -0500 Subject: [PATCH 4/4] More specific exceptions if client credentials client is misconfigured --- .../ClientCredentialsTokenEndpointService.cs | 17 +++- ...ClientCredentialsTokenManagementBuilder.cs | 77 +++++++++---------- test/Tests/ClientTokenManagementTests.cs | 40 ++++++---- 3 files changed, 77 insertions(+), 57 deletions(-) diff --git a/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs b/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs index e59a4e6..9305719 100755 --- a/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs +++ b/src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs @@ -58,9 +58,24 @@ public virtual async Task RequestToken( { var client = _options.Get(clientName); + var clientIdMissing = string.IsNullOrWhiteSpace(client.ClientId); + var tokenEndpointMissing = string.IsNullOrWhiteSpace(client.TokenEndpoint); + + // If both are missing, we infer that this client is just not set up at all + if (clientIdMissing && tokenEndpointMissing) + { + throw new InvalidOperationException($"Unknown client {clientName}"); + } + + // Otherwise, if we don't have a specific value that is required, throw an appropriate exception + if (string.IsNullOrWhiteSpace(client.ClientId)) + { + throw new InvalidOperationException($"No ClientId configured for client {clientName}"); + } + if (string.IsNullOrWhiteSpace(client.TokenEndpoint)) { - throw new InvalidOperationException("unknown client"); + throw new InvalidOperationException($"No TokenEndpoint configured for client {clientName}"); } var request = new ClientCredentialsTokenRequest diff --git a/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs b/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs index e931e96..91d3a0b 100644 --- a/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs +++ b/src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs @@ -1,43 +1,36 @@ -// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -using System; -using Duende.AccessTokenManagement; - -namespace Microsoft.Extensions.DependencyInjection; - -/// -/// Builder for client credential clients -/// -public class ClientCredentialsTokenManagementBuilder -{ - private readonly IServiceCollection _services; - - /// - /// ctor - /// - /// - public ClientCredentialsTokenManagementBuilder(IServiceCollection services) - { - _services = services; - } - - /// - /// Adds a client credentials client to the token management system - /// - /// - /// - /// - public ClientCredentialsTokenManagementBuilder AddClient(string name, Action configureOptions) - { - _services.Configure(name, configureOptions); - _services.PostConfigure(name, client => - { - if (string.IsNullOrEmpty(client.ClientId)) - { - throw new InvalidOperationException("ClientId must not be empty"); - } - }); - return this; - } +// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +using System; +using Duende.AccessTokenManagement; + +namespace Microsoft.Extensions.DependencyInjection; + +/// +/// Builder for client credential clients +/// +public class ClientCredentialsTokenManagementBuilder +{ + private readonly IServiceCollection _services; + + /// + /// ctor + /// + /// + public ClientCredentialsTokenManagementBuilder(IServiceCollection services) + { + _services = services; + } + + /// + /// Adds a client credentials client to the token management system + /// + /// + /// + /// + public ClientCredentialsTokenManagementBuilder AddClient(string name, Action configureOptions) + { + _services.Configure(name, configureOptions); + return this; + } } \ No newline at end of file diff --git a/test/Tests/ClientTokenManagementTests.cs b/test/Tests/ClientTokenManagementTests.cs index 20b87f7..cf5d468 100644 --- a/test/Tests/ClientTokenManagementTests.cs +++ b/test/Tests/ClientTokenManagementTests.cs @@ -24,13 +24,10 @@ public async Task Unknown_client_should_throw_exception() var provider = services.BuildServiceProvider(); var sut = provider.GetRequiredService(); - async Task action() - { - var token = await sut.GetAccessTokenAsync("unknown"); - } + var action = async () => await sut.GetAccessTokenAsync("unknown"); (await Should.ThrowAsync(action)) - .Message.ShouldBe("unknown client"); + .Message.ShouldBe("Unknown client unknown"); } [Fact] @@ -44,23 +41,38 @@ public async Task Missing_client_id_throw_exception() { client.TokenEndpoint = "https://as/connect/token"; client.ClientId = null; - client.ClientSecret = "client_secret"; + }); - client.Scope = "scope"; - client.Resource = "resource"; - client.Parameters.Add("audience", "audience"); + var provider = services.BuildServiceProvider(); + var sut = provider.GetRequiredService(); + + var action = async () => await sut.GetAccessTokenAsync("test"); + + (await Should.ThrowAsync(action)) + .Message.ShouldBe("No ClientId configured for client test"); + } + + + [Fact] + public async Task Missing_tokenEndpoint_throw_exception() + { + var services = new ServiceCollection(); + + services.AddDistributedMemoryCache(); + services.AddClientCredentialsTokenManagement() + .AddClient("test", client => + { + client.TokenEndpoint = null; + client.ClientId = "test"; }); var provider = services.BuildServiceProvider(); var sut = provider.GetRequiredService(); - async Task action() - { - var token = await sut.GetAccessTokenAsync("test"); - } + var action = async () => await sut.GetAccessTokenAsync("test"); (await Should.ThrowAsync(action)) - .Message.ShouldBe("ClientId must not be empty"); + .Message.ShouldBe("No TokenEndpoint configured for client test"); } [Theory]