Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

request.getUserPrincipal() does not return CustomPrincipal #290

Closed
Circkumflex opened this issue May 20, 2015 · 5 comments
Closed

request.getUserPrincipal() does not return CustomPrincipal #290

Circkumflex opened this issue May 20, 2015 · 5 comments

Comments

@Circkumflex
Copy link

I've an own javax.security.auth.message.module.ServerAuthModule implementation.
In validateRequest() I put an instance of custom Principal to the callbackhandler

CustomPrincipal myprincipal = ...;
callbackHandler.handle(new Callback[]
{ new CallerPrincipalCallback(clientSubject, myprincipal), new GroupPrincipalCallback(...) }
);

In the application request.getUserPrincipal() returns an instance of com.sun.enterprise.security.web.integration.WebPrincipal and NOT an instance of CustomPrincipal!
In an ejb the call of ejbContext.getCallerPrincipal() does return an instance of CustomPrincipal!

That is strange, request.getUserPrincipal() should return the principal which is set in the ServerAuthModule

Dependent issue: https://java.net/jira/browse/GLASSFISH-16587

@Circkumflex
Copy link
Author

Depends on this bug made that workaround:

private static Principal glassfishWorkAround(HttpServletRequest request) {
        Principal principal = null;
        try {
            Principal webPrincipal = request.getUserPrincipal();
            if (webPrincipal != null) {
                Class glassfishWrapper = Class.forName("com.sun.enterprise.security.web.integration.WebPrincipal");
                if (glassfishWrapper.isInstance(webPrincipal)) {
                    Field customPrincipal = glassfishWrapper.getDeclaredField("customPrincipal");
                    customPrincipal.setAccessible(true);
                    principal = (Principal) customPrincipal.get(webPrincipal);
                } else {
                    principal = webPrincipal;
                }
            }
        } catch (IllegalArgumentException | IllegalAccessException | NoSuchFieldException | SecurityException | ClassNotFoundException ex) {
            LOGGER.throwing("SecurityConstraint", "glassfishWorkAround", ex);
        }
        return principal;
}

public static Principal getPrincipal(HttpServletRequest request) {
        return glassfishWorkAround(request);
}

Hope it helps somebody! Anyway hope this bug will be resolved soon.

@smillidge
Copy link
Contributor

Could you provide an example of the call you are making in the web application to get the Principal. Also how you are configuring your custom JAAS provider for use in Payara/GlassFish

@arjantijms
Copy link
Contributor

@sultry

Thanks a lot for reporting this, I had this on my mind to test for a long time but for some reason it slipped off my radar.

@smillidge

I've added a reproducer for this here: https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/custom-principal

Oleg is indeed right, the test fails immediately on GlassFish 4.1. JBoss WildFly 8.2 passes.

There's nothing special to be done for the call. Just HttpServletRequest#getUserPrincipal() from a plain Servlet is enough to demonstrate it.

Also how you are configuring your custom JAAS provider for use in Payara/GlassFish

JASPIC doesn't need any custom JAAS provider. It can optionally delegate to it (if needed even in a semi-standard way via the Bridge Profile), but this it unnecessary for reproducing this bug. The simplest SAM which just handles the CallerPrincipalCallback with a custom Principal is enough.

The fix suggested by Oleg really isn't that bad. This can be done directly in GlassFish without reflection.

Just note it's not a complete fix, as that would entail storing the custom Principal in the Subject as well (such that e.g. a JACC provider can use it).

But for practical purposes just unwrapping just before returning from HttpServletRequest#getUserPrincipal() may get us there for 90%.

@smillidge
Copy link
Contributor

OK thanks. We'll look into it. Also getting the Principal into the subject as well.

@smillidge smillidge added this to the Payara Server 4.1.153 milestone May 24, 2015
smillidge added a commit that referenced this issue May 31, 2015
Fixes #290 by ensuring the Facade uses the custom principal if available
@arjantijms
Copy link
Contributor

@smillidge @Circkumflex

I unfortunately found a problem with the solution as committed here. In case a custom principal is used AND the JASPIC 1.1 "javax.servlet.http.registerSession" directive is used, the groups (and possibly any credentials) are not restored as they should be.

This happens because the protocol for "javax.servlet.http.registerSession" requires getting the Principal from the HttpServletRequest and then feeding it back to the handler in a SAM. Subsequently BaseContainerCallbackHandler#processCallerPrincipal checks if the type is WebPrincipal.

If this is not the case, no "reuse" will take place.

Now I'm the first to admit this "getting principal from request and feeding it back" is a little brittle, but that's how it works in JASPIC 1.1.

What I guess what needs to happen is that when in a SAM's validateRequest method the unwrapping should not take place. There are a few characteristics one can check for, e.g. if the principal has reference equality to org.apache.catalina.Session#getPrincipal, but perhaps the more robust way is setting a request attribute to indicate we're in a SAM:

//JSR 196 is enabled for this application
try {
     context.fireContainerEvent(ContainerEvent.BEFORE_AUTHENTICATION, null);
     ((HttpServletRequest)request.getRequest()).setAttribute("...", true);
     result = validate(request, response, config, authenticator, calledFromAuthenticate);
 } finally {
     ((HttpServletRequest)request.getRequest()).removeAttribute("...");
     context.fireContainerEvent(ContainerEvent.AFTER_AUTHENTICATION, null);
 }

Any other mechanism like the internal notes may work as well, as long as the getUserPrincipal() can internally check for it.

What do you think? I'm also going to ask Ron Monzillo if he has any opinion on this matter and let you know if he has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants