Skip to content

Commit 1b75038

Browse files
strehlestrehle
committed
Fix the client_auth_method check (#3243)
* Test to ensure client credentials auth check * Fix the client_auth_method check Ensure for client_credentials that either secret or private_key_jwt is used If value from claim client_auth_method is none or empty we have to reject token requests * Fix test Wanted is bad request (cherry picked from commit e7a9334)
1 parent 8b32ac8 commit 1b75038

File tree

5 files changed

+70
-3
lines changed

5 files changed

+70
-3
lines changed

‎model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ static public List<String> getStringValues() {
6666

6767
public static final String CLIENT_AUTH_NONE = ClientAuthentication.NONE;
6868
public static final String CLIENT_AUTH_EMPTY = "empty";
69+
public static final String CLIENT_AUTH_SECRET = "secret";
6970
public static final String CLIENT_AUTH_PRIVATE_KEY_JWT = ClientAuthentication.PRIVATE_KEY_JWT;
7071

7172
public static final String ID_TOKEN_HINT_PROMPT = "prompt";

‎server/src/main/java/org/cloudfoundry/identity/uaa/oauth/provider/client/ClientCredentialsTokenGranter.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
import org.cloudfoundry.identity.uaa.oauth.provider.token.AuthorizationServerTokenServices;
99
import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetailsService;
1010

11+
import java.util.List;
12+
13+
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_PRIVATE_KEY_JWT;
14+
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_SECRET;
1115
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_CLIENT_CREDENTIALS;
1216

1317
/**
@@ -20,6 +24,8 @@
2024
*/
2125
public class ClientCredentialsTokenGranter extends AbstractTokenGranter {
2226

27+
private static final List<String> ALLOWED_AUTH_METHODS = List.of(CLIENT_AUTH_SECRET, CLIENT_AUTH_PRIVATE_KEY_JWT);
28+
2329
public ClientCredentialsTokenGranter(AuthorizationServerTokenServices tokenServices,
2430
ClientDetailsService clientDetailsService, OAuth2RequestFactory requestFactory) {
2531
this(tokenServices, clientDetailsService, requestFactory, GRANT_TYPE_CLIENT_CREDENTIALS);
@@ -33,7 +39,7 @@ protected ClientCredentialsTokenGranter(AuthorizationServerTokenServices tokenSe
3339
@Override
3440
public OAuth2AccessToken grant(String grantType, TokenRequest tokenRequest) {
3541
OAuth2AccessToken token = super.grant(grantType, tokenRequest);
36-
if (token != null) {
42+
if (token != null && isValidClientAuthentication(ALLOWED_AUTH_METHODS)) {
3743
DefaultOAuth2AccessToken norefresh = new DefaultOAuth2AccessToken(token);
3844
// The spec says that client credentials should not be allowed to get a refresh token
3945
norefresh.setRefreshToken(null);

‎server/src/main/java/org/cloudfoundry/identity/uaa/oauth/provider/token/AbstractTokenGranter.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import org.apache.commons.logging.Log;
44
import org.apache.commons.logging.LogFactory;
5+
import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
56
import org.cloudfoundry.identity.uaa.oauth.common.OAuth2AccessToken;
7+
import org.cloudfoundry.identity.uaa.oauth.common.exceptions.InvalidRequestException;
68
import org.cloudfoundry.identity.uaa.oauth.provider.OAuth2Authentication;
79
import org.cloudfoundry.identity.uaa.oauth.provider.OAuth2Request;
810
import org.cloudfoundry.identity.uaa.oauth.provider.OAuth2RequestFactory;
@@ -11,8 +13,16 @@
1113
import org.cloudfoundry.identity.uaa.oauth.common.exceptions.InvalidClientException;
1214
import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetails;
1315
import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetailsService;
16+
import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants;
17+
import org.cloudfoundry.identity.uaa.util.UaaSecurityContextUtils;
18+
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
19+
import org.springframework.security.core.Authentication;
20+
import org.springframework.security.core.context.SecurityContext;
21+
import org.springframework.security.core.context.SecurityContextHolder;
1422

1523
import java.util.Collection;
24+
import java.util.List;
25+
import java.util.Optional;
1626

1727
/**
1828
* Moved class AbstractTokenGranter implementation of from spring-security-oauth2 into UAA
@@ -76,6 +86,26 @@ protected void validateGrantType(String grantType, ClientDetails clientDetails)
7686
}
7787
}
7888

89+
protected boolean isValidClientAuthentication(List<String> allowedMethods) {
90+
SecurityContext context = SecurityContextHolder.getContext();
91+
if (allowedMethods.contains(Optional.ofNullable(getUaaClientAuthenticationMethod(context.getAuthentication()))
92+
.orElse(TokenConstants.CLIENT_AUTH_SECRET))) {
93+
// internal null for client authentication means secret based (authorization header or post body)
94+
return true;
95+
} else {
96+
throw new InvalidRequestException("Client credentials required");
97+
}
98+
}
99+
100+
protected String getUaaClientAuthenticationMethod(Authentication authentication) {
101+
if (authentication instanceof UsernamePasswordAuthenticationToken && authentication.isAuthenticated() &&
102+
authentication.getDetails() instanceof UaaAuthenticationDetails) {
103+
return ((UaaAuthenticationDetails) authentication.getDetails()).getAuthenticationMethod();
104+
} else {
105+
return UaaSecurityContextUtils.getClientAuthenticationMethod(authentication);
106+
}
107+
}
108+
79109
protected AuthorizationServerTokenServices getTokenServices() {
80110
return tokenServices;
81111
}

‎server/src/test/java/org/cloudfoundry/identity/uaa/oauth/provider/client/ClientCredentialsTokenGranterTests.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package org.cloudfoundry.identity.uaa.oauth.provider.client;
22

3+
import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
4+
import org.cloudfoundry.identity.uaa.oauth.common.DefaultOAuth2AccessToken;
35
import org.cloudfoundry.identity.uaa.oauth.common.OAuth2AccessToken;
6+
import org.cloudfoundry.identity.uaa.oauth.common.exceptions.InvalidRequestException;
47
import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetails;
58
import org.cloudfoundry.identity.uaa.oauth.provider.ClientDetailsService;
69
import org.cloudfoundry.identity.uaa.oauth.provider.OAuth2Request;
@@ -10,9 +13,13 @@
1013
import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants;
1114
import org.junit.Before;
1215
import org.junit.Test;
16+
import org.junit.jupiter.api.AfterEach;
17+
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
18+
import org.springframework.security.core.context.SecurityContextHolder;
1319

1420
import java.util.Collections;
1521

22+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
1623
import static org.junit.Assert.assertNotNull;
1724
import static org.junit.Assert.assertNull;
1825
import static org.mockito.ArgumentMatchers.any;
@@ -36,6 +43,11 @@ public void setUp() throws Exception {
3643
clientCredentialsTokenGranter = new ClientCredentialsTokenGranter(tokenServices, clientDetailsService, requestFactory);
3744
}
3845

46+
@AfterEach
47+
void cleanUp() {
48+
SecurityContextHolder.clearContext();
49+
}
50+
3951
@Test
4052
public void grant() {
4153
OAuth2Request oAuth2Request = mock(OAuth2Request.class);
@@ -55,4 +67,22 @@ public void grantNoToken() {
5567
when(oAuth2Request.getAuthorities()).thenReturn(Collections.EMPTY_LIST);
5668
assertNull(clientCredentialsTokenGranter.grant(TokenConstants.GRANT_TYPE_CLIENT_CREDENTIALS, tokenRequest));
5769
}
70+
71+
@Test
72+
public void grantNoSecretFails() {
73+
UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken("username", null, null);
74+
SecurityContextHolder.getContext().setAuthentication(authentication);
75+
OAuth2Request oAuth2Request = mock(OAuth2Request.class);
76+
UaaAuthenticationDetails uaaAuthenticationDetails = mock(UaaAuthenticationDetails.class);
77+
authentication.setDetails(uaaAuthenticationDetails);
78+
when(clientDetailsService.loadClientByClientId(any())).thenReturn(mock(ClientDetails.class));
79+
when(requestFactory.createOAuth2Request(any(), any())).thenReturn(oAuth2Request);
80+
when(tokenServices.createAccessToken(any())).thenReturn(new DefaultOAuth2AccessToken("eyJx"));
81+
when(oAuth2Request.getAuthorities()).thenReturn(Collections.emptyList());
82+
when(uaaAuthenticationDetails.getAuthenticationMethod()).thenReturn("none");
83+
assertThatThrownBy(() -> clientCredentialsTokenGranter
84+
.grant(TokenConstants.GRANT_TYPE_CLIENT_CREDENTIALS, tokenRequest))
85+
.isInstanceOf(InvalidRequestException.class)
86+
.hasMessage("Client credentials required");
87+
}
5888
}

‎uaa/src/test/java/org/cloudfoundry/identity/uaa/security/UaaUsingDelegatingPasswordEncoderMockMvcTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ void getClientCredentialsTokenWithValidPassword(String clientId) throws Exceptio
5656
@ValueSource(strings = {
5757
"client_id_with_empty_password"
5858
})
59-
void tryToGetTokenWithEmtpyPasswordSucceeds(String clientId) throws Exception {
59+
void tryToGetTokenWithEmtpyPasswordMustFail(String clientId) throws Exception {
6060
mockMvc.perform(post("/oauth/token")
6161
.contentType(MediaType.APPLICATION_FORM_URLENCODED)
6262
.accept(MediaType.APPLICATION_JSON)
6363
.param("client_id", clientId)
6464
.param("client_secret", "")
6565
.param("grant_type", "client_credentials"))
66-
.andExpect(status().isOk());
66+
.andExpect(status().isBadRequest());
6767
}
6868

6969
@ParameterizedTest

0 commit comments

Comments
 (0)