From 5f75e9bf9a893a70cf5339dafd4c151f9118f396 Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Wed, 22 Jun 2005 06:30:46 +0000 Subject: [PATCH] Refactor Authentication.isAuthenticated() handling to be more performance (as per developer list discussion). --- .../org/acegisecurity/Authentication.java | 53 ++++++++++++++++--- .../ContextPropagatingRemoteInvocation.java | 15 ++++++ .../AbstractSecurityInterceptor.java | 50 ++++++++++------- .../UsernamePasswordAuthenticationToken.java | 45 ++++++++++------ .../AnonymousAuthenticationToken.java | 26 ++++----- .../providers/cas/CasAuthenticationToken.java | 28 ++++------ .../jaas/JaasAuthenticationProvider.java | 18 ++++--- .../RememberMeAuthenticationToken.java | 16 ++---- .../acegisecurity/runas/RunAsUserToken.java | 18 ++----- .../MethodSecurityInterceptorTests.java | 14 ++--- ...rnamePasswordAuthenticationTokenTests.java | 40 ++++++++------ .../AnonymousAuthenticationTokenTests.java | 5 +- .../cas/CasAuthenticationTokenTests.java | 7 ++- .../RememberMeAuthenticationTokenTests.java | 5 +- .../runas/RunAsUserTokenTests.java | 6 +-- doc/xdocs/changes.xml | 19 +++---- 16 files changed, 211 insertions(+), 154 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/Authentication.java b/core/src/main/java/org/acegisecurity/Authentication.java index fcc211950a..4fcce02941 100644 --- a/core/src/main/java/org/acegisecurity/Authentication.java +++ b/core/src/main/java/org/acegisecurity/Authentication.java @@ -39,16 +39,55 @@ import java.security.Principal; public interface Authentication extends Principal, Serializable { //~ Methods ================================================================ - public void setAuthenticated(boolean isAuthenticated); + /** + * See {@link #isAuthenticated()} for a full description. + * + *

+ * Implementations should always allow this method to be called with + * a false parameter, as this is used by various classes to + * specify the authentication token should not be trusted. If an + * implementation wishes to reject an invocation with a true + * parameter (which would indicate the authentication token is trusted - a + * potential security risk) the implementation should throw an {@link + * IllegalArgumentException}. + *

+ * + * @param isAuthenticated true if the token should be trusted + * (which may result in an exception) or false if the + * token should not be trusted + * + * @throws IllegalArgumentException if an attempt to make the + * authentication token trusted (by passing true as + * the argument) is rejected due to the implementation being + * immutable or implementing its own alternative approach to + * {@link #isAuthenticated()} + */ + public void setAuthenticated(boolean isAuthenticated) + throws IllegalArgumentException; /** - * Indicates whether or not authentication was attempted by the {@link - * net.sf.acegisecurity.intercept.AbstractSecurityInterceptor}. Note that - * classes should not rely on this value as being valid unless it has been - * set by a trusted AbstractSecurityInterceptor. + * Used to indicate to AbstractSecurityInterceptor whether it + * should present the authentication token to the + * AuthenticationManager. Typically an + * AuthenticationManager (or, more often, one of its + * AuthenticationProviders) will return an immutable + * authentication token after successful authentication, in which case + * that token can safely return true to this method. + * Returning true will improve performance, as calling the + * AuthenticationManager for every request will no longer be + * necessary. + * + *

+ * For security reasons, implementations of this interface should be very + * careful about returning true to this method unless they + * are either immutable, or have some way of ensuring the properties have + * not been changed since original creation. + *

* - * @return true if authenticated by the - * AbstractSecurityInterceptor + * @return true if the token has been authenticated and the + * AbstractSecurityInterceptor does not need to + * represent the token for re-authentication to the + * AuthenticationManager */ public boolean isAuthenticated(); diff --git a/core/src/main/java/org/acegisecurity/context/rmi/ContextPropagatingRemoteInvocation.java b/core/src/main/java/org/acegisecurity/context/rmi/ContextPropagatingRemoteInvocation.java index 592798ec7c..404dff74ba 100644 --- a/core/src/main/java/org/acegisecurity/context/rmi/ContextPropagatingRemoteInvocation.java +++ b/core/src/main/java/org/acegisecurity/context/rmi/ContextPropagatingRemoteInvocation.java @@ -82,6 +82,15 @@ public class ContextPropagatingRemoteInvocation extends RemoteInvocation { /** * Invoked on the server-side as described in the class JavaDocs. + * + *

+ * Invocations will always have their {@link + * net.sf.acegisecurity.Authentication#setAuthenticated(boolean)} set to + * false, which is guaranteed to always be accepted by + * Authentication implementations. This ensures that even + * remotely authenticated Authentications will be untrusted + * by the server-side, which is an appropriate security measure. + *

* * @param targetObject the target object to apply the invocation to * @@ -97,6 +106,12 @@ public class ContextPropagatingRemoteInvocation extends RemoteInvocation { InvocationTargetException { SecurityContextHolder.setContext(securityContext); + if ((SecurityContextHolder.getContext() != null) + && (SecurityContextHolder.getContext().getAuthentication() != null)) { + SecurityContextHolder.getContext().getAuthentication() + .setAuthenticated(false); + } + if (logger.isDebugEnabled()) { logger.debug("Set SecurityContextHolder to contain: " + securityContext); diff --git a/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java b/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java index e3aea473b5..8587cfcc84 100644 --- a/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java +++ b/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java @@ -364,30 +364,42 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, object, attr); } - // Attempt authentication + // Attempt authentication if not already authenticated Authentication authenticated; - try { - authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext() - .getAuthentication()); - } catch (AuthenticationException authenticationException) { - AuthenticationFailureEvent event = new AuthenticationFailureEvent(object, - attr, - SecurityContextHolder.getContext().getAuthentication(), - authenticationException); - this.context.publishEvent(event); + if (!SecurityContextHolder.getContext().getAuthentication() + .isAuthenticated()) { + try { + authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext() + .getAuthentication()); + } catch (AuthenticationException authenticationException) { + AuthenticationFailureEvent event = new AuthenticationFailureEvent(object, + attr, + SecurityContextHolder.getContext() + .getAuthentication(), + authenticationException); + this.context.publishEvent(event); - throw authenticationException; + throw authenticationException; + } + + // We don't authenticated.setAuthentication(true), because each provider should do that + if (logger.isDebugEnabled()) { + logger.debug("Successfully Authenticated: " + + authenticated.toString()); + } + + SecurityContextHolder.getContext().setAuthentication(authenticated); + } else { + authenticated = SecurityContextHolder.getContext() + .getAuthentication(); + + if (logger.isDebugEnabled()) { + logger.debug("Previously Authenticated: " + + authenticated.toString()); + } } - authenticated.setAuthenticated(true); - - if (logger.isDebugEnabled()) { - logger.debug("Authenticated: " + authenticated.toString()); - } - - SecurityContextHolder.getContext().setAuthentication(authenticated); - // Attempt authorization try { this.accessDecisionManager.decide(authenticated, object, attr); diff --git a/core/src/main/java/org/acegisecurity/providers/UsernamePasswordAuthenticationToken.java b/core/src/main/java/org/acegisecurity/providers/UsernamePasswordAuthenticationToken.java index 4a613fbdef..ce155a22ed 100644 --- a/core/src/main/java/org/acegisecurity/providers/UsernamePasswordAuthenticationToken.java +++ b/core/src/main/java/org/acegisecurity/providers/UsernamePasswordAuthenticationToken.java @@ -1,4 +1,4 @@ -/* Copyright 2004 Acegi Technology Pty Limited +/* Copyright 2004, 2005 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,21 +40,42 @@ public class UsernamePasswordAuthenticationToken private Object details = null; private Object principal; private GrantedAuthority[] authorities; - private boolean authenticated = false; + private boolean authenticated; //~ Constructors =========================================================== + /** + * This constructor can be safely used by any code that wishes to create a + * UsernamePasswordAuthenticationToken, as the {@link + * #isAuthenticated()} will return false. + * + * @param principal DOCUMENT ME! + * @param credentials DOCUMENT ME! + */ public UsernamePasswordAuthenticationToken(Object principal, Object credentials) { this.principal = principal; this.credentials = credentials; + this.authenticated = false; } + /** + * This constructor should only be used by + * AuthenticationManager or + * AuthenticationProvider implementations that are satisfied + * with producing a trusted (ie {@link #isAuthenticated()} = + * true) authentication token. + * + * @param principal + * @param credentials + * @param authorities + */ public UsernamePasswordAuthenticationToken(Object principal, Object credentials, GrantedAuthority[] authorities) { this.principal = principal; this.credentials = credentials; this.authorities = authorities; + this.authenticated = true; } protected UsernamePasswordAuthenticationToken() { @@ -63,7 +84,13 @@ public class UsernamePasswordAuthenticationToken //~ Methods ================================================================ - public void setAuthenticated(boolean isAuthenticated) { + public void setAuthenticated(boolean isAuthenticated) + throws IllegalArgumentException { + if (isAuthenticated) { + throw new IllegalArgumentException( + "Cannot set this token to trusted - use constructor containing GrantedAuthority[]s instead"); + } + this.authenticated = isAuthenticated; } @@ -71,18 +98,6 @@ public class UsernamePasswordAuthenticationToken return this.authenticated; } - /** - * Generally you should not call this method, because on subsequent - * requests the Authentication will be recreated by the - * relevant AuthenticationManager. This method is mostly of - * interest to AuthenticationManagers and unit tests. - * - * @param authorities the new authorities to apply - */ - public void setAuthorities(GrantedAuthority[] authorities) { - this.authorities = authorities; - } - public GrantedAuthority[] getAuthorities() { return this.authorities; } diff --git a/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationToken.java b/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationToken.java index 51d75b6fa9..f1d9cff43f 100644 --- a/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationToken.java +++ b/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationToken.java @@ -18,10 +18,10 @@ package net.sf.acegisecurity.providers.anonymous; import net.sf.acegisecurity.GrantedAuthority; import net.sf.acegisecurity.providers.AbstractAuthenticationToken; -import java.io.Serializable; - import org.springframework.util.Assert; +import java.io.Serializable; + /** * Represents an anonymous Authentication. @@ -35,6 +35,7 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken private Object principal; private GrantedAuthority[] authorities; + private boolean authenticated; private int keyHash; //~ Constructors =========================================================== @@ -58,14 +59,15 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken } for (int i = 0; i < authorities.length; i++) { - Assert.notNull(authorities[i], "Granted authority element " - + i - + " is null - GrantedAuthority[] cannot contain any null elements"); + Assert.notNull(authorities[i], + "Granted authority element " + i + + " is null - GrantedAuthority[] cannot contain any null elements"); } this.keyHash = key.hashCode(); this.principal = principal; this.authorities = authorities; + this.authenticated = true; } protected AnonymousAuthenticationToken() { @@ -74,22 +76,12 @@ public class AnonymousAuthenticationToken extends AbstractAuthenticationToken //~ Methods ================================================================ - /** - * Ignored (always true). - * - * @param isAuthenticated ignored - */ public void setAuthenticated(boolean isAuthenticated) { - // ignored + this.authenticated = isAuthenticated; } - /** - * Always returns true. - * - * @return true - */ public boolean isAuthenticated() { - return true; + return this.authenticated; } public GrantedAuthority[] getAuthorities() { diff --git a/core/src/main/java/org/acegisecurity/providers/cas/CasAuthenticationToken.java b/core/src/main/java/org/acegisecurity/providers/cas/CasAuthenticationToken.java index 9b894b6858..dc22ef8272 100644 --- a/core/src/main/java/org/acegisecurity/providers/cas/CasAuthenticationToken.java +++ b/core/src/main/java/org/acegisecurity/providers/cas/CasAuthenticationToken.java @@ -1,4 +1,4 @@ -/* Copyright 2004 Acegi Technology Pty Limited +/* Copyright 2004, 2005 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,12 +19,12 @@ import net.sf.acegisecurity.GrantedAuthority; import net.sf.acegisecurity.UserDetails; import net.sf.acegisecurity.providers.AbstractAuthenticationToken; +import org.springframework.util.Assert; + import java.io.Serializable; import java.util.List; -import org.springframework.util.Assert; - /** * Represents a successful CAS Authentication. @@ -42,6 +42,7 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken private String proxyGrantingTicketIou; private UserDetails userDetails; private GrantedAuthority[] authorities; + private boolean authenticated; private int keyHash; //~ Constructors =========================================================== @@ -79,9 +80,9 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken } for (int i = 0; i < authorities.length; i++) { - Assert.notNull(authorities[i], "Granted authority element " - + i - + " is null - GrantedAuthority[] cannot contain any null elements"); + Assert.notNull(authorities[i], + "Granted authority element " + i + + " is null - GrantedAuthority[] cannot contain any null elements"); } this.keyHash = key.hashCode(); @@ -91,6 +92,7 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken this.userDetails = userDetails; this.proxyList = proxyList; this.proxyGrantingTicketIou = proxyGrantingTicketIou; + this.authenticated = true; } protected CasAuthenticationToken() { @@ -99,22 +101,12 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken //~ Methods ================================================================ - /** - * Ignored (always true). - * - * @param isAuthenticated ignored - */ public void setAuthenticated(boolean isAuthenticated) { - // ignored + this.authenticated = isAuthenticated; } - /** - * Always returns true. - * - * @return true - */ public boolean isAuthenticated() { - return true; + return this.authenticated; } public GrantedAuthority[] getAuthorities() { diff --git a/core/src/main/java/org/acegisecurity/providers/jaas/JaasAuthenticationProvider.java b/core/src/main/java/org/acegisecurity/providers/jaas/JaasAuthenticationProvider.java index 231f592004..706550b8e1 100644 --- a/core/src/main/java/org/acegisecurity/providers/jaas/JaasAuthenticationProvider.java +++ b/core/src/main/java/org/acegisecurity/providers/jaas/JaasAuthenticationProvider.java @@ -327,7 +327,7 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, public Authentication authenticate(Authentication auth) throws AuthenticationException { if (auth instanceof UsernamePasswordAuthenticationToken) { - UsernamePasswordAuthenticationToken token = (UsernamePasswordAuthenticationToken) auth; + UsernamePasswordAuthenticationToken request = (UsernamePasswordAuthenticationToken) auth; try { //Create the LoginContext object, and pass our InternallCallbackHandler @@ -340,8 +340,8 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, //create a set to hold the authorities, and add any that have already been applied. Set authorities = new HashSet(); - if (token.getAuthorities() != null) { - authorities.addAll(Arrays.asList(token.getAuthorities())); + if (request.getAuthorities() != null) { + authorities.addAll(Arrays.asList(request.getAuthorities())); } //get the subject principals and pass them to each of the AuthorityGranters @@ -368,19 +368,21 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, } //Convert the authorities set back to an array and apply it to the token. - token.setAuthorities((GrantedAuthority[]) authorities.toArray( - new GrantedAuthority[authorities.size()])); + UsernamePasswordAuthenticationToken result = new UsernamePasswordAuthenticationToken(request + .getPrincipal(), request.getCredentials(), + (GrantedAuthority[]) authorities.toArray( + new GrantedAuthority[authorities.size()])); //Publish the success event - publishSuccessEvent(token); + publishSuccessEvent(result); //we're done, return the token. - return token; + return result; } catch (LoginException loginException) { AcegiSecurityException ase = loginExceptionResolver .resolveException(loginException); - publishFailureEvent(token, ase); + publishFailureEvent(request, ase); throw ase; } } diff --git a/core/src/main/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationToken.java b/core/src/main/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationToken.java index 08219f9d0e..1618d8748c 100644 --- a/core/src/main/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationToken.java +++ b/core/src/main/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationToken.java @@ -42,6 +42,7 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken private Object principal; private GrantedAuthority[] authorities; private int keyHash; + private boolean authenticated; //~ Constructors =========================================================== @@ -72,6 +73,7 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken this.keyHash = key.hashCode(); this.principal = principal; this.authorities = authorities; + this.authenticated = true; } protected RememberMeAuthenticationToken() { @@ -80,22 +82,12 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken //~ Methods ================================================================ - /** - * Ignored (always true). - * - * @param isAuthenticated ignored - */ public void setAuthenticated(boolean isAuthenticated) { - // ignored + this.authenticated = isAuthenticated; } - /** - * Always returns true. - * - * @return true - */ public boolean isAuthenticated() { - return true; + return this.authenticated; } public GrantedAuthority[] getAuthorities() { diff --git a/core/src/main/java/org/acegisecurity/runas/RunAsUserToken.java b/core/src/main/java/org/acegisecurity/runas/RunAsUserToken.java index e68f37848a..ed6c256596 100644 --- a/core/src/main/java/org/acegisecurity/runas/RunAsUserToken.java +++ b/core/src/main/java/org/acegisecurity/runas/RunAsUserToken.java @@ -34,6 +34,7 @@ public class RunAsUserToken extends AbstractAuthenticationToken { private Object principal; private GrantedAuthority[] authorities; private int keyHash; + private boolean authenticated; //~ Constructors =========================================================== @@ -45,6 +46,7 @@ public class RunAsUserToken extends AbstractAuthenticationToken { this.principal = principal; this.credentials = credentials; this.originalAuthentication = originalAuthentication; + this.authenticated = true; } protected RunAsUserToken() { @@ -53,22 +55,12 @@ public class RunAsUserToken extends AbstractAuthenticationToken { //~ Methods ================================================================ - /** - * Setting is ignored. Always considered authenticated. - * - * @param ignored DOCUMENT ME! - */ - public void setAuthenticated(boolean ignored) { - // ignored + public void setAuthenticated(boolean isAuthenticated) { + this.authenticated = isAuthenticated; } - /** - * Always returns true. - * - * @return DOCUMENT ME! - */ public boolean isAuthenticated() { - return true; + return this.authenticated; } public GrantedAuthority[] getAuthorities() { diff --git a/core/src/test/java/org/acegisecurity/intercept/method/aopalliance/MethodSecurityInterceptorTests.java b/core/src/test/java/org/acegisecurity/intercept/method/aopalliance/MethodSecurityInterceptorTests.java index b6d1b0f25f..7f4a604ad4 100644 --- a/core/src/test/java/org/acegisecurity/intercept/method/aopalliance/MethodSecurityInterceptorTests.java +++ b/core/src/test/java/org/acegisecurity/intercept/method/aopalliance/MethodSecurityInterceptorTests.java @@ -83,14 +83,14 @@ public class MethodSecurityInterceptorTests extends TestCase { SecurityContextHolder.getContext().setAuthentication(null); } - public void testCallingAPublicMethodWhenPresentingAnAuthenticationObjectWillProperlySetItsIsAuthenticatedProperty() + public void testCallingAPublicMethodWhenPresentingAnAuthenticationObjectWillNotChangeItsIsAuthenticatedProperty() throws Exception { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", - "Password", - new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_THIS_IS_NOT_REQUIRED_AS_IT_IS_PUBLIC")}); + "Password"); assertTrue(!token.isAuthenticated()); SecurityContextHolder.getContext().setAuthentication(token); + // The associated MockAuthenticationManager WILL accept the above UsernamePasswordAuthenticationToken ITargetObject target = makeInterceptedTarget(); String result = target.publicMakeLowerCase("HELLO"); assertEquals("hello net.sf.acegisecurity.providers.UsernamePasswordAuthenticationToken false", @@ -158,13 +158,13 @@ public class MethodSecurityInterceptorTests extends TestCase { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_LOWER")}); - assertTrue(!token.isAuthenticated()); + assertTrue(token.isAuthenticated()); SecurityContextHolder.getContext().setAuthentication(token); ITargetObject target = makeInterceptedTargetWithoutAnAfterInvocationManager(); String result = target.makeLowerCase("HELLO"); - // Note we check the isAuthenticated becomes true in following line + // Note we check the isAuthenticated remained true in following line assertEquals("hello net.sf.acegisecurity.providers.UsernamePasswordAuthenticationToken true", result); @@ -203,11 +203,11 @@ public class MethodSecurityInterceptorTests extends TestCase { public void testRejectsCallsWhenAuthenticationIsIncorrect() throws Exception { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", - "Password", - new GrantedAuthority[] {new GrantedAuthorityImpl("MOCK_LOWER")}); + "Password"); assertTrue(!token.isAuthenticated()); SecurityContextHolder.getContext().setAuthentication(token); + // NB: The associated MockAuthenticationManager WILL reject the above UsernamePasswordAuthenticationToken ITargetObject target = makeInterceptedTargetRejectsAuthentication(); try { diff --git a/core/src/test/java/org/acegisecurity/providers/UsernamePasswordAuthenticationTokenTests.java b/core/src/test/java/org/acegisecurity/providers/UsernamePasswordAuthenticationTokenTests.java index 4f0ba485cd..5f15296de0 100644 --- a/core/src/test/java/org/acegisecurity/providers/UsernamePasswordAuthenticationTokenTests.java +++ b/core/src/test/java/org/acegisecurity/providers/UsernamePasswordAuthenticationTokenTests.java @@ -1,4 +1,4 @@ -/* Copyright 2004 Acegi Technology Pty Limited +/* Copyright 2004, 2005 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,9 +51,30 @@ public class UsernamePasswordAuthenticationTokenTests extends TestCase { public void testAuthenticated() { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password", null); - assertTrue(!token.isAuthenticated()); - token.setAuthenticated(true); + + // check default given we passed some GrantedAuthorty[]s (well, we passed null) assertTrue(token.isAuthenticated()); + + // check explicit set to untrusted (we can safely go from trusted to untrusted, but not the reverse) + token.setAuthenticated(false); + assertTrue(!token.isAuthenticated()); + + // Now let's create a UsernamePasswordAuthenticationToken without any GrantedAuthorty[]s (different constructor) + token = new UsernamePasswordAuthenticationToken("Test", "Password"); + + assertTrue(!token.isAuthenticated()); + + // check we're allowed to still set it to untrusted + token.setAuthenticated(false); + assertTrue(!token.isAuthenticated()); + + // check denied changing it to trusted + try { + token.setAuthenticated(true); + fail("Should have prohibited setAuthenticated(true)"); + } catch (IllegalArgumentException expected) { + assertTrue(true); + } } public void testGetters() { @@ -67,19 +88,6 @@ public class UsernamePasswordAuthenticationTokenTests extends TestCase { assertEquals("ROLE_TWO", token.getAuthorities()[1].getAuthority()); } - public void testNewAuthorities() { - UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", - "Password", null); - assertEquals("Test", token.getPrincipal()); - assertEquals("Password", token.getCredentials()); - assertEquals(null, token.getAuthorities()); - - token.setAuthorities(new GrantedAuthority[] {new GrantedAuthorityImpl( - "ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")}); - assertEquals("ROLE_ONE", token.getAuthorities()[0].getAuthority()); - assertEquals("ROLE_TWO", token.getAuthorities()[1].getAuthority()); - } - public void testNoArgConstructor() { try { new UsernamePasswordAuthenticationToken(); diff --git a/core/src/test/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationTokenTests.java b/core/src/test/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationTokenTests.java index a631756173..d76f91290c 100644 --- a/core/src/test/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationTokenTests.java +++ b/core/src/test/java/org/acegisecurity/providers/anonymous/AnonymousAuthenticationTokenTests.java @@ -159,7 +159,6 @@ public class AnonymousAuthenticationTokenTests extends TestCase { "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}); - token2.setAuthenticated(true); assertFalse(token1.equals(token2)); } @@ -184,7 +183,7 @@ public class AnonymousAuthenticationTokenTests extends TestCase { new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}); assertTrue(token.isAuthenticated()); - token.setAuthenticated(false); // ignored - assertTrue(token.isAuthenticated()); + token.setAuthenticated(false); + assertTrue(!token.isAuthenticated()); } } diff --git a/core/src/test/java/org/acegisecurity/providers/cas/CasAuthenticationTokenTests.java b/core/src/test/java/org/acegisecurity/providers/cas/CasAuthenticationTokenTests.java index 09e3dccd05..9cb9d231ca 100644 --- a/core/src/test/java/org/acegisecurity/providers/cas/CasAuthenticationTokenTests.java +++ b/core/src/test/java/org/acegisecurity/providers/cas/CasAuthenticationTokenTests.java @@ -223,7 +223,6 @@ public class CasAuthenticationTokenTests extends TestCase { "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}); - token2.setAuthenticated(true); assertTrue(!token1.equals(token2)); } @@ -295,15 +294,15 @@ public class CasAuthenticationTokenTests extends TestCase { assertTrue(!token1.equals(token2)); } - public void testSetAuthenticatedIgnored() { + public void testSetAuthenticated() { CasAuthenticationToken token = new CasAuthenticationToken("key", "Test", "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}, makeUserDetails(), new Vector(), "PGTIOU-0-R0zlgrl4pdAQwBvJWO3vnNpevwqStbSGcq3vKB2SqSFFRnjPHt"); assertTrue(token.isAuthenticated()); - token.setAuthenticated(false); // ignored - assertTrue(token.isAuthenticated()); + token.setAuthenticated(false); + assertTrue(!token.isAuthenticated()); } public void testToString() { diff --git a/core/src/test/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationTokenTests.java b/core/src/test/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationTokenTests.java index 1544d9ae3e..845737f664 100644 --- a/core/src/test/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationTokenTests.java +++ b/core/src/test/java/org/acegisecurity/providers/rememberme/RememberMeAuthenticationTokenTests.java @@ -159,7 +159,6 @@ public class RememberMeAuthenticationTokenTests extends TestCase { "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}); - token2.setAuthenticated(true); assertFalse(token1.equals(token2)); } @@ -184,7 +183,7 @@ public class RememberMeAuthenticationTokenTests extends TestCase { new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}); assertTrue(token.isAuthenticated()); - token.setAuthenticated(false); // ignored - assertTrue(token.isAuthenticated()); + token.setAuthenticated(false); + assertTrue(!token.isAuthenticated()); } } diff --git a/core/src/test/java/org/acegisecurity/runas/RunAsUserTokenTests.java b/core/src/test/java/org/acegisecurity/runas/RunAsUserTokenTests.java index 4637680473..7d31edc48c 100644 --- a/core/src/test/java/org/acegisecurity/runas/RunAsUserTokenTests.java +++ b/core/src/test/java/org/acegisecurity/runas/RunAsUserTokenTests.java @@ -1,4 +1,4 @@ -/* Copyright 2004 Acegi Technology Pty Limited +/* Copyright 2004, 2005 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -49,14 +49,14 @@ public class RunAsUserTokenTests extends TestCase { junit.textui.TestRunner.run(RunAsUserTokenTests.class); } - public void testAuthenticationSettingAlwaysReturnsTrue() { + public void testAuthenticationSetting() { RunAsUserToken token = new RunAsUserToken("my_password", "Test", "Password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl( "ROLE_TWO")}, UsernamePasswordAuthenticationToken.class); assertTrue(token.isAuthenticated()); token.setAuthenticated(false); - assertTrue(token.isAuthenticated()); + assertTrue(!token.isAuthenticated()); } public void testGetters() { diff --git a/doc/xdocs/changes.xml b/doc/xdocs/changes.xml index 09e11f5438..9b08df8a6e 100644 --- a/doc/xdocs/changes.xml +++ b/doc/xdocs/changes.xml @@ -26,17 +26,18 @@ - JdbcDaoImpl modified to support synthetic primary keys - Greatly improve BasicAclEntryAfterInvocationCollectionFilteringProvider performance with large collections (if the principal has access to relatively few collection elements) - Reorder DaoAuthenticationProvider exception logic as per developer list discussion - ContextHolder refactored and replaced by SecurityContextHolder - Made AclEntry Serializable (correct issue with BasicAclEntryCache) - Changed order of credentials verification and expiry checking in DaoAuthenticationProvider. Password must now be successfully verified before expired credentials are reported. + JdbcDaoImpl modified to support synthetic primary keys + Greatly improve BasicAclEntryAfterInvocationCollectionFilteringProvider performance with large collections (if the principal has access to relatively few collection elements) + Reorder DaoAuthenticationProvider exception logic as per developer list discussion + ContextHolder refactored and replaced by SecurityContextHolder + Made AclEntry Serializable (correct issue with BasicAclEntryCache) + Changed order of credentials verification and expiry checking in DaoAuthenticationProvider. Password must now be successfully verified before expired credentials are reported. AnonymousProcessingFilter offers protected method to control when it should execute AbstractAuthenticationToken.getName() now returns username alone if UserDetails present - AuthorityGranter.grant now returns a java.util.Set of role names, instead of a single role name - JavaDoc improvements - Correct synchronization issue with FilterToBeanProxy initialization + AuthorityGranter.grant now returns a java.util.Set of role names, instead of a single role name + JavaDoc improvements + Correct synchronization issue with FilterToBeanProxy initialization (as per developer list discussion) + Refactor Authentication.isAuthenticated() handling to be more performance (as per developer list discussion) Correct location of AuthenticationSimpleHttpInvokerRequestExecutor in clientContext.xml