Payara icon indicating copy to clipboard operation
Payara copied to clipboard

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

Open rcuprak opened this issue 4 years ago • 16 comments

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)

rcuprak avatar Jun 21 '20 03:06 rcuprak

@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.

phillipross avatar Jun 21 '20 04:06 phillipross

@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).

rcuprak avatar Jun 21 '20 04:06 rcuprak

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 avatar Jun 21 '20 04:06 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.

phillipross avatar Jun 29 '20 22:06 phillipross

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 avatar Nov 16 '20 15:11 AlanRoth

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

AlanRoth avatar Nov 23 '20 12:11 AlanRoth

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

AlanRoth avatar Nov 30 '20 09:11 AlanRoth

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).

rcuprak avatar Jan 09 '21 06:01 rcuprak

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

AlanRoth avatar Jan 11 '21 09:01 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.

AlanRoth avatar Jan 11 '21 13:01 AlanRoth

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

hzometa avatar Jun 16 '21 16:06 hzometa

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

hberton avatar Sep 09 '21 13:09 hberton

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.

douglasmartim avatar Oct 22 '21 03:10 douglasmartim

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?

hberton avatar Nov 24 '21 14:11 hberton

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() https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java#L171-L221 together with this change for #290 https://github.com/payara/Payara/blob/d5635a89e45e2382f68e4eefbdf6c37940604c31/appserver/web/web-core/src/main/java/org/apache/catalina/connector/RequestFacade.java#L858-L864 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 https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java#L174 and this effectively calls down to RequestFacade.getUserPrincipal() https://github.com/payara/Payara/blob/b263fb8b7eab9fee664cd8b0a9457ed3dc96cdd0/appserver/web/web-core/src/main/java/org/apache/catalina/connector/RequestFacade.java#L828-L843 and therefore sets prin to the custom principal (unwrapped from the WebPrincipal). However basePrincipal is set to the WebPrincipal here https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java#L197-L203 The servlet request is of type RequestFacadeWrapper here and therefore the underlying Coyote request is unwrapped and the base principal is obtained from there https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-core/src/main/java/org/apache/catalina/connector/Request.java#L2814 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() https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java#L212-L219 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 https://github.com/payara/Payara/issues/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 https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-glue/src/main/java/com/sun/web/server/J2EEInstanceListener.java#L212 wil be false again and the SecurityContext won't be reset.
  2. Adjust com.sun.enterprise.security.SecurityContext.getSecurityContextForPrincipal() https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/nucleus/security/core/src/main/java/com/sun/enterprise/security/SecurityContext.java#L449 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 https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/security/webintegration/src/main/java/com/sun/web/security/RealmAdapter.java#L1261 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 https://github.com/payara/Payara/blob/97decf6a5fca66515fcb4dd1852226a593f86307/appserver/web/web-core/src/main/java/org/apache/catalina/connector/Request.java#L2814 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?

georgwolf avatar Nov 22 '22 10:11 georgwolf