Skip to content

Commit

Permalink
feature: add client_auth_method=none into tokens for clients with emp…
Browse files Browse the repository at this point in the history
…ty secret

e.g. cf client.
This allows clients to see, whats really used.
According to https://oauth.net/2.1/ the empty secret can be treated as public usage
  • Loading branch information
strehle committed Aug 8, 2023
1 parent 0ddff2c commit f743c47
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
2 changes: 1 addition & 1 deletion scripts/cargo/uaa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jwt:
revocable: false
refresh:
format: opaque
rotate: false
rotate: true
unique: false

login:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

package org.cloudfoundry.identity.uaa.authentication;

import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager;
import org.cloudfoundry.identity.uaa.util.SessionUtils;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
Expand Down Expand Up @@ -130,6 +131,12 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
if (clientAuth.isAuthenticated()) {
// Ensure the OAuth2Authentication is authenticated
authorizationRequest.setApproved(true);
if (clientAuth.getDetails() instanceof UaaAuthenticationDetails) {
String clientAuthentication = ((UaaAuthenticationDetails) clientAuth.getDetails()).getAuthenticationMethod();
if (clientAuthentication != null) {
authorizationRequest.getExtensions().put(ClaimConstants.CLIENT_AUTH_METHOD, clientAuthentication);
}
}
}

OAuth2Request storedOAuth2Request = oAuth2RequestFactory.createOAuth2Request(authorizationRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.userdetails.User;
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;
Expand Down Expand Up @@ -57,14 +59,19 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
for(String pwd: passwordList) {
try {
User user = new User(userDetails.getUsername(), pwd, userDetails.isEnabled(), userDetails.isAccountNonExpired(), userDetails.isCredentialsNonExpired(), userDetails.isAccountNonLocked(), userDetails.getAuthorities());
if (authentication.getCredentials() == null && isPublicGrantTypeUsageAllowed(authentication.getDetails()) && userDetails instanceof UaaClient) {
// in case of grant_type=authorization_code and code_verifier passed (PKCE) we check if client has option allowpublic with true and proceed even if no secret is provided
if (userDetails instanceof UaaClient) {
UaaClient uaaClient = (UaaClient) userDetails;
Object allowPublic = uaaClient.getAdditionalInformation().get(ClientConstants.ALLOW_PUBLIC);
if ((allowPublic instanceof String && Boolean.TRUE.toString().equalsIgnoreCase((String)allowPublic)) ||
(allowPublic instanceof Boolean && Boolean.TRUE.equals(allowPublic))) {
((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(CLIENT_AUTH_NONE);
break;
if (authentication.getCredentials() == null && isPublicGrantTypeUsageAllowed(authentication.getDetails())) {
// in case of grant_type=authorization_code and code_verifier passed (PKCE) we check if client has option allowpublic with true and proceed even if no secret is provided
Object allowPublic = uaaClient.getAdditionalInformation().get(ClientConstants.ALLOW_PUBLIC);
if ((allowPublic instanceof String && Boolean.TRUE.toString().equalsIgnoreCase((String) allowPublic)) ||
(allowPublic instanceof Boolean && Boolean.TRUE.equals(allowPublic))) {
setAuthenticationMethodNone(authentication);
break;
}
} else if (ObjectUtils.isEmpty(authentication.getCredentials())) {
// set none as client_auth_method for all usage of empty secrets, e.g. cf client
setAuthenticationMethodNone(authentication);
}
}
super.additionalAuthenticationChecks(user, authentication);
Expand All @@ -79,6 +86,10 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
}
}

private static void setAuthenticationMethodNone(AbstractAuthenticationToken authentication) {
((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(CLIENT_AUTH_NONE);
}

private boolean isPublicGrantTypeUsageAllowed(Object uaaAuthenticationDetails) {
UaaAuthenticationDetails authenticationDetails = uaaAuthenticationDetails instanceof UaaAuthenticationDetails ?
(UaaAuthenticationDetails) uaaAuthenticationDetails : new UaaAuthenticationDetails();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
package org.cloudfoundry.identity.uaa.oauth;

import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants;
import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning;
import org.cloudfoundry.identity.uaa.security.beans.SecurityContextAccessor;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.util.UaaSecurityContextUtils;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.cloudfoundry.identity.uaa.util.UaaTokenUtils;
import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices;
Expand Down Expand Up @@ -397,6 +398,10 @@ public UaaTokenRequest(Map<String, String> requestParameters, String clientId, C
@Override
public OAuth2Request createOAuth2Request(ClientDetails client) {
OAuth2Request request = super.createOAuth2Request(client);
String clientAuthentication = UaaSecurityContextUtils.getClientAuthenticationMethod();
if (clientAuthentication != null) {
request.getExtensions().put(ClaimConstants.CLIENT_AUTH_METHOD, clientAuthentication);
}
return new OAuth2Request(
request.getRequestParameters(),
client.getClientId(),
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 @@ -42,6 +51,14 @@ public class PasswordGrantIntegrationTests {
public void testUserLoginViaPasswordGrant() {
ResponseEntity<String> responseEntity = makePasswordGrantRequest(testAccounts.getUserName(), testAccounts.getPassword(), "cf", "", serverRunning.getAccessTokenUri());
assertEquals(HttpStatus.OK, responseEntity.getStatusCode());
validateClientAuthenticationMethod(responseEntity, true);
}

@Test
public void testUserLoginViaPasswordGrantButOwnClient() {
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 @@ -135,4 +152,16 @@ public void handleError(ClientHttpResponse response) {
});
return template;
}

private void 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)));
}
}
}

0 comments on commit f743c47

Please sign in to comment.