Re: Welcome to the new JSR-375 mailing list


Arjan Tijms
 

Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

 
So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.
 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)
 
I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.
 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

 
  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.


  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?
 
  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

public class HttpMessageContextWrapper implements HttpMessageContext {

    private final HttpMessageContext httpMessageContext;
 
    public HttpMessageContextWrapper(HttpMessageContext httpMessageContext) {
        this.httpMessageContext = httpMessageContext;
    }
    
    public HttpMessageContext getWrapped() {
        return httpMessageContext;
    }

    @Override
    public boolean isProtected() {
        return getWrapped().isProtected();
    }

It's modelled directly after the Servlet wrappers, e.g. HttpServletRequestWrapper:

public class HttpServletRequestWrapper extends ServletRequestWrapper implements HttpServletRequest {

    /** 
     * Constructs a request object wrapping the given request.
     * @throws java.lang.IllegalArgumentException if the request is null
     */
    public HttpServletRequestWrapper(HttpServletRequest request) {
        super(request);
    }
    
    private HttpServletRequest _getHttpServletRequest() {
        return (HttpServletRequest) super.getRequest();
    }

    /**
     * The default behavior of this method is to return getAuthType()
     * on the wrapped request object.
     */
    @Override
    public String getAuthType() {
        return this._getHttpServletRequest().getAuthType();
    }

 
  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Kind regards,
Arjan Tijms



 

Thanks,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Join javaee-security-spec@javaee.groups.io to automatically receive all group messages.