> On April 6, 2017, 11:07 p.m., Jared Stewart wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java > > Line 56 (original), 53 (patched) > > <https://reviews.apache.org/r/58238/diff/1/?file=1686145#file1686145line56> > > > > This doesn't relate to your changes per se, but I am curious why ENV > > needs to be ThreadLocal while securityService does not. Do you know how > > Spring manages the instances of this class? I.e. is there a singleton > > instance, or one instance per session, or something else? > > Jinmei Liao wrote: > I believe it's just a singleton instance in the web container. It's > defined in the servlet xml: > > <mvc:interceptors> > <bean > class="org.apache.geode.management.internal.web.controllers.support.LoginHandlerInterceptor"/> > </mvc:interceptors>
Hm, that seems possibly worrisome to me. Maybe you can clarify. When someone logs in, I believe Shrio sets the subject on the (ThreadLocal) ThreadContext. But since this is a thread in a web server, can't that thread later potentially server requests from a different user? What stops this later user from getting the subject set on the thread earlier? - Jared ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58238/#review171268 ----------------------------------------------------------- On April 6, 2017, 9:12 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58238/ > ----------------------------------------------------------- > > (Updated April 6, 2017, 9:12 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-2756: Do not put security-* properties in the env. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java > 9e051748f48b6d792724706535b120f1f4b0d1be > > geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java > 63d410f6335d9eabbb2547551e32dab8631c7871 > > geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java > 118e3853750d7ea3ef9af6ac092664a4dabb0108 > > > Diff: https://reviews.apache.org/r/58238/diff/1/ > > > Testing > ------- > > precheckin successful > > > Thanks, > > Jinmei Liao > >
