On Tue, Feb 16, 2010 at 7:55 PM, Kalle Korhonen
<[email protected]> wrote:
> On Tue, Feb 16, 2010 at 7:12 AM, Les Hazlewood <[email protected]> wrote:
>> Yes, in my opinion, the SecurityManager#getSubject() call is redundant
>> and should be removed and SecurityUtils should be updated to have the
>> fallback - it could probably just use the Subject.Builder() directly.
Integration tests save the day! Since I would have already broken it
had I gone ahead with the changes I had prepared. The issue is that
eventually the principals need to be bound to the session. Les, I
assume it was you who had added this TODO to
DefaultSecurityManager.login():
//TODO - is binding necessary anymore? Shouldn't the Builders
or Builder callers do this now?
Unfortunately it's still needed since in the builder we are still
building the subject so we cannot just move the
SessionSubjectBinder.bindToSession implementation to the builder. We
could move it to DefaultSecurityManager.bind() pretty much as is but
then we'd still need to pass the session around in several
DefaultSubjectFactory calls; getPrincipal() being the most critical
one (as we check the session for principals there). Soo... anybody
(Les?) have any better ideas for this or should I just go ahead with
what I suggested above and we can improve later? Overall, it still
feels to me that the internals for this could be simplified.
Kalle
