geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Mulder <ammul...@alumni.princeton.edu>
Subject Re: svn commit: r123495 - /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
Date Tue, 28 Dec 2004 18:11:23 GMT
	Well, I think an == would do the trick.  We could also use some
kind of temporary Subject manipulations, but I don't think a major change
is necessary.

	I'll grant you we're kind of "enhancing" JAAS.  I'm not 100% sure 
how useful this will be in practice, but we agreed on the features we 
want, and I'm not sure we should take them off the table just because JAAS 
doesn't support them natively.

Aaron

On Tue, 28 Dec 2004, David Jencks wrote:
> 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 adc@apache.org 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 {
> >>
> >
> 
> 

Mime
View raw message