[Bug] OAuth 2 client_credentials breaks Shield.

[hskrasek]

I have discovered that if you authenticate via OAuth using the client_credentials flow instead of the password flow, Shield basically explodes because it tries to do something like User::findOrFail('oauth_client_id'), which if you're using the Dingo\Controllers, means bad business.

Not sure if this is intended or not, but thought I would bring it to your attention.

[jasonlewis]

Good point. I'll look into this.
On 19 May 2014 11:14, "Hunter Skrasek" notifications@github.com wrote:

I have discovered that if you authenticate via OAuth using the
client_credentials flow instead of the password flow, Shield basically
explodes because it tries to do something like
User::findOrFail('oauth_client_id'), which if you're using the
Dingo\Controllers, means bad business.

Not sure if this is intended or not, but thought I would bring it to your
attention.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/56
.

[kevindierkx]

I have the same problem, it happens when I access a route that has the protected => true attribute using the client_credentials grant. It tries to validate the user in this case however it doesn't take Grant types in account, since client_credential grants don't have an user it will fail with a null exception.

[hipsterjazzbo]

What's the status of this? I was thinking of forking and having a go, but I won't if there's already stuff being done.

[jasonlewis]

It's next on my list but if you want to take a look then go for it mate. 😄 I'm doing a few other things at the moment.

[hipsterjazzbo]

Just to update this, I had a look but I don't have the time right now to dig in and figure this out unfortunately. If it's still broken in a few weeks when I've got some more time, I'll try to figure it out.

[hipsterjazzbo]

I've had a dig through, and I've found the issue: Shield::check().

/**
 * Check if a user has authenticated with the API.
 * 
 * @return bool
 */
public function check()
{
    return ! is_null($this->user());
}

It's just assuming that there is a user to be authed. In here it should probably check in some way to see if the current auth method/grant type needs a user associated, but I'm not 100% sure the best way to do this.

I thought perhaps of adding an abstract function to AuthorizationProvider, which each provider would then implement to answer whether it needs a user account for the current request, by checking the grant type in the case of the two oauth providers, or just returning true in the case of the basic provider.

If this sounds reasonable I'll get stuck in and make the change.

[kukat]

same problem +1

[jasonlewis]

Is Shield::check being called somewhere automatically?

[hipsterjazzbo]

Yes it is, via RateLimit::isAuthenticatedRequest().

[jasonlewis]

Ah! Gosh I have a bad memory. Must've been the only place I didn't look. Cheers @cfidecaro.

[hipsterjazzbo]

It took me a while to track it down. It's quite a convoluted route :)

[hipsterjazzbo]

Just to remind, I have a pull request open with a "solution" to this issue, but I doubt if it's the way you'd want it done. It was a quick job.

[jasonlewis]

Yeah I've been looking at that. Going to tinker with some things first.