Includes the nullability annotations from community PR.

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/Models/RefreshTokenCreationRequest.cs

@@ -16,12 +16,12 @@ public class RefreshTokenCreationRequest
     /// <summary>
     /// The client.
     /// </summary>
-    public Client Client { get; set; } = default!;
+    public required Client Client { get; set; }
 
     /// <summary>
     /// The subject.
     /// </summary>
-    public ClaimsPrincipal Subject { get; set; } = default!;
+    public required ClaimsPrincipal Subject { get; set; }
 
     /// <summary>
     /// The description.
@@ -31,7 +31,7 @@ public class RefreshTokenCreationRequest
     /// <summary>
     /// The scopes.
     /// </summary>
-    public IEnumerable<string> AuthorizedScopes { get; set; } = default!;
+    public required IEnumerable<string> AuthorizedScopes { get; set; }
 
     /// <summary>
     /// The resource indicators. Null indicates there was no authorization step, thus no restrictions. 
@@ -47,7 +47,7 @@ public class RefreshTokenCreationRequest
     /// <summary>
     /// The access token.
     /// </summary>
-    public Token AccessToken { get; set; } = default!;
+    public required Token AccessToken { get; set; }
 
     /// <summary>
     /// The proof type used.

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,27 @@ namespace Duende.IdentityServer.Validation;
 /// <summary>
 /// Default implementation of IScopeParser.
 /// </summary>
-public class DefaultScopeParser : IScopeParser
+/// <remarks>
+/// Ctor.
+/// </remarks>
+/// <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 +49,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;
@@ -46,7 +45,7 @@ public async Task CreateRefreshToken_token_exists_in_store()
         var client = new Client();
         var accessToken = new Token();
 
-        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = accessToken, Client = client });
+        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = accessToken, Client = client, AuthorizedScopes = [] });
 
         (await _store.GetRefreshTokenAsync(handle)).ShouldNotBeNull();
     }
@@ -62,7 +61,7 @@ public async Task CreateRefreshToken_should_match_absolute_lifetime()
             AbsoluteRefreshTokenLifetime = 10
         };
 
-        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client });
+        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client, AuthorizedScopes = [] });
 
         var refreshToken = (await _store.GetRefreshTokenAsync(handle));
 
@@ -82,7 +81,7 @@ public async Task CreateRefreshToken_should_cap_sliding_lifetime_that_exceeds_ab
             AbsoluteRefreshTokenLifetime = 10
         };
 
-        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client });
+        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client, AuthorizedScopes = [] });
 
         var refreshToken = (await _store.GetRefreshTokenAsync(handle));
 
@@ -101,15 +100,14 @@ public async Task CreateRefreshToken_should_match_sliding_lifetime()
             SlidingRefreshTokenLifetime = 10
         };
 
-        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client });
+        var handle = await _subject.CreateRefreshTokenAsync(new RefreshTokenCreationRequest { Subject = _user, AccessToken = new Token(), Client = client, AuthorizedScopes = [] });
 
         var refreshToken = (await _store.GetRefreshTokenAsync(handle));
 
         refreshToken.ShouldNotBeNull();
         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();
+    }
+}