diff --git a/cas/src/test/java/org/springframework/security/cas/userdetails/GrantedAuthorityFromAssertionAttributesUserDetailsServiceTests.java b/cas/src/test/java/org/springframework/security/cas/userdetails/GrantedAuthorityFromAssertionAttributesUserDetailsServiceTests.java new file mode 100644 index 0000000000..0dcc456a80 --- /dev/null +++ b/cas/src/test/java/org/springframework/security/cas/userdetails/GrantedAuthorityFromAssertionAttributesUserDetailsServiceTests.java @@ -0,0 +1,49 @@ +package org.springframework.security.cas.userdetails; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.jasig.cas.client.authentication.AttributePrincipal; +import org.jasig.cas.client.validation.Assertion; +import org.junit.Test; +import org.springframework.security.cas.authentication.CasAssertionAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.userdetails.UserDetails; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +/** + * @author Luke Taylor + */ +public class GrantedAuthorityFromAssertionAttributesUserDetailsServiceTests { + + @Test + public void correctlyExtractsNamedAttributesFromAssertionAndConvertsThemToAuthorities() { + GrantedAuthorityFromAssertionAttributesUserDetailsService uds = + new GrantedAuthorityFromAssertionAttributesUserDetailsService(new String[] {"a", "b", "c", "d"}); + uds.setConvertToUpperCase(false); + Assertion assertion = mock(Assertion.class); + AttributePrincipal principal = mock(AttributePrincipal.class); + Map attributes = new HashMap(); + attributes.put("a", Arrays.asList("role_a1", "role_a2")); + attributes.put("b", "role_b"); + attributes.put("c", "role_c"); + attributes.put("d", null); + attributes.put("someother", "unused"); + when(assertion.getPrincipal()).thenReturn(principal); + when(principal.getAttributes()).thenReturn(attributes); + when(principal.getName()).thenReturn("somebody"); + CasAssertionAuthenticationToken token = new CasAssertionAuthenticationToken(assertion, "ticket"); + UserDetails user = uds.loadUserDetails(token); + Set roles = AuthorityUtils.authorityListToSet(user.getAuthorities()); + assertTrue(roles.size() == 4); + assertTrue(roles.contains("role_a1")); + assertTrue(roles.contains("role_a2")); + assertTrue(roles.contains("role_b")); + assertTrue(roles.contains("role_c")); + } +} diff --git a/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java b/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java index e3221b722e..efb8325838 100644 --- a/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java +++ b/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java @@ -16,12 +16,15 @@ package org.springframework.security.cas.web; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.cas.ServiceProperties; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -35,14 +38,17 @@ public class CasAuthenticationFilterTests { //~ Methods ======================================================================================================== @Test - public void testGetters() { + public void testGettersSetters() { CasAuthenticationFilter filter = new CasAuthenticationFilter(); assertEquals("/j_spring_cas_security_check", filter.getFilterProcessesUrl()); + filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class)); + filter.setProxyReceptorUrl("/someurl"); + filter.setServiceProperties(new ServiceProperties()); } @Test public void testNormalOperation() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/j_spring_cas_security_check"); request.addParameter("ticket", "ST-0-ER94xMJmn6pha35CQRoZ"); CasAuthenticationFilter filter = new CasAuthenticationFilter(); @@ -52,6 +58,8 @@ public class CasAuthenticationFilterTests { } }); + assertTrue(filter.requiresAuthentication(request, new MockHttpServletResponse())); + Authentication result = filter.attemptAuthentication(request, new MockHttpServletResponse()); assertTrue(result != null); } diff --git a/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java b/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java index a313bc8a24..81a07dfe98 100644 --- a/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java +++ b/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java @@ -15,40 +15,43 @@ package org.springframework.security.cas.web; -import org.springframework.security.cas.ServiceProperties; - -import junit.framework.TestCase; +import static junit.framework.Assert.*; +import org.junit.Test; +import org.springframework.security.cas.SamlServiceProperties; +import org.springframework.security.cas.ServiceProperties; /** * Tests {@link ServiceProperties}. * * @author Ben Alex */ -public class ServicePropertiesTests extends TestCase { +public class ServicePropertiesTests { //~ Methods ======================================================================================================== - public void testDetectsMissingLoginFormUrl() throws Exception { + @Test(expected=IllegalArgumentException.class) + public void detectsMissingService() throws Exception { ServiceProperties sp = new ServiceProperties(); - - try { - sp.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - assertEquals("service must be specified.", expected.getMessage()); - } + sp.afterPropertiesSet(); } + @Test public void testGettersSetters() throws Exception { - ServiceProperties sp = new ServiceProperties(); - sp.setSendRenew(false); - assertFalse(sp.isSendRenew()); - sp.setSendRenew(true); - assertTrue(sp.isSendRenew()); - - sp.setService("https://mycompany.com/service"); - assertEquals("https://mycompany.com/service", sp.getService()); + ServiceProperties[] sps = {new ServiceProperties(), new SamlServiceProperties()}; + for(ServiceProperties sp : sps) { + sp.setSendRenew(false); + assertFalse(sp.isSendRenew()); + sp.setSendRenew(true); + assertTrue(sp.isSendRenew()); + sp.setArtifactParameter("notticket"); + assertEquals("notticket", sp.getArtifactParameter()); + sp.setServiceParameter("notservice"); + assertEquals("notservice", sp.getServiceParameter()); + + sp.setService("https://mycompany.com/service"); + assertEquals("https://mycompany.com/service", sp.getService()); - sp.afterPropertiesSet(); + sp.afterPropertiesSet(); + } } } diff --git a/gradle/emma.gradle b/gradle/emma.gradle index fbc2950f8e..0cd8bedf71 100644 --- a/gradle/emma.gradle +++ b/gradle/emma.gradle @@ -8,8 +8,8 @@ dependencies{ emma "emma:emma_ant:2.0.5312" } -def emmaMetaDataFile = "${rootProject.buildDir}/emma/coverage.em" -def emmaCoverageFile = "${rootProject.buildDir}/emma/coverage.ec" +def emmaMetaDataFile = "${rootProject.buildDir}/emma/emma.em" +def emmaCoverageFile = "${rootProject.buildDir}/emma/emma.ec" task emmaInstrument { dependsOn classes @@ -18,7 +18,7 @@ task emmaInstrument { ant.path(id: "emmarun.classpath") { pathelement(location: sourceSets.main.classesDir.absolutePath) } - ant.emma(verbosity: "info") { + ant.emma(verbosity: "quiet") { // use "info, verbose, trace1, trace2, trace3 for more info" instr(merge: "true", destdir: "$buildDir/emma/classes", instrpathref: "emmarun.classpath", metadatafile: "$emmaMetaDataFile") { instrpath { fileset(dir: sourceSets.main.classesDir.absolutePath, includes: "*.class") @@ -35,6 +35,7 @@ afterEvaluate { task.dependsOn emmaInstrument task.configure() { jvmArgs '-Dsec.log.level=DEBUG', "-Demma.coverage.out.file=$emmaCoverageFile" + jvmArgs '-Demma.verbosity.level=quiet' } task.doFirst { classpath = files("$buildDir/emma/classes") + configurations.emma + classpath @@ -53,7 +54,7 @@ if (rootProject.getTasksByName('coverageReport', false).isEmpty()) { } } } - ant.emma(enabled: "true", verbosity: "trace1") { // use "verbose, trace1, trace2, trace3 for more info" + ant.emma(enabled: "true", verbosity: "info") { report(sourcepathref:"src.path") { fileset(dir: rootProject.buildDir) { include: '*.ec' diff --git a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java index eff869f074..6a49eabf25 100644 --- a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java @@ -94,9 +94,7 @@ public class ExceptionTranslationFilter extends GenericFilterBean { try { chain.doFilter(request, response); - if (logger.isDebugEnabled()) { - logger.debug("Chain processed normally"); - } + logger.debug("Chain processed normally"); } catch (IOException ex) { throw ex; @@ -112,7 +110,7 @@ public class ExceptionTranslationFilter extends GenericFilterBean { } if (ase != null) { - handleException(request, response, chain, ase); + handleSpringSecurityException(request, response, chain, ase); } else { // Rethrow ServletExceptions and RuntimeExceptions as-is if (ex instanceof ServletException) { @@ -122,7 +120,8 @@ public class ExceptionTranslationFilter extends GenericFilterBean { throw (RuntimeException) ex; } - // Wrap other Exceptions. These are not expected to happen + // Wrap other Exceptions. This shouldn't actually happen + // as we've already covered all the possibilities for doFilter throw new RuntimeException(ex); } } @@ -136,30 +135,23 @@ public class ExceptionTranslationFilter extends GenericFilterBean { return authenticationTrustResolver; } - private void handleException(HttpServletRequest request, HttpServletResponse response, FilterChain chain, + private void handleSpringSecurityException(HttpServletRequest request, HttpServletResponse response, FilterChain chain, RuntimeException exception) throws IOException, ServletException { if (exception instanceof AuthenticationException) { - if (logger.isDebugEnabled()) { - logger.debug("Authentication exception occurred; redirecting to authentication entry point", exception); - } + logger.debug("Authentication exception occurred; redirecting to authentication entry point", exception); sendStartAuthentication(request, response, chain, (AuthenticationException) exception); } else if (exception instanceof AccessDeniedException) { if (authenticationTrustResolver.isAnonymous(SecurityContextHolder.getContext().getAuthentication())) { - if (logger.isDebugEnabled()) { - logger.debug("Access is denied (user is anonymous); redirecting to authentication entry point", + logger.debug("Access is denied (user is anonymous); redirecting to authentication entry point", exception); - } sendStartAuthentication(request, response, chain, new InsufficientAuthenticationException( "Full authentication is required to access this resource")); } else { - if (logger.isDebugEnabled()) { - logger.debug("Access is denied (user is not anonymous); delegating to AccessDeniedHandler", - exception); - } + logger.debug("Access is denied (user is not anonymous); delegating to AccessDeniedHandler", exception); accessDeniedHandler.handle(request, response, (AccessDeniedException) exception); } diff --git a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java b/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java index 13501d9112..475b79c3f9 100644 --- a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java @@ -17,15 +17,8 @@ package org.springframework.security.web.access; import static org.junit.Assert.*; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.*; - -import java.io.IOException; - -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import org.junit.After; import org.junit.Before; @@ -43,10 +36,16 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.WebAttributes; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; -import org.springframework.security.web.savedrequest.DefaultSavedRequest; import org.springframework.security.web.savedrequest.SavedRequest; import org.springframework.security.web.util.ThrowableAnalyzer; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.io.IOException; + /** * Tests {@link ExceptionTranslationFilter}. * @@ -134,7 +133,7 @@ public class ExceptionTranslationFilterTests { } @Test - public void testRedirectedToLoginFormAndSessionShowsOriginalTargetWhenAuthenticationException() throws Exception { + public void redirectedToLoginFormAndSessionShowsOriginalTargetWhenAuthenticationException() throws Exception { // Setup our HTTP request MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secure/page.html"); @@ -159,7 +158,7 @@ public class ExceptionTranslationFilterTests { } @Test - public void testRedirectedToLoginFormAndSessionShowsOriginalTargetWithExoticPortWhenAuthenticationException() + public void redirectedToLoginFormAndSessionShowsOriginalTargetWithExoticPortWhenAuthenticationException() throws Exception { // Setup our HTTP request MockHttpServletRequest request = new MockHttpServletRequest(); @@ -188,7 +187,7 @@ public class ExceptionTranslationFilterTests { } @Test(expected=IllegalArgumentException.class) - public void testStartupDetectsMissingAuthenticationEntryPoint() throws Exception { + public void startupDetectsMissingAuthenticationEntryPoint() throws Exception { ExceptionTranslationFilter filter = new ExceptionTranslationFilter(); filter.setThrowableAnalyzer(mock(ThrowableAnalyzer.class)); @@ -196,14 +195,15 @@ public class ExceptionTranslationFilterTests { } @Test(expected=IllegalArgumentException.class) - public void testStartupDetectsMissingRequestCache() throws Exception { + public void startupDetectsMissingRequestCache() throws Exception { ExceptionTranslationFilter filter = new ExceptionTranslationFilter(); filter.setAuthenticationEntryPoint(mockEntryPoint()); filter.setRequestCache(null); } - public void testSuccessfulAccessGrant() throws Exception { + @Test + public void successfulAccessGrant() throws Exception { // Setup our HTTP request MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secure/page.html"); @@ -217,36 +217,44 @@ public class ExceptionTranslationFilterTests { } @Test - public void testThrowIOException() throws Exception { + public void thrownIOExceptionServletExceptionAndRuntimeExceptionsAreRethrown() throws Exception { ExceptionTranslationFilter filter = new ExceptionTranslationFilter(); filter.setAuthenticationEntryPoint(mockEntryPoint()); filter.afterPropertiesSet(); - FilterChain fc = mock(FilterChain.class); - doThrow(new IOException()).when(fc).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); - try { - filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), fc); - fail("Should have thrown IOException"); - } - catch (IOException e) { - assertNull("The IOException thrown should not have been wrapped", e.getCause()); + Exception[] exceptions = {new IOException(), new ServletException(), new RuntimeException()}; + for (Exception e : exceptions) { + FilterChain fc = mock(FilterChain.class); + + doThrow(e).when(fc).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); + try { + filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), fc); + fail("Should have thrown Exception"); + } + catch (Exception expected) { + assertSame("The exception thrown should not have been wrapped", e, expected); + } } } @Test - public void testThrowServletException() throws Exception { + public void unexpectedExceptionsAreWrappedAsRuntime() throws Exception { ExceptionTranslationFilter filter = new ExceptionTranslationFilter(); filter.setAuthenticationEntryPoint(mockEntryPoint()); - filter.afterPropertiesSet(); + + Exception e = new Exception(""); + FilterChain fc = mock(FilterChain.class); - doThrow(new ServletException()).when(fc).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); + + doThrow(e).when(fc).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class)); + try { filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), fc); - fail("Should have thrown ServletException"); + fail("Should have thrown Exception"); } - catch (ServletException e) { - assertNull("The ServletException thrown should not have been wrapped", e.getCause()); + catch (RuntimeException expected) { + assertSame(e, expected.getCause()); } }