Polish spring-security-cas main code

Manually polish `spring-security-cas` following the formatting
and checkstyle fixes.

Issue gh-8945
This commit is contained in:
Phillip Webb 2020-08-05 22:22:41 -07:00 committed by Rob Winch
parent fdc66d45d5
commit 4e5aa6c9d3
9 changed files with 64 additions and 143 deletions

View File

@ -39,7 +39,6 @@ public final class CasAssertionAuthenticationToken extends AbstractAuthenticatio
public CasAssertionAuthenticationToken(final Assertion assertion, final String ticket) {
super(new ArrayList<>());
this.assertion = assertion;
this.ticket = ticket;
}

View File

@ -26,6 +26,7 @@ import org.springframework.beans.factory.InitializingBean;
import org.springframework.context.MessageSource;
import org.springframework.context.MessageSourceAware;
import org.springframework.context.support.MessageSourceAccessor;
import org.springframework.core.log.LogMessage;
import org.springframework.security.authentication.AccountStatusUserDetailsChecker;
import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.authentication.BadCredentialsException;
@ -94,7 +95,6 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
if (!supports(authentication.getClass())) {
return null;
}
if (authentication instanceof UsernamePasswordAuthenticationToken
&& (!CasAuthenticationFilter.CAS_STATEFUL_IDENTIFIER.equals(authentication.getPrincipal().toString())
&& !CasAuthenticationFilter.CAS_STATELESS_IDENTIFIER
@ -102,16 +102,13 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
// UsernamePasswordAuthenticationToken not CAS related
return null;
}
// If an existing CasAuthenticationToken, just check we created it
if (authentication instanceof CasAuthenticationToken) {
if (this.key.hashCode() == ((CasAuthenticationToken) authentication).getKeyHash()) {
return authentication;
}
else {
if (this.key.hashCode() != ((CasAuthenticationToken) authentication).getKeyHash()) {
throw new BadCredentialsException(this.messages.getMessage("CasAuthenticationProvider.incorrectKey",
"The presented CasAuthenticationToken does not contain the expected key"));
}
return authentication;
}
// Ensure credentials are presented
@ -120,38 +117,30 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
"Failed to provide a CAS service ticket to validate"));
}
boolean stateless = false;
if (authentication instanceof UsernamePasswordAuthenticationToken
&& CasAuthenticationFilter.CAS_STATELESS_IDENTIFIER.equals(authentication.getPrincipal())) {
stateless = true;
}
boolean stateless = (authentication instanceof UsernamePasswordAuthenticationToken
&& CasAuthenticationFilter.CAS_STATELESS_IDENTIFIER.equals(authentication.getPrincipal()));
CasAuthenticationToken result = null;
if (stateless) {
// Try to obtain from cache
result = this.statelessTicketCache.getByTicketId(authentication.getCredentials().toString());
}
if (result == null) {
result = this.authenticateNow(authentication);
result.setDetails(authentication.getDetails());
}
if (stateless) {
// Add to cache
this.statelessTicketCache.putTicketInCache(result);
}
return result;
}
private CasAuthenticationToken authenticateNow(final Authentication authentication) throws AuthenticationException {
try {
final Assertion assertion = this.ticketValidator.validate(authentication.getCredentials().toString(),
Assertion assertion = this.ticketValidator.validate(authentication.getCredentials().toString(),
getServiceUrl(authentication));
final UserDetails userDetails = loadUserByAssertion(assertion);
UserDetails userDetails = loadUserByAssertion(assertion);
this.userDetailsChecker.check(userDetails);
return new CasAuthenticationToken(this.key, userDetails, authentication.getCredentials(),
this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion);
@ -172,22 +161,14 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
private String getServiceUrl(Authentication authentication) {
String serviceUrl;
if (authentication.getDetails() instanceof ServiceAuthenticationDetails) {
serviceUrl = ((ServiceAuthenticationDetails) authentication.getDetails()).getServiceUrl();
}
else if (this.serviceProperties == null) {
throw new IllegalStateException(
"serviceProperties cannot be null unless Authentication.getDetails() implements ServiceAuthenticationDetails.");
}
else if (this.serviceProperties.getService() == null) {
throw new IllegalStateException(
"serviceProperties.getService() cannot be null unless Authentication.getDetails() implements ServiceAuthenticationDetails.");
}
else {
serviceUrl = this.serviceProperties.getService();
}
if (logger.isDebugEnabled()) {
logger.debug("serviceUrl = " + serviceUrl);
return ((ServiceAuthenticationDetails) authentication.getDetails()).getServiceUrl();
}
Assert.state(this.serviceProperties != null,
"serviceProperties cannot be null unless Authentication.getDetails() implements ServiceAuthenticationDetails.");
Assert.state(this.serviceProperties.getService() != null,
"serviceProperties.getService() cannot be null unless Authentication.getDetails() implements ServiceAuthenticationDetails.");
serviceUrl = this.serviceProperties.getService();
logger.debug(LogMessage.format("serviceUrl = %s", serviceUrl));
return serviceUrl;
}

View File

@ -93,12 +93,10 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
final Collection<? extends GrantedAuthority> authorities, final UserDetails userDetails,
final Assertion assertion) {
super(authorities);
if ((principal == null) || "".equals(principal) || (credentials == null) || "".equals(credentials)
|| (authorities == null) || (userDetails == null) || (assertion == null)) {
throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
}
this.keyHash = keyHash;
this.principal = principal;
this.credentials = credentials;
@ -117,21 +115,16 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
if (!super.equals(obj)) {
return false;
}
if (obj instanceof CasAuthenticationToken) {
CasAuthenticationToken test = (CasAuthenticationToken) obj;
if (!this.assertion.equals(test.getAssertion())) {
return false;
}
if (this.getKeyHash() != test.getKeyHash()) {
return false;
}
return true;
}
return false;
}
@ -174,7 +167,6 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
sb.append(super.toString());
sb.append(" Assertion: ").append(this.assertion);
sb.append(" Credentials (Service/Proxy Ticket): ").append(this.credentials);
return (sb.toString());
}

View File

@ -22,6 +22,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
/**
@ -44,11 +45,7 @@ public class EhCacheBasedTicketCache implements StatelessTicketCache, Initializi
@Override
public CasAuthenticationToken getByTicketId(final String serviceTicket) {
final Element element = this.cache.get(serviceTicket);
if (logger.isDebugEnabled()) {
logger.debug("Cache hit: " + (element != null) + "; service ticket: " + serviceTicket);
}
logger.debug(LogMessage.of(() -> "Cache hit: " + (element != null) + "; service ticket: " + serviceTicket));
return (element != null) ? (CasAuthenticationToken) element.getValue() : null;
}
@ -59,20 +56,13 @@ public class EhCacheBasedTicketCache implements StatelessTicketCache, Initializi
@Override
public void putTicketInCache(final CasAuthenticationToken token) {
final Element element = new Element(token.getCredentials().toString(), token);
if (logger.isDebugEnabled()) {
logger.debug("Cache put: " + element.getKey());
}
logger.debug(LogMessage.of(() -> "Cache put: " + element.getKey()));
this.cache.put(element);
}
@Override
public void removeTicketFromCache(final CasAuthenticationToken token) {
if (logger.isDebugEnabled()) {
logger.debug("Cache remove: " + token.getCredentials().toString());
}
logger.debug(LogMessage.of(() -> "Cache remove: " + token.getCredentials().toString()));
this.removeTicketFromCache(token.getCredentials().toString());
}

View File

@ -20,6 +20,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cache.Cache;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
/**
@ -43,31 +44,20 @@ public class SpringCacheBasedTicketCache implements StatelessTicketCache {
@Override
public CasAuthenticationToken getByTicketId(final String serviceTicket) {
final Cache.ValueWrapper element = (serviceTicket != null) ? this.cache.get(serviceTicket) : null;
if (logger.isDebugEnabled()) {
logger.debug("Cache hit: " + (element != null) + "; service ticket: " + serviceTicket);
}
logger.debug(LogMessage.of(() -> "Cache hit: " + (element != null) + "; service ticket: " + serviceTicket));
return (element != null) ? (CasAuthenticationToken) element.get() : null;
}
@Override
public void putTicketInCache(final CasAuthenticationToken token) {
String key = token.getCredentials().toString();
if (logger.isDebugEnabled()) {
logger.debug("Cache put: " + key);
}
logger.debug(LogMessage.of(() -> "Cache put: " + key));
this.cache.put(key, token);
}
@Override
public void removeTicketFromCache(final CasAuthenticationToken token) {
if (logger.isDebugEnabled()) {
logger.debug("Cache remove: " + token.getCredentials().toString());
}
logger.debug(LogMessage.of(() -> "Cache remove: " + token.getCredentials().toString()));
this.removeTicketFromCache(token.getCredentials().toString());
}

View File

@ -54,35 +54,28 @@ public final class GrantedAuthorityFromAssertionAttributesUserDetailsService
@SuppressWarnings("unchecked")
@Override
protected UserDetails loadUserDetails(final Assertion assertion) {
final List<GrantedAuthority> grantedAuthorities = new ArrayList<>();
for (final String attribute : this.attributes) {
final Object value = assertion.getPrincipal().getAttributes().get(attribute);
if (value == null) {
continue;
}
if (value instanceof List) {
final List list = (List) value;
for (final Object o : list) {
grantedAuthorities.add(new SimpleGrantedAuthority(
this.convertToUpperCase ? o.toString().toUpperCase() : o.toString()));
List<GrantedAuthority> grantedAuthorities = new ArrayList<>();
for (String attribute : this.attributes) {
Object value = assertion.getPrincipal().getAttributes().get(attribute);
if (value != null) {
if (value instanceof List) {
for (Object o : (List<?>) value) {
grantedAuthorities.add(createSimpleGrantedAuthority(o));
}
}
else {
grantedAuthorities.add(createSimpleGrantedAuthority(value));
}
}
else {
grantedAuthorities.add(new SimpleGrantedAuthority(
this.convertToUpperCase ? value.toString().toUpperCase() : value.toString()));
}
}
return new User(assertion.getPrincipal().getName(), NON_EXISTENT_PASSWORD_VALUE, true, true, true, true,
grantedAuthorities);
}
private SimpleGrantedAuthority createSimpleGrantedAuthority(Object o) {
return new SimpleGrantedAuthority(this.convertToUpperCase ? o.toString().toUpperCase() : o.toString());
}
/**
* Converts the returned attribute values to uppercase values.
* @param convertToUpperCase true if it should convert, false otherwise.

View File

@ -68,14 +68,11 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
}
@Override
public final void commence(final HttpServletRequest servletRequest, final HttpServletResponse response,
final AuthenticationException authenticationException) throws IOException {
final String urlEncodedService = createServiceUrl(servletRequest, response);
final String redirectUrl = createRedirectUrl(urlEncodedService);
public final void commence(final HttpServletRequest servletRequest, HttpServletResponse response,
AuthenticationException authenticationException) throws IOException {
String urlEncodedService = createServiceUrl(servletRequest, response);
String redirectUrl = createRedirectUrl(urlEncodedService);
preCommence(servletRequest, response);
response.sendRedirect(redirectUrl);
}
@ -86,7 +83,7 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
* @param response the HttpServlet Response
* @return the constructed service url. CANNOT be NULL.
*/
protected String createServiceUrl(final HttpServletRequest request, final HttpServletResponse response) {
protected String createServiceUrl(HttpServletRequest request, HttpServletResponse response) {
return CommonUtils.constructServiceUrl(null, response, this.serviceProperties.getService(), null,
this.serviceProperties.getArtifactParameter(), this.encodeServiceUrlWithSessionId);
}
@ -97,7 +94,7 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
* @param serviceUrl the service url that should be included.
* @return the redirect url. CANNOT be NULL.
*/
protected String createRedirectUrl(final String serviceUrl) {
protected String createRedirectUrl(String serviceUrl) {
return CommonUtils.constructRedirectUrl(this.loginUrl, this.serviceProperties.getServiceParameter(), serviceUrl,
this.serviceProperties.isSendRenew(), false);
}
@ -107,7 +104,7 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
* @param request the HttpServletRequest
* @param response the HttpServletResponse
*/
protected void preCommence(final HttpServletRequest request, final HttpServletResponse response) {
protected void preCommence(HttpServletRequest request, HttpServletResponse response) {
}
@ -124,11 +121,11 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
return this.serviceProperties;
}
public final void setLoginUrl(final String loginUrl) {
public final void setLoginUrl(String loginUrl) {
this.loginUrl = loginUrl;
}
public final void setServiceProperties(final ServiceProperties serviceProperties) {
public final void setServiceProperties(ServiceProperties serviceProperties) {
this.serviceProperties = serviceProperties;
}
@ -137,7 +134,7 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
* @param encodeServiceUrlWithSessionId whether to encode the service url with the
* session id or not.
*/
public final void setEncodeServiceUrlWithSessionId(final boolean encodeServiceUrlWithSessionId) {
public final void setEncodeServiceUrlWithSessionId(boolean encodeServiceUrlWithSessionId) {
this.encodeServiceUrlWithSessionId = encodeServiceUrlWithSessionId;
}

View File

@ -27,6 +27,7 @@ import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage;
import org.jasig.cas.client.util.CommonUtils;
import org.jasig.cas.client.validation.TicketValidator;
import org.springframework.core.log.LogMessage;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.authentication.AuthenticationDetailsSource;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@ -216,23 +217,17 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
super.successfulAuthentication(request, response, chain, authResult);
return;
}
if (this.logger.isDebugEnabled()) {
this.logger.debug("Authentication success. Updating SecurityContextHolder to contain: " + authResult);
}
this.logger.debug(
LogMessage.format("Authentication success. Updating SecurityContextHolder to contain: %s", authResult));
SecurityContextHolder.getContext().setAuthentication(authResult);
// Fire event
if (this.eventPublisher != null) {
this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass()));
}
chain.doFilter(request, response);
}
@Override
public Authentication attemptAuthentication(final HttpServletRequest request, final HttpServletResponse response)
public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response)
throws AuthenticationException, IOException {
// if the request is a proxy request process it and return null to indicate the
// request has been processed
@ -241,21 +236,15 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
CommonUtils.readAndRespondToProxyReceptorRequest(request, response, this.proxyGrantingTicketStorage);
return null;
}
final boolean serviceTicketRequest = serviceTicketRequest(request, response);
final String username = serviceTicketRequest ? CAS_STATEFUL_IDENTIFIER : CAS_STATELESS_IDENTIFIER;
boolean serviceTicketRequest = serviceTicketRequest(request, response);
String username = serviceTicketRequest ? CAS_STATEFUL_IDENTIFIER : CAS_STATELESS_IDENTIFIER;
String password = obtainArtifact(request);
if (password == null) {
this.logger.debug("Failed to obtain an artifact (cas ticket)");
password = "";
}
final UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken(username,
password);
UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken(username, password);
authRequest.setDetails(this.authenticationDetailsSource.buildDetails(request));
return this.getAuthenticationManager().authenticate(authRequest);
}
@ -272,7 +261,7 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
* Overridden to provide proxying capabilities.
*/
@Override
protected boolean requiresAuthentication(final HttpServletRequest request, final HttpServletResponse response) {
protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
final boolean serviceTicketRequest = serviceTicketRequest(request, response);
final boolean result = serviceTicketRequest || proxyReceptorRequest(request)
|| (proxyTicketRequest(serviceTicketRequest, request));
@ -320,11 +309,9 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
* @param response
* @return
*/
private boolean serviceTicketRequest(final HttpServletRequest request, final HttpServletResponse response) {
private boolean serviceTicketRequest(HttpServletRequest request, HttpServletResponse response) {
boolean result = super.requiresAuthentication(request, response);
if (this.logger.isDebugEnabled()) {
this.logger.debug("serviceTicketRequest = " + result);
}
this.logger.debug(LogMessage.format("serviceTicketRequest = %s", result));
return result;
}
@ -333,14 +320,12 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
* @param request
* @return
*/
private boolean proxyTicketRequest(final boolean serviceTicketRequest, final HttpServletRequest request) {
private boolean proxyTicketRequest(boolean serviceTicketRequest, HttpServletRequest request) {
if (serviceTicketRequest) {
return false;
}
final boolean result = this.authenticateAllArtifacts && obtainArtifact(request) != null && !authenticated();
if (this.logger.isDebugEnabled()) {
this.logger.debug("proxyTicketRequest = " + result);
}
boolean result = this.authenticateAllArtifacts && obtainArtifact(request) != null && !authenticated();
this.logger.debug(LogMessage.format("proxyTicketRequest = %s", result));
return result;
}
@ -359,11 +344,9 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
* @param request
* @return
*/
private boolean proxyReceptorRequest(final HttpServletRequest request) {
private boolean proxyReceptorRequest(HttpServletRequest request) {
final boolean result = proxyReceptorConfigured() && this.proxyReceptorMatcher.matches(request);
if (this.logger.isDebugEnabled()) {
this.logger.debug("proxyReceptorRequest = " + result);
}
this.logger.debug(LogMessage.format("proxyReceptorRequest = %s", result));
return result;
}
@ -374,9 +357,7 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
*/
private boolean proxyReceptorConfigured() {
final boolean result = this.proxyGrantingTicketStorage != null && this.proxyReceptorMatcher != null;
if (this.logger.isDebugEnabled()) {
this.logger.debug("proxyReceptorConfigured = " + result);
}
this.logger.debug(LogMessage.format("proxyReceptorConfigured = %s", result));
return result;
}
@ -387,8 +368,6 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
* will be used for proxy requests that fail. The value
* {@link CasAuthenticationFilter#setAuthenticationFailureHandler(AuthenticationFailureHandler)}
* will be used for service tickets that fail.
*
* @author Rob Winch
*/
private class CasAuthenticationFailureHandler implements AuthenticationFailureHandler {

View File

@ -108,7 +108,7 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails
if (query == null) {
return null;
}
final String result = artifactPattern.matcher(query).replaceFirst("");
String result = artifactPattern.matcher(query).replaceFirst("");
if (result.length() == 0) {
return null;
}