aerni / statamic-advanced-seo

Comprehensive SEO addon for Statamic with flexibility in mind
https://statamic.com/addons/aerni/advanced-seo
10 stars 5 forks source link

App\Models\User database user error when viewing setting pages. #108

Closed JorisOrangeStudio closed 7 months ago

JorisOrangeStudio commented 7 months ago

I'm using database users in Statamic and with this addon I get the following error when opening SEO related pages (for example: /cp/advanced-seo/collections). The error not showing up for super users, only users with permissions (all SEO permissions are enabled).

Aerni\AdvancedSeo\Policies\SeoVariablesPolicy::index(): Argument #1 ($user) must be of type Statamic\Contracts\Auth\User, App\Models\User given, called in /var/www/html/vendor/laravel/framework/src/Illuminate/Auth/Access/Gate.php on line 811

Here is the full stacktrace of this error: stacktrace.txt

How to fix this? Is this something in my install or addon-related?

aerni commented 7 months ago

This issue happens because I'm type-hinting Statamic\Contracts\Auth\User in the policy methods. Can you delete the User type in all methods and try if that resolves the issue?

https://github.com/aerni/statamic-advanced-seo/blob/69d82fbedf28f783480bc7ff9693a223d83baaa6/src/Policies/SeoVariablesPolicy.php#L11-L16

I'm not sure if this could be fixed on the Statamic side of things by making sure that the $user is a Statamic\Auth\Eloquent\User (which implements the Statamic\Contracts\Auth\User interface) instead of a App\Models\User.

aerni commented 7 months ago

Are you using the Statamic eloquent driver for your users? Can you please reproduce this on a fresh installation and share the repo with me?

JorisOrangeStudio commented 7 months ago

Can you delete the User type in all methods and try if that resolves the issue?

This fixes it, but then we are overriding your addon and that is not our preference. Is this something you could implement in a new update?

Yes we use statamic's eloquent model for users.

This is our App\Models\User on the current install:

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Laravel\Sanctum\HasApiTokens;

class User extends Authenticatable
{
    use HasApiTokens, HasFactory, Notifiable;

    /**
     * The attributes that are mass assignable.
     *
     * @var array<int, string>
     */
    protected $fillable = [
        'name',
        'email',
        'password',
    ];

    /**
     * The attributes that should be hidden for serialization.
     *
     * @var array<int, string>
     */
    protected $hidden = [
        'password',
        'remember_token',
    ];

    /**
     * The attributes that should be cast.
     *
     * @var array<string, string>
     */
    protected $casts = [
        'email_verified_at' => 'datetime',
    ];
}

This is the relevant part from the auth.php config file:

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

And this is a part of the /statamic/users.php config file:

    'repository' => 'eloquent',

    'repositories' => [
        'file' => [
            'driver' => 'file',
            'paths' => [
                'users' => base_path('users'),
                'roles' => resource_path('users/roles.yaml'),
                'groups' => resource_path('users/groups.yaml'),
            ],
        ],

        'eloquent' => [
            'driver' => 'eloquent',
        ],
    ],

I'm curious if you can find the problem, or build a solution into the addon. Thanks in advance! If you need extra info please let me know.

aerni commented 7 months ago

Ok, thanks for testing. I might just remove the type then. I'll take a look at it.