10000 🔒 Handle cancel for SASL AUTHENTICATE by nevans · Pull Request #324 · ruby/net-imap · GitHub
[go: up one dir, main page]
Skip to content

🔒 Handle cancel for SASL AUTHENTICATE #324

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.

Sign up for GitHub

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
+110 −12
Draft

🔒 Handle cancel for SASL AUTHENTICATE #324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d67bf0
🔒 Don't disconnect for AuthenticationCanceled [🚧 tests]
nevans Jun 30, 2024
d80daa8
🥅 Cancel AUTHENTICATE if process raises an error [🚧 server error]
nevans Jun 30, 2024
525574f
🚧 Handle cancellations more carefully...
nevans Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
🥅 Cancel AUTHENTICATE if process raises an error [🚧 server error] 
The exception will be re-raised after the protocol cancel response has
been sent.
  • Loading branch information
@nevans
nevans committed Apr 24, 2025
commit d80daa8aac31e19b8f2f9901694444d613a24a26
21 changes: 17 additions & 4 deletions lib/net/imap/sasl/authentication_exchange.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ module SASL
# AuthenticationExchange is used internally by Net::IMAP#authenticate.
# But the API is still *experimental*, and may change.
#
# TODO: catch exceptions in #process and send #cancel_response.
# TODO: raise an error if the command succeeds after being canceled.
# TODO: use with more clients, to verify the API can accommodate them.
# TODO: pass ClientAdapter#service to SASL.authenticator
#
Expand Down Expand Up @@ -81,6 +79,9 @@ def self.build(client, mechanism, *args, sasl_ir: true, **kwargs, &block)

attr_reader :mechanism, :authenticator

# An exception that has been raised by <tt>authenticator.process</tt>.
attr_reader :process_error

def initialize(client, mechanism, authenticator, sasl_ir: true)
@client = client
@mechanism = Authenticators.normalize_name(mechanism)
Expand All @@ -93,8 +94,17 @@ def initialize(client, mechanism, authenticator, sasl_ir: true)
# using #authenticator. Authentication failures will raise an
# exception. Any exceptions other than AuthenticationCanceled or those
# in <tt>client.response_errors</tt> will drop the connection.
#
# When <tt>authenticator.process</tt> raises any StandardError
# (including AuthenticationCanceled), the authentication exchange will
# be canceled before re-raising the exception. The server will usually
# respond with an error response, and the client will most likely raise
# that error. This client error will supercede the original error.
# Unfortunately, the original error will not be the +#cause+ for the
# client error. But it will be available on #process_error.
def authenticate
client.run_command(mechanism, initial_response) { process _1 }
.tap { raise process_error if process_error }
.tap { raise AuthenticationIncomplete, _1 unless done? }
rescue AuthenticationCanceled, *client.response_errors
raise # but don't drop the connection
Expand Down Expand Up @@ -128,9 +138,12 @@ def initial_response
end

def process(challenge)
client.encode authenticator.process client.decode challenge
ensure
@processed = true
return client.cancel_response if process_error
client.encode authenticator.process client.decode challenge
rescue => process_error
@process_error = process_error
client.cancel_response
end

end
Expand Down
5 changes: 3 additions & 2 deletions test/net/imap/fake_server/command_router.rb
View file
Open in desktop
< B4FD /tbody>
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ def handler_for(command)
response_b64 = resp.request_continuation("") || ""
state.commands << {continuation: response_b64}
end
response = Base64.decode64(response_b64)
response.empty? and return resp.fail_bad "canceled"
response_b64.strip == ?* and return resp.fail_bad "canceled"
response = Base64.decode64(response_b64) rescue :decode64_failed
response == :decode64_failed and return resp.fail_bad "invalid b64"
# TODO: support mechanisms other than PLAIN.
parts = response.split("\0")
parts.length == 3 or return resp.fail_bad "invalid"
Expand Down
19 changes: 19 additions & 0 deletions test/net/imap/test_imap.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,25 @@ def test_id
end
end

test "#authenticate can be canceled with SASL::AuthenticationCanceled" do
with_fake_server(
preauth: false, cleartext_auth: true,
sasl_ir: false, sasl_mechanisms: %i[PLAIN]
) do |server, imap|
registry = Net::IMAP::SASL::Authenticators.new(use_defaults: false)
registry.add_authenticator :plain, ->(*a, **kw, &b) {
->(challenge) {
raise(Net::IMAP::SASL::AuthenticationCanceled,
"a: %p, kw: %p, b: %p" % [a, kw, b])
}
}
assert_raise_with_message(Net::IMAP::BadResponseError, "canceled") do
imap.authenticate(:plain, hello: :world, registry: registry)
end
refute imap.disconnected?
end
end

test "#uid_expunge with EXPUNGE responses" do
with_fake_server(select: "INBOX",
extensions: %i[UIDPLUS]) do |server, imap|
Expand Down
0