2016-02-11 12:59 GMT+01:00 Mark Thomas <ma...@apache.org>: > JASPIC requires that an authentication module can wrap the > HttpServletRequest and/or HttpServletResponse passed to it and that the > wrapped instances are passed to the application. This creates a problem > since the authenticators are implemented as Valves which pass Tomcat's > o.a.c.connector.[Request|Response] objects. Tomcat's objects implement > the Servlet spec interfaces so, as long as no wrapping occurs, all is fine. >
Tomcat could simply use the already wrapped request/response later when invoking the filter chain. It would be set on the MessageInfo, right ? In that case, JASPIC should get Request.getRequest() and Response.getResponse() as its request/response objects and the wrapped request/response can go into two new fields in Request/Response. Would that work ? Honestly, whatever JASPIC would do with its wrapping is likely irrelevant for the rest of the Cataline pipeline, so as long as the application sees it it should be fine. There is a long standing enhancement request [1] for being able to use > wrapping with Valves. Rémy is -1 on implementing it. > > I'd still like to support JASPIC but I do understand where Rémy is > coming from w.r.t. the risks of wrapping. With all that in mind I have > been thinking through different implementation options and I have > reached the point where I think it makes sense to set out the options as > I see them for discussion. > > 1. Give up on JASPIC. > Pros: All the design / integration issues go away > No performance risk > Cons: We lose the opportunity for a simple OAuth / SAML solution > > 2. Don't support wrapping with JASPIC. > Pros: All the design / integration issues go away > Cons: Not specification compliant > We don't know how much implementations rely on this > > 3. Switch the Valve interface to use HttpServletRequest and > HttpServletResponse. > Pros: Enables the use of the associated Wrapper classes > Cons: Valves need access to the internals. ValveBase could > provide utility methods for accessing the wrapped > o.a.c.connector.[Request|Response] implementations > Breaks existing custom Valves > > 4. Make o.a.c.connector.[Request|Response] interfaces and provide > separate concrete implementations. > Pros: No change to Valve interface > Cons: JASPIC would need additional code to bridge between these > interfaces and the HttpServlet.*Wrapper classes > > 5. Move JASPIC processing to the start of the FilterChain since it > uses HttpServlet[Request|Response] > Pros: No / minimal API changes for Tomcat > Cons: Would result in authentication happening in multiple places. > Would need to be very careful to ensure requests couldn't > bypass authentication, particularly during JASPIC provider > (de)registratiob > JASPIC would end up duplciating a lot of the authorization > code in AuthenticatorBase > > > Of all of these, I think 4 holds the most promise. The first step for > that option would be reducing the current public interface of > o.a.c.connector.[Request|Response] to the minimum that is required. From > a design point of view that is a good thing to do anyway so I plan to > work on that while these options are discussed. Even if we don't go for > option 4, the work would be entirely pointless. > > If we do follow option 4 then the Javadoc will need updating to make it > very clear what is supported and what isn't. Generally, I think the > message needs to be "If you wrap these you are messing with Tomcat's > internals. Tread very, very carefully. Correct operation of Tomcat > depends on the correct behaviour of these methods. Before providing an > alternative implementation of any method you should review the standard > implementation and how the method is used." > > Mark > > > [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014 > > On the other proposed solutions: 1) Well, we did so much with this junk, it would be a problem to give up. 2) It should be properly supported. 3) Meh. OTOH it's probably just some unwrapping. 4) I think 3 is better. 5) Servlet 3 did a lot to move some auth to the application layer, so if there's an auth step at this point, I don't think it is so horrible conceptually. So maybe I like 3 better if my comment at the beginning cannot work. Rémy
