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.
Great, then we are perfectly in sync - that's what I expected and was reading from between the lines. > We're ultimately pushing the responsibility of managing the runtime > Subject instance based on the client technology, which I think is a > smart idea - the runtime environment could vary dramatically based on > client and it is best managed based on the deployment environment. > ThreadLocal binding is not always the best approach (for example, > maybe not necessary or even desirable in a standalone client > application). Attempting to manage state is always risky business. > In reviewing this code a month ago, I thought heavily about > introducing something like a SubjectAccessor interface that would be > used by SecurityUtils > (SecurityUtils.setSubjectAccessor/getSubjectAccessor) to allow > SecurityUtils to be configured with the lookup strategy appropriate > for the application. > Thoughts on this? An improvement we can make later. If you don't absolutely need it and I don't absolutely need it, the chances are that the community as well can get by without it for 1.0.0. Kalle > On Sun, Feb 14, 2010 at 12:55 AM, Kalle Korhonen > <[email protected]> wrote: >> As we are marching towards the 1.0.0 I decided assign a few more >> issues to myself, one of them being Regarding SHIRO-110: Remove >> org.apache.shiro.mgt.SubjectBinder and its usages. There really >> weren't too many places left so it's an easy removal. The only only >> area where I'm not completely sure what to do is >> SecurityManager.getSubject(). Supposedly an implementation could >> always create a subject but there's createSubject for that purpose so >> getSubject() might be redundant and somewhat misleading. >> SecurityManager.getSubject was added in 0.9. Should we deprecate >> SecurityManager.getSubject() or outright remove it? If we leave it >> alone or just deprecate, DefaultSecurityManager.getSubject() could >> call ThreadContext.getSubject() - same as SecurityUtils and as a >> fallback createSubject(new HashMap()). SecurityManager.getSubject() is >> only called from SecurityUtils within Shiro but SecurityManager is a >> fairly visible interface. >> >> Les, do you think we should getSubject() as is and provide the default >> implementation as suggested? If it's simpler to talk when you see the >> code, I could commit the suggested implementation and we could review >> afterwards. >> >> Kalle >> >
