-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a shared SASL implementation #69
Draft
nevans
wants to merge
9
commits into
ruby:master
Choose a base branch
from
nevans:sasl/net-imap-fallback
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nevans
force-pushed
the
sasl/net-imap-fallback
branch
2 times, most recently
from
October 9, 2023 22:14
6bb82aa
to
a5b654b
Compare
nevans
changed the title
Use
Use a shared SASL implementation
Oct 9, 2023
net-imap
SASL implementation
nevans
force-pushed
the
sasl/net-imap-fallback
branch
7 times, most recently
from
October 14, 2023 17:06
dcfc541
to
4913bf4
Compare
nevans
force-pushed
the
sasl/net-imap-fallback
branch
8 times, most recently
from
October 21, 2023 00:28
7e8fa6f
to
08eff09
Compare
nevans
force-pushed
the
sasl/net-imap-fallback
branch
2 times, most recently
from
November 9, 2023 15:42
45f893a
to
92b67d4
Compare
This adds a new `auth` keyword param to `Net::SMTP.start` and `#start` that can be used to pass any arbitrary keyword parameters to `#authenticate`. The pre-existing `username`, `secret`, etc keyword params will retain their existing behavior as positional arguments to `#authenticate`.
Although "user" is a reasonable abbreviation, the parameter is more accurately described as a "username" or an "authentication identity". They are synonomous here, with "username" winning when both are present.
Username can be set by args[0], authcid, username, or user. Secret can be set by args[1], secret, or password. Since all of the existing authenticators have the same API, it is sufficient to update `check_args` in the base class.
This API is a little bit confusing, IMO. But it does preserve backward compatibility, while allowing authenticators that don't allow positional parameters to work without crashing. But, authenticators that require only one parameter—or more than three—will still be inaccessible.
This is convenient for `smtp.start auth: {type:, **etc}`.
Although `#authenticate` can be updated to make username and secret _both_ optional, by placing the mechanism last and making it optional, it's not possible to use an authenticator with a _single_ positional parameter or with more than two positional parameters. By placing `type` first among positional parameters or as a keyword argument, we avoid this problem.
The net-imap dependency requires 2.7.3, to deal with kwargs.
This commit adds the `net-imap` as a default fallback for mechanisms that haven't otherwise been added. In this commit, the original implementation is still used by `#authenticate` for the `PLAIN`, `XOAUTH2`, `LOGIN`, and `CRAM-MD5` mechanisms. Every other mechanism supported by `net-imap` v0.4.0 is added here: * `ANONYMOUS` * `DIGEST-MD5` _(deprecated)_ * `EXTERNAL` * `OAUTHBEARER` * `SCRAM-SHA-1` and `SCRAM-SHA-256` **TODO:** Ideally, `net-smtp` and `net-imap` should both depend on a shared `sasl` or `net-sasl` gem, rather than keep the SASL implementation inside one or the other. See ruby/net-imap#23.
nevans
force-pushed
the
sasl/net-imap-fallback
branch
from
May 5, 2024 15:49
92b67d4
to
78e1da8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on the following other PRs:
#auth_capable?
public #63Wrap#authenticate
withcritical
#65username
keyword param tostart
methods #72auth_method
#67As currently written, this also depends on the following
net-imap
PRs:secret
alias (forpassword
,oauth2_token
, etc) to relevant SASL mechanisms net-imap#195After those PRs are applied, this PR creates an adapter that is compatible with
Authenticator
and#authenticate
and registers it as a generic default for mechanisms that haven't otherwise been added (as subclasses ofAuthenticator
). This adapter then delegates tonet-imap
's SASL implementation. Every other mechanism supported bynet-imap
v0.4.1 is added here:ANONYMOUS
DIGEST-MD5
(deprecated)EXTERNAL
OAUTHBEARER
SCRAM-SHA-1
andSCRAM-SHA-256
XOAUTH
TODO: Ideally,
net-smtp
andnet-imap
should both depend on a sharedsasl
ornet-sasl
gem, rather than keep the SASL implementation inside one or the other. See ruby/net-imap#23.In this PR, the current
Net::SMTP::Authenticator
implementation is still used by thePLAIN
,LOGIN
, andCRAM-MD5
mechanisms. PR #70 removes the current authenticators are replaces them with this.Related issues: