DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

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

Open Bartizan opened 4 days ago

Bartizan commented 4 days ago

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 commented 23 hours ago

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?