shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan D. Cabrera" <>
Subject Re: Password hashing - getting the salt from AuthenticationInfo
Date Sat, 23 Oct 2010 19:40:16 GMT

On Oct 23, 2010, at 11:59 AM, Les Hazlewood wrote:

> Hi Alan and Kalle,
> Thanks for chiming in on this.
> But I'm not sure that inheritance is used too often.  Sure, it is used
> in some places where it shouldn't be and that there are definitely
> places where we can and should eliminate it.  For example, the
> CodecSupport class and the HashedCredentialsMatcher hierarchy can be
> deprecated now and then removed in a 2.0 release (to retain backwards
> compatibility).
> But if you look at pretty much all of Shiro's inheritance mechanisms
> (SecurityManager and its subclasses, Realm and its subclasses,
> SessionManager and its subclasses, etc), almost _all_ of those things
> are purely wrapper classes that delegate to internal pluggable
> components.  Almost all of it is composition.

I snipped most of this email because there are a number of good things discussed here.  Using
a single thread to discuss them all would be confusing and inefficient.   I'll post a reply
to them all in separate emails.  :)

I will add a few comments below to give a taste of my thinking.

> Anyway, I'm all for cleaning up things where we can, and making it
> even more pluggable - but the level of customization you can do today
> due to almost everything relying on delegation is *huge* compared to
> most frameworks.

Customization is fine. Delegation is fine.  But these capabilities don't require that the
implementation be based on abstract classes with complex and brittle interactions with their
implementations.  I might even go so far as to say that extensive use of delegation via inheritance
is an anti-pattern. 

Also, having all those protected methods out there makes the API very brittle.  We saw this
when we converted the code to start using StringBuilder. 

> About versions:  I think we should do as much of this type of cleanup
> as is possible for 1.2 where it is %100 backwards-compatible based on
> APR versioning guidelines.  We're a TLP project, and a lot more people
> are using our software today - I would like to ensure that upgrades
> don't cause people API grief (not including the very occasional change
> of internal code that is backwards compatible, but wasn't expected to
> be used by end-users).

Yep, hence my point in discussing "2.0".

> In fact, I think the HashedCredentialsMatcher changes can be done now,
> for 1.1.  I was in the process of making similar changes when working
> on the Salt support, but kept those changes in a separate change list
> and didn't commit them.  I can probably have that stuff in in an hour
> or two for your review.

Minor releases for backward compatible cleanups are fine.  

I think Kalle and I are talking about a new architecture based on what we've learned so far.
 I'm sure 2.0 is a long way off but it doesn't hurt to chat about it.


View raw message