User form: Always show external auth field, update access control

Updated old user management routes to only be accessible with permission
to manage users, so also removed old content controls checking for that
permission.

diff --git a/app/Users/Controllers/UserController.php b/app/Users/Controllers/UserController.php
index 0cd48948f8ccc79a461931f141f8cb333bf41043..507c7cf064ebeaf793f805424fd78ce3f108e212 100644 (file)
--- a/app/Users/Controllers/UserController.php
+++ b/app/Users/Controllers/UserController.php
@@ -103,8 +103,7 @@ class UserController extends Controller
      */
     public function edit(int $id, SocialAuthService $socialAuthService)
     {
-        $this->preventGuestAccess();
-        $this->checkPermissionOrCurrentUser('users-manage', $id);
+        $this->checkPermission('users-manage');
 
         $user = $this->userRepo->getById($id);
         $user->load(['apiTokens', 'mfaValues']);
@@ -134,8 +133,7 @@ class UserController extends Controller
     public function update(Request $request, int $id)
     {
         $this->preventAccessInDemoMode();
-        $this->preventGuestAccess();
-        $this->checkPermissionOrCurrentUser('users-manage', $id);
+        $this->checkPermission('users-manage');
 
         $validated = $this->validate($request, [
             'name'             => ['min:2', 'max:100'],
@@ -150,7 +148,7 @@ class UserController extends Controller
         ]);
 
         $user = $this->userRepo->getById($id);
-        $this->userRepo->update($user, $validated, userCan('users-manage'));
+        $this->userRepo->update($user, $validated, true);
 
         // Save profile image if in request
         if ($request->hasFile('profile_image')) {
@@ -168,9 +166,7 @@ class UserController extends Controller
             $user->save();
         }
 
-        $redirectUrl = userCan('users-manage') ? '/settings/users' : "/settings/users/{$user->id}";
-
-        return redirect($redirectUrl);
+        return redirect('/settings/users');
     }
 
     /**
@@ -178,8 +174,7 @@ class UserController extends Controller
      */
     public function delete(int $id)
     {
-        $this->preventGuestAccess();
-        $this->checkPermissionOrCurrentUser('users-manage', $id);
+        $this->checkPermission('users-manage');
 
         $user = $this->userRepo->getById($id);
         $this->setPageTitle(trans('settings.users_delete_named', ['userName' => $user->name]));
@@ -195,8 +190,7 @@ class UserController extends Controller
     public function destroy(Request $request, int $id)
     {
         $this->preventAccessInDemoMode();
-        $this->preventGuestAccess();
-        $this->checkPermissionOrCurrentUser('users-manage', $id);
+        $this->checkPermission('users-manage');
 
         $user = $this->userRepo->getById($id);
         $newOwnerId = intval($request->get('new_owner_id')) ?: null;

diff --git a/lang/en/settings.php b/lang/en/settings.php
index 579c4b5c856f17ec558c7c8149c78a619524e48b..dfd0f7841e3b0315a45b34e3ae33af954f94c12f 100644 (file)
--- a/lang/en/settings.php
+++ b/lang/en/settings.php
@@ -193,7 +193,7 @@ return [
     'users_send_invite_text' => 'You can choose to send this user an invitation email which allows them to set their own password otherwise you can set their password yourself.',
     'users_send_invite_option' => 'Send user invite email',
     'users_external_auth_id' => 'External Authentication ID',
-    'users_external_auth_id_desc' => 'This is the ID used to match this user when communicating with your external authentication system.',
+    'users_external_auth_id_desc' => 'When an external authentication system is in use (such as SAML2, OIDC or LDAP) this is the ID which links this BookStack user to the authentication system account. You can ignore this field if using the default email-based authentication.',
     'users_password_warning' => 'Only fill the below if you would like to change the password for this user.',
     'users_system_public' => 'This user represents any guest users that visit your instance. It cannot be used to log in but is assigned automatically.',
     'users_delete' => 'Delete User',

diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php
index e6b477a120f0946aaa272045e8c246075679e069..1254a13307ae3c2f7acd580b313c6609f7a6123d 100644 (file)
--- a/resources/views/users/edit.blade.php
+++ b/resources/views/users/edit.blade.php
@@ -38,7 +38,7 @@
                 </div>
 
                 <div class="text-right">
-                    <a href="{{  url(userCan('users-manage') ? "/settings/users" : "/") }}"
+                    <a href="{{  url("/settings/users") }}"
                        class="button outline">{{ trans('common.cancel') }}</a>
                     @if($authMethod !== 'system')
                         <a href="{{ url("/settings/users/{$user->id}/delete") }}"
@@ -100,9 +100,7 @@
             </section>
         @endif
 
-        @if((user()->id === $user->id && userCan('access-api')) || userCan('users-manage'))
-            @include('users.api-tokens.parts.list', ['user' => $user])
-        @endif
+        @include('users.api-tokens.parts.list', ['user' => $user])
     </div>
 
 @stop

diff --git a/resources/views/users/parts/form.blade.php b/resources/views/users/parts/form.blade.php
index d9f9588377621539b6ca4e7e8717c7fb57ff743e..bf1eb08a7602b081660ec174edddea0bf3cce7b4 100644 (file)
--- a/resources/views/users/parts/form.blade.php
+++ b/resources/views/users/parts/form.blade.php
@@ -11,7 +11,7 @@
     @if($authMethod === 'ldap' || $authMethod === 'system')
         <p class="small">{{ trans('settings.users_details_desc_no_email') }}</p>
     @endif
-    <div class="grid half mt-m gap-xl">
+    <div class="grid half mt-m gap-xl mb-l">
         <div>
             <label for="name">{{ trans('auth.name') }}</label>
             @include('form.text', ['name' => 'name'])
@@ -23,29 +23,26 @@
             @endif
         </div>
     </div>
-</div>
-
-@if(in_array($authMethod, ['ldap', 'saml2', 'oidc']) && userCan('users-manage'))
-    <div class="grid half gap-xl v-center">
-        <div>
-            <label class="setting-list-label">{{ trans('settings.users_external_auth_id') }}</label>
-            <p class="small">{{ trans('settings.users_external_auth_id_desc') }}</p>
-        </div>
-        <div>
-            @include('form.text', ['name' => 'external_auth_id'])
+    <div>
+        <div class="form-group collapsible mb-none" component="collapsible" id="external-auth-field">
+            <button refs="collapsible@trigger" type="button" class="collapse-title text-link" aria-expanded="false">
+                <label for="external-auth">{{ trans('settings.users_external_auth_id') }}</label>
+            </button>
+            <div refs="collapsible@content" class="collapse-content stretch-inputs">
+                <p class="small">{{ trans('settings.users_external_auth_id_desc') }}</p>
+                @include('form.text', ['name' => 'external_auth_id'])
+            </div>
         </div>
     </div>
-@endif
+</div>
 
-@if(userCan('users-manage'))
-    <div>
-        <label for="role" class="setting-list-label">{{ trans('settings.users_role') }}</label>
-        <p class="small">{{ trans('settings.users_role_desc') }}</p>
-        <div class="mt-m">
-            @include('form.role-checkboxes', ['name' => 'roles', 'roles' => $roles])
-        </div>
+<div>
+    <label for="role" class="setting-list-label">{{ trans('settings.users_role') }}</label>
+    <p class="small">{{ trans('settings.users_role_desc') }}</p>
+    <div class="mt-m">
+        @include('form.role-checkboxes', ['name' => 'roles', 'roles' => $roles])
     </div>
-@endif
+</div>
 
 @if($authMethod === 'standard')
     <div component="new-user-password">