Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow token authentication header with text value. #14213

Open
wants to merge 1 commit into from

4 participants

@tylerhunt

The field content is allowed to be *TEXT, and does not have to contain
key value pairs. Update the test to account for this.

See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 for the
relevant section of the specification.

@tylerhunt tylerhunt Allow token authentication header with text value.
The field content is allowed to be *TEXT, and does not have to contain
key value pairs. Update the test to account for this.

See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 for the
relevant section of the specification.
1214264
@robin850 robin850 added the actionpack label
@repinel repinel commented on the diff
...ack/test/controller/http_token_authentication_test.rb
@@ -79,12 +79,12 @@ def authenticate_long_credentials
end
end
- test "authentication request with badly formatted header" do
- @request.env['HTTP_AUTHORIZATION'] = "Token foobar"
+ test "authentication request with text" do
@repinel
repinel added a note

You should probably have this as a new test.

@arthurnn Collaborator

Right, is there any reason that you didnt create a new test, and instead it is changing an existent one?

My impression was that the existing test was testing for a badly formatted header to fail authentication, when in reality the "badly formatted header" in question is actually valid according to the spec, so the existing (invalid) test was removed, and a new one was added to ensure authorization headers without key/value pairs are acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 26, 2014
  1. @tylerhunt

    Allow token authentication header with text value.

    tylerhunt authored
    The field content is allowed to be *TEXT, and does not have to contain
    key value pairs. Update the test to account for this.
    
    See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 for the
    relevant section of the specification.
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 4 deletions.
  1. +4 −4 actionpack/test/controller/http_token_authentication_test.rb
View
8 actionpack/test/controller/http_token_authentication_test.rb
@@ -79,12 +79,12 @@ def authenticate_long_credentials
end
end
- test "authentication request with badly formatted header" do
- @request.env['HTTP_AUTHORIZATION'] = "Token foobar"
+ test "authentication request with text" do
@repinel
repinel added a note

You should probably have this as a new test.

@arthurnn Collaborator

Right, is there any reason that you didnt create a new test, and instead it is changing an existent one?

My impression was that the existing test was testing for a badly formatted header to fail authentication, when in reality the "badly formatted header" in question is actually valid according to the spec, so the existing (invalid) test was removed, and a new one was added to ensure authorization headers without key/value pairs are acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ @request.env['HTTP_AUTHORIZATION'] = "Token lifo"
get :index
- assert_response :unauthorized
- assert_equal "HTTP Token: Access denied.\n", @response.body, "Authentication header was not properly parsed"
+ assert_response :success
+ assert_equal 'Hello Secret', @response.body
end
test "authentication request without credential" do
Something went wrong with that request. Please try again.