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

Artisan command make:policy generates invalid policy code with a non-default user provider configuration #40345

Closed
slavicd opened this issue Jan 11, 2022 · 10 comments · Fixed by #40348
Labels

Comments

@slavicd
Copy link
Contributor

slavicd commented Jan 11, 2022

  • Laravel Version: 8.78.1
  • PHP Version: 7.4, 8.0

Description:

php artisan make:policy will produce invalid output, similar to what's been reported here, here and here (missing user class) if a non-default auth.providers.users configuration is in place.

Steps To Reproduce:

  1. composer create-project laravel/laravel
  2. Change the user provider configuration. E.G:
// config/auth.php
// ...
    'providers' => [
        'users' => [
            'driver' => 'database',
            'table' => 'users',
        ],
    ],
// ...
  1. php artisan make:policy CommentPolicy --model=Comment

Note the generated code in Policies: it is missing the User class and is invalid.

I realized this happens when the provider configuration is missing the model key, which was my case too (except I had a custom driver, and I never needed the model key).

Expected behaviour

The make:policy command throws an error if it needs explicit user class configuration or otherwise inserts a default user class.

@hmennen90
Copy link

Just tested it with a new Laravel Project:

'providers' => [
        //'users' => [
        //    'driver' => 'eloquent',
        //    'model' => App\Models\User::class,
        //],

         'users' => [
             'driver' => 'database',
             'table' => 'users',
         ],
    ],

Output:


❯ php artisan make:policy CommentPolicy --model=Comment
Policy created successfully.

@slavicd
Copy link
Contributor Author

slavicd commented Jan 11, 2022

@hmennen90 check the generated policy code.

@slavicd slavicd changed the title Artisan command make:policy fails for a non-default user provider configuration Artisan command make:policy generates invalid policy code with a non-default user provider configuration Jan 11, 2022
@hmennen90
Copy link

The Target Models are in use Statements using App\Models Namespace in both cases: -m and --model

If no Model is specified, there is a blank Policy created

use App\Models\Comment;
use App\Models\User;
use Illuminate\Auth\Access\HandlesAuthorization;

@hmennen90
Copy link

hmm i try it on php 8.0 - i'm currently on 8.1.1

@hmennen90
Copy link

Results on PHP8.0 and PHP7.4 are same as at PHP8.1.1

Maybe anyone else can reproduce your issue?

@hmennen90
Copy link

@slavicd command back ;-) Can now confirm your issue also on PHP 8.1.1 - had the config cached...

Did you mean the following Output?

namespace App\Policies;

use App\Models\Comment;
use Illuminate\Auth\Access\HandlesAuthorization;
use ;

class CommentPolicy
{
    use HandlesAuthorization;

    /**
     * Determine whether the user can view any models.
     *
     * @param  \  $
     * @return \Illuminate\Auth\Access\Response|bool
     */
    public function viewAny( $)
    {
        //
    }

@driesvints
Copy link
Member

You're changing the guard to one that doesn't have a model. Then it's normal that you'll get an error. A policy always requires a model.

@slavicd
Copy link
Contributor Author

slavicd commented Jan 11, 2022

@driesvints the command is generating invalid code. The problem is that I am not getting an error. The fact that I have a custom user provider is not documented anywhere as related to this, and even if it were, spitting out an error or warning or an explanatory message all seem more appropriate than generating a half-stub that is really hard to decode.

@driesvints
Copy link
Member

@slavicd ah I see what you mean. What would need to happen here is that the if (! $model) { throws an exception I think instead of the stub. Because a model is clearly required here.

@driesvints driesvints reopened this Jan 11, 2022
@driesvints driesvints added the bug label Jan 11, 2022
@slavicd
Copy link
Contributor Author

slavicd commented Jan 11, 2022

Precisely. I'll try to write a PR now.

slavicd added a commit to slavicd/framework that referenced this issue Jan 11, 2022
taylorotwell added a commit that referenced this issue Jan 13, 2022
#40348)

* Fixes issue #40345

* Addresses coding style complaint.

* Update PolicyMakeCommand.php

* Update PolicyMakeCommand.php

* use default user model

Co-authored-by: Taylor Otwell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants