Skip to content

Commit

Permalink
🥅 Cancel AUTHENTICATE if process raises an error [🚧 server error]
Browse files Browse the repository at this point in the history
The exception will be re-raised after the protocol cancel response has
been sent.
  • Loading branch information
nevans committed Sep 16, 2024
1 parent 9312d90 commit 5e075ae
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
21 changes: 17 additions & 4 deletions lib/net/imap/sasl/authentication_exchange.rb
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,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
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,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

def test_uidplus_uid_expunge
with_fake_server(select: "INBOX",
extensions: %i[UIDPLUS]) do |server, imap|
Expand Down

0 comments on commit 5e075ae

Please sign in to comment.