Fix parsed token value with header `Authorization token=`. #15683

Merged
merged 1 commit into from Jun 13, 2014

Conversation

Projects
None yet
6 participants
Contributor

larrylv commented Jun 13, 2014

Related issue: #14846

With header Authorization: Token token=, we get token value as nil instead of token.

The current implementation with params.shift.last or param.last assume params or param is a two-elements array. But with malformed request, they are not, so instead of last, we should use [1]

cc @simonbnrd @robin850

@egilburg egilburg and 2 others commented on an outdated diff Jun 13, 2014

...ck/lib/action_controller/metal/http_authentication.rb
@@ -449,7 +449,7 @@ def token_and_options(request)
authorization_request = request.authorization.to_s
if authorization_request[TOKEN_REGEX]
params = token_params_from authorization_request
- [params.shift.last, Hash[params].with_indifferent_access]
+ [(params.shift)[1], Hash[params].with_indifferent_access]
@egilburg

egilburg Jun 13, 2014

Contributor

why do you need the () parentheses here?

@larrylv

larrylv Jun 13, 2014

Contributor

It is needless. I put it just for making code clear.

@guilleiguaran

guilleiguaran Jun 13, 2014

Owner

too Lispy in my opinion 😄

Contributor

larrylv commented Jun 13, 2014

@guilleiguaran ok. removed the parentheses.

Contributor

tumayun commented Jun 13, 2014

@larrylv Great! How do you find it? work?

Contributor

larrylv commented Jun 13, 2014

@tumayun no, find this in an old issue.

Contributor

larrylv commented Jun 13, 2014

ping @guilleiguaran . Travis build passed. :-)

@arthurnn arthurnn commented on the diff Jun 13, 2014

...ack/test/controller/http_token_authentication_test.rb
end
+
+ private
+
+ def sample_request(token)
+ @sample_request ||= OpenStruct.new authorization: %{Token token="#{token}", nonce="def"}
@arthurnn

arthurnn Jun 13, 2014

Member

why did you need to add the nonce in this method.

@larrylv

larrylv Jun 13, 2014

Contributor

@arthurnn Good catch. I was doing some tests on normal authorization parsing, and forgot to remove that key-value pair. Would you want me send another PR?

matthewd merged commit b71d46a into rails:master Jun 13, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

larrylv deleted the larrylv:fix-token-with-empty-value branch Jun 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment