From 3cec698dfa06abe06e50fd32eebd621dba8b5438 Mon Sep 17 00:00:00 2001 From: strehle Date: Wed, 11 Nov 2026 21:05:01 +0100 Subject: [PATCH] Ignore Csrf only if bearer token send --- docs/UAA-Client-Authentication.md | 14 ++ .../ClientAdminSecurityConfiguration.java | 23 ++- .../source/index.html.md.erb | 7 + .../clients/ClientAdminCsrfMockMvcTests.java | 193 ++++++++++++++++++ .../mock/clients/ClientAdminEndpointDocs.java | 14 +- 5 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminCsrfMockMvcTests.java diff --git a/docs/UAA-Client-Authentication.md b/docs/UAA-Client-Authentication.md index 1266cac7445..d90dc4f34fa 100644 --- a/docs/UAA-Client-Authentication.md +++ b/docs/UAA-Client-Authentication.md @@ -23,6 +23,20 @@ standards define token-based authentication mechanisms for OAuth2 clients. They [RFC 7523 from OAuth2 standard](https://www.rfc-editor.org/info/rfc7523) * tls_client_auth [RFC 8705](https://www.rfc-editor.org/rfc/rfc8705) +## Client administration API and CSRF + +The `/oauth/clients/**` administration endpoints use stateless OAuth2 authentication, not login sessions or +HTTP Basic credentials. Send an access token with the required client-management scopes in an +`Authorization: Bearer ` header. These explicitly authenticated requests do not require a CSRF token. +The access token is still validated, and an invalid bearer header does not fall back to a login session. + +For state-changing requests without a nonempty bearer header, CSRF protection is enabled. Legacy callers using +only the `access_token` request parameter must also supply the `X-Uaa-Csrf` cookie (named `__Host-X-Uaa-Csrf` +over HTTPS) and its matching value in the `X-CSRF-TOKEN` header or `X-Uaa-Csrf` request parameter. +Missing or mismatched CSRF tokens result in HTTP 403. Prefer the bearer header instead of sending access tokens +in request parameters. Read-only requests do not require a CSRF token, but still require OAuth2 authentication. +The CSRF token alone never authorizes a request. + ## New methods The new methods are based on asymmetric trust relation, so that the keys are divided into a private and a public one. diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminSecurityConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminSecurityConfiguration.java index 6711ba4e910..a2a631eca99 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminSecurityConfiguration.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminSecurityConfiguration.java @@ -4,10 +4,12 @@ import org.springframework.security.authorization.AuthorizationManagers; import org.cloudfoundry.identity.uaa.oauth.tls.MtlsTrustBundleEndpoints; import org.cloudfoundry.identity.uaa.oauth.UaaTokenServices; +import org.cloudfoundry.identity.uaa.oauth.common.OAuth2AccessToken; import org.cloudfoundry.identity.uaa.oauth.provider.authentication.OAuth2AuthenticationManager; import org.cloudfoundry.identity.uaa.oauth.provider.authentication.OAuth2AuthenticationProcessingFilter; import org.cloudfoundry.identity.uaa.oauth.provider.error.OAuth2AccessDeniedHandler; import org.cloudfoundry.identity.uaa.oauth.provider.error.OAuth2AuthenticationEntryPoint; +import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository; import org.cloudfoundry.identity.uaa.web.FilterChainOrder; import org.cloudfoundry.identity.uaa.web.UaaFilterChain; import org.springframework.beans.factory.annotation.Qualifier; @@ -15,14 +17,18 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter; +import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler; +import org.springframework.util.StringUtils; + +import java.util.Collections; import static org.cloudfoundry.identity.uaa.web.AuthorizationManagersUtils.anyOf; @@ -36,7 +42,8 @@ UaaFilterChain clientAdminSecret( HttpSecurity http, @Qualifier("oauthAuthenticationEntryPoint") OAuth2AuthenticationEntryPoint oauthAuthenticationEntryPoint, @Qualifier("oauthAccessDeniedHandler") OAuth2AccessDeniedHandler oauthAccessDeniedHandler, - @Qualifier("clientAdminOAuth2ResourceFilter") FilterRegistrationBean resourceFilter + @Qualifier("clientAdminOAuth2ResourceFilter") FilterRegistrationBean resourceFilter, + CookieBasedCsrfTokenRepository csrfTokenRepository ) throws Exception { var originalChain = http .securityMatcher("/oauth/clients/**", MtlsTrustBundleEndpoints.PATH) @@ -66,7 +73,17 @@ UaaFilterChain clientAdminSecret( }) .addFilterAt(resourceFilter.getFilter(), AbstractPreAuthenticatedProcessingFilter.class) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) - .csrf(CsrfConfigurer::disable) + .csrf(csrf -> { + csrf.csrfTokenRepository(csrfTokenRepository); + csrf.csrfTokenRequestHandler(new CsrfTokenRequestAttributeHandler()); + // Browsers do not automatically attach bearer headers. The stateless resource + // filter still validates the token and never falls back to session authentication. + // Match BearerTokenExtractor's legacy support for multiple headers and no separating space. + csrf.ignoringRequestMatchers(request -> Collections.list(request.getHeaders(HttpHeaders.AUTHORIZATION)) + .stream().anyMatch(authorization -> + StringUtils.startsWithIgnoreCase(authorization, OAuth2AccessToken.BEARER_TYPE) + && StringUtils.hasText(authorization.substring(OAuth2AccessToken.BEARER_TYPE.length())))); + }) .exceptionHandling(exception -> { exception.authenticationEntryPoint(oauthAuthenticationEntryPoint); exception.accessDeniedHandler(oauthAccessDeniedHandler); diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 0de47f67a70..60699d56539 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -2584,6 +2584,13 @@ _Response Fields_ <%= render('ClientAdminEndpointDocs/createClient/response-fields.md') %> +State-changing client-administration requests require CSRF protection unless they supply a nonempty +`Authorization: Bearer ` header. Callers using only the legacy `access_token` parameter must also +send the `X-Uaa-Csrf` cookie (`__Host-X-Uaa-Csrf` over HTTPS) and its matching value in the +`X-CSRF-TOKEN` header or `X-Uaa-Csrf` parameter. Missing or mismatched CSRF tokens return HTTP 403: + +<%= render('ClientAdminEndpointDocs/createClientWithoutBearerHeaderRequiresCsrf/http-response.md') %> + ## Retrieve <%= render('ClientAdminEndpointDocs/retrieveClient/curl-request.md') %> diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminCsrfMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminCsrfMockMvcTests.java new file mode 100644 index 00000000000..0f805eb0f4c --- /dev/null +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminCsrfMockMvcTests.java @@ -0,0 +1,193 @@ +package org.cloudfoundry.identity.uaa.mock.clients; + +import jakarta.servlet.http.Cookie; +import org.cloudfoundry.identity.uaa.DefaultTestContext; +import org.cloudfoundry.identity.uaa.client.UaaClientDetails; +import org.cloudfoundry.identity.uaa.oauth.UaaTokenServices; +import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository; +import org.cloudfoundry.identity.uaa.test.TestClient; +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.test.web.servlet.MockMvc; + +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.http.HttpHeaders.AUTHORIZATION; +import static org.springframework.http.MediaType.APPLICATION_JSON; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.request; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@DefaultTestContext +class ClientAdminCsrfMockMvcTests { + @Autowired + private MockMvc mockMvc; + @Autowired + private TestClient testClient; + @Autowired + private UaaTokenServices tokenServices; + + private String adminToken; + private UaaClientDetails client; + private boolean clientCreated; + + @BeforeEach + void setUp() throws Exception { + adminToken = testClient.getClientCredentialsOAuthAccessToken("admin", "adminsecret", "clients.admin"); + client = new UaaClientDetails(UUID.randomUUID().toString(), null, "uaa.none", "client_credentials", "uaa.none"); + client.setClientSecret("secret"); + } + + @AfterEach + void removeCreatedClient() throws Exception { + if (clientCreated) { + mockMvc.perform(delete("/oauth/clients/" + client.getClientId()) + .header(AUTHORIZATION, "Bearer " + adminToken)) + .andExpect(status().isOk()); + } + } + + @ParameterizedTest + @CsvSource({ + "POST, /oauth/clients", + "POST, /oauth/clients/", + "PUT, /oauth/clients/example", + "DELETE, /oauth/clients/example", + "PUT, /oauth/clients/example/secret", + "PUT, /oauth/clients/example/clientjwt", + "POST, /oauth/clients/tx", + "PUT, /oauth/clients/tx", + "DELETE, /oauth/clients/tx" + }) + void mutationWithoutBearerHeader_requiresCsrf(String method, String path) throws Exception { + var response = mockMvc.perform(request(HttpMethod.valueOf(method), path) + .param("access_token", adminToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isForbidden()) + .andReturn().getResponse(); + + assertThat(response.getContentAsString()).contains("CSRF"); + } + + @ParameterizedTest + @ValueSource(strings = {"Basic YWRtaW46YWRtaW5zZWNyZXQ=", "Bearer", "Bearer ", "Bearer "}) + void nonBearerOrEmptyHeader_doesNotBypassCsrf(String authorization) throws Exception { + var response = mockMvc.perform(post("/oauth/clients") + .header(AUTHORIZATION, authorization) + .param("access_token", adminToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isForbidden()) + .andReturn().getResponse(); + + assertThat(response.getContentAsString()).contains("CSRF"); + } + + @ParameterizedTest + @ValueSource(strings = {"Bearer ", "bearer ", "BEARER ", "Bearer"}) + void bearerHeader_allowsCreationWithoutCsrf(String scheme) throws Exception { + var result = mockMvc.perform(post("/oauth/clients") + .header(AUTHORIZATION, scheme + adminToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isCreated()) + .andReturn(); + + clientCreated = true; + assertThat(result.getRequest().getSession(false)).isNull(); + } + + @Test + void bearerAmongMultipleHeaders_allowsCreationWithoutCsrf() throws Exception { + mockMvc.perform(post("/oauth/clients") + .header(AUTHORIZATION, "Basic YWRtaW46YWRtaW5zZWNyZXQ=", "Bearer " + adminToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isCreated()); + clientCreated = true; + } + + @Test + void invalidBearerToken_isNotAuthorized() throws Exception { + mockMvc.perform(post("/oauth/clients") + .header(AUTHORIZATION, "Bearer invalid-token") + .param("access_token", adminToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isUnauthorized()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void accessTokenParameterWithMatchingCsrf_allowsCreationWithoutSession(boolean csrfAsParameter) throws Exception { + var createRequest = post("/oauth/clients") + .param("access_token", adminToken) + .cookie(new Cookie(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "csrf-token")) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client)); + if (csrfAsParameter) { + createRequest.param(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "csrf-token"); + } else { + createRequest.header(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME, "csrf-token"); + } + var result = mockMvc.perform(createRequest) + .andExpect(status().isCreated()) + .andReturn(); + + clientCreated = true; + assertThat(result.getRequest().getSession(false)).isNull(); + } + + @Test + void mismatchedCsrfToken_isRejected() throws Exception { + mockMvc.perform(post("/oauth/clients") + .param("access_token", adminToken) + .cookie(new Cookie(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "csrf-token")) + .header(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME, "wrong-token") + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client))) + .andExpect(status().isForbidden()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void adminSessionWithoutBearerToken_isRejected(boolean includeCsrf) throws Exception { + var session = new MockHttpSession(); + session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, + new SecurityContextImpl(tokenServices.loadAuthentication(adminToken))); + + var createRequest = post("/oauth/clients") + .session(session) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(client)); + if (includeCsrf) { + createRequest.cookie(new Cookie(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME, "csrf-token")) + .header(CookieBasedCsrfTokenRepository.DEFAULT_CSRF_HEADER_NAME, "csrf-token"); + } + var response = mockMvc.perform(createRequest) + .andExpect(status().isForbidden()) + .andReturn().getResponse(); + + assertThat(response.getContentAsString()).contains(includeCsrf ? "insufficient_scope" : "CSRF"); + } + + @Test + void readWithAccessTokenParameter_doesNotRequireCsrf() throws Exception { + mockMvc.perform(get("/oauth/clients").param("access_token", adminToken)) + .andExpect(status().isOk()); + } +} diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java index 1206e8d2f2c..6601560bacb 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointDocs.java @@ -52,7 +52,7 @@ class ClientAdminEndpointDocs extends AdminClientCreator { private static final FieldDescriptor clientSecretField = fieldWithPath("client_secret").constrained("Required if the client allows `authorization_code` or `client_credentials` grant type").type(STRING).description("A secret string used for authenticating as this client."); private static final FieldDescriptor secondaryClientSecretField = fieldWithPath("secondary_client_secret").optional(null).type(STRING).description("An optional, secondary secret string used for authenticating as this client to support secret rotation."); private static final FieldDescriptor actionField = fieldWithPath("action").constrained("Always required.").description("Set to `secret` to change client secret, `delete` to delete the client or `add` to add the client"); - private static final HeaderDescriptor authorizationHeader = headerWithName("Authorization").description("Bearer token containing `clients.write`, `clients.admin` or `zones.{zone.id}.admin`"); + private static final HeaderDescriptor authorizationHeader = headerWithName("Authorization").description("Bearer token containing `clients.write`, `clients.admin` or `zones.{zone.id}.admin`. Explicit bearer headers do not require a CSRF token; mutations using only an `access_token` parameter require a matching CSRF cookie and token, otherwise they return HTTP 403."); private static final HeaderDescriptor IDENTITY_ZONE_ID_HEADER = headerWithName(IdentityZoneSwitchingFilter.HEADER).optional().description("If using a `zones..admin` scope/token, indicates what zone this request goes to by supplying a zone_id."); private static final HeaderDescriptor IDENTITY_ZONE_SUBDOMAIN_HEADER = headerWithName(IdentityZoneSwitchingFilter.SUBDOMAIN_HEADER).optional().description("If using a `zones..admin` scope/token, indicates what zone this request goes to by supplying a subdomain."); @@ -144,6 +144,18 @@ void setup() throws Exception { "uaa.admin clients.admin clients.secret"); } + @Test + void createClientWithoutBearerHeaderRequiresCsrf() throws Exception { + ClientDetails client = createBaseClient(null, SECRET, Collections.singleton("client_credentials"), null, null); + mockMvc.perform(post("/oauth/clients") + .param("access_token", clientAdminToken) + .contentType(APPLICATION_JSON) + .content(writeValueAsString(client))) + .andExpect(status().isForbidden()) + .andDo(document("{ClassName}/{methodName}", + preprocessResponse(prettyPrint()))); + } + @Test void createClient() throws Exception { Snippet requestFields = requestFields(