emacs-circe / circe

Circe, a Client for IRC in Emacs
GNU General Public License v3.0
390 stars 51 forks source link

Add support for SASL EXTERNAL and TLS client certificates #386

Closed AitorATuin closed 3 years ago

AitorATuin commented 3 years ago

This PR adds support for:

Tested in Freenode and OFTC. OFTC does not support sasl and Freenode allows SASL EXTERNAL though the client certificate authentication works without using it.

wasamasa commented 3 years ago

Could you please reindent the changed parts of the file? This is jarring to read otherwise.

Thaodan commented 3 years ago

The last commit fixes the change from commit before? If so please squash the last with the first commit with git rebase --interactive.

wasamasa commented 3 years ago

The logic looks fine to me. I've tested plain SASL after this change successfully. One more cosmetic change plus squash and this should be ready to merge.

AitorATuin commented 3 years ago

I think all the indentation issues are fixed now (my emacs was mixing tabs and spaces) and commits are squashed.

This PR does not check if EXTERNAL is returned in the CAPS by the server, it just checks that sasl is available, but SIMPLE is doing the same I think it's enough for now. In another PR it could be improved though.

Also this PR did not try to hook authentication errors returned by SAML but I think that another PR in the future could do that. Right now on SASL authentication error circe will continue without login but it will be nice to have sasl-strict mode or something like that.

wasamasa commented 3 years ago

Sure, feel free to tackle this in a follow-up PR.