On Wed, Jan 20, 2016 at 06:28:21PM -0600, Endi Sukma Dewata wrote:

On 12/3/2015 12:31 AM, Fraser Tweedale wrote:

...

The attached patch was written as part of work implementing GSS-API authentication. We actually might not end up using SSLAuthenticatorWithFallback to interpret the authentication data but I think this refactor is worthwhile on its own, so here's the patch.

Cheers, Fraser

Ideally the SSLAuthenticatorWithFallback for Tomcat 7 should not store the LoginConfig as an attribute because potentially it can be shared by multiple web applications. Since each PKI web application has a separate instance of the SSLAuthenticatorWithFallback this is probably not a problem. So the patch is ACKed as is.

But if you want, the code probably can be modified like this:

public boolean authenticate(..., config) { HashMap attributes = new HashMap(); attributes.put("loginConfig", config); return doAuthenticate(..., attributes); }

public boolean doSubAuthenticate(..., attributes) { LoginConfig config = attributes.get("loginConfig"); return auth.authenticate(..., loginConfig); }

public String goGetRealName(..., attributes) { LoginConfig config = attributes.get("loginConfig"); return loginConfig.getRealName(); }

For Tomcat 8 the attributes map can be null.

It might even be possible to merge the two implementations. Recent Tomcat 7 contains additional stuff that reduces the differences with Tomcat 8. But we will need to make sure the latest Tomcat 7 is available on all platforms that we support. This can be done later though.

Since loginConfig is immediately set with the passed LoginConfig on each call to authenticate(), we can use ThreadLocal[1] to prevent interference in a less intrusive way that is still type-safe. [1] http://docs.oracle.com/javase/7/docs/api/java/lang/ThreadLocal.html Updated patch attached. Thanks for reviewing, Fraser

On Thu, Jan 21, 2016 at 08:54:20AM -0600, Endi Sukma Dewata wrote:

On 1/21/2016 4:03 AM, Fraser Tweedale wrote:

...

Since loginConfig is immediately set with the passed LoginConfig on each call to authenticate(), we can use ThreadLocal[1] to prevent interference in a less intrusive way that is still type-safe.

[1] http://docs.oracle.com/javase/7/docs/api/java/lang/ThreadLocal.html

Updated patch attached.

Thanks for reviewing, Fraser

ACK.

Thanks; pushed to master 67ac39227e5db83c7a4a7ad72364f3dcd30db05e Fraser