Merge pull request #73 from kallayj/clientid-null-exception

Adds a specific exception when the clientId is empty.

Author: josephdecock

src/Duende.AccessTokenManagement/ClientCredentialsTokenEndpointService.cs

@@ -58,9 +58,24 @@ public virtual async Task<ClientCredentialsToken> RequestToken(
     {
         var client = _options.Get(clientName);
 
-        if (string.IsNullOrWhiteSpace(client.TokenEndpoint) || string.IsNullOrEmpty(client.ClientId))
+        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

src/Duende.AccessTokenManagement/ClientCredentialsTokenManagementBuilder.cs

@@ -1,36 +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;
-
-/// <summary>
-/// Builder for client credential clients
-/// </summary>
-public class ClientCredentialsTokenManagementBuilder
-{
-    private readonly IServiceCollection _services;
-
-    /// <summary>
-    /// ctor
-    /// </summary>
-    /// <param name="services"></param>
-    public ClientCredentialsTokenManagementBuilder(IServiceCollection services)
-    {
-        _services = services;
-    }
-
-    /// <summary>
-    /// Adds a client credentials client to the token management system
-    /// </summary>
-    /// <param name="name"></param>
-    /// <param name="configureOptions"></param>
-    /// <returns></returns>
-    public ClientCredentialsTokenManagementBuilder AddClient(string name, Action<ClientCredentialsClient> configureOptions)
-    {
-        _services.Configure(name, configureOptions);
-        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;
+
+/// <summary>
+/// Builder for client credential clients
+/// </summary>
+public class ClientCredentialsTokenManagementBuilder
+{
+    private readonly IServiceCollection _services;
+
+    /// <summary>
+    /// ctor
+    /// </summary>
+    /// <param name="services"></param>
+    public ClientCredentialsTokenManagementBuilder(IServiceCollection services)
+    {
+        _services = services;
+    }
+
+    /// <summary>
+    /// Adds a client credentials client to the token management system
+    /// </summary>
+    /// <param name="name"></param>
+    /// <param name="configureOptions"></param>
+    /// <returns></returns>
+    public ClientCredentialsTokenManagementBuilder AddClient(string name, Action<ClientCredentialsClient> configureOptions)
+    {
+        _services.Configure(name, configureOptions);
+        return this;
+    }
 }

test/Tests/ClientTokenManagementTests.cs

@@ -24,12 +24,55 @@ public async Task Unknown_client_should_throw_exception()
         var provider = services.BuildServiceProvider();
         var sut = provider.GetRequiredService<IClientCredentialsTokenManagementService>();
 
-        async Task action()
-        {
-            var token = await sut.GetAccessTokenAsync("unknown");
-        }
+        var action = async () => await sut.GetAccessTokenAsync("unknown");
+
+        (await Should.ThrowAsync<InvalidOperationException>(action))
+            .Message.ShouldBe("Unknown client unknown");
+    }
+
+    [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;
+            });
+
+        var provider = services.BuildServiceProvider();
+        var sut = provider.GetRequiredService<IClientCredentialsTokenManagementService>();
+
+        var action = async () => await sut.GetAccessTokenAsync("test");
+
+        (await Should.ThrowAsync<InvalidOperationException>(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<IClientCredentialsTokenManagementService>();
+
+        var action = async () => await sut.GetAccessTokenAsync("test");
 
-        await Should.ThrowAsync<InvalidOperationException>(action);
+        (await Should.ThrowAsync<InvalidOperationException>(action))
+            .Message.ShouldBe("No TokenEndpoint configured for client test");
     }
 
     [Theory]