securityContext.isCallerInRole() incorrectly returning false after authenticating until next page reload / FISH-991

[rcuprak]

Payara Server 5.2020.2 (Full)

I have run into a problem where after logging into an application checking with securityContext.isCallerInRole("admin") incorrectly returns false whereas request.isUserInRole("admin") returns true. Refreshing the page results in securityContext.isCallerInRole("admin") then correctly returning true. Note, the EJB container is affected by this bug, if a Singleton bean requires the admin role, after logging in the principal (which was created) is "missing" the role until you refresh the page. I worked backwards to create a simpler test case with just the securityContext.

I have attached a sample application which demonstrates the problem along with a video. In creating the video, I think I might have stumbled upon another issue where if I hit back and then re-login, I get a 404.

The Maven project uses toolchain to define the Java 8 JVM. The username/password to log into the demo applications are "test/password". Application is deployed under "/security".

security-bug.tar.gz
(url)

[phillipross]

@rcuprak I haven't looked at your reproducer closely, but I saw the video shows testing against 127.0.01 loopback ip. I've run into situations where the browsers have other stuff cached from other environments that had been listening on localhost. It manifested as mysterious and inconsistent results that I thought were solteria bugs or something. But then I discovered it worked on brand new browser sessions or after i had recently cleared cookies and cache.

The way I get around this now is to use a dedicated browser profile for each environment that I'm testing. I do this with chrome and firefox, but I'm not sure offhand how to do this with safari. A way to do it without using browser profiles is to go in and clear out any cache, cookies, etc that might be associated with localhost or loopback ip addresses.

[rcuprak]

@phillipross I can reproduce it on Chrome and FireFox as well as in incognito mode as well. request.isUserInRole("admin") will report true after logging in whereas securityContext.isCallerInRole("admin") reports false. SecurityContext is a new feature of Jakarta EE 8 (JSR 375).

[phillipross]

I seem to recall incognito mode not quite working either when I was trying it... at least with chrome. The guest profile worked, but not incognito.

Soteria is Payara's JSR375 implementation... I myself haven't done any regression testing on Payara 5.2020.2 yet, but I'll try your reproducer on my 5.201 environments and see if I observe the same as you.

[phillipross]

I've tested v5.201 and v5.2020.2 and can confirm that I see the same behavior even on a clean browser.
I still haven't looked at the reproducer code in it's entirety... only the identity store to see what the credentials are for logging in. My tests are with Centos7, Payara 5.201/5.2020.2 running on zulu 11.0.7, and the code built with zulu 8u252.

The interesting thing for me about this is that I have very simple test case like this that seems to work fine up to 5.201 but haven't tested with 5.2020.2 yet. When I get more time I'll do some comparisons for my tests and this reproducer and see if I can isolate it further.

[AlanRoth]

Hi @rcuprak, are you able to reproduce this issue with the latest Payara Server Community Release 5.2020.6 with the latest Zulu JDK?

[AlanRoth]

Hi @rcuprak, do you have any updates on this issue?

[AlanRoth]

Hi @rcuprak, I will be closing this issue due to inactivity. Feel free to reopen. Thank you.

[rcuprak]

Hi @AlanRoth - the issue still exists on 5.20.7 running on Java 11 (Java(TM) SE Runtime Environment GraalVM EE 20.2.0 (build 11.0.8.0.2+1-LTS-jvmci-20.2-b03).

[AlanRoth]

Hi @rcuprak, thank you for getting back to me, I will reopen the issue and attempt to reproduce.

[AlanRoth]

Hi @rcuprak, I was able to reproduce both issues, however, I will accept the original issue first and if you wish to submit a new issue for the 404 then feel free. I have created internal issue FISH-991. Thank you.

[hzometa]

Same problem still with Payara 5.2021.4 deploying this project: https://github.com/fturizo/SecureWebApplication/

[hberton]

Same problem still with Payara 5.2021.4 deploying this project: https://github.com/fturizo/SecureWebApplication/

I Have the same problem with Payara 5.2021.5, let's wait for the next releases

[douglasmartim]

I also have similar problems, related to security, logins failing and lack of permissions, problems are intermittent, time appear time work correctly, I'm waiting for new versions.

[hberton]

Hi @rcuprak, I was able to reproduce both issues, however, I will accept the original issue first and if you wish to submit a new issue for the 404 then feel free. I have created internal issue FISH-991. Thank you.

Hello @AlanRoth , do you think you can help us find the defect for a fix?

[georgwolf]

This bug is still present in current Payara versions and myabe the following analysis can help in getting this fixed:

Issue is that the LoginToContinueInterceptor sets a wrapped request (HttpServletRequestDelegator) on the HttpMessageContext for container initialized logins (method processContainerInitiatedAuthentication) when the original URL is requested after login: https://github.com/eclipse-ee4j/soteria/blob/24a427c19f3600e1738946c2d46ec904842e0549/impl/src/main/java/org/glassfish/soteria/cdi/LoginToContinueInterceptor.java#L238

If this .withRequest(new HttpServletRequestDelegator(request, requestData)) is commented out, SecurityContext.isCallerInRole() works as expected. But of course this misses some properties of the original request (headers, cookies, method) then, so it's not a viable fix.

However this indicates that there must be some logic that relies on the actual request that is working differently when the request is wrapped.

After doing a deep dive with the debugger I think the root cause is in J2EEInstanceListener.handleBeforeEvent()

Payara/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java

Lines 171 to 221 in 97decf6

	HttpServletRequest hreq = (HttpServletRequest) request;
	HttpServletRequest base = hreq;
	Principal prin = hreq.getUserPrincipal();
	Principal basePrincipal = prin;
	boolean wrapped = false;
	while (prin != null) {
	if (base instanceof ServletRequestWrapper) {
	// unwarp any wrappers to find the base object
	ServletRequest sr = ((ServletRequestWrapper) base).getRequest();
	if (sr instanceof HttpServletRequest) {
	base = (HttpServletRequest) sr;
	wrapped = true;
	continue;
	}
	}
	if (wrapped) {
	basePrincipal = base.getUserPrincipal();
	}
	else if (base instanceof RequestFacade) {
	// try to avoid the getUnWrappedCoyoteRequest call
	// when we can identify see we have the texact class.
	if (base.getClass() != RequestFacade.class) {
	basePrincipal = ((RequestFacade) base).getUnwrappedCoyoteRequest().getUserPrincipal();
	}
	} else {
	basePrincipal = base.getUserPrincipal();
	}
	break;
	}
	if (prin != null && prin == basePrincipal && prin.getClass().getName().equals(SecurityConstants.WEB_PRINCIPAL_CLASS)) {
	securityContext.setSecurityContextWithPrincipal(prin);
	} else if (prin != basePrincipal) {
	// the wrapper has overridden getUserPrincipal
	// reject the request if the wrapper does not have
	// the necessary permission.
	checkObjectForDoAsPermission(hreq);
	securityContext.setSecurityContextWithPrincipal(prin);
	}

together with this change for #290

Payara/appserver/web/web-core/src/main/java/org/apache/catalina/connector/RequestFacade.java

Lines 858 to 864 in d5635a8

	Principal p = request.getUserPrincipal();
	if (p instanceof WebPrincipal) {
	WebPrincipal wp = (WebPrincipal)p;
	if (wp.getCustomPrincipal() != null) {
	p = wp.getCustomPrincipal();
	}
	}

J2EEInstanceListener.handleBeforeEvent() is checking if a different principal has been set on the wrapped request and if that's the case sets this overridden principal on the SecurityContext.
Due to the change for PAYARA-290 handleBeforeEvent() is wrongly detecting an overridden principal on the wrapper set by the LoginToContinueInterceptor.
prin is obtained from the wrapped request

Payara/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java

Line 174 in 97decf6

Principal prin = hreq.getUserPrincipal();

and this effectively calls down to RequestFacade.getUserPrincipal()

Payara/appserver/web/web-core/src/main/java/org/apache/catalina/connector/RequestFacade.java

Lines 828 to 843 in b263fb8

	public java.security.Principal getUserPrincipal() {
	if (request == null) {
	throw new IllegalStateException(rb.getString(LogFacade.CANNOT_USE_REQUEST_OBJECT_OUTSIDE_SCOPE_EXCEPTION));
	}
	// Fix for GLASSFISH-16587
	Principal p = request.getUserPrincipal();
	if (p instanceof WebPrincipal) {
	WebPrincipal wp = (WebPrincipal) p;
	if (wp.getCustomPrincipal() != null) {
	p = wp.getCustomPrincipal();
	}
	}
	return p;

and therefore sets prin to the custom principal (unwrapped from the WebPrincipal). However basePrincipal is set to the WebPrincipal here

Payara/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java

Lines 197 to 203 in 97decf6

	else if (base instanceof RequestFacade) {
	// try to avoid the getUnWrappedCoyoteRequest call
	// when we can identify see we have the texact class.
	if (base.getClass() != RequestFacade.class) {
	basePrincipal = ((RequestFacade) base).getUnwrappedCoyoteRequest().getUserPrincipal();
	}
	} else {

The servlet request is of type RequestFacadeWrapper here and therefore the underlying Coyote request is unwrapped and the base principal is obtained from there

Payara/appserver/web/web-core/src/main/java/org/apache/catalina/connector/Request.java

Line 2814 in 97decf6

return userPrincipal;

This method doesn't do any unwrapping of the custom principal and returns the WebPrincipal as is.

This now leads to the condition in handleBeforeEvent()

Payara/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java

Lines 212 to 219 in 97decf6

	} else if (prin != basePrincipal) {
	// the wrapper has overridden getUserPrincipal
	// reject the request if the wrapper does not have
	// the necessary permission.
	checkObjectForDoAsPermission(hreq);
	securityContext.setSecurityContextWithPrincipal(prin);

being true and resets the SecurityContext to the custom principal which is lacking all roles.
When SecurityContext.isCallerInRole() is called now it returns false because there are no group principals on the subject. They have been erased by setting the custom principal on the SecurityContext before.

A related issue is #2898 It's closed due to inactivity, but as this issue shows it's still present, otherwise this issue described here probably wouldn't manifest itself or would go unnoticed.

I'm not really sure how to best fix this, but I think there are two options:

1. Adjust J2EEInstanceListener.handleBeforeEvent() to also unwrap the custom principal from the WebPrincipal returned by ((RequestFacade)base).getUnwrappedCoyoteRequest().getUserPrincipal() in line 201. That way
   

   Payara/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java

   Line 212 in 97decf6

   } else if (prin != basePrincipal) {

   wil be false again and the SecurityContext won't be reset.
2. Adjust com.sun.enterprise.security.SecurityContext.getSecurityContextForPrincipal()
   

   Payara/nucleus/security/core/src/main/java/com/sun/enterprise/security/SecurityContext.java

   Line 449 in 97decf6

   private SecurityContext getSecurityContextForPrincipal(Principal principal) {

   to copy the group principals from the current SecurityContext to the newly created subject. The same method in RealmAdapter may need to be adjusted too

   Payara/appserver/security/webintegration/src/main/java/com/sun/web/security/RealmAdapter.java

   Line 1261 in 97decf6

   private SecurityContext getSecurityContextForPrincipal(Principal principal) {

   This could possibly also fix PAYARA-2898.

Theoretically it could also be fixed by adding the logic for unwrapping the custom principal from the WebPrincipal here

Payara/appserver/web/web-core/src/main/java/org/apache/catalina/connector/Request.java

Line 2814 in 97decf6

return userPrincipal;

but as this method has many callers (of which surely some depend on getting the WebPrincipal) I don't think it's a good idea.

Maybe @arjantijms has an idea on how to best fix this?

[sosuisen]

It appears that this issue has been resolved in Glassfish as noted in eclipse-ee4j/glassfish#24196
However, it has not yet been reflected in Payara 7.2024.1.alpha. Wouldn't it be beneficial to reflect this fix?