Just catching up after the long weekend off... 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.
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). 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? 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 >
