Handle null secret in HashedSharedSecretValidator #3404

IdentityServer · Jul 9, 2019 · 7be6a8d · 7be6a8d
1 parent fd15f76
commit 7be6a8d
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 0 deletions.
diff --git a/src/IdentityServer4/src/Validation/Default/HashedSharedSecretValidator.cs b/src/IdentityServer4/src/Validation/Default/HashedSharedSecretValidator.cs
@@ -82,6 +82,11 @@ public Task<SecretValidationResult> ValidateAsync(IEnumerable<Secret> secrets, P
                     _logger.LogInformation("Secret: {description} uses invalid hashing algorithm.", secretDescription);
                     return fail;
                 }
+                catch (ArgumentNullException)
+                {
+                    _logger.LogInformation("Secret: {description} is null.", secretDescription);
+                    return fail;
+                }
 
                 if (secretBytes.Length == 32)
                 {

diff --git a/...yServer4/test/IdentityServer.UnitTests/Validation/Secrets/HashedSharedSecretValidation.cs b/...yServer4/test/IdentityServer.UnitTests/Validation/Secrets/HashedSharedSecretValidation.cs
@@ -140,5 +140,23 @@ public async Task Client_with_no_Secret_Should_Fail()
             var result = await _validator.ValidateAsync(client.ClientSecrets, secret);
             result.Success.Should().BeFalse();
         }
+
+        [Fact]
+        [Trait("Category", Category)]
+        public async Task Client_with_null_Secret_Should_Fail()
+        {
+            var clientId = "null_secret_client";
+            var client = await _clients.FindEnabledClientByIdAsync(clientId);
+
+            var secret = new ParsedSecret
+            {
+                Id = clientId,
+                Type = IdentityServerConstants.ParsedSecretTypes.SharedSecret,
+                Credential = "secret"
+            };
+
+            var result = await _validator.ValidateAsync(client.ClientSecrets, secret);
+            result.Success.Should().BeFalse();
+        }
     }
 }
diff --git a/...rver4/test/IdentityServer.UnitTests/Validation/Secrets/PlainTextClientSecretValidation.cs b/...rver4/test/IdentityServer.UnitTests/Validation/Secrets/PlainTextClientSecretValidation.cs
@@ -140,5 +140,23 @@ public async Task Client_with_no_Secret_Should_Fail()
             var result = await _validator.ValidateAsync(client.ClientSecrets, secret);
             result.Success.Should().BeFalse();
         }
+
+        [Fact]
+        [Trait("Category", Category)]
+        public async Task Client_with_null_Secret_Should_Fail()
+        {
+            var clientId = "null_secret_client";
+            var client = await _clients.FindEnabledClientByIdAsync(clientId);
+
+            var secret = new ParsedSecret
+            {
+                Id = clientId,
+                Type = IdentityServerConstants.ParsedSecretTypes.SharedSecret,
+                Credential = "secret"
+            };
+
+            var result = await _validator.ValidateAsync(client.ClientSecrets, secret);
+            result.Success.Should().BeFalse();
+        }
     }
 }
diff --git a/...tityServer4/test/IdentityServer.UnitTests/Validation/Setup/ClientValidationTestClients.cs b/...tityServer4/test/IdentityServer.UnitTests/Validation/Setup/ClientValidationTestClients.cs
@@ -36,6 +36,14 @@ public static List<Client> Get()
                     Enabled = true
                 },
 
+                new Client
+                {
+                    ClientName = "Client with null secret set",
+                    ClientId = "null_secret_client",
+                    Enabled = true,
+                    ClientSecrets = { new Secret(null) }
+                },
+
                 new Client
                 {
                     ClientName = "Client with single secret, no protection, no expiration",