lynndylanhurley / lynndylanhurley/devise_token_auth
`set_user_token`: Incompatibility between the Authorization header and having a param clashing with the auth header names.
- Dominant language
- Ruby
- Stars
- 3.6k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
Hi,
There seems to be a bug in how we read the authentication attributes in [concerns/set_user_by_token.rb](https://github.com/lynndylanhurley/devise_token_auth/blob/aedef3ca68d4e69bdc4a3e8987fa9d5f45420eec/app/controllers/devise_token_auth/concerns/set_user_by_token.rb#L52).
On line 52 and after this is how it reads:
```ruby
uid = request.headers[uid_name] || params[uid_name] || parsed_auth_cookie[uid_name] || decoded_authorization_token[uid_name]
other_uid = other_uid_name && request.headers[other_uid_name] || params[other_uid_name] || parsed_auth_cookie[other_uid_name]
@token = DeviseTokenAuth::TokenFactory.new unless @token
@token.token ||= request.headers[access_token_name] || params[access_token_name] || parsed_auth_cookie[access_token_name] || decoded_authorization_token[access_token_name]
@token.client ||= request.headers[client_name] || params[client_name] || parsed_auth_cookie[client_name] || decoded_authorization_token[client_name]
```
The problem exists where: if someone uses the `decoded_authorization_token` values coming from the one `Authorization` header instead of relying on the other 4 headers, the order of reading values is interrupted by reading from the `params`.
As a result, if either a path, query or body param contains an attribute with a name clashing with the defined `client_name`, `uid_name` or `access_token_name`, this will get evaluated before we can even try to evaluate the `decoded_authorization_token` (and this would be the same with the cookie).
As a result, auth fails for these requests.
```
GET /api/protected/some_resource?client=123
Authorization: Bearer a3dxd.....
===> Fails because client = 123 instead of the value in the Bearer token.
```
Could we issue a fix where we read from `params` after any other option has been tried ?
```ruby
uid = request.headers[uid_name] || parsed_auth_cookie[uid_name] || decoded_authorization_token[uid_name] || params[uid_name]
other_uid = other_uid_name && request.headers[other_uid_name] || parsed_auth_cookie[other_uid_name] || params[other_uid_name]
@token = DeviseTokenAuth::TokenFactory.new unless @token
@token.token ||= request.headers[access_token_name] || parsed_auth_cookie[access_token_name] || decoded_authorization_token[access_token_name] || params[access_token_name]
@token.client ||= request.headers[client_name] || parsed_auth_cookie[client_name] || decoded_authorization_token[client_name] || params[client_name]
```
Contributor guide
Research direction
Start in app/controllers/devise_token_auth/concerns/set_user_by_token.rb around line 52 and trace how authentication attributes are selected from headers, cookies, the decoded Authorization token, and params. Reproduce the documented request with a colliding client parameter, then verify that authentication still uses the Authorization token while preserving fallback behavior for params.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- api, authentication, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100