Skip to content

Conversation

@reynir
Copy link
Contributor

@reynir reynir commented Feb 19, 2022

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 ?sasl takes a polymorphic variant [`None | `Plain | `External instead of a bool.

@reynir
Copy link
Contributor Author

reynir commented Feb 19, 2022

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 ()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants