Skip to content

SEC-1987 & SEC-1986 #14

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

Closed
wants to merge 3 commits into from
Closed
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 @@ -56,6 +56,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
//~ Static fields/initializers =====================================================================================

private static final Log logger = LogFactory.getLog(CasAuthenticationProvider.class);
public static final String DEFAULT_REMEMBERME_ATTRIBUTE_NAME = "longTermAuthenticationRequestTokenUsed";

//~ Instance fields ================================================================================================

Expand All @@ -68,6 +69,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
private TicketValidator ticketValidator;
private ServiceProperties serviceProperties;
private GrantedAuthoritiesMapper authoritiesMapper = new NullAuthoritiesMapper();
private String rememberMeAttributeName = DEFAULT_REMEMBERME_ATTRIBUTE_NAME;


//~ Methods ========================================================================================================
Expand Down Expand Up @@ -141,7 +143,7 @@ private CasAuthenticationToken authenticateNow(final Authentication authenticati
final UserDetails userDetails = loadUserByAssertion(assertion);
userDetailsChecker.check(userDetails);
return new CasAuthenticationToken(this.key, userDetails, authentication.getCredentials(),
authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion);
authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion, rememberMeAttributeName);
} catch (final TicketValidationException e) {
throw new BadCredentialsException(e.getMessage(), e);
}
Expand Down Expand Up @@ -234,6 +236,14 @@ public void setAuthoritiesMapper(GrantedAuthoritiesMapper authoritiesMapper) {
this.authoritiesMapper = authoritiesMapper;
}

public String getRememberMeAttributeName() {
return rememberMeAttributeName;
}

public void setRememberMeAttributeName(String rememberMeAttributeName) {
this.rememberMeAttributeName = rememberMeAttributeName;
}

public boolean supports(final Class<?> authentication) {
return (UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication)) ||
(CasAuthenticationToken.class.isAssignableFrom(authentication)) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.jasig.cas.client.validation.Assertion;
import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.authentication.RememberMeAware;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.SpringSecurityCoreVersion;
import org.springframework.security.core.userdetails.UserDetails;
Expand All @@ -30,7 +31,7 @@
* @author Ben Alex
* @author Scott Battaglia
*/
public class CasAuthenticationToken extends AbstractAuthenticationToken implements Serializable {
public class CasAuthenticationToken extends AbstractAuthenticationToken implements Serializable, RememberMeAware {

private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID;

Expand All @@ -40,6 +41,7 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
private final UserDetails userDetails;
private final int keyHash;
private final Assertion assertion;
private final boolean rememberMe;

//~ Constructors ===================================================================================================

Expand All @@ -57,15 +59,18 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
* org.springframework.security.core.userdetails.UserDetailsService}) (cannot be <code>null</code>)
* @param assertion the assertion returned from the CAS servers. It contains the principal and how to obtain a
* proxy ticket for the user.
* @param rememberMeAttributeName the CAS remember-me attribute name
*
* @throws IllegalArgumentException if a <code>null</code> was passed
*/
public CasAuthenticationToken(final String key, final Object principal, final Object credentials,
final Collection<? extends GrantedAuthority> authorities, final UserDetails userDetails, final Assertion assertion) {
final Collection<? extends GrantedAuthority> authorities, final UserDetails userDetails, final Assertion assertion,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this constructor receives way too many parameters. Maybe consider a builder?

String rememberMeAttributeName) {
super(authorities);

if ((key == null) || ("".equals(key)) || (principal == null) || "".equals(principal) || (credentials == null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this long if for parameter validations could be used
Assert.notNull(myParameter, "myParamter must not be null"); which is a replacement for already java 8 Objects.requireNonNull()

|| "".equals(credentials) || (authorities == null) || (userDetails == null) || (assertion == null)) {
|| "".equals(credentials) || (authorities == null) || (userDetails == null) || (assertion == null) ||
(rememberMeAttributeName == null)) {
throw new IllegalArgumentException("Cannot pass null or empty values to constructor");
}

Expand All @@ -74,6 +79,8 @@ public CasAuthenticationToken(final String key, final Object principal, final Ob
this.credentials = credentials;
this.userDetails = userDetails;
this.assertion = assertion;
String rememberMeStringValue = (String) assertion.getPrincipal().getAttributes().get(rememberMeAttributeName);
this.rememberMe = rememberMeStringValue != null && Boolean.parseBoolean(rememberMeStringValue);
setAuthenticated(true);
}

Expand Down Expand Up @@ -121,9 +128,14 @@ public UserDetails getUserDetails() {
return userDetails;
}

public boolean isRememberMe() {
return rememberMe;
}

public String toString() {
StringBuilder sb = new StringBuilder();
sb.append(super.toString());
sb.append(" RememberMe: ").append(this.rememberMe);
sb.append(" Assertion: ").append(this.assertion);
sb.append(" Credentials (Service/Proxy Ticket): ").append(this.credentials);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@
* The user's browser will be redirected to the JA-SIG CAS enterprise-wide login page.
* This page is specified by the <code>loginUrl</code> property. Once login is complete, the CAS login page will
* redirect to the page indicated by the <code>service</code> property. The <code>service</code> is a HTTP URL
* belonging to the current application. The <code>service</code> URL is monitored by the {@link CasAuthenticationFilter},
* which will validate the CAS login was successful.
* belonging to the current application. The <code>renew</code> parameter can be forced to true when redirecting
* to the CAS server. The <code>service</code> URL is monitored by the {@link CasAuthenticationFilter}, which will
* validate the CAS login was successful.
*
* @author Ben Alex
* @author Scott Battaglia
Expand Down Expand Up @@ -68,11 +69,18 @@ public void afterPropertiesSet() throws Exception {
Assert.notNull(this.serviceProperties.getService(),"serviceProperties.getService() cannot be null.");
}


public final void commence(final HttpServletRequest servletRequest, final HttpServletResponse response,
final AuthenticationException authenticationException) throws IOException, ServletException {
// by default, don't force the renew parameter to true
commence(servletRequest, response, authenticationException, false);
}

public final void commence(final HttpServletRequest servletRequest, final HttpServletResponse response,
final AuthenticationException authenticationException) throws IOException, ServletException {
final AuthenticationException authenticationException, boolean forceRenew) throws IOException, ServletException {

final String urlEncodedService = createServiceUrl(servletRequest, response);
final String redirectUrl = createRedirectUrl(urlEncodedService);
final String redirectUrl = createRedirectUrl(urlEncodedService, forceRenew);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a good practice to pass boolean as parameter, just add another explicit method for this case


preCommence(servletRequest, response);

Expand All @@ -91,12 +99,19 @@ protected String createServiceUrl(final HttpServletRequest request, final HttpSe

/**
* Constructs the Url for Redirection to the CAS server. Default implementation relies on the CAS client to do the bulk of the work.
* The renew parameter can be forced to true if the forceRenew parameter is true.
*
* @param serviceUrl the service url that should be included.
* @param forceRenew if the renew parameter should be forced to true
* @return the redirect url. CANNOT be NULL.
*/
protected String createRedirectUrl(final String serviceUrl) {
return CommonUtils.constructRedirectUrl(this.loginUrl, this.serviceProperties.getServiceParameter(), serviceUrl, this.serviceProperties.isSendRenew(), false);
protected String createRedirectUrl(final String serviceUrl, final boolean forceRenew) {
boolean renew = this.serviceProperties.isSendRenew();
// if forceRenew is true, the renew parameter is forced to true
if (forceRenew) {
renew = true;
}
return CommonUtils.constructRedirectUrl(this.loginUrl, this.serviceProperties.getServiceParameter(), serviceUrl, renew, false);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright 2012 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.security.cas.web;

import java.io.IOException;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.beans.factory.InitializingBean;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.InsufficientAuthenticationException;
import org.springframework.security.cas.authentication.CasAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.access.AccessDeniedHandlerImpl;
import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
import org.springframework.security.web.savedrequest.RequestCache;
import org.springframework.util.Assert;

/**
* This class represents the <code>AccessDeniedHandlerImpl</code> dedicated to CAS authentication with remember-me support.
*
* @author Jerome Leleu
* @since 3.2.0
*/
public class CasRememberMeAccessDeniedHandler extends AccessDeniedHandlerImpl implements InitializingBean {

private RequestCache requestCache = new HttpSessionRequestCache();

private CasAuthenticationEntryPoint casAuthenticationEntryPoint;

public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException accessDeniedException)
throws IOException, ServletException {

Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
// if CAS authentication token
if (authentication != null && authentication instanceof CasAuthenticationToken) {
CasAuthenticationToken casAuthenticationToken = (CasAuthenticationToken) authentication;
// in remember-me mode
if (casAuthenticationToken.isRememberMe()) {
requestCache.saveRequest(request, response);
logger.debug("Calling Authentication entry point with renew=true.");
casAuthenticationEntryPoint.commence(request, response,
new InsufficientAuthenticationException("Full CAS authentication is required to access this resource"), true);
return;
}
}

super.handle(request, response, accessDeniedException);
}

public RequestCache getRequestCache() {
return requestCache;
}

public void setRequestCache(RequestCache requestCache) {
this.requestCache = requestCache;
}

public CasAuthenticationEntryPoint getCasAuthenticationEntryPoint() {
return casAuthenticationEntryPoint;
}

public void setCasAuthenticationEntryPoint(CasAuthenticationEntryPoint casAuthenticationEntryPoint) {
this.casAuthenticationEntryPoint = casAuthenticationEntryPoint;
}

public void afterPropertiesSet() throws Exception {
Assert.notNull(this.casAuthenticationEntryPoint, "casAuthenticationEntryPoint must be specified");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected CasAuthenticationToken getToken() {
final Assertion assertion = new AssertionImpl("rod");

return new CasAuthenticationToken("key", user, "ST-0-ER94xMJmn6pha35CQRoZ",
AuthorityUtils.createAuthorityList("ROLE_ONE", "ROLE_TWO"), user, assertion);
AuthorityUtils.createAuthorityList("ROLE_ONE", "ROLE_TWO"), user, assertion, CasAuthenticationProviderTests.TEST_REMEMBERME_ATTRIBUTE_NAME);
}

}
Loading