spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.42k stars 5.74k forks source link

When token expires_in is less then 62 seconds two requests are made instead of 1 #14943

Closed tkoval83 closed 1 week ago

tkoval83 commented 1 week ago

Describe the bug When token expires_in is less then 62 seconds two requests are made instead of 1.

To Reproduce In token response for client_credentials set expires_in to less then 62.

Expected behavior Only single request to token endpoint should be made.

Sample

{
  "request": {
    "method": "POST",
    "headers": {
      "Content-Type": {
        "contains": "application/x-www-form-urlencoded"
      },
      "Authorization": {
        "matches": "Basic .+"
      }
    },
    "bodyPatterns": [
      {
        "contains": "grant_type=client_credentials"
      },
      {
        "contains": "scope=OAUTH2_TOKEN_INTROSPECTION"
      }
    ]
  },
  "response": {
    "status": 200,
    "headers": {
      "Content-Type": "application/json"
    },
    "jsonBody": {
      "access_token": "Uox5PYIFqhKmNzey7PUpxRjJiTET",
      "token_type": "Bearer",
      "expires_in": 5,
      "scope": "OAUTH2_TOKEN_INTROSPECTION"
    }
  }
}

I am using WebClient configuration for OAuth2. Application itself is NOT reactive.

@Bean
public ReactiveOAuth2AuthorizedClientManager authorizedClientManager(
  @Qualifier("introspectOAuth") final WebClient introspectOAuthClient,
  final ReactiveClientRegistrationRepository registrations,
  final ReactiveOAuth2AuthorizedClientService clients) {
  final Converter<OAuth2ClientCredentialsGrantRequest, HttpHeaders> headersConverter = request -> {
    final ClientRegistration registration = request.getClientRegistration();
    final HttpHeaders headers = new HttpHeaders();
    headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
    headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
    headers.setBasicAuth(registration.getClientId(), registration.getClientSecret());
    headers.set(SERVICE_ID, TOKEN_INTROSPECTION_OAUTH2);
    return headers;
  };

  final var client = new WebClientReactiveClientCredentialsTokenResponseClient();
  client.setHeadersConverter(headersConverter);
  client.setWebClient(introspectOAuthClient);

  final var authorizedClientProvider = ReactiveOAuth2AuthorizedClientProviderBuilder.builder()
    .clientCredentials(clientCredentials -> clientCredentials.accessTokenResponseClient(client))
    .build();

  final var authorizedClientManager =
    new AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager(registrations, clients);
  authorizedClientManager.setAuthorizedClientProvider(authorizedClientProvider);
  return authorizedClientManager;
}

@Bean
public ReactiveClientRegistrationRepository reactiveClientRegistrationRepository(
  ClientRegistrationRepository clientRegistrations) {
  return new InMemoryReactiveClientRegistrationRepository(
    clientRegistrations.findByRegistrationId(CLIENT_REGISTRATION_ID));
}

@Bean
public ReactiveOAuth2AuthorizedClientService reactiveOAuth2AuthorizedClientService(
  ReactiveClientRegistrationRepository registrations) {
  return new InMemoryReactiveOAuth2AuthorizedClientService(registrations);
}

@Bean(name = "introspect")
public WebClient introspectWebApiClient(
  final ReactiveOAuth2AuthorizedClientManager authorizedClientManager,
  final ReactiveOAuth2AuthorizedClientService authorizedClientService,
  final IntrospectApiClientProperties properties,
  final ObjectMapper objectMapper
) {
  final var oauth = new ServerOAuth2AuthorizedClientExchangeFilterFunction(authorizedClientManager);
  oauth.setDefaultClientRegistrationId(CLIENT_REGISTRATION_ID);
  oauth.setAuthorizationFailureHandler(new RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler(
      (clientRegistrationId, principal, attributes) -> {
        final String principalName = Optional.ofNullable(principal)
          .map(Principal::getName)
          .orElse(null);

        log.debug(
          "Removing stale authorization for {} and principal {}",
          clientRegistrationId,
          principalName
        );

        return authorizedClientService.removeAuthorizedClient(
          clientRegistrationId,
          principalName
        );
      }
    )
  );

  final long timeout = Duration.ofSeconds(properties.getTimeout()).toMillis();
  final HttpClient httpClient = HttpClient.create()
    .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, (int) timeout)
    .doOnConnected(
      conn -> conn.addHandlerLast(new ReadTimeoutHandler(timeout, TimeUnit.MILLISECONDS)));

  final ExchangeStrategies jacksonStrategy = ExchangeStrategies.builder()
    .codecs(config -> {
      config.defaultCodecs().jackson2JsonEncoder(new Jackson2JsonEncoder(objectMapper, MediaType.APPLICATION_JSON));
      config.defaultCodecs().jackson2JsonDecoder(new Jackson2JsonDecoder(objectMapper, MediaType.APPLICATION_JSON));
    }).build();

  return WebClient.builder()
    .filter(oauth)
    .exchangeStrategies(jacksonStrategy)
    .clientConnector(new ReactorClientHttpConnector(httpClient))
    .build();
}
sjohnr commented 1 week ago

Hi @tkoval83, thanks for reaching out.

You mention:

I am using WebClient configuration for OAuth2. Application itself is NOT reactive.

yet your configuration is using all reactive components which is not intended. Please see core interfaces and classes and WebClient integration for servlet applications in the docs.

Only single request to token endpoint should be made.

It's not clear from your issue whether you're referring to (1) the framework making two calls to the token endpoint in response to a single request to the application, or (2) something else. If (1), please provide a minimal, reproducible sample or an integration test that reproduces the issue.

If you're referring to multiple requests to the application, I don't feel that the expectation of a single atomic request to the token endpoint is reasonable for the framework out of the box. Please see discussions on this in gh-11461 and gh-14123, including suggestions for how to address this in this and this comment.

Please consider providing a minimal reproducible example and more details if you believe this is a bug in the framework. Otherwise I plan to close this issue.

sjohnr commented 1 week ago

@tkoval83 as I mentioned, your configuration is incorrect. Please use components from the servlet documentation to configure your application and provide a minimal sample. As part of the minimal sample, I’d ask that you remove as many of the customizations as you are able, while still reproducing the problem.

sjohnr commented 1 week ago

@tkoval83 thank you for providing such a detailed sample, it is quite impressive. Unfortunately, the sample is not minimal and includes extra dependencies, configuration and modules. I'm going to close this issue. If you would like us to look further at this issue, please take some time to simplify the sample so that only the minimal dependencies, configuration and a single module are required.