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 14 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 ID_TOKEN_HINT_PROMPT = "prompt";
public static final String ID_TOKEN_HINT_PROMPT_NONE = "none";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@

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.UaaSecurityContextUtils;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -130,6 +132,10 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
if (clientAuth.isAuthenticated()) {
// Ensure the OAuth2Authentication is authenticated
authorizationRequest.setApproved(true);
String clientAuthentication = UaaSecurityContextUtils.getClientAuthenticationMethod(clientAuth);
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 @@ -17,18 +17,21 @@
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.BadCredentialsException;
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.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 static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_EMPTY;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;

public class ClientDetailsAuthenticationProvider extends DaoAuthenticationProvider {
Expand Down Expand Up @@ -62,6 +65,9 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(CLIENT_AUTH_NONE);
break;
}
} else if (ObjectUtils.isEmpty(authentication.getCredentials())) {
// set none as client_auth_method for all usage of empty secrets, e.g. cf client
setAuthenticationMethod(authentication, CLIENT_AUTH_EMPTY);
}
if (uaaClient.getPassword() == null) {
error = new BadCredentialsException("Missing credentials");
Expand All @@ -79,6 +85,12 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
}
}

private static void setAuthenticationMethod(AbstractAuthenticationToken authentication, String method) {
if (authentication.getDetails() instanceof UaaAuthenticationDetails) {
((UaaAuthenticationDetails) authentication.getDetails()).setAuthenticationMethod(method);
}
}

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 @@ -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 @@ -319,13 +320,14 @@ private static String getAuthenticationMethod(OAuth2Request oAuth2Request) {
}

private void addAuthenticationMethod(Claims claims, Map<String, Object> additionalRootClaims, UserAuthenticationData authenticationData) {
if (authenticationData.clientAuth != null && CLIENT_AUTH_NONE.equals(authenticationData.clientAuth)) {
if (authenticationData.clientAuth != null) {
// public refresh flow, allowed if access_token before was also without authentication (claim: client_auth_method=none) and refresh token is one time use (rotate it in refresh)
if (refreshTokenCreator.shouldRotateRefreshTokens() && CLIENT_AUTH_NONE.equals(claims.getClientAuth())) {
addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth);
} else {
if (CLIENT_AUTH_NONE.equals(authenticationData.clientAuth) && // current authentication
(!CLIENT_AUTH_NONE.equals(claims.getClientAuth()) || // authentication before
!refreshTokenCreator.shouldRotateRefreshTokens())) {
throw new TokenRevokedException("Refresh without client authentication not allowed.");
}
addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth);
}
}

Expand Down Expand Up @@ -489,7 +491,7 @@ 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);
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
@@ -1,5 +1,6 @@
package org.cloudfoundry.identity.uaa.util;

import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.provider.OAuth2Authentication;
Expand All @@ -14,8 +15,13 @@ public final class UaaSecurityContextUtils {
private UaaSecurityContextUtils() {}

public static String getClientAuthenticationMethod() {
Authentication a = SecurityContextHolder.getContext().getAuthentication();
return getClientAuthenticationMethod(SecurityContextHolder.getContext().getAuthentication());
}
public static String getClientAuthenticationMethod(Authentication a) {
if (!(a instanceof OAuth2Authentication)) {
if (a != null && a.isAuthenticated() && a.getDetails() instanceof UaaAuthenticationDetails) {
return ((UaaAuthenticationDetails) a.getDetails()).getAuthenticationMethod();
}
return null;
}
OAuth2Authentication oAuth2Authentication = (OAuth2Authentication) a;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.cloudfoundry.identity.uaa.authentication;

import org.cloudfoundry.identity.uaa.oauth.TokenTestSupport;
import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager;
import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthCodeToken;
import org.cloudfoundry.identity.uaa.util.SessionUtils;
Expand All @@ -31,24 +32,31 @@
import org.springframework.security.authentication.InsufficientAuthenticationException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
import org.springframework.security.oauth2.provider.OAuth2Authentication;
import org.springframework.security.oauth2.provider.OAuth2Request;
import org.springframework.security.oauth2.provider.OAuth2RequestFactory;
import org.springframework.security.saml.SAMLProcessingFilter;
import org.springframework.security.web.AuthenticationEntryPoint;

import javax.servlet.FilterChain;
import java.util.Collections;
import java.util.Map;

import static java.util.Optional.ofNullable;
import static org.cloudfoundry.identity.uaa.oauth.TokenTestSupport.OPENID;
import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.GRANT_TYPE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.CLIENT_AUTH_NONE;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_JWT_BEARER;
import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_SAML2_BEARER;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -124,13 +132,46 @@ public void attempt_password_authentication() throws Exception {
request.addParameter(GRANT_TYPE, "password");
request.addParameter("username", "marissa");
request.addParameter("password", "koala");
when(passwordAuthManager.authenticate(any())).thenReturn(mock(UaaAuthentication.class));
OAuth2Authentication clientAuthentication = mock(OAuth2Authentication.class);
OAuth2Request oAuth2Request = mock(OAuth2Request.class);
AuthorizationRequest authorizationRequest = mock(AuthorizationRequest.class);
when(clientAuthentication.isAuthenticated()).thenReturn(true);
when(requestFactory.createAuthorizationRequest(anyMap())).thenReturn(authorizationRequest);
when(clientAuthentication.getOAuth2Request()).thenReturn(oAuth2Request);
when(oAuth2Request.getExtensions()).thenReturn(Map.of(ClaimConstants.CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
SecurityContextHolder.getContext().setAuthentication(clientAuthentication);
filter.doFilter(request, response, chain);
verify(clientAuthentication, times(0)).getDetails();
verify(filter, times(1)).attemptTokenAuthentication(same(request), same(response));
verify(passwordAuthManager, times(1)).authenticate(any());
verify(oAuth2Request, times(1)).getExtensions();
verifyNoInteractions(samlAuthFilter);
verifyNoInteractions(externalOAuthAuthenticationManager);
}

@Test
public void attempt_password_authentication_with_details() throws Exception {
request.addParameter(GRANT_TYPE, "password");
request.addParameter("username", "marissa");
request.addParameter("password", "koala");
when(passwordAuthManager.authenticate(any())).thenReturn(mock(UaaAuthentication.class));
UaaAuthentication clientAuthentication = mock(UaaAuthentication.class);
UaaAuthenticationDetails uaaAuthenticationDetails = mock(UaaAuthenticationDetails.class);
AuthorizationRequest authorizationRequest = mock(AuthorizationRequest.class);
when(clientAuthentication.getDetails()).thenReturn(uaaAuthenticationDetails);
when(clientAuthentication.isAuthenticated()).thenReturn(true);
when((uaaAuthenticationDetails.getAuthenticationMethod())).thenReturn(CLIENT_AUTH_NONE);
when(requestFactory.createAuthorizationRequest(anyMap())).thenReturn(authorizationRequest);
SecurityContextHolder.getContext().setAuthentication(clientAuthentication);
filter.doFilter(request, response, chain);
verify(clientAuthentication, atLeast(1)).getDetails();
verify(filter, times(1)).attemptTokenAuthentication(same(request), same(response));
verify(passwordAuthManager, times(1)).authenticate(any());
verify(authorizationRequest, times(1)).getExtensions();
verifyNoInteractions(samlAuthFilter);
verifyNoInteractions(externalOAuthAuthenticationManager);
}

@Test
public void attempt_saml_assertion_authentication() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,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()), a);
assertNotNull(a);
}

@Test
void provider_refresh_client_without_password_public_boolean() {
client = createClient(ClientConstants.ALLOW_PUBLIC, true);
Expand Down
Loading