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

Soteria on Tomcat sometimes fails to register itself during server startup #315

Open
pizzi80 opened this issue Dec 21, 2021 · 9 comments
Open

Comments

@pizzi80
Copy link

pizzi80 commented Dec 21, 2021

JSF 3.0.2 + Weld 4.0.2 + Soteria 2.0.1 on Tomcat 10.0.14 on latest OpenJDK 17

Soteria print the following line on the Log during Tomcat startup:

[org.glassfish.soteria.servlet.SamRegistrationInstaller onStartup] Initializing Soteria 2.0.1 for context ...

BUT Sometimes it fails to register itself, in fact the Log line is missing

When it happens the Login mechanism do not work and you will get:

  1. if your try to manually login from the login page you get the same output of securityContext.authenticate always returns SUCCESS when I used a custom form in JSF 2.3 #194

  2. If you try to navigate directly to a page which requires a role to diplay you will get a 403 instead of a redirect to the login page!

In this case I have to stop the server + remove the deploy + delete work/Catalina directory + restart

@pizzi80
Copy link
Author

pizzi80 commented Feb 19, 2022

The problem is inside

org.glassfish.soteria.servlet.SamRegistrationInstaller

which in turns use

org.glassfish.soteria.cdi.CdiUtils

which uses JNDI to get the CDI BeanManager instead of

CDI.current().getBeanManager()

which is standard from CDI v1.1

and to get the instance of CdiExtension I've used:

CdiExtension cdiExtension = CDI.current().select(CdiExtension.class).get();

in this way soteria never fails to startup

@arjantijms
Copy link
Contributor

CDI.current().getBeanManager()
which is standard from CDI v1.1

Indeed, and originally I tried to use that exclusively. But then a lot of reports came in about issues with that, often getting the wrong bean archive.

See also: https://lists.jboss.org/pipermail/cdi-dev/2016-May/008297.html

Did you see what exactly happened? Was the BeanManager null, or did the BeanManager obtained from JNDI fail to find the CdiExtension class?

@pizzi80
Copy link
Author

pizzi80 commented Feb 28, 2022

Ok,
I found that the problem is not JNDI vs CDI.current() but the ServletContainerInitializer ordering

https://stackoverflow.com/questions/19729370/servletcontextlistener-execution-order

so if I define a web-fragment.xml like the one inside OmniFaces everything works

<?xml version="1.0" encoding="UTF-8"?>
<web-fragment
        xmlns="https://jakarta.ee/xml/ns/jakartaee"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-fragment_5_0.xsd"
        version="5.0"
>
  <name>soteria</name>

  <distributable />

  <ordering>
      <after>
          <others />
      </after>
  </ordering>
</web-fragment>

@arjantijms
Copy link
Contributor

I found that the problem is not JNDI vs CDI.current() but the ServletContainerInitializer ordering

Interesting indeed. Yes, for an issue quite like this I've started a discussion about that before. See https://eclipse.org/lists/servlet-dev/msg00170.html

If you want, maybe it's a good idea to restart that discussion by posting about this case to the servlet list?

@pizzi80
Copy link
Author

pizzi80 commented Mar 3, 2022

After many experiments the cleanest solution is to transform

SamRegistrationInstaller from a ServletContainerInitializer / ServletContextListener
to a CDI Extension because Soteria is a de facto CDI Extension ...

https://github.com/pizzi80/soteria/blob/2.0.10/impl/src/main/java/org/glassfish/soteria/cdi/SamRegistrationInstaller.java

in this way we don't need the web-fragment.xml file hack or beans.xml or @ApplicationScoped on SamRegistrationInstaller ...

@arjantijms
Copy link
Contributor

arjantijms commented Mar 8, 2022

It's a bit of a chicken and egg issue there, since a CDI extension does not have the servlet context available.

That context is needed to register the ServerAuthModule as per:

AuthConfigFactory
    .getFactory()
    .registerServerAuthModule(new HttpBridgeServerAuthModule(cdiPerRequestInitializer), ctx);

@pizzi80
Copy link
Author

pizzi80 commented Mar 8, 2022

Actually CDI spec v3.0 says that,
if the @initialized event is about the ApplicationScoped.class,
the payload of the event is exactly the ServletContext instance

If you take a look at my implementation, the signature of the method is exactly:

void onCdiStartup(@Observes @Initialized(ApplicationScoped.class) ServletContext context)

void onCdiShutdown(@Observes @BeforeDestroyed(ApplicationScoped.class) ServletContext context)

Even more on CDI v4.0 finally they made the @startup event which is a shortcut for @initialized(ApplicationScoped.class)
and the counterpart @shutdown wich is equivalent to @BeforeDestroyed(ApplicationScoped.class)

I'm using my fork in production right now for a side project and is working fine
event on tomcat redeploy.

The only problem with tomcat redeploy / parallel deploy it's on the exousia side

eclipse-ee4j/exousia#16

@arjantijms
Copy link
Contributor

Interesting idea indeed. I guess the extension interface is not needed here, as SamRegistrationInstaller is seen as a bean in this case. Here this may work, although the CDI spec is silent on whether the ServletContext instance is in restricted mode or not. See jakartaee/servlet#416

@sergey-morenets
Copy link

Hi

Any updates on that issue? It seems that Soteria has migrated to Jakarta EE 10.

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