[WFCORE-3963] Fix of WFCORE-3826 breaks plain authentic… #3391
Conversation
|
@JiriOndrusek [WFCORE-3963] is mentioned twice in commit and PR title messages |
|
@xstefank Bad ctrl+c, I'll fix it, thanks |
JiriOndrusek
force-pushed
the
WFCORE-3963_anonymous_ejb_DIGEST
branch
from
0c68bab
to
ef02212
Compare
JiriOndrusek
changed the title
| @@ -590,7 +590,7 @@ public SaslAuthenticationFactory getSaslAuthenticationFactory(final String[] mec | |||
|
|
|||
| for(Iterator<String> iter = requestedMechanisms.iterator();iter.hasNext();) { | |||
| AuthMechanism authMechanism = toAuthMechanism("SASL", iter.next()); | |||
| if(authMechanism != null) { | |||
| if(authMechanism != null && (registeredServices.containsKey(authMechanism) || registeredServices.containsKey(AuthMechanism.DIGEST))) { | |||
There was a problem hiding this comment.
@JiriOndrusek Why is there a special case for DIGEST here and below? Could you explain the reason for these changes a bit? Thanks!
There was a problem hiding this comment.
@fjuma I admit that I'm not sure with this particular piece of code. I was trying to add new mechanisms defined by SASL_MECHANISMS. Originally I wanted use mechanism names as realm names for all types. But it wasn't possible. This is a workaround to make it working. But as I'm thinking more about it, it seems wrong.
My fix would work only if elytron is defined with digest mechanism - which may not be true always.
I have to probably rework this solution.
@darranl May I ask for your opinion?
Currently e.g. if user wants PLAIN mechanism and there is no plain mechanism defined in elytron, how should it work? (goal of this issue is to don't do any elytron configuration at all and work only with legacy one)
There was a problem hiding this comment.
I think going back to the original issue the main problem was that users previously could use this property to activate ANONYMOUS authentication.
If they are using something like LDAP they will never be able to override the config and enable DIGEST, the two are just completely incompatible.
On the other hand, I am not sure if it has been used in the past but a realm that supports DIGEST should always be compatible to support PLAIN, this should be east to verify with a test.
There was a problem hiding this comment.
Now I understand it correctly, (I hope):
- fix should allow anonymous - no problem
- fix should allow only these mechanisms, which are currently enabled by elytron, with exception for PLAIN which could use DIGEST (this substitution works, I've tested it)
@darranl Is my understanding correct?
There was a problem hiding this comment.
IMO that clarification sounds good, this is adding a property to be used with a legacy configuration - even if we could go further we should not as users would not have been able to do it previously and if a more advanced config is required nowadays they should be switching to an Elytron based configuration where they get full control of the mechanism configurations.
|
@fjuma I'll change fix according to result of discussion:
|
… using legacy configuration
JiriOndrusek
force-pushed
the
WFCORE-3963_anonymous_ejb_DIGEST
branch
from
ef02212
to
f8f8c69
Compare
|
@fjuma changes are commited |
…ation for ejbs using legacy configuration
Issue: https://issues.jboss.org/browse/WFCORE-3963
Test: wildfly/wildfly#11432