Forward-compatible SASL API #68
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
|
I've been using a personal fork of |
nevans
force-pushed
the
sasl/forward-compatible-api
branch
2 times, most recently
from
baf9035 to
628f113
Compare
|
This PR is still fairly large. So I've converted it to draft, and I'm going to split it into several more bite-sized PRs which can mostly be applied directly to I'd still welcome feedback on this PR as it is. Unless I hear feedback otherwise, I don't expect the final form will be significantly different from this. |
nevans
force-pushed
the
sasl/forward-compatible-api
branch
9 times, most recently
from
4efd8a4 to
745c320
Compare
nevans
force-pushed
the
sasl/forward-compatible-api
branch
from
745c320 to
5e2e2ef
Compare
nevans
force-pushed
the
sasl/forward-compatible-api
branch
2 times, most recently
from
846e9cb to
510c293
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.
nevans
force-pushed
the
sasl/forward-compatible-api
branch
from
510c293 to
d8cc256
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.
It is my belief that the current API for
#startand#authenticatecan't fully support every SASL mechanism. Most of the necessary changes have been implemented by #71, and are described in that ticket.This PR adds a new
Net::SMTP#authmethod which is similar to#authenticate, but uses a different call signature:typeis passed in either as a keyword argument or as the first positional argument. All other arguments are forwarded to the authenticator.Although #71 updates
#authenticateto forward keyword args and a block and makes username and secret both optional, there are limitations to maintaining backward-compatibility with the existing#authenticatemethod. Because the mechanism is last and optional it's not possible to use an authenticator with a single positional parameter or with more than two positional parameters. The call signature used by#authavoids this problem.As currently written, this PR depends on the following other PRs:
#auth_capable?public #63Wrap#authenticatewithcritical#65usernamekeyword param tostartmethods #72auth_method#67This PR is intended as a transition between the v0.4.0
#authenticatemethod and a shared SASL implementation, either usingnet-imap's implementation or extracting it to anet-saslgem (see ruby/net-imap#23). The#authand#startAPI in this PR are both compatible with all of the SASL mechanisms supported bynet-imapv0.4.1. The following PRs depend on this one:PLAIN,CRAM-MD5, andLOGINimplementations #70