Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: add client_auth_method=none into tokens for clients with empty secret #2504

Merged
merged 17 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ static public List<String> getStringValues() {
public static final String GRANT_TYPE_IMPLICIT = "implicit";

public static final String CLIENT_AUTH_NONE = "none";
public static final String CLIENT_AUTH_EMPTY = "empty";
public static final String CLIENT_AUTH_PRIVATE_KEY_JWT = "private_key_jwt";

public static final String ID_TOKEN_HINT_PROMPT = "prompt";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

import java.util.Collections;
import java.util.Map;
import java.util.Optional;

import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_EMPTY;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_PRIVATE_KEY_JWT;
import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.getSafeParameterValue;
Expand Down Expand Up @@ -74,6 +76,9 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
setAuthenticationMethod(authentication, CLIENT_AUTH_PRIVATE_KEY_JWT);
break;
}
} else if (ObjectUtils.isEmpty(authentication.getCredentials())) {
// set internally empty as client_auth_method e.g. cf client
setAuthenticationMethod(authentication, CLIENT_AUTH_EMPTY);
}
if (uaaClient.getPassword() == null) {
error = new BadCredentialsException("Missing credentials");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.ZONE_ID;
import static org.cloudfoundry.identity.uaa.oauth.token.RevocableToken.TokenType.ACCESS_TOKEN;
import static org.cloudfoundry.identity.uaa.oauth.token.RevocableToken.TokenType.REFRESH_TOKEN;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_EMPTY;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_CLIENT_CREDENTIALS;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN;
Expand Down Expand Up @@ -490,7 +491,8 @@ private CompositeToken createCompositeToken(String tokenId,

private static Map<String, Object> addRootClaimEntry(Map<String, Object> additionalRootClaims, String entry, String value) {
Map<String, Object> claims = additionalRootClaims != null ? additionalRootClaims : new HashMap<>();
claims.put(entry, value);
// set externally none as client_auth_method if internally empty
claims.put(entry, CLIENT_AUTH_EMPTY.equals(value) ? CLIENT_AUTH_NONE : value);
strehle marked this conversation as resolved.
Show resolved Hide resolved
return claims;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,19 @@ void provider_authenticate_client_without_password_public_string() {
assertNotNull(a);
}

@Test
void provider_authenticate_client_with_empty_password_public_string() {
IdentityZoneHolder.get().getConfig().getTokenPolicy().setRefreshTokenRotate(true);
BaseClientDetails clientDetails = new BaseClientDetails(generator.generate(), "", "", "password", "uaa.resource");
clientDetails.setClientSecret("");
jdbcClientDetailsService.addClientDetails(clientDetails);
client = clientDetails;
UsernamePasswordAuthenticationToken a = getAuthenticationToken("password");
when(a.getCredentials()).thenReturn("");
authenticationProvider.additionalAuthenticationChecks(new UaaClient("cf", passwordEncoder.encode(""), Collections.emptyList(), client.getAdditionalInformation(), null), a);
assertNotNull(a);
}

@Test
void provider_refresh_client_without_password_public_boolean() {
client = createClient(ClientConstants.ALLOW_PUBLIC, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import static org.cloudfoundry.identity.uaa.oauth.TokenTestSupport.CLIENT_ID;
import static org.cloudfoundry.identity.uaa.oauth.TokenTestSupport.GRANT_TYPE;
import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.CLIENT_AUTH_METHOD;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_EMPTY;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_REFRESH_TOKEN;
Expand Down Expand Up @@ -138,6 +139,39 @@ void testRefreshPublicClientWithRotation() {
assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
}

@Test
@DisplayName("Refresh Token from public to empty authentication")
void testRefreshPublicClientWithRotationAndEmpyAuthentication() {
BaseClientDetails clientDetails = new BaseClientDetails(tokenSupport.defaultClient);
clientDetails.setAutoApproveScopes(singleton("true"));
tokenSupport.clientDetailsService.setClientDetailsStore(IdentityZoneHolder.get().getId(), Collections.singletonMap(CLIENT_ID, clientDetails));
AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID, tokenSupport.requestedAuthScopes);
authorizationRequest.setResourceIds(new HashSet<>(tokenSupport.resourceIds));
Map<String, String> azParameters = new HashMap<>(authorizationRequest.getRequestParameters());
azParameters.put(GRANT_TYPE, GRANT_TYPE_AUTHORIZATION_CODE);
authorizationRequest.setRequestParameters(azParameters);
authorizationRequest.setExtensions(Map.of(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
OAuth2Request oAuth2Request = authorizationRequest.createOAuth2Request();
OAuth2Authentication authentication = new OAuth2Authentication(oAuth2Request, tokenSupport.defaultUserAuthentication);
new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(true);
CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication);

assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
String refreshTokenValue = accessToken.getRefreshToken().getValue();
assertThat(refreshTokenValue, is(notNullValue()));

authorizationRequest.setExtensions(Map.of(CLIENT_AUTH_METHOD, CLIENT_AUTH_EMPTY));
setupOAuth2Authentication(authorizationRequest.createOAuth2Request());
OAuth2AccessToken refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN));
assertThat(refreshedToken, is(notNullValue()));
assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue());
assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));

refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN));
assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue());
assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
}

@Test
@DisplayName("Refresh Token with allowpublic but without rotation")
void testRefreshPublicClientWithoutRotation() {
Expand All @@ -164,6 +198,35 @@ void testRefreshPublicClientWithoutRotation() {
assertEquals("Refresh without client authentication not allowed.", exception.getMessage());
}

@Test
@DisplayName("Refresh with allowpublic and rotation but existing token was not public")
void testRefreshPublicClientButExistingTokenWasEmptyAuthentication() {
BaseClientDetails clientDetails = new BaseClientDetails(tokenSupport.defaultClient);
clientDetails.setAutoApproveScopes(singleton("true"));
tokenSupport.clientDetailsService.setClientDetailsStore(IdentityZoneHolder.get().getId(), Collections.singletonMap(CLIENT_ID, clientDetails));
AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID, tokenSupport.requestedAuthScopes);
authorizationRequest.setResourceIds(new HashSet<>(tokenSupport.resourceIds));
Map<String, String> azParameters = new HashMap<>(authorizationRequest.getRequestParameters());
azParameters.put(GRANT_TYPE, GRANT_TYPE_AUTHORIZATION_CODE);
authorizationRequest.setRequestParameters(azParameters);
authorizationRequest.setExtensions(Map.of(CLIENT_AUTH_METHOD, CLIENT_AUTH_EMPTY));
OAuth2Request oAuth2Request = authorizationRequest.createOAuth2Request();
OAuth2Authentication authentication = new OAuth2Authentication(oAuth2Request, tokenSupport.defaultUserAuthentication);
new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(true);
strehle marked this conversation as resolved.
Show resolved Hide resolved
CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication);

assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
String refreshTokenValue = accessToken.getRefreshToken().getValue();
assertThat(refreshTokenValue, is(notNullValue()));

new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(false);
strehle marked this conversation as resolved.
Show resolved Hide resolved
authorizationRequest.setExtensions(Map.of(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
setupOAuth2Authentication(authorizationRequest.createOAuth2Request());
RuntimeException exception = assertThrows(TokenRevokedException.class, () ->
tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN)));
assertEquals("Refresh without client authentication not allowed.", exception.getMessage());
}

private static Authentication setupOAuth2Authentication(OAuth2Request auth2Request) {
OAuth2Authentication authentication = mock(OAuth2Authentication.class);
SecurityContextHolder.getContext().setAuthentication(authentication);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.springframework.security.oauth2.common.AuthenticationScheme;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.client.BaseClientDetails;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.util.StringUtils;

import java.net.URLEncoder;
Expand Down Expand Up @@ -137,7 +136,7 @@ public String getAuthorizationHeader(String prefix, String defaultUsername, Stri
* @param password client_secret
* @return OAuth2 encoded client_id and client_secret, e.g. https://datatracker.ietf.org/doc/html/rfc2617#section-2
*/
public String getAuthorizationHeader(String username, String password) {
public static String getAuthorizationHeader(String username, String password) {
String credentials =
String.format("%s:%s", URLEncoder.encode(username, StandardCharsets.UTF_8), URLEncoder.encode(password, StandardCharsets.UTF_8));
return String.format("Basic %s", new String(Base64.encode(credentials.getBytes())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import org.cloudfoundry.identity.uaa.ServerRunning;
import org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.UaaTokenUtils;
import org.junit.Rule;
import org.junit.Test;
import org.springframework.http.HttpEntity;
Expand All @@ -25,7 +27,14 @@
import java.util.HashMap;
import java.util.Map;

import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasKey;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.springframework.http.MediaType.APPLICATION_JSON;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;

Expand All @@ -39,9 +48,17 @@ public class PasswordGrantIntegrationTests {
RandomValueStringGenerator generator = new RandomValueStringGenerator(36);

@Test
public void testUserLoginViaPasswordGrant() {
public void testUserLoginViaPasswordGrant_usingClientWithEmptyClientSecret() {
ResponseEntity<String> responseEntity = makePasswordGrantRequest(testAccounts.getUserName(), testAccounts.getPassword(), "cf", "", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
validateClientAuthenticationMethod(responseEntity, true);
}

@Test
public void testUserLoginViaPasswordGrant_usingConfidentialClient() {
ResponseEntity<String> responseEntity = makePasswordGrantRequest(testAccounts.getUserName(), testAccounts.getPassword(), "app", "appclientsecret", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
validateClientAuthenticationMethod(responseEntity, false);
}

@Test
Expand Down Expand Up @@ -104,10 +121,10 @@ protected BaseClientDetails addUserGroupsRequiredClient() {
return JsonUtils.readValue(response.getBody(), BaseClientDetails.class);
}

private ResponseEntity<String> makePasswordGrantRequest(String userName, String password, String clientId, String clientSecret, String url) {
protected static ResponseEntity<String> makePasswordGrantRequest(String userName, String password, String clientId, String clientSecret, String url) {
HttpHeaders headers = new HttpHeaders();
headers.setAccept(Collections.singletonList(APPLICATION_JSON));
headers.add("Authorization", testAccounts.getAuthorizationHeader(clientId, clientSecret));
headers.add("Authorization", UaaTestAccounts.getAuthorizationHeader(clientId, clientSecret));

MultiValueMap<String, String> params = new LinkedMultiValueMap<>();
params.add("grant_type", "password");
Expand All @@ -120,7 +137,7 @@ private ResponseEntity<String> makePasswordGrantRequest(String userName, String
return template.postForEntity(url, request, String.class);
}

private RestTemplate getRestTemplate() {
protected static RestTemplate getRestTemplate() {
RestTemplate template = new RestTemplate();
template.setErrorHandler(new ResponseErrorHandler() {
@Override
Expand All @@ -135,4 +152,17 @@ public void handleError(ClientHttpResponse response) {
});
return template;
}

protected static String validateClientAuthenticationMethod(ResponseEntity<String> responseEntity, boolean isNone) {
Map<String, Object> jsonBody = JsonUtils.readValue(responseEntity.getBody(), new TypeReference<Map<String,Object>>() {});
String accessToken = (String) jsonBody.get("access_token");
assertThat(accessToken, is(notNullValue()));
Map<String, Object> claims = UaaTokenUtils.getClaims(accessToken);
if (isNone) {
assertThat(claims, hasEntry(ClaimConstants.CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
} else {
assertThat(claims, not(hasKey(ClaimConstants.CLIENT_AUTH_METHOD)));
}
return (String) jsonBody.get("refresh_token");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.cloudfoundry.identity.uaa.test.UaaTestAccounts;
import org.junit.Rule;
import org.junit.Test;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand All @@ -32,12 +33,14 @@
import org.springframework.web.client.RestTemplate;

import java.net.URI;
import java.util.Collections;
import java.util.Map;

import static org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils.getHeaders;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE;
import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME;
import static org.junit.Assert.*;
import static org.springframework.http.MediaType.APPLICATION_JSON;
import static org.springframework.security.oauth2.common.util.OAuth2Utils.USER_OAUTH_APPROVAL;

/**
Expand Down Expand Up @@ -186,4 +189,39 @@ public void testRefreshTokenWithInactiveZone() {
ResponseEntity<Map> tokenResponse = serverRunning.postForMap(serverRunning.getAccessTokenUri().replace("localhost", "testzoneinactive.localhost"), formData, new HttpHeaders());
assertEquals(HttpStatus.NOT_FOUND, tokenResponse.getStatusCode());
}

@Test
public void testUserLoginViaPasswordGrantAndRefresh_usingClientWithEmptyClientSecret() {
ResponseEntity<String> responseEntity = PasswordGrantIntegrationTests.makePasswordGrantRequest(testAccounts.getUserName(), testAccounts.getPassword(), "cf", "", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
String refreshToken = PasswordGrantIntegrationTests.validateClientAuthenticationMethod(responseEntity, true);
responseEntity = makeRefreshGrantRequest(refreshToken, "cf", "", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
PasswordGrantIntegrationTests.validateClientAuthenticationMethod(responseEntity, true);
}

@Test
public void testUserLoginViaPasswordGrantAndRefresh_usingConfidentialClient() {
ResponseEntity<String> responseEntity = PasswordGrantIntegrationTests.makePasswordGrantRequest(testAccounts.getUserName(), testAccounts.getPassword(), "app", "appclientsecret", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
String refreshToken = PasswordGrantIntegrationTests.validateClientAuthenticationMethod(responseEntity, false);
responseEntity = makeRefreshGrantRequest(refreshToken, "app", "appclientsecret", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
PasswordGrantIntegrationTests.validateClientAuthenticationMethod(responseEntity, false);
}

protected static ResponseEntity<String> makeRefreshGrantRequest(String refreshToken, String clientId, String clientSecret, String url) {
HttpHeaders headers = new HttpHeaders();
headers.setAccept(Collections.singletonList(APPLICATION_JSON));
headers.add("Authorization", UaaTestAccounts.getAuthorizationHeader(clientId, clientSecret));

MultiValueMap<String, String> params = new LinkedMultiValueMap<>();
params.add("grant_type", "refresh_token");
params.add("refresh_token", refreshToken);

HttpEntity<MultiValueMap<String, String>> request = new HttpEntity<>(params, headers);

RestTemplate template = PasswordGrantIntegrationTests.getRestTemplate();
return template.postForEntity(url, request, String.class);
}
}