As Aaron points out this fix doesn't work, see the updated test.
IMNSHO fixing this problem will require major changes in the login module subject sharing that
1. may be inconsistent with the expected JAAS semantics
2. may be more trouble than it's worth.
With a single set of principals, you cannot detect multiple login modules adding the same principal to the set. The only solutions will involve:
1. giving each login module a separate subject
or
2. removing the unwrapped principals from the single shared subject as we wrap them.
or
3. stop trying to identify which login module added a principal.
To me it is starting to seem like we are saying, "JAAS isn't good enough for us, we need to extend it" but our extension isn't working and seems to me is working against the philosophy behind JAAS. What are we actually gaining from this login domain tracking?
thanks david jencks
On Dec 28, 2004, at 6:55 AM, Aaron Mulder wrote:
I'm not sure I agree with this fix. The comparison now checks if
the RealmPrincipal is in the list of processed principals before
processing it, but realm principals are never added to the processed
principal list (only original principals), so this will add everything it
sees, yeah?
It might be better to maintain a list and manually check if the
new principal == each one in the old list rather than relying on .equals,
though I guess checking the realm principals should work too.
Aaron
On Tue, 28 Dec 2004 [EMAIL PROTECTED] wrote:Author: adc Date: Tue Dec 28 03:37:31 2004 New Revision: 123495
URL: http://svn.apache.org/viewcvs?view=rev&rev=123495
Log:
Fixed problem with missing realm principals.
Modified:
geronimo/trunk/modules/security/src/java/org/apache/geronimo/ security/jaas/JaasSecurityContext.java
geronimo/trunk/modules/security/src/test/org/apache/geronimo/ security/jaas/MultipleLoginDomainTest.java
Modified: geronimo/trunk/modules/security/src/java/org/apache/geronimo/ security/jaas/JaasSecurityContext.java
Url: http://svn.apache.org/viewcvs/geronimo/trunk/modules/security/src/ java/org/apache/geronimo/security/jaas/JaasSecurityContext.java? view=diff&rev=123495&p1=geronimo/trunk/modules/security/src/java/org/ apache/geronimo/security/jaas/ JaasSecurityContext.java&r1=123494&p2=geronimo/trunk/modules/ security/src/java/org/apache/geronimo/security/jaas/ JaasSecurityContext.java&r2=123495
====================================================================== ========
--- geronimo/trunk/modules/security/src/java/org/apache/geronimo/ security/jaas/JaasSecurityContext.java (original)
+++ geronimo/trunk/modules/security/src/java/org/apache/geronimo/ security/jaas/JaasSecurityContext.java Tue Dec 28 03:37:31 2004
@@ -77,9 +77,12 @@
List list = new LinkedList();
for (Iterator it = subject.getPrincipals().iterator(); it.hasNext();) {
Principal p = (Principal) it.next();
- if(!(p instanceof RealmPrincipal) && !processedPrincipals.contains(p)) {
- list.add(ContextManager.registerPrincipal(new RealmPrincipal(loginDomainName, p, realmName)));
- processedPrincipals.add(p);
+ if(!(p instanceof RealmPrincipal)) {
+ RealmPrincipal rp = new RealmPrincipal(loginDomainName, p, realmName);
+ if (!processedPrincipals.contains(rp)) {
+ list.add(ContextManager.registerPrincipal(rp));
+ processedPrincipals.add(p);
+ }
}
}
subject.getPrincipals().addAll(list);
Modified: geronimo/trunk/modules/security/src/test/org/apache/geronimo/ security/jaas/MultipleLoginDomainTest.java
Url: http://svn.apache.org/viewcvs/geronimo/trunk/modules/security/src/ test/org/apache/geronimo/security/jaas/MultipleLoginDomainTest.java? view=diff&rev=123495&p1=geronimo/trunk/modules/security/src/test/org/ apache/geronimo/security/jaas/ MultipleLoginDomainTest.java&r1=123494&p2=geronimo/trunk/modules/ security/src/test/org/apache/geronimo/security/jaas/ MultipleLoginDomainTest.java&r2=123495
====================================================================== ========
--- geronimo/trunk/modules/security/src/test/org/apache/geronimo/ security/jaas/MultipleLoginDomainTest.java (original)
+++ geronimo/trunk/modules/security/src/test/org/apache/geronimo/ security/jaas/MultipleLoginDomainTest.java Tue Dec 28 03:37:31 2004
@@ -53,7 +53,7 @@
c.processPrincipals("D2");
//Uncomment the following line to verify that the subject will have only 2 principals rather than the desired 3 after both
//login modules have tried to add the same principal to the subject.
-// assertEquals(3, s.getPrincipals().size());
+ assertEquals(3, s.getPrincipals().size());
}
public static class MockLoginModule implements LoginModule {
