You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shiro.apache.org by Brian Demers <br...@gmail.com> on 2012/03/09 18:56:39 UTC

Making the ModularRealmAuthenticator not check all realms for FirstSuccessfulStrategy

The ModularRealmAuthenticator always loops through all the configure
realms.  This can get expense if you have multiple external realms
configured (JDBC, LDAP, etc), and you are using
the FirstSuccessfulStrategy.  In this case where you had both a JDBC and
LDAP realm configured you would want to drop out of the loop if the Subject
was authenticated in the JDBC realm.  This way you do not need to eat the
cost of the LDAP query (which the results would be ignored anyway)

To solve this I added a new method to the AuthenticationStrategy Interface,
this makes the change simple, but would break backwards compatibility with
any custom AuthenticationStrategy NOT extending from
AbstractAuthenticationStrategy.
https://github.com/apache/shiro/compare/trunk...first-successful-authc-strategy#L1R99

I cannot think of any (non hackie way) alternative to this for 1.3.  Anyone
have any thoughts?  Or any thoughts on including this in 1.3?

Re: Making the ModularRealmAuthenticator not check all realms for FirstSuccessfulStrategy

Posted by Brian Demers <br...@gmail.com>.
I'm with you on the API compatibility concern.  We could do something like
you suggest (ShortCircuitIterationException) or another proposed
suggestion? doc the heck out of it, remove it in 2.x, replacing it with an
API change.


On Fri, Mar 9, 2012 at 4:21 PM, Les Hazlewood <lh...@apache.org> wrote:

> Adding the new method for 1.3 is difficult:  while not backwards
> compatible, it is certainly better than changing the behavior of an
> existing API.  But if we want to guarantee APR versioning guidelines -
> which reinforces perceived stability (highly desirable for a security
> API/framework) - it should probably go into 2.x only.
>
> If we are to guarantee backwards compatibility, a new strategy should
> be implemented that throws an Exception from the afterAttempt method.
> That exception is inspected (e.g. ShortCircuitIterationException) to
> trigger whether or not to continue to further realms.
>
> Yes, this is hacky, yes, it's not ideal, but it does guarantee
> backwards compatibility while still retaining existing functionality.
>
> Because I favor stability in a security framework, my preference is to
> do the hacky exception approach and then clean this up permanently in
> 2.x.  But I could be in the minority here.


> What are peoples' thoughts on this?
>
> Les
>
> On Fri, Mar 9, 2012 at 9:56 AM, Brian Demers <br...@gmail.com>
> wrote:
> > The ModularRealmAuthenticator always loops through all the configure
> > realms.  This can get expense if you have multiple external realms
> > configured (JDBC, LDAP, etc), and you are using
> > the FirstSuccessfulStrategy.  In this case where you had both a JDBC and
> > LDAP realm configured you would want to drop out of the loop if the
> Subject
> > was authenticated in the JDBC realm.  This way you do not need to eat the
> > cost of the LDAP query (which the results would be ignored anyway)
> >
> > To solve this I added a new method to the AuthenticationStrategy
> Interface,
> > this makes the change simple, but would break backwards compatibility
> with
> > any custom AuthenticationStrategy NOT extending from
> > AbstractAuthenticationStrategy.
> >
> https://github.com/apache/shiro/compare/trunk...first-successful-authc-strategy#L1R99
> >
> > I cannot think of any (non hackie way) alternative to this for 1.3.
>  Anyone
> > have any thoughts?  Or any thoughts on including this in 1.3?
>

Re: Making the ModularRealmAuthenticator not check all realms for FirstSuccessfulStrategy

Posted by Les Hazlewood <lh...@apache.org>.
Adding the new method for 1.3 is difficult:  while not backwards
compatible, it is certainly better than changing the behavior of an
existing API.  But if we want to guarantee APR versioning guidelines -
which reinforces perceived stability (highly desirable for a security
API/framework) - it should probably go into 2.x only.

If we are to guarantee backwards compatibility, a new strategy should
be implemented that throws an Exception from the afterAttempt method.
That exception is inspected (e.g. ShortCircuitIterationException) to
trigger whether or not to continue to further realms.

Yes, this is hacky, yes, it's not ideal, but it does guarantee
backwards compatibility while still retaining existing functionality.

Because I favor stability in a security framework, my preference is to
do the hacky exception approach and then clean this up permanently in
2.x.  But I could be in the minority here.

What are peoples' thoughts on this?

Les

On Fri, Mar 9, 2012 at 9:56 AM, Brian Demers <br...@gmail.com> wrote:
> The ModularRealmAuthenticator always loops through all the configure
> realms.  This can get expense if you have multiple external realms
> configured (JDBC, LDAP, etc), and you are using
> the FirstSuccessfulStrategy.  In this case where you had both a JDBC and
> LDAP realm configured you would want to drop out of the loop if the Subject
> was authenticated in the JDBC realm.  This way you do not need to eat the
> cost of the LDAP query (which the results would be ignored anyway)
>
> To solve this I added a new method to the AuthenticationStrategy Interface,
> this makes the change simple, but would break backwards compatibility with
> any custom AuthenticationStrategy NOT extending from
> AbstractAuthenticationStrategy.
> https://github.com/apache/shiro/compare/trunk...first-successful-authc-strategy#L1R99
>
> I cannot think of any (non hackie way) alternative to this for 1.3.  Anyone
> have any thoughts?  Or any thoughts on including this in 1.3?