ARTEMIS-5316 XOAUTH2 SASL mechanism for broker connection #5721
Conversation
| @@ -536,6 +538,7 @@ private static int parseDefaultVariable(String variableName, int defaultValue) { | |||
| allowableConnectorKeys.add(TransportConstants.TRUST_MANAGER_FACTORY_PLUGIN_PROP_NAME); | |||
| allowableConnectorKeys.add(TransportConstants.HANDSHAKE_TIMEOUT); | |||
| allowableConnectorKeys.add(TransportConstants.CRL_PATH_PROP_NAME); | |||
| allowableConnectorKeys.add(TransportConstants.SASL_MECHANISMS); | |||
There was a problem hiding this comment.
Same issue as before, we should not be adding SASL mechanisms to netty options as AMQP is the protocol that's implementing the SASL bits, others do not so the options should be handled as other AMQP specific options are.
|
|
||
| private List<String> getAcceptedMechanisms(List<String> availableMechanisms) { | ||
| TransportConfiguration connectorConfig = connection.getConnectorConfig(); | ||
| if(connectorConfig == null){ |
There was a problem hiding this comment.
I believe the formatting in this method breaks checkstyle rules which needs to be fixed beyond fixing the bit about not putting the sasl bits into the netty transport options
| @@ -115,6 +115,31 @@ public void testConnectsWithPlain() throws Exception { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Testing is insufficient as there are routes through the is applicable code that involved checking if user and passwords are set or not which should be tested.
|
@bstlukasiewicz, do you plan on addressing the issues that Tim identified? |
|
Yes, I'm working on those issuses |
No description provided.