Avoid unhandled argument null exception for refresh_token grant in token validation when scopes are missing

[bartizan]

Which version of Duende IdentityServer are you using?
v7.0.6

Which version of .NET are you using?
.net v8

Describe the bug

On call to /connect/token endpoint Identity Server returns an HTML page due to an internal error instead of a response in JSON format.

To Reproduce

1. an invalid refresh token is created by a 3rd party app. (not Duende IdentityServer). Its scopes property equals null.
2. Requesting a new refresh token

POST /connect/token

    client_id=client&
    grant_type=refresh_token&
    refresh_token=hdh922

3. Processing the request, a refresh token is restored from PersistedGrants table, deserialized, and validated.
4. The scope validation fails (Duende.IdentityServer.Validation.DefaultScopeParser.ParseScopeValues()). The error is System.ArgumentNullException: Value cannot be null. (Parameter 'scopeValues').

Expected behavior

According to OAuth 2 specification the response from the API should be in JSON format.
A token error response is like

{
   "error": "invalid_request"
}

Log output/exception with stacktrace

{
  "Level": "Fatal",
  "SpanId": "b44fe99cdd32a6d6",
  "TraceId": "c388ac98b921cb23b1d5e546c9ff286f",
  "Exception": "System.ArgumentNullException: Value cannot be null. (Parameter 'scopeValues')\r\n   at Duende.IdentityServer.Validation.DefaultScopeParser.ParseScopeValues(IEnumerable`1 scopeValues) in /_/src/IdentityServer/Validation/Default/DefaultScopeParser.cs:line 34\r\n   
  at Duende.IdentityServer.Validation.DefaultResourceValidator.ValidateRequestedResourcesAsync(ResourceValidationRequest request) in /_/src/IdentityServer/Validation/Default/DefaultResourceValidator.cs:line 46\r\n   
  at Duende.IdentityServer.Validation.TokenRequestValidator.ValidateRefreshTokenRequestAsync(NameValueCollection parameters) in /_/src/IdentityServer/Validation/Default/TokenRequestValidator.cs:line 787\r\n   
  at Duende.IdentityServer.Validation.TokenRequestValidator.RunValidationAsync(Func`2 validationFunc, NameValueCollection parameters) in /_/src/IdentityServer/Validation/Default/TokenRequestValidator.cs:line 273\r\n   
  at Duende.IdentityServer.Validation.TokenRequestValidator.ValidateRequestAsync(TokenRequestValidationContext context) in /_/src/IdentityServer/Validation/Default/TokenRequestValidator.cs:line 196\r\n   
  at Duende.IdentityServer.Endpoints.TokenEndpoint.ProcessTokenRequestAsync(HttpContext context) in /_/src/IdentityServer/Endpoints/TokenEndpoint.cs:line 120\r\n   
  at Duende.IdentityServer.Endpoints.TokenEndpoint.ProcessAsync(HttpContext context) in /_/src/IdentityServer/Endpoints/TokenEndpoint.cs:line 81\r\n   
  at Duende.IdentityServer.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IdentityServerOptions options, IEndpointRouter router, IUserSession userSession, IEventService events, IIssuerNameService issuerNameService, ISessionCoordinationService sessionCoordinationService) in /_/src/IdentityServer/Hosting/IdentityServerMiddleware.cs:line 106\r\n   at Duende.IdentityServer.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IdentityServerOptions options, IEndpointRouter router, IUserSession userSession, IEventService events, IIssuerNameService issuerNameService, ISessionCoordinationService sessionCoordinationService) in /_/src/IdentityServer/Hosting/IdentityServerMiddleware.cs:line 128\r\n   at Duende.IdentityServer.Hosting.MutualTlsEndpointMiddleware.Invoke(HttpContext context, IAuthenticationSchemeProvider schemes) in /_/src/IdentityServer/Hosting/MutualTlsEndpointMiddleware.cs:line 95\r\n   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)\r\n   at Duende.IdentityServer.Hosting.DynamicProviders.DynamicSchemeAuthenticationMiddleware.Invoke(HttpContext context) in /_/src/IdentityServer/Hosting/DynamicProviders/DynamicSchemes/DynamicSchemeAuthenticationMiddleware.cs:line 51\r\n   at Duende.IdentityServer.Hosting.BaseUrlMiddleware.Invoke(HttpContext context) in /_/src/IdentityServer/Hosting/BaseUrlMiddleware.cs:line 27\r\n   at  Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddlewareImpl.<Invoke>g__Awaited|10_0(ExceptionHandlerMiddlewareImpl middleware, HttpContext context, Task task)",
  "Timestamp": "2024-09-16T15:16:42.6281034+03:00",
  "Properties": {
    "RequestId": "8000285b-0000-f600-b63f-84710c7967bb",
    "exception": "Value cannot be null. (Parameter 'scopeValues')",
    "RequestPath": "/connect/token",
    "SourceContext": "Duende.IdentityServer.Hosting.IdentityServerMiddleware",
  },
  "MessageTemplate": "Unhandled exception: {exception}"
}

Additional context

Please, revise the necessity of the improvement similar to what we find for authorization_code grant validation, taking into account that the problem occurred due to an error in 3rd party app. after migration from IDS v4.

[josephdecock]

What's the intended semantic of a refresh token without any scopes? @RolandGuijt are you able to repro this - do we actually allow the refresh token to be created in the first place?

[josephdecock]

@bartizan, can you describe how the refresh token is getting created in more detail? Is some out of band mechanism inserting this refresh token into the grants store?

[bartizan]

@josephdecock, it seems there is nothing out of band. It's used IRefreshTokenService api for a refresh token creation (or an update).
In RefreshTokenCreationRequest and RefreshToken relevant classes, there are the AuthorizedScopes property defined as

 public IEnumerable<string> AuthorizedScopes { get; set; } = default!; 

It resolves to null by default and ignores nullable warnings in code analysis in VS. Thus it could be unintentionally created/updated with the property equals null (with an empty list it would be fine).

    class Service
    {
       Duende.IdentityServer.Services.IRefreshTokenService _refreshTokenService;

       async Task CreateSomeRefreshToken()
       {
            //...
            var request = new Duende.IdentityServer.Models.RefreshTokenCreationRequest()
            {
                AccessToken = my_token,
                Subject = my_principal,
                Client = my_client,
                AuthorizedScopes = my_client.AllowedScopes,  // <<<--- fix for v7. The scopes have to be assigned
            };

            var refresh_token = await _refreshTokenService.CreateRefreshTokenAsync(request);
       }
    }

[josephdecock]

I think that we should rather improve the nullability annotations on the RefreshTokenCreationRequest, as a refresh token with null scopes is not expected.

[RolandGuijt]

I have created an issue in IdentityServer's repo. Please track the issue there from now on.

Thanks for reporting this!