From 08e84b0ee3e93871eae0d6c4fbdd65ca1a7de131 Mon Sep 17 00:00:00 2001 From: Fabien Crespel Date: Mon, 21 Jan 2019 01:24:25 +0100 Subject: [PATCH] Fix login redirect loop caused by SECURITY-901 changes in Jenkins core --- .../plugins/cas/CasSecurityRealm.java | 21 ++++++++++++----- .../plugins/cas/spring/CasEventListener.java | 23 ++++++------------- .../plugins/cas/CasSecurityRealm.groovy | 1 + 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/cas/CasSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/cas/CasSecurityRealm.java index 63893d0..1c1ef0c 100644 --- a/src/main/java/org/jenkinsci/plugins/cas/CasSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/cas/CasSecurityRealm.java @@ -16,12 +16,14 @@ import org.acegisecurity.AuthenticationException; import org.acegisecurity.AuthenticationManager; import org.acegisecurity.BadCredentialsException; +import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken; import org.apache.commons.lang.StringUtils; import org.jasig.cas.client.session.SessionMappingStorage; import org.jasig.cas.client.util.CommonUtils; import org.jenkinsci.plugins.cas.spring.security.AcegiAuthenticationManager; +import org.jenkinsci.plugins.cas.spring.security.CasAuthentication; import org.jenkinsci.plugins.cas.spring.security.CasRestAuthenticator; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; @@ -29,7 +31,7 @@ import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.interceptor.RequirePOST; import org.springframework.security.cas.ServiceProperties; -import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.cas.authentication.CasAuthenticationToken; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.context.WebApplicationContext; @@ -40,10 +42,10 @@ import hudson.model.Descriptor; import hudson.security.ChainedServletFilter; import hudson.security.SecurityRealm; -import hudson.security.SecurityRealm.SecurityComponents; import hudson.util.FormValidation; import hudson.util.spring.BeanBuilder; import jenkins.model.Jenkins; +import jenkins.security.SecurityListener; /** * CAS Single Sign-On security realm. @@ -244,7 +246,7 @@ public Filter createFilter(FilterConfig filterConfig) { @Override public void doLogout(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { // Clear Spring Security context - SecurityContextHolder.clearContext(); + org.springframework.security.core.context.SecurityContextHolder.clearContext(); // Remove session from CAS single sign-out storage HttpSession session = req.getSession(false); @@ -270,13 +272,20 @@ public void doCommenceLogin(StaplerRequest req, StaplerResponse rsp) throws IOEx } /** - * The login process finishes here, although by the time this action is called - * everything has already been taken care of by filters. + * The login process finishes here, by mapping the Spring Security + * authentication back to Acegi and by firing the authenticated event. * @param req request * @param rsp response */ public void doFinishLogin(StaplerRequest req, StaplerResponse rsp) { - // Nothing to do + org.springframework.security.core.Authentication authentication = org.springframework.security.core.context.SecurityContextHolder.getContext().getAuthentication(); + if (authentication instanceof CasAuthenticationToken) { + org.springframework.security.core.context.SecurityContextHolder.clearContext(); + CasAuthenticationToken casToken = (CasAuthenticationToken) authentication; + CasAuthentication casAuth = CasAuthentication.newInstance(casToken); + SecurityContextHolder.getContext().setAuthentication(casAuth); + SecurityListener.fireAuthenticated(casAuth.getUserDetails()); + } } // ~ SecurityRealm descriptor ======================================================================================= diff --git a/src/main/java/org/jenkinsci/plugins/cas/spring/CasEventListener.java b/src/main/java/org/jenkinsci/plugins/cas/spring/CasEventListener.java index a133b63..a32e8f9 100644 --- a/src/main/java/org/jenkinsci/plugins/cas/spring/CasEventListener.java +++ b/src/main/java/org/jenkinsci/plugins/cas/spring/CasEventListener.java @@ -4,14 +4,12 @@ import java.util.Collection; import java.util.Map; -import org.acegisecurity.context.SecurityContextHolder; -import org.jenkinsci.plugins.cas.spring.security.CasAuthentication; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.security.authentication.event.AuthenticationSuccessEvent; -import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent; import org.springframework.security.cas.authentication.CasAuthenticationToken; import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import hudson.tasks.Mailer; @@ -36,16 +34,20 @@ public class CasEventListener implements ApplicationListener { public void onApplicationEvent(ApplicationEvent event) { if (event instanceof AuthenticationSuccessEvent) { onSuccessfulAuthentication(((AuthenticationSuccessEvent) event).getAuthentication()); - } else if (event instanceof InteractiveAuthenticationSuccessEvent) { - onSuccessfulInteractiveAuthentication(((InteractiveAuthenticationSuccessEvent) event).getAuthentication()); } } /** * Successful authentication event handler. + * This event is fired immediately after authentication, before filter chain + * is invoked and before Spring security context is updated. * @param authentication the successful authentication object */ protected void onSuccessfulAuthentication(Authentication authentication) { + // Set Spring security context early to allow Acegi mapping in CasSecurityRealm.doFinishLogin() + SecurityContextHolder.getContext().setAuthentication(authentication); + + // Map user attributes if (authentication instanceof CasAuthenticationToken) { CasAuthenticationToken casToken = (CasAuthenticationToken) authentication; try { @@ -56,17 +58,6 @@ protected void onSuccessfulAuthentication(Authentication authentication) { } } - /** - * Successful interactive authentication event handler. - * @param authentication the successful authentication object - */ - protected void onSuccessfulInteractiveAuthentication(Authentication authentication) { - if (authentication instanceof CasAuthenticationToken) { - CasAuthenticationToken casToken = (CasAuthenticationToken) authentication; - SecurityContextHolder.getContext().setAuthentication(CasAuthentication.newInstance(casToken)); - } - } - /** * Sync user attributes with a CAS authentication token. * @param casToken CAS authentication token diff --git a/src/main/resources/org/jenkinsci/plugins/cas/CasSecurityRealm.groovy b/src/main/resources/org/jenkinsci/plugins/cas/CasSecurityRealm.groovy index 0fdb43d..acd1585 100644 --- a/src/main/resources/org/jenkinsci/plugins/cas/CasSecurityRealm.groovy +++ b/src/main/resources/org/jenkinsci/plugins/cas/CasSecurityRealm.groovy @@ -87,6 +87,7 @@ casFilter(ChainedServletFilter) { serviceProperties = casServiceProperties authenticationFailureHandler = bean(SimpleUrlAuthenticationFailureHandler, "/" + securityRealm.failedLoginUrl) authenticationSuccessHandler = bean(SessionUrlAuthenticationSuccessHandler, "/") + continueChainBeforeSuccessfulAuthentication = true } ] }