Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.5] Support for multiword models to authorize resource controllers #19821

Conversation

Hesto
Copy link
Contributor

@Hesto Hesto commented Jun 29, 2017

Not sure if this should be pushed to 5.4 branch or master branch.

Method authorizeResource in framework/src/Illuminate/Foundation/Auth/Access/AuthorizesRequests.php can be used to authorize resource controllers. So if we have model Order and resource controller OrdersController we can use
authorizeResorce(Order::class) method in controller to authorize each REST method with OrderPolicy.

It works fine but if we want to authorize controller for multiword model for example: OrderDetail it will not work. The problem is this code

public function authorizeResource($model, $parameter = null, array $options = [], $request = null)
{
    $parameter = $parameter ?: lcfirst(class_basename($model));
    ...
}

So when the second param is null, it will take model name and change the first letter to lowercase.
For example $parameter for model Order will be order but for model OrderDetail it will be orderDetail.
Unfortunatly this is wrong value because as default laravel params in route model binding are in snake_case not camelCase.
This issue returns always 401 Unauthorized. Of course we can specify the second param maually like so:
authorizeResorce(OrderDetail::class, 'order_detail') but nobody knows about that during implementing ACL feature in his laravel app.

To save time for future users, support for multiword models should be done automatically like so:

public function authorizeResource($model, $parameter = null, array $options = [], $request = null)
{
    $parameter = $parameter ?: snake_case(lcfirst(class_basename($model))); 
    ...
}

Result for models:

  • Order -> order (still the same)
  • OrderDetail -> order_detail (snake case instead of camel case)

@Hesto Hesto changed the title Support for multiword models to authorize resource controllers [5.4] Support for multiword models to authorize resource controllers Jun 29, 2017
@taylorotwell taylorotwell changed the base branch from 5.4 to master June 29, 2017 13:17
@taylorotwell taylorotwell merged commit 8cdd49e into laravel:master Jun 29, 2017
@taylorotwell
Copy link
Member

Looks good. Re-targeted to master in case its a breaking change for anyone.

@deleugpn
Copy link
Contributor

Should probably update the title so it doesn't go to the wrong changelog

@Hesto Hesto changed the title [5.4] Support for multiword models to authorize resource controllers [5.5] Support for multiword models to authorize resource controllers Jun 29, 2017
@Hesto
Copy link
Contributor Author

Hesto commented Jun 29, 2017

@deleugpn title changed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants