Roles: Added max validation for role external auth id field

For #5037
BookStackApp · Jun 8, 2024 · bddc6ae · bddc6ae
1 parent 6019d2e
commit bddc6ae
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 4 deletions.
diff --git a/app/Users/Controllers/RoleApiController.php b/app/Users/Controllers/RoleApiController.php
@@ -21,15 +21,15 @@ class RoleApiController extends ApiController
             'display_name'  => ['required', 'string', 'min:3', 'max:180'],
             'description'   => ['string', 'max:180'],
             'mfa_enforced'  => ['boolean'],
-            'external_auth_id' => ['string'],
+            'external_auth_id' => ['string', 'max:180'],
             'permissions'   => ['array'],
             'permissions.*' => ['string'],
         ],
         'update' => [
             'display_name'  => ['string', 'min:3', 'max:180'],
             'description'   => ['string', 'max:180'],
             'mfa_enforced'  => ['boolean'],
-            'external_auth_id' => ['string'],
+            'external_auth_id' => ['string', 'max:180'],
             'permissions'   => ['array'],
             'permissions.*' => ['string'],
         ]

diff --git a/app/Users/Controllers/RoleController.php b/app/Users/Controllers/RoleController.php
@@ -75,7 +75,7 @@ public function store(Request $request)
         $data = $this->validate($request, [
             'display_name' => ['required', 'min:3', 'max:180'],
             'description'  => ['max:180'],
-            'external_auth_id' => ['string'],
+            'external_auth_id' => ['string', 'max:180'],
             'permissions'  => ['array'],
             'mfa_enforced' => ['string'],
         ]);
@@ -109,7 +109,7 @@ public function update(Request $request, string $id)
         $data = $this->validate($request, [
             'display_name' => ['required', 'min:3', 'max:180'],
             'description'  => ['max:180'],
-            'external_auth_id' => ['string'],
+            'external_auth_id' => ['string', 'max:180'],
             'permissions'  => ['array'],
             'mfa_enforced' => ['string'],
         ]);

diff --git a/tests/User/RoleManagementTest.php b/tests/User/RoleManagementTest.php
@@ -96,6 +96,31 @@ public function test_role_create_update_delete_flow()
         $this->assertActivityExists(ActivityType::ROLE_DELETE);
     }
 
+    public function test_role_external_auth_id_validation()
+    {
+        config()->set('auth.method', 'oidc');
+        $role = Role::query()->first();
+        $routeByMethod = [
+            'post' => '/settings/roles/new',
+            'put' => "/settings/roles/{$role->id}",
+        ];
+
+        foreach ($routeByMethod as $method => $route) {
+            $resp = $this->asAdmin()->get($route);
+            $resp->assertDontSee('The external auth id');
+
+            $resp = $this->asAdmin()->call($method, $route, [
+                'display_name' => 'Test role for auth id validation',
+                'description'  => '',
+                'external_auth_id' => str_repeat('a', 181),
+            ]);
+
+            $resp->assertRedirect($route);
+            $resp = $this->followRedirects($resp);
+            $resp->assertSee('The external auth id may not be greater than 180 characters.');
+        }
+    }
+
     public function test_admin_role_cannot_be_removed_if_user_last_admin()
     {
         /** @var Role $adminRole */