Commit ff65fe04 authored by Tiago's avatar Tiago
Browse files

auth: do not concatenate same token when a request is retried for a non-authorization error 

while the token was being popped only on auth errors, the logic to add the token to the request was still running, the current token ending up concatenated with itself

the token is now memoized in the request and only discarded on auth errors, ensuring that connection resets won't reset the token either
parent d7cabaae

lib/httpx/plugins/auth.rb

+21 −2
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ module HTTPX 
        private



        def send_request(request, *)

          return super if @skip_auth_header_value

          return super if @skip_auth_header_value || request.authorized?



          @auth_header_value ||= generate_auth_token
@@ -92,12 +92,31 @@ module HTTPX 
      end



      module RequestMethods

        def initialize(*)

          super

          @auth_token_value = nil

        end



        def authorized?

          !@auth_token_value.nil?

        end



        def unauthorize!

          return unless (auth_value = @auth_token_value)



          @headers.get("authorization").delete(auth_value)



          @auth_token_value = nil

        end



        def authorize(auth_value)

          if (auth_type = @options.auth_header_type)

            auth_value = "#{auth_type} #{auth_value}"

          end



          @headers.add("authorization", auth_value)



          @auth_token_value = auth_value

        end

      end
@@ -119,7 +138,7 @@ module HTTPX 
            return unless auth_error?(response, request.options) ||

                          (@options.generate_auth_value_on_retry && @options.generate_auth_value_on_retry.call(response))



            request.headers.get("authorization").pop

            request.unauthorize!

            @auth_header_value = generate_auth_token

          end

lib/httpx/plugins/ntlm_auth.rb

+1 −1
Original line number Diff line number Diff line
@@ -46,7 +46,7 @@ module HTTPX 


              if probe_response.status == 401 && ntlm.can_authenticate?(probe_response.headers["www-authenticate"])

                request.transition(:idle)

                request.headers.get("authorization").pop

                request.unauthorize!

                request.authorize(ntlm.authenticate(request, probe_response.headers["www-authenticate"]).encode("utf-8"))

                super(request)

              else

sig/plugins/auth.rbs

+6 −0
Original line number Diff line number Diff line
@@ -29,6 +29,12 @@ module HTTPX 
      end



      module RequestMethods

        @auth_token_value: String?



        def authorized?: () -> bool



        def unauthorize!: () -> void



        def authorize: (String auth_value) -> void

      end

test/support/requests/plugins/auth.rb

+28 −3
Original line number Diff line number Diff line
@@ -41,19 +41,35 @@ module Requests 
          body = json_body(response)

          verify_header(body["headers"], "Authorization", "TOKEN1")

        end



        # proves that token is discarded

        authed.reset_auth_header_value!

        # proves that token is reused

        response = authed.get(get_uri)

        verify_status(response, 200)

        body = json_body(response)

        verify_header(body["headers"], "Authorization", "TOKEN2")

      end



      def test_plugin_auth_generate_token_once_for_multi_request

        get_uri = build_uri("/get")

        authed = HTTPX.plugin(:auth)

        i = 0

        r1, r2 = authed.authorization { "TOKEN#{i += 1}" }.get(get_uri, get_uri)

        verify_status(r1, 200)

        body = json_body(r1)

        verify_header(body["headers"], "Authorization", "TOKEN1")



        verify_status(r2, 200)

        body = json_body(r2)

        verify_header(body["headers"], "Authorization", "TOKEN1")

      end



      def test_plugin_auth_regenerate_on_retry

        i = 0

        session = HTTPX.plugin(RequestInspector)

                       .plugin(:retries, max_retries: 1, retry_on: ->(res) { res.status == 400 })

                       .plugin(:auth, generate_auth_value_on_retry: ->(res) { res.status == 400 })

                       .plugin(:retries, max_retries: 1, retry_on: ->(res) { res.respond_to?(:status) && res.status == 400 })

                       .plugin(:auth, generate_auth_value_on_retry: ->(res) { res.respond_to?(:status) && res.status == 400 })

                       .with(timeout: { request_timeout: 3 })

                       .authorization { "TOKEN#{i += 1}" }



        response = session.get(build_uri("/status/400"))
@@ -71,6 +87,15 @@ module Requests 
        req1, req2 = session.total_requests

        assert req1.headers["authorization"] == "TOKEN2", "the last successful token should have been reused"

        assert req2.headers["authorization"] == "TOKEN3"

        session.reset



        # on regular errors, it should try to reuse the same token

        response = session.get(build_uri("/delay/10"))

        verify_error_response(response, HTTPX::RequestTimeoutError)

        assert session.calls == 1, "expected two errors to have been sent"

        req1, req2 = session.total_requests

        assert req1.headers["authorization"] == "TOKEN3", "the last successful token should have been reused"

        assert req2.headers["authorization"] == "TOKEN3", "the previous token should have been reused"

      end



      # Bearer Auth