spring-projects / spring-projects/spring-security

Token endpoint: a converter that declines gets the same answer as an unknown grant type

Open
#19,741 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

We run the authorization server in production for a large multi-tenant deployment, and we have had our own grant type for four years, because of a legacy system we must stay compatible with. Writing the AuthenticationConverter and the AuthenticationProvider is no problem. The problem starts when it does not work.

GrantTypeRegistryTest sends the same request with a custom grant_type twice:

  • shipped defaults, so no converter for it
  • a converter that is registered for it and returns null, because a required parameter is missing

Status, content type and body are identical, and the body says unsupported_grant_type. A control test in the same class shows the response becomes invalid_grant as soon as the same converter accepts. So the error code that means "I do not know this grant type" is also what a client gets when your converter ran and declined. In production the calling team then reports that the grant type is not registered on the server. It is.

The reason is that nothing is keyed on the grant type:

  • exactly one collection of AuthorizationGrantType exists in the whole jar, on RegisteredClient. AuthorizationServerSettings, OAuth2TokenEndpointFilter and the configurer have none.
  • grant_types_supported comes from a fixed list in OidcProviderConfigurationEndpointFilter and not from the registered converters, so an extension grant is never advertised. (read from the source, no test here, it needs a full server context)
  • provider selection is supports(Class<?>) only. No built-in provider has a method taking an AuthorizationGrantType, four of them claim OAuth2ClientAuthenticationToken, and all are final. (ProviderSelectionTest)
  • the only key you can share is the base class OAuth2AuthorizationGrantAuthenticationToken. A provider on it claims every grant token in the jar, also the ones added in a later release, with no compile error and no startup error, and supports() never sees the grant type to decline it. (BaseClassClaimsFutureGrantsTest)
  • overriding a built-in grant is only list position. Behind the built-in converter mine is never called, in front of it it wins, and accessTokenRequestConverters(...) hands out the raw List. (OverridingBuiltInGrantTest)

What we do today: our own filter in front of OAuth2TokenEndpointFilter that validates the grant type first, providers placed by position, and a grant type check inside authenticate() because supports() cannot do it. All three are position based or defensive, and none of them survives a refactoring in the library without us noticing late.

Two things would help:

  1. Distinguish "no converter claimed this request" from "a converter was called and declined". The same error code with a different error_description is already enough.
  2. Document that the order of converters and providers matters, and that supports() is class based.

Tests for each point: https://github.com/macstab/spring-authorization-server-issues - green on 7.1.1, on 7.0.7 and on standalone 1.5.3. Not a regression, identical on all three.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with GrantTypeRegistryTest, ProviderSelectionTest, BaseClassClaimsFutureGrantsTest, and OverridingBuiltInGrantTest, then inspect OAuth2TokenEndpointFilter and OidcProviderConfigurationEndpointFilter. Determine how to distinguish an unclaimed request from a converter that declines and how to document converter/provider ordering and class-based supports(). Done means the requested behavior and documentation are covered by tests for each listed case.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
api, authentication, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.