You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by lh...@apache.org on 2012/07/22 02:56:28 UTC
svn commit: r1364198 - in /shiro/branches/1.2.x/core/src:
main/java/org/apache/shiro/subject/support/DelegatingSubject.java
test/java/org/apache/shiro/subject/DelegatingSubjectTest.java
Author: lhazlewood
Date: Sun Jul 22 00:56:28 2012
New Revision: 1364198
URL: http://svn.apache.org/viewvc?rev=1364198&view=rev
Log:
SHIRO-344: fixed runAs bug found for 1.2, with integration test update.
Modified:
shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java
shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java
Modified: shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java
URL: http://svn.apache.org/viewvc/shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java?rev=1364198&r1=1364197&r2=1364198&view=diff
==============================================================================
--- shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java (original)
+++ shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/subject/support/DelegatingSubject.java Sun Jul 22 00:56:28 2012
@@ -39,10 +39,10 @@ import org.apache.shiro.util.StringUtils
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
/**
* Implementation of the {@code Subject} interface that delegates
@@ -60,7 +60,7 @@ import java.util.concurrent.Callable;
* data is desired (to eliminate EIS round trips and therefore improve database performance), it is considered
* much more elegant to let the underlying {@code SecurityManager} implementation or its delegate components
* manage caching, not this class. A {@code SecurityManager} is considered a business-tier component,
- * where caching strategies are better suited.
+ * where caching strategies are better managed.
* <p/>
* Applications from large and clustered to simple and JVM-local all benefit from
* stateless architectures. This implementation plays a part in the stateless programming
@@ -83,7 +83,6 @@ public class DelegatingSubject implement
* @since 1.2
*/
protected boolean sessionCreationEnabled;
- private List<PrincipalCollection> runAsPrincipals; //supports assumed identities (aka 'run as')
protected transient SecurityManager securityManager;
@@ -108,7 +107,6 @@ public class DelegatingSubject implement
this.host = host;
if (session != null) {
this.session = decorate(session);
- this.runAsPrincipals = getRunAsPrincipals(this.session);
}
this.sessionCreationEnabled = sessionCreationEnabled;
}
@@ -152,7 +150,8 @@ public class DelegatingSubject implement
}
public PrincipalCollection getPrincipals() {
- return CollectionUtils.isEmpty(this.runAsPrincipals) ? this.principals : this.runAsPrincipals.get(0);
+ List<PrincipalCollection> runAsPrincipals = getRunAsPrincipalsStack();
+ return CollectionUtils.isEmpty(runAsPrincipals) ? this.principals : runAsPrincipals.get(0);
}
public boolean isPermitted(String permission) {
@@ -253,7 +252,7 @@ public class DelegatingSubject implement
}
public void login(AuthenticationToken token) throws AuthenticationException {
- clearRunAsIdentities();
+ clearRunAsIdentitiesInternal();
Subject subject = securityManager.login(this, token);
PrincipalCollection principals;
@@ -285,7 +284,6 @@ public class DelegatingSubject implement
Session session = subject.getSession(false);
if (session != null) {
this.session = decorate(session);
- this.runAsPrincipals = getRunAsPrincipals(this.session);
} else {
this.session = null;
}
@@ -316,7 +314,9 @@ public class DelegatingSubject implement
public Session getSession(boolean create) {
if (log.isTraceEnabled()) {
- log.trace("attempting to get session; create = " + create + "; session is null = " + (this.session == null) + "; session has id = " + (this.session != null && session.getId() != null));
+ log.trace("attempting to get session; create = " + create +
+ "; session is null = " + (this.session == null) +
+ "; session has id = " + (this.session != null && session.getId() != null));
}
if (this.session == null && create) {
@@ -347,21 +347,24 @@ public class DelegatingSubject implement
return sessionContext;
}
+ private void clearRunAsIdentitiesInternal() {
+ //try/catch added for SHIRO-298
+ try {
+ clearRunAsIdentities();
+ } catch (SessionException se) {
+ log.debug("Encountered session exception trying to clear 'runAs' identities during logout. This " +
+ "can generally safely be ignored.", se);
+ }
+ }
+
public void logout() {
try {
- //try/catch added for SHIRO-298
- try {
- clearRunAsIdentities();
- } catch (SessionException se) {
- log.debug("Encountered session exception trying to clear 'runAs' identities during logout. This " +
- "can generally safely be ignored.", se);
- }
+ clearRunAsIdentitiesInternal();
this.securityManager.logout(this);
} finally {
this.session = null;
this.principals = null;
this.authenticated = false;
- this.runAsPrincipals = null;
//Don't set securityManager to null here - the Subject can still be
//used, it is just considered anonymous at this point. The SecurityManager instance is
//necessary if the subject would log in again or acquire a new session. This is in response to
@@ -435,19 +438,33 @@ public class DelegatingSubject implement
}
public boolean isRunAs() {
- return !CollectionUtils.isEmpty(this.runAsPrincipals);
+ List<PrincipalCollection> stack = getRunAsPrincipalsStack();
+ return !CollectionUtils.isEmpty(stack);
}
public PrincipalCollection getPreviousPrincipals() {
- return isRunAs() ? this.principals : null;
+ PrincipalCollection previousPrincipals = null;
+ List<PrincipalCollection> stack = getRunAsPrincipalsStack();
+ int stackSize = stack != null ? stack.size() : 0;
+ if (stackSize > 0) {
+ if (stackSize == 1) {
+ previousPrincipals = this.principals;
+ } else {
+ //always get the one behind the current:
+ assert stack != null;
+ previousPrincipals = stack.get(1);
+ }
+ }
+ return previousPrincipals;
}
public PrincipalCollection releaseRunAs() {
return popIdentity();
}
- @SuppressWarnings({"unchecked"})
- private List<PrincipalCollection> getRunAsPrincipals(Session session) {
+ @SuppressWarnings("unchecked")
+ private List<PrincipalCollection> getRunAsPrincipalsStack() {
+ Session session = getSession(false);
if (session != null) {
return (List<PrincipalCollection>) session.getAttribute(RUN_AS_PRINCIPALS_SESSION_KEY);
}
@@ -455,8 +472,6 @@ public class DelegatingSubject implement
}
private void clearRunAsIdentities() {
- //setting to null must occur before interacting with the session in case it throws an exception (SHIRO-298)
- this.runAsPrincipals = null;
Session session = getSession(false);
if (session != null) {
session.removeAttribute(RUN_AS_PRINCIPALS_SESSION_KEY);
@@ -468,29 +483,29 @@ public class DelegatingSubject implement
String msg = "Specified Subject principals cannot be null or empty for 'run as' functionality.";
throw new NullPointerException(msg);
}
- if (this.runAsPrincipals == null) {
- this.runAsPrincipals = new ArrayList<PrincipalCollection>();
+ List<PrincipalCollection> stack = getRunAsPrincipalsStack();
+ if (stack == null) {
+ stack = new CopyOnWriteArrayList<PrincipalCollection>();
}
- this.runAsPrincipals.add(0, principals);
+ stack.add(0, principals);
Session session = getSession();
- session.setAttribute(RUN_AS_PRINCIPALS_SESSION_KEY, this.runAsPrincipals);
+ session.setAttribute(RUN_AS_PRINCIPALS_SESSION_KEY, stack);
}
private PrincipalCollection popIdentity() {
PrincipalCollection popped = null;
- if (!CollectionUtils.isEmpty(this.runAsPrincipals)) {
- popped = this.runAsPrincipals.remove(0);
+
+ List<PrincipalCollection> stack = getRunAsPrincipalsStack();
+ if (!CollectionUtils.isEmpty(stack)) {
+ popped = stack.remove(0);
Session session;
- if (!CollectionUtils.isEmpty(this.runAsPrincipals)) {
- //persist the changed deque to the session
+ if (!CollectionUtils.isEmpty(stack)) {
+ //persist the changed stack to the session
session = getSession();
- session.setAttribute(RUN_AS_PRINCIPALS_SESSION_KEY, this.runAsPrincipals);
+ session.setAttribute(RUN_AS_PRINCIPALS_SESSION_KEY, stack);
} else {
- //deque is empty, remove it from the session:
- session = getSession(false);
- if (session != null) {
- session.removeAttribute(RUN_AS_PRINCIPALS_SESSION_KEY);
- }
+ //stack is empty, remove it from the session:
+ clearRunAsIdentities();
}
}
Modified: shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java
URL: http://svn.apache.org/viewvc/shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java?rev=1364198&r1=1364197&r2=1364198&view=diff
==============================================================================
--- shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java (original)
+++ shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java Sun Jul 22 00:56:28 2012
@@ -140,8 +140,8 @@ public class DelegatingSubjectTest {
}
/**
- * TODO: This test fails on JDK 1.5 from the command line but not from within the IDE
- * and I have no idea why at the moment - Les - 29 Oct 2010.
+ * This test fails on JDK 1.5 from the command line but not from within the IDE
+ * and I have no idea why at the moment - Les - 29 Oct 2010. Works fine on JDK 1.6 (JDK 1.6 is required to build).
*/
@Test
public void testRunAs() {
@@ -150,34 +150,75 @@ public class DelegatingSubjectTest {
Ini.Section users = ini.addSection("users");
users.put("user1", "user1,role1");
users.put("user2", "user2,role2");
+ users.put("user3", "user3,role3");
IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
SecurityManager sm = factory.getInstance();
+ //login as user1
Subject subject = new Subject.Builder(sm).buildSubject();
subject.login(new UsernamePasswordToken("user1", "user1"));
- assertTrue(subject.getPrincipal().equals("user1"));
- assertTrue(subject.hasRole("role1"));
assertFalse(subject.isRunAs());
- assertNull(subject.getPreviousPrincipals());
+ assertEquals("user1", subject.getPrincipal());
+ assertTrue(subject.hasRole("role1"));
+ assertFalse(subject.hasRole("role2"));
+ assertFalse(subject.hasRole("role3"));
+ assertNull(subject.getPreviousPrincipals()); //no previous principals since we haven't called runAs yet
+ //runAs user2:
subject.runAs(new SimplePrincipalCollection("user2", IniSecurityManagerFactory.INI_REALM_NAME));
+ assertTrue(subject.isRunAs());
+ assertEquals("user2", subject.getPrincipal());
+ assertTrue(subject.hasRole("role2"));
+ assertFalse(subject.hasRole("role1"));
+ assertFalse(subject.hasRole("role3"));
- assertFalse(subject.getPrincipal().equals("user1"));
+ //assert we still have the previous (user1) principals:
+ PrincipalCollection previous = subject.getPreviousPrincipals();
+ assertFalse(CollectionUtils.isEmpty(previous));
+ assertTrue(previous.getPrimaryPrincipal().equals("user1"));
+
+ //test the stack functionality: While as user2, run as user3:
+ subject.runAs(new SimplePrincipalCollection("user3", IniSecurityManagerFactory.INI_REALM_NAME));
+ assertTrue(subject.isRunAs());
+ assertEquals("user3", subject.getPrincipal());
+ assertTrue(subject.hasRole("role3"));
assertFalse(subject.hasRole("role1"));
- assertTrue(subject.getPrincipal().equals("user2"));
- assertTrue(subject.hasRole("role2"));
+ assertFalse(subject.hasRole("role2"));
+
+ //assert we still have the previous (user2) principals in the stack:
+ previous = subject.getPreviousPrincipals();
+ assertFalse(CollectionUtils.isEmpty(previous));
+ assertTrue(previous.getPrimaryPrincipal().equals("user2"));
+
+ //drop down to user2:
+ subject.releaseRunAs();
+
+ //assert still run as:
assertTrue(subject.isRunAs());
- assertFalse(CollectionUtils.isEmpty(subject.getPreviousPrincipals()));
- assertTrue(subject.getPreviousPrincipals().getPrimaryPrincipal().equals("user1"));
+ assertEquals("user2", subject.getPrincipal());
+ assertTrue(subject.hasRole("role2"));
+ assertFalse(subject.hasRole("role1"));
+ assertFalse(subject.hasRole("role3"));
+
+ //assert we still have the previous (user1) principals:
+ previous = subject.getPreviousPrincipals();
+ assertFalse(CollectionUtils.isEmpty(previous));
+ assertTrue(previous.getPrimaryPrincipal().equals("user1"));
+ //drop down to original user1:
subject.releaseRunAs();
- assertTrue(subject.getPrincipal().equals("user1"));
- assertTrue(subject.hasRole("role1"));
+
+ //assert we're no longer runAs:
assertFalse(subject.isRunAs());
- assertNull(subject.getPreviousPrincipals());
+ assertEquals("user1", subject.getPrincipal());
+ assertTrue(subject.hasRole("role1"));
+ assertFalse(subject.hasRole("role2"));
+ assertFalse(subject.hasRole("role3"));
+ assertNull(subject.getPreviousPrincipals()); //no previous principals in orig state
subject.logout();
+
LifecycleUtils.destroy(sm);
}
}