Repository: spatie/laravel-permission (php)

Issue:
Cannot save the user model twice in the same request
**Describe the bug**
Hi,
Sorry if this has already been asked but I'm using Laravel fortify to register new users, once user is registered I automatically log him in but I have a listener that listens for Login and automatically edits the user IP and last login and saves the user again. When the user is saved again I get an error Duplicate entry '7-153-User' for key 'model_has_roles.PRIMARY'. 
I fixed the issue by saving quietly but is this expected behavior ?
Now everytime I save a user in the same lifetime cycle it just crashes.
It seems to come from the use of attach instead of sync in the HasRoles trait line 168. see #2420 
Did not have this issue before because I migrated from v4 to v6.

**Versions**
- spatie/laravel-permission package version: 6.4.0
- laravel/framework package: 10.48.3

PHP version: 8.2.16

Database version: mysql 8.0.32


**To Reproduce**

- Create a new User
- Sync or Assign role to the user
- Save the User
- Save the User again

My listener:
```php
public function login(Login $event): void
    {
        try {
            /** @var User $user */
            $user = $event->user;
            activity('Auth')->by($user)
                ->withProperty('IP', request()->getClientIp())
                ->log('Login');

            $user->last_login = Carbon::now();
            $user->last_ip = request()->getClientIp();
            $user->saveQuietly();
        } catch (\Exception $exception) {
            report($exception);
        }
    }
```

**Expected behavior**
I would expect the save to not throw an error if I save the User twice.

**Environment (please complete the following information, because it helps us investigate better):**
 - OS: ubuntu
 - Version: 22.04

 


Pull request:
Fix can't save the same model twice
- Closes #2638

~This doesn't solve the problem completely~ **Another approach seems to work**
 
```php
$user->save();
//$user = $user->fresh();
$user->save();
```


Patch:
diff --git a/.github/ISSUE_TEMPLATE/1_Bug_report.yml b/.github/ISSUE_TEMPLATE/1_Bug_report.yml
new file mode 100644
index 0000000..96b2d65
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/1_Bug_report.yml
@@ -0,0 +1,55 @@
+name: Bug Report
+description: "Report a reproducible bug."
+body:
+  - type: markdown
+    attributes:
+      value: |
+          Before creating a new Bug Report, please check that there isn't already a similar issue on [the issue tracker](https://github.com/spatie/laravel-permission/issues) or in [the discussions](https://github.com/spatie/laravel-permission/discussions).
+          Also, **many issues/questions/problems are already answered** in the [documentation](https://spatie.be/docs/laravel-permission) already. **Please be sure to check the docs** because it will save you time!
+
+  - type: textarea
+    attributes:
+      label: Description
+      description: A clear and concise description of what the bug is.
+    validations:
+      required: true
+  - type: textarea
+    attributes:
+      label: Steps To Reproduce
+      description: How do you trigger this bug? Please walk us through it step by step.
+      value: |
+        1.
+        2.
+        3.
+        ...
+    validations:
+      required: true
+  - type: input
+    attributes:
+      label: Example Application
+      description: "Here is a link to my Github repo containing a minimal Laravel application which shows my problem:"
+  - type: markdown
+    attributes:
+      value: |
+          You can use `composer show` to get package version numbers:
+  - type: input
+    attributes:
+      label: "Version of spatie/laravel-permission package:"
+    validations:
+      required: true
+  - type: input
+    attributes:
+      label: "Version of laravel/framework package:"
+    validations:
+      required: true
+  - type: input
+    attributes:
+      label: "PHP version:"
+    validations:
+      required: true
+  - type: input
+    attributes:
+      label: "Database engine and version:"
+  - type: input
+    attributes:
+      label: "OS: Windows/Mac/Linux version:" 
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
deleted file mode 100644
index 6b62c8f..0000000
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ /dev/null
@@ -1,44 +0,0 @@
----
-name: Bug report
-about: Report a reproducible bug
-title: ''
-labels: ''
-assignees: ''
-
----
-
-**Before creating a new bug report**
-Please check that there isn't already a similar issue on [the issue tracker](https://github.com/spatie/laravel-permission/issues) or in [the discussions](https://github.com/spatie/laravel-permission/discussions).
-
-**Describe the bug**
-A clear and concise description of what the bug is.
-
-**Versions**
-You can use `composer show` to get the version numbers of:
-- spatie/laravel-permission package version:
-- laravel/framework package
-
-PHP version:
-
-Database version:
-
-
-**To Reproduce**
-Steps to reproduce the behavior:
-
-Here is my example code and/or tests showing the problem in my app:
-
-**Example Application**
-Here is a link to my Github repo containing a minimal Laravel application which shows my problem:
-
-**Expected behavior**
-A clear and concise description of what you expected to happen.
-
- **Additional context**
-Add any other context about the problem here.
-
-**Environment (please complete the following information, because it helps us investigate better):**
- - OS: [e.g. macOS]
- - Version [e.g. 22]
-
- 
diff --git a/database/migrations/create_permission_tables.php.stub b/database/migrations/create_permission_tables.php.stub
index b865d48..9c7044b 100644
--- a/database/migrations/create_permission_tables.php.stub
+++ b/database/migrations/create_permission_tables.php.stub
@@ -25,22 +25,24 @@ return new class extends Migration
         }
 
         Schema::create($tableNames['permissions'], function (Blueprint $table) {
+            //$table->engine('InnoDB');
             $table->bigIncrements('id'); // permission id
-            $table->string('name');       // For MySQL 8.0 use string('name', 125);
-            $table->string('guard_name'); // For MySQL 8.0 use string('guard_name', 125);
+            $table->string('name');       // For MyISAM use string('name', 225); // (or 166 for InnoDB with Redundant/Compact row format)
+            $table->string('guard_name'); // For MyISAM use string('guard_name', 25);
             $table->timestamps();
 
             $table->unique(['name', 'guard_name']);
         });
 
         Schema::create($tableNames['roles'], function (Blueprint $table) use ($teams, $columnNames) {
+            //$table->engine('InnoDB');
             $table->bigIncrements('id'); // role id
             if ($teams || config('permission.testing')) { // permission.testing is a fix for sqlite testing
                 $table->unsignedBigInteger($columnNames['team_foreign_key'])->nullable();
                 $table->index($columnNames['team_foreign_key'], 'roles_team_foreign_key_index');
             }
-            $table->string('name');       // For MySQL 8.0 use string('name', 125);
-            $table->string('guard_name'); // For MySQL 8.0 use string('guard_name', 125);
+            $table->string('name');       // For MyISAM use string('name', 225); // (or 166 for InnoDB with Redundant/Compact row format)
+            $table->string('guard_name'); // For MyISAM use string('guard_name', 25);
             $table->timestamps();
             if ($teams || config('permission.testing')) {
                 $table->unique([$columnNames['team_foreign_key'], 'name', 'guard_name']);
diff --git a/docs/basic-usage/super-admin.md b/docs/basic-usage/super-admin.md
index 3539277..4eefee9 100644
--- a/docs/basic-usage/super-admin.md
+++ b/docs/basic-usage/super-admin.md
@@ -11,21 +11,18 @@ Then you can implement the best-practice of primarily using permission-based con
 ## `Gate::before`
 If you want a "Super Admin" role to respond `true` to all permissions, without needing to assign all those permissions to a role, you can use [Laravel's `Gate::before()` method](https://laravel.com/docs/master/authorization#intercepting-gate-checks). For example:
 
+In Laravel 11 this would go in the `boot()` method of `AppServiceProvider`:
+In Laravel 10 and below it would go in the `boot()` method of `AuthServiceProvider.php`:
 ```php
 use Illuminate\Support\Facades\Gate;
-
-class AuthServiceProvider extends ServiceProvider
+// ...
+public function boot()
 {
-    public function boot()
-    {
-//...
-
-        // Implicitly grant "Super Admin" role all permissions
-        // This works in the app by using gate-related functions like auth()->user->can() and @can()
-        Gate::before(function ($user, $ability) {
-            return $user->hasRole('Super Admin') ? true : null;
-        });
-    }
+    // Implicitly grant "Super Admin" role all permissions
+    // This works in the app by using gate-related functions like auth()->user->can() and @can()
+    Gate::before(function ($user, $ability) {
+        return $user->hasRole('Super Admin') ? true : null;
+    });
 }
 ```
 
@@ -37,11 +34,11 @@ Jeffrey Way explains the concept of a super-admin (and a model owner, and model
 
 If you aren't using `Gate::before()` as described above, you could alternatively grant super-admin control by checking the role in individual Policy classes, using the `before()` method.
 
-Here is an example from the [Laravel Documentation on Policy Filters](https://laravel.com/docs/master/authorization#policy-filters)
+Here is an example from the [Laravel Documentation on Policy Filters](https://laravel.com/docs/master/authorization#policy-filters), where you can define `before()` in your Policy where needed:
 
 ```php
-use App\Models\User; // could be any model
- 
+use App\Models\User; // could be any Authorizable model
+
 /**
  * Perform pre-authorization checks on the model.
  */
diff --git a/docs/best-practices/using-policies.md b/docs/best-practices/using-policies.md
index 607f90e..504042e 100644
--- a/docs/best-practices/using-policies.md
+++ b/docs/best-practices/using-policies.md
@@ -9,4 +9,64 @@ Using Policies allows you to simplify things by abstracting your "control" rules
 
 Jeffrey Way explains the concept simply in the [Laravel 6 Authorization Filters](https://laracasts.com/series/laravel-6-from-scratch/episodes/51) and [policies](https://laracasts.com/series/laravel-6-from-scratch/episodes/63) videos and in other related lessons in that chapter. He also mentions how to set up a super-admin, both in a model policy and globally in your application.
 
-You can find an example of implementing a model policy with this Laravel Permissions package in this demo app: [https://github.com/drbyte/spatie-permissions-demo/blob/master/app/Policies/PostPolicy.php](https://github.com/drbyte/spatie-permissions-demo/blob/master/app/Policies/PostPolicy.php)
+Here's an example of a PostPolicy which could control access to Post model records:
+```php
+<?php
+namespace App\Policies;
+
+use App\Models\Post;
+use App\Models\User;
+use Illuminate\Auth\Access\HandlesAuthorization;
+
+class PostPolicy
+{
+    use HandlesAuthorization;
+
+    public function view(?User $user, Post $post)
+    {
+        if ($post->published) {
+            return true;
+        }
+
+        // visitors cannot view unpublished items
+        if ($user === null) {
+            return false;
+        }
+
+        // admin overrides published status
+        if ($user->can('view unpublished posts')) {
+            return true;
+        }
+
+        // authors can view their own unpublished posts
+        return $user->id == $post->user_id;
+    }
+
+    public function create(User $user)
+    {
+        return ($user->can('create posts'));
+    }
+
+    public function update(User $user, Post $post)
+    {
+        if ($user->can('edit own posts')) {
+            return $user->id == $post->user_id;
+        }
+
+        if ($user->can('edit all posts')) {
+            return true;
+        }
+    }
+
+    public function delete(User $user, Post $post)
+    {
+        if ($user->can('delete own posts')) {
+            return $user->id == $post->user_id;
+        }
+
+        if ($user->can('delete any post')) {
+            return true;
+        }
+    }
+}
+```
diff --git a/docs/prerequisites.md b/docs/prerequisites.md
index 88f3ad3..0b0cf30 100644
--- a/docs/prerequisites.md
+++ b/docs/prerequisites.md
@@ -43,14 +43,26 @@ This package publishes a `config/permission.php` file. If you already have a fil
 
 ## Schema Limitation in MySQL
 
-MySQL 8.0 limits index keys to 1000 characters. This package publishes a migration which combines multiple columns in single index. With `utf8mb4` the 4-bytes-per-character requirement of `mb4` means the max length of the columns in the hybrid index can only be `125` characters.
+Potential error message: "1071 Specified key was too long; max key length is 1000 bytes"
 
-Thus in your AppServiceProvider you will need to set `Schema::defaultStringLength(125)`. [See the Laravel Docs for instructions](https://laravel.com/docs/migrations#index-lengths-mysql-mariadb).
+MySQL 8.0 limits index key lengths, which might be too short for some compound indexes used by this package.
+This package publishes a migration which combines multiple columns in a single index. With `utf8mb4` the 4-bytes-per-character requirement of `mb4` means the total length of the columns in the hybrid index can only be `25%` of that maximum index length.
 
-You may be able to bypass setting `defaultStringLength(125)` by editing the migration and specifying the `125` in 4 fields. There are 2 instances of this code snippet where you can explicitly set the `125`:
+- MyISAM tables limit the index to 1000 characters (which is only 250 total chars in `utf8mb4`)
+- InnoDB tables using ROW_FORMAT of 'Redundant' or 'Compact' limit the index to 767 characters (which is only 191 total chars in `utf8mb4`)
+- InnoDB tables using ROW_FORMAT of 'Dynamic' or 'Compressed' have a 3072 character limit (which is 768 total chars in `utf8mb4`).
+
+Depending on your MySQL or MariaDB configuration, you may implement one of the following approaches:
+
+1. Ideally, configure the database to use InnoDB by default, and use ROW FORMAT of 'Dynamic' by default for all new tables. (See [MySQL](https://dev.mysql.com/doc/refman/8.0/en/innodb-limits.html) and [MariaDB](https://mariadb.com/kb/en/innodb-dynamic-row-format/) docs.)
+
+2. OR if your app doesn't require a longer default, in your AppServiceProvider you can set `Schema::defaultStringLength(125)`. [See the Laravel Docs for instructions](https://laravel.com/docs/migrations#index-lengths-mysql-mariadb). This will have Laravel set all strings to 125 characters by default.
+
+3. OR you could edit the migration and specify a shorter length for 4 fields. Then in your app be sure to manually impose validation limits on any form fields related to these fields. 
+There are 2 instances of this code snippet where you can explicitly set the length.:
 ```php
-    $table->string('name');       // For MySQL 8.0 use string('name', 125);
-    $table->string('guard_name'); // For MySQL 8.0 use string('guard_name', 125);
+    $table->string('name');       // For MyISAM use string('name', 225); // (or 166 for InnoDB with Redundant/Compact row format)
+    $table->string('guard_name'); // For MyISAM use string('guard_name', 25);
 ```
 
 ## Note for apps using UUIDs/ULIDs/GUIDs
diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php
index a71da3a..4e38f8b 100644
--- a/src/Traits/HasPermissions.php
+++ b/src/Traits/HasPermissions.php
@@ -401,14 +401,16 @@ trait HasPermissions
             $model->unsetRelation('permissions');
         } else {
             $class = \get_class($model);
+            $saved = false;
 
             $class::saved(
-                function ($object) use ($permissions, $model, $teamPivot) {
-                    if ($model->getKey() != $object->getKey()) {
+                function ($object) use ($permissions, $model, $teamPivot, &$saved) {
+                    if ($saved || $model->getKey() != $object->getKey()) {
                         return;
                     }
                     $model->permissions()->attach($permissions, $teamPivot);
                     $model->unsetRelation('permissions');
+                    $saved = true;
                 }
             );
         }
diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php
index c7a4b80..88013e8 100644
--- a/src/Traits/HasRoles.php
+++ b/src/Traits/HasRoles.php
@@ -159,14 +159,16 @@ trait HasRoles
             $model->unsetRelation('roles');
         } else {
             $class = \get_class($model);
+            $saved = false;
 
             $class::saved(
-                function ($object) use ($roles, $model, $teamPivot) {
-                    if ($model->getKey() != $object->getKey()) {
+                function ($object) use ($roles, $model, $teamPivot, &$saved) {
+                    if ($saved || $model->getKey() != $object->getKey()) {
                         return;
                     }
                     $model->roles()->attach($roles, $teamPivot);
                     $model->unsetRelation('roles');
+                    $saved = true;
                 }
             );
         }


Test patch:
diff --git a/tests/HasPermissionsTest.php b/tests/HasPermissionsTest.php
index ea156a2..b785f01 100644
--- a/tests/HasPermissionsTest.php
+++ b/tests/HasPermissionsTest.php
@@ -636,6 +636,7 @@ class HasPermissionsTest extends TestCase
         $user = new User(['email' => 'test@user.com']);
         $user->syncPermissions('edit-articles');
         $user->save();
+        $user->save(); // test save same model twice 
 
         $this->assertTrue($user->hasPermissionTo('edit-articles'));
 
diff --git a/tests/HasRolesTest.php b/tests/HasRolesTest.php
index a6ebef1..9cc13a7 100644
--- a/tests/HasRolesTest.php
+++ b/tests/HasRolesTest.php
@@ -341,6 +341,7 @@ class HasRolesTest extends TestCase
         $user = new User(['email' => 'test@user.com']);
         $user->syncRoles([$this->testUserRole]);
         $user->save();
+        $user->save(); // test save same model twice 
 
         $this->assertTrue($user->hasRole($this->testUserRole));