Conversation
1379e25 to
9a78c84
Compare
|
Rebased on main which should fix the CI issue. |
| | _, Some password -> send_pass ~connection ~password | ||
| send_auth_sasl_external ~connection ~user | ||
| | _, Some password, None -> send_pass ~connection ~password | ||
| | _ -> return () |
There was a problem hiding this comment.
Maybe we should match on etc None, _, Some Plain | _, None, Some Plain and issue an error instead of silently skipping SASL authentication. What do you think, @johnelse ?
There was a problem hiding this comment.
Since this is going to be a breaking change anyway, we could go even further and do something like
type auth = {
password : string;
sasl : [`Plain | `External] option
}
and have connect take an auth option. That way, there's no way of requesting any kind of SASL without also supplying a password.
There was a problem hiding this comment.
I think that makes sense. Can I suggest that auth also carries a username field? It is not always the case that you want to authenticate with the same username as the IRC user.
The SASL external mechanism does not use the password for anything. Maybe the type should have a different shape.
This adds support for SASL EXTERNAL mechanism. I have successfully used it with libera.chat using certfp.
It is a breaking change since the optional argument
?sasltakes a polymorphic variant[`None | `Plain | `Externalinstead of a bool.