Drop @LoginConfig

[rmannibucau]

Follow up issue of #70 (hoping the scope of this one is clear and narrowed :))

This annotation has a lot of pitfalls:

1. it is not defined in a way which enables it to be portable,
2. it requires to hardcode the security config in the application (same as @*Definition are never used anywhere else than in demo in JakartaEE land, we should learn from this error),
3. it is not needed to make it work (only case it can be relevant is for a container providing all spec, even the ones the user don't need/want - container philosophy - and in this case the vendor can trivially provide a way to disable it if needed so the toggle is not in jwt-auth spec level).

So overall, there is not a single case which justify this annotation and it does not bring any configuration the implementation would require so there is no blocker to drop it (note that dropping an annotation keeps the application functional without recompilation in terms of API).

Side note: adding a mp-config key to enable/disable this spec can still be done as a consequence of the removal of this annotation IMHO.

[sberyozkin]

@rmannibucau Sure, this is more concrete, no doubt about it :-)

[sberyozkin]

Hey @rmannibucau I've added a 2.0 milestone, though I'll resist as much as possible :-) because we have the users (and likely not only us) using it, unfortunately with more than MP-JWT, so there has to be a strong reason to make them update their applications. Few quick questions:

it is not defined in a way which enables it to be portable,

What is not portable about sticking it to the JAX-RS Application ?

it requires to hardcode the security config in the application

Do you mean its optional realm property ?

it is not needed to make it work

MP-JWT ? I certainly know one project where authMethod=MP-JWT is checked.

I wonder if a starting point can be to drop the references to all but MP-JWT method, and make MP-JWT a default value. IMHO it can be more realistic than just dropping LoginConfig

Cheers

[rmannibucau]

there has to be a strong reason to make them update their applications

It is an annotation so if not in the classpath it is ignored, no update to do :).

What is not portable about sticking it to the JAX-RS Application ?

It is not defined yet ;). It also has the issue to conflict with other EE specs (servlet and security out of my mind) which have their own security mecanism/annotations. Who wins? Who wraps who?

Do you mean its optional realm property ?

No, the jwt-auth activation itself. With all the proxies we have these days in k8s and friends, it is likely we'll have deployment with JWT Auth and deployments without when going accross cloud providers, using a mp-config toggle is a neat solution to be able to deactivate that feature and can completly replace the annotation.

MP-JWT ? I certainly know one project where authMethod=MP-JWT is checked.

I guess it is the case of a project going beyond the spec (keycloak usage?) and this would be solved the same way with the mp-config toggle for this app (+ redhat ecosystem using jandex does not need the annotation to exist to see it ;)).

[sberyozkin]

Hi Romain,

It is an annotation so if not in the classpath it is ignored, no update to do :).

But the legacy project is relying on a user application telling it via LoginConfig what it wants

What is not portable about sticking it to the JAX-RS Application ? It is not defined yet ;)

Then its docs can be updated

using a mp-config toggle is a neat solution to be able to deactivate that feature and can completly replace the annotation.

It can - but we are talking about the new projects here.

guess it is the case of a project going beyond the spec (keycloak usage?)

No, it is not related to KC.

Some immediate clarity would definitely helped though. We can say:

• the use of LoginConfig is optional - the implementations which depend on it may check JAX-RS Application
• remove the list of non MP JWT methods from the docs and make MP-JWT a default value

making optional should really work for all

[rmannibucau]

@sberyozkin before saying I agree or not, can you clarify you propose to make this annotation support optional as "implementation can or not support it" (I'm ok with it) or something else?

[sberyozkin]

@rmannibucau Yes, optional would imply it, but I'm not 100% to be honest right now.

[rmannibucau]

@sberyozkin nothing urges but think we should try to relax that soon, if bothersome (= we get negative feedback of making it optional) we could rethink about an useful and not conflicting configuration (can be as simple as enabling to handle a JAXRS interceptors set programmatically in an user feature to enable/disable that, it could even enable to make it dynamic in apps compared to today ;)).

[sberyozkin]

@rmannibucau Are you OK with renaming this issue to Make @LoginConfig optional ? Once we fix this one we can open Deprecate LoginConfig if needed, if having optional will not be enough.

[rmannibucau]

@sberyozkin works for me (maybe with another wording since "make @loginConfig optional" sounds like it can be there or not whereas we want to emphasis it can be supported or not).

[sberyozkin]

@rmannibucau Making it optional is the very first step - once it is optional it effectively means no one has to support it but only those containers which prefer it will do it.

By the way, the package.info has:

LoginConfig: an annotation that is used by a JAX-RS Application to define the authentication method and associated security realm

So there is a reference to Application. So not sure how to untangle it, because as soon as we make it optional, some users will start saying, wait, we already use it, as well some implementers may assert the same.

But in any case, I'm fine with updating its docs as the 1step as suggested earlier (drop the ref to all but MP JWT methods)

[rmannibucau]

Yep works for me.

Side note: resources are used by jaxrs apps but not always referenced so javadoc does not remove all the ambiguity and clearly encourages the one with EE ecosystem so good to slowly move to dropping it from default required api.

[sberyozkin]

CC @teddyjtorres.