Addresses an unhandled exception path & nullability concerns (#1930)

Updates the `DefaultScopeParser` to return gracefully rather than throwing an exception (which is ultimately unhandled).
Introduces a new unit test to cover `AuthorizedScopes` being null (as apposed to an empty collection).

Author: StuFrankish

identity-server/src/IdentityServer/Validation/Default/DefaultScopeParser.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Duende Software. All rights reserved.
 // See LICENSE in the project root for license information.
 
-
 using Duende.IdentityServer.Extensions;
 using Microsoft.Extensions.Logging;
 
@@ -10,29 +9,24 @@ namespace Duende.IdentityServer.Validation;
 /// <summary>
 /// Default implementation of IScopeParser.
 /// </summary>
-public class DefaultScopeParser : IScopeParser
+/// <param name="logger"></param>
+public class DefaultScopeParser(ILogger<DefaultScopeParser> logger) : IScopeParser
 {
-    private readonly ILogger<DefaultScopeParser> _logger;
-
-    /// <summary>
-    /// Ctor.
-    /// </summary>
-    /// <param name="logger"></param>
-    public DefaultScopeParser(ILogger<DefaultScopeParser> logger)
-    {
-        _logger = logger;
-    }
-
     /// <inheritdoc/>
     public ParsedScopesResult ParseScopeValues(IEnumerable<string> scopeValues)
     {
         using var activity = Tracing.ValidationActivitySource.StartActivity("DefaultScopeParser.ParseScopeValues");
         activity?.SetTag(Tracing.Properties.Scope, scopeValues.ToSpaceSeparatedString());
 
-        if (scopeValues == null) throw new ArgumentNullException(nameof(scopeValues));
-
         var result = new ParsedScopesResult();
 
+        if (scopeValues is null)
+        {
+            logger.LogError("A collection of scopes cannot be null.");
+            result.Errors.Add(new ParsedScopeValidationError("null", "A collection of scopes cannot be null."));
+            return result;
+        }
+
         foreach (var scopeValue in scopeValues)
         {
             var ctx = new ParseScopeContext(scopeValue);
@@ -52,7 +46,7 @@ public ParsedScopesResult ParseScopeValues(IEnumerable<string> scopeValues)
             }
             else
             {
-                _logger.LogDebug("Scope parsing ignoring scope {scope}", scopeValue.SanitizeLogParameter());
+                logger.LogDebug("Scope parsing ignoring scope {scope}", scopeValue.SanitizeLogParameter());
             }
         }
 

identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultRefreshTokenServiceTests.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Duende Software. All rights reserved.
 // See LICENSE in the project root for license information.
 
-
 using System.Security.Claims;
 using Duende.IdentityServer;
 using Duende.IdentityServer.Models;
@@ -109,7 +108,6 @@ public async Task CreateRefreshToken_should_match_sliding_lifetime()
         refreshToken.Lifetime.ShouldBe(client.SlidingRefreshTokenLifetime);
     }
 
-
     [Fact]
     public async Task UpdateRefreshToken_one_time_use_should_create_new_token()
     {

identity-server/test/IdentityServer.UnitTests/Validation/TokenRequest Validation/TokenRequestValidation_Invalid.cs

@@ -0,0 +1,58 @@
+// Copyright (c) Duende Software. All rights reserved.
+// See LICENSE in the project root for license information.
+
+using System.Collections.Specialized;
+using System.Security.Claims;
+using Duende.IdentityModel;
+using Duende.IdentityServer;
+using Duende.IdentityServer.Models;
+using Duende.IdentityServer.Stores;
+using UnitTests.Validation.Setup;
+
+namespace UnitTests.Validation.TokenRequest_Validation;
+
+public class TokenRequestValidation_Invalid
+{
+    private const string Category = "TokenRequest Validation - General - Invalid";
+
+    private readonly IClientStore _clients = Factory.CreateClientStore();
+
+    [Fact]
+    [Trait("Category", Category)]
+    public async Task Invalid_refresh_token_request_should_fail()
+    {
+        var subjectClaim = new Claim(JwtClaimTypes.Subject, "foo");
+
+        var refreshToken = new RefreshToken
+        {
+            ClientId = "roclient",
+            Subject = new IdentityServerUser(subjectClaim.Value).CreatePrincipal(),
+            AuthorizedScopes = null, // Passing a null value to AuthorizedScopes should be invalid
+            Lifetime = 600,
+            CreationTime = DateTime.UtcNow
+        };
+
+        refreshToken.SetAccessToken(new Token("access_token")
+        {
+            Claims = [subjectClaim],
+            ClientId = "roclient"
+        });
+
+        var grants = Factory.CreateRefreshTokenStore();
+        var handle = await grants.StoreRefreshTokenAsync(refreshToken);
+
+        var client = await _clients.FindEnabledClientByIdAsync("roclient");
+
+        var validator = Factory.CreateTokenRequestValidator(refreshTokenStore: grants);
+
+        var parameters = new NameValueCollection
+        {
+            { OidcConstants.TokenRequest.GrantType, "refresh_token" },
+            { OidcConstants.TokenRequest.RefreshToken, handle }
+        };
+
+        var result = await validator.ValidateRequestAsync(parameters, client.ToValidationResult());
+
+        result.IsError.ShouldBeTrue();
+    }
+}