eclipse-ee4j / glassfish

Eclipse GlassFish
https://eclipse-ee4j.github.io/glassfish/
385 stars 143 forks source link

Request login/logout/relogin fails to load roles (isUserInRole) #23757

Closed ctabin closed 1 year ago

ctabin commented 2 years ago

Hello,

By making some tests to upgrade from Payara 5/Java EE 8 to GlassFish 6.2.3/JakartaEE 9.1, we hit a strange bug with HttpServletRequest.login(user,pass) and HttpServletRequest.isUserInRole(role). Even if the login works after a logout, it seems the isUserInRole returns always false.

Here is some code snippet that we have:

request.login(myUser, myPassword); //ok
request.isUserInRole("ADMIN"); //true
//...
request.logout(); //principal is reset
//...
request.login(myUser, myPassword); //ok
request.isUserInRole("ADMIN"); //false => should (still) be true

After diving in the GlassFish's code, we can find out that during the logout, PolicyContext.setContextId(null) is invoked (however the context is not set during login). Hence, any futher invokation of isUserInRole does not work because the context id is needed.

We are using a simple JDBC Realm and there is no change in the user/roles between the first and second calls. All the invokations are in the same thread.

Here is a working workaround, however seems (very) hacky because we have to use GlassFish internal classes:

String ctxId = PolicyContext.getContextId();
request.login(myUser, myPassword); //ok
request.isUserInRole("ADMIN"); //true
//...
request.logout(); //principal is reset
//...
PolicyContext.setContextId(ctxId);
request.login(myUser, myPassword); //ok
request.isUserInRole("ADMIN"); //true

That was working with Payara 5, but wasn't tested in previous version of GlassFish.

Environment Details

github-actions[bot] commented 1 year ago

This issue has been marked as inactive and old and will be closed in 7 days if there is no further activity. If you want the issue to remain open please add a comment

ctabin commented 1 year ago

This issue is still valid in 6.2.5. Not tested in 7.0.0 for now.

zistrong commented 1 year ago

7.0.3 is still NOT OK now.

arjantijms commented 1 year ago

Thanks a bunch for reporting this. I've worked on exactly this issue in Piranha Cloud and I think Payara. So in my mind I already fixed this two times.

The mid-request login/logout/login has been a source of errors in the past, and it's somewhat surprising giving that, that we still don't have a clear test covering this. As login(username, password) is not directly supported via the Jakarta Security APIs, I think we should add a Servlet TCK test for this.

I'll take a look where to add such test in the meantime.

Btw, technically speaking the proposed workaround is not GlassFish internal code, as PolicyContext is a standard type from Jakarta Authorization, but of course user code should not have to do this.

arjantijms commented 1 year ago

As a start, I added an example for this scenario here. It has to be moved/copied to e.g. the TCK later.

https://github.com/arjantijms/jakartaee-examples/tree/master/focused/servlet/relogin

run with

mvn clean install -pl :relogin

And observe the output on GlassFish 7.0.4:

1:john
1:true
2:null
3:john
3:false
ctabin commented 1 year ago

@arjantijms Great to see you working on this :thumbsup: I can confirm that this is exactly the behavior that we observed.