Skip to content

Commit

Permalink
[5.3] Update TokenGuard.php to look for key in query string items onl…
Browse files Browse the repository at this point in the history
…y. (#14985)

* Update TokenGuard.php to look for key in query string items only.

Because in Larvel's combined input system, the body items take precedence over query string items. If an item appears in the body that uses the same key as the one being used for the API token, then this body item is then assumed to be the token which could lead to authentication errors especially if the key is being set to a more generic custom name with a high risk of conflict, e.g. 'password'. This file has been edited to restrict the API token to being in the query string only by using request->query instead of request->input which I think is the expected behaviour for token authentication.

* Update TokenGuard.php
  • Loading branch information
malhal authored and taylorotwell committed Aug 24, 2016
1 parent 65927fc commit cbc8d6e
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/Illuminate/Auth/TokenGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class TokenGuard implements Guard
protected $request;

/**
* The name of the field on the request containing the API token.
* The name of the query string item from the request containing the API token.
*
* @var string
*/
Expand Down Expand Up @@ -80,7 +80,7 @@ public function user()
*/
public function getTokenForRequest()
{
$token = $this->request->input($this->inputKey);
$token = $this->request->query($this->inputKey);

if (empty($token)) {
$token = $this->request->bearerToken();
Expand Down

2 comments on commit cbc8d6e

@alexpli
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, i've an issue with this change on some request with the HTTP networking library Alamofire on iOS. I don't know why and I try everything.

@graphem
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this has cause problem for me, cause we have a huge network of api user who rely on this and the instruction has been given to them to pass the key in the body. Now, with upgrade to 5.3, it has become a problem for us cause, the request are not being authenticated as the missing query string. Would it be possible to maybe have a config for this? So we can decide or not the api key must be passed in the query or not.

Please sign in to comment.