[WFCORE-3826] - anonymous authentication for ejbs using legacy config…

[JiriOndrusek]

…uration

Issue: https://issues.jboss.org/browse/WFCORE-3826

Test: wildfly/wildfly#11205

[wildfly-ci]

Core - Full Integration Build 7364 outcome was FAILURE using a merge of 5d8cdde
Summary: Compilation error: Compiler (new) Build time: 00:00:51

[jmesnil]

@JiriOndrusek I don't understand why the test for this PR is in WildFly and uses EJB as the issue is located in the remoting subsystem.

Should you be able to assert the new behaviour without using EJB (i.e. with a simple JNDI lookup or a remoting connection?)

[dmlloyd]

This seems like the wrong place for this logic to me. Why not put it in the Remoting subsystem directly, below?

[dmlloyd]

I think rather than expanding this particular API, this should probably done using one or more wrapping SaslServerFactory around the normal SaslAuthenticationFactory SaslServerFactory. We don't want XNIO options adding mechanisms that were not allowed by the global configuration: if anything we only want to reduce the set IMO.

[JiriOndrusek]

@dmlloyd
Thank you for your thoughts about this issue. I was discussing it firstly with Darran L. and the result of the discussion was to add getSaslAuthenticcationFactory with more parameters to propagate XNIO options.
It seems to me, that wrapping factory could be used only for reducing mechanisms, but this case should allow also adding new mechanisms. You can see that there is a validation of newly added mechanisms, that all of them are allowed: https://github.com/wildfly/wildfly-core/pull/3282/files#diff-b00c9f17a7ad69e0b3b498c79882888fR584

@darranl may I ask about you opinion on this issue?

@dmlloyd As for putting this logic into remoting subsystem, it seems reasonable. I can try it, but decision on the above question can influence it.

[dmlloyd]

I was discussing it firstly with Darran L. and the result of the discussion was to add getSaslAuthenticcationFactory with more parameters to propagate XNIO options. It seems to me, that wrapping factory could be used only for reducing mechanisms, but this case should allow also adding new mechanisms.

OK we can continue that discussion. One factor to consider is that we should not extend APIs for legacy constructs.

[darranl]

So in this case as @JiriOndrusek says we needed the legacy realm to add support for two legacy configuration options, firstly we needed to make sure anonymous would be authorized and then secondly we needed to add it to the list of mechanism advertised. In the Remoting subsystem I also believe we only have the option to reduce the set of mechanisms not increase it.

In the Remoting subsystem we probably should make sure the properties are deprecated if they have not already been as we don't use them with pure Elytron capabilities.

But for the remainder this is a bug fix in legacy code to resurrect some legacy behaviour, having the majority of the fix in the legacy code seems more appropriate as when possible that entire block of legacy code will be removed whilst the Remoting subsystem will predominantly remain.

[darranl]

The SecurityRealm API however is a legacy API, and the getSaslAuthenticationFactory is specifically to get an Elytron SASL authentication factory that emulates legacy behaviour.

[dmlloyd]

Ah okay, that makes it more clear. As long as this isn't a problem for the realms code then I'm fine with it.

[JiriOndrusek]

@jmesnil about the test:

The main reason why I've created test in wildfly with ejb is:
I wanted to achieve assertion on a real prinicipal name (return context.getCallerPrincipal().getName();) - during investigation I was once sure, that there is no problem, but in backrground server was using local authentication and getting real principal helped me to distinguish it.
If I use remote lookup for non existing EJB, which could end with SaslException: Authentication failed (before fix was used) or with NoSuchEJBException: No such EJB: (with fix). So I could distinguish, that authentication is correct or not. But I won't be sure that e.g. local authentication wasn't used...

Should I try to remove EJB from test?

From discussion above I'm not sure that remoting is the best location as majority of code is not in remoting.

[darranl]

Regarding the test I think in this situation having an EJB test is the way to go.

The Remoting subsystem by itself does not have the ability to deploy any secured endpoint so we would always need something to call, for core I suppose something could be written to test against management / JMX and use the whoami operation.

However this bug fix is being written based on an issue a user is experiencing migrating to a later version and in their situation they are using EJBs so we do need to ensure the anonymous identity can reach the EJB container as well so if we really did want anything in Core it would be in parallel to the test in WildFly.

Overall I think this is just the nature of the WildFly Core / WildFly split and security in general, the security implementation lives in core but in many cases it is the integration with a subsystem in WildFly that is really being fixed.