michaelklishin/langohr

EXTERNAL for sasl-config

visibletrap opened this issue · 6 comments

I'm the one who made this PR ruby-amqp/rubybunny.info#27, but when it comes to langohr, the story is different.

We had an error when trying to make a rabbbitmq connection to rabbitmq broker that has ssl authentication plugin enabled. We figured through looking at Java client source code that we have to set DefaultSaslConfig/EXTERNAL to sasl-config to make it works, as in the snippet below.

(rmq/connect { ...
               :sasl-config DefaultSaslConfig/External }

I need your thought and help on these.

  • What's an appropriate place to put this example in the langohr.docs?
  • Should we wrap DefaultSaslConfig/External in a more friendly fashion? Like for example
{:sasl-config "EXTERNAL"}

I can try to come up with a change PR if you think it's a good way to go.

@visibletrap yes, a new option (:authentication-mechanism) would be great. The right place to cover this is next to other TLS options.

Thank you!

Sorry for being slow on implementing this. Finally I get a chance to dig into it. I have enough implementation to make this work here a1067ed. But I'm struggling and need some advices whether do I need to write test for this?

On one side, I think it's good to have test as a documentation as you mention here. But on the other side, test for this is required a bit of setup. It needs a client certificate that have CN matches with user name in rabbitmq server. I'm not sure the amount of complexity introduced with the test is worth covering the tiny bit of code.

Also I do fix/improve the existing tls-test here ee13462...22be9aa, would you interest in merging those in?

@visibletrap different authentication mechanisms are a pain to test, so given how trivial this change is, I'm OK with not having tests for it, given that you test it adequately manually.

@michaelklishin I have manually tested this (with / without the option) extensively.

Should be fixed by #70. Thanks again, @visibletrap :)

@michaelklishin Great. Thanks for you help and super fast response!