Fix: remove admin.logentry perm, use admin (staff) status (#6380)

2025-04-02 13:45:10 -05:00 · 2024-04-13 17:35:34 -07:00 · 2024-04-13 17:35:34 -07:00 · f812f2af4d
commit f812f2af4d
parent 47b4a602a7
15 changed files with 81 additions and 47 deletions
--- a/docs/usage.md
+++ b/docs/usage.md
@ -241,6 +241,11 @@ permissions can be granted to limit access to certain parts of the UI (and corre

    Superusers can access all parts of the front and backend application as well as any and all objects.

+#### Admin Status
+
+Admin status (Django 'staff status') grants access to viewing the paperless logs and the system status dialog
+as well as accessing the Django backend.
+
 #### Detailed Explanation of Global Permissions {#global-permissions}

 Global permissions define what areas of the app and API endpoints the user can access. For example, they
@ -249,7 +254,6 @@ still have "object-level" permissions.

 | Type          | Details                                                                                                                                                                                             |
 | ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| Admin         | _View_ or higher permissions grants access to the logs view as well as the system status.                                                                                                           |
 | AppConfig     | _Change_ or higher permissions grants access to the "Application Configuration" area.                                                                                                               |
 | Correspondent | Grants global permissions to add, edit, delete or view Correspondents.                                                                                                                              |
 | CustomField   | Grants global permissions to add, edit, delete or view Custom Fields.                                                                                                                               |
--- a/src-ui/src/app/app-routing.module.ts
+++ b/src-ui/src/app/app-routing.module.ts
@ -141,10 +141,7 @@ export const routes: Routes = [
        component: LogsComponent,
        canActivate: [PermissionsGuard],
        data: {
-          requiredPermission: {
-            action: PermissionAction.View,
-            type: PermissionType.Admin,
-          },
+          requireAdmin: true,
        },
      },
      // redirect old paths
--- a/src-ui/src/app/components/admin/settings/settings.component.html
+++ b/src-ui/src/app/components/admin/settings/settings.component.html
@ -7,29 +7,30 @@
  <button class="btn btn-sm btn-outline-primary" (click)="tourService.start()">
    <i-bs class="me-1" name="airplane"></i-bs>&nbsp;<ng-container i18n>Start tour</ng-container>
  </button>
-  <button class="btn btn-sm btn-outline-primary position-relative ms-md-5 me-1" (click)="showSystemStatus()"
-    [disabled]="!systemStatus"
-    *pngxIfPermissions="{ action: PermissionAction.View, type: PermissionType.Admin }">
-    @if (!systemStatus) {
-      <div class="spinner-border spinner-border-sm me-1 h-75" role="status"></div>
-    } @else {
-      <i-bs class="me-2" name="card-checklist"></i-bs>
-      @if (systemStatusHasErrors) {
-        <span class="badge bg-body position-absolute top-0 start-100 translate-middle rounded-pill p-0">
-          <i-bs name="exclamation-circle-fill" class="text-danger" width="1.75em" height="1.75em"></i-bs>
-        </span>
+  @if (permissionsService.isAdmin()) {
+    <button class="btn btn-sm btn-outline-primary position-relative ms-md-5 me-1" (click)="showSystemStatus()"
+      [disabled]="!systemStatus">
+      @if (!systemStatus) {
+        <div class="spinner-border spinner-border-sm me-1 h-75" role="status"></div>
      } @else {
-        <span class="badge bg-body position-absolute top-0 start-100 translate-middle rounded-pill p-0">
-          <i-bs name="check-circle-fill" class="text-primary" width="1.75em" height="1.75em"></i-bs>
-        </span>
+        <i-bs class="me-2" name="card-checklist"></i-bs>
+        @if (systemStatusHasErrors) {
+          <span class="badge bg-body position-absolute top-0 start-100 translate-middle rounded-pill p-0">
+            <i-bs name="exclamation-circle-fill" class="text-danger" width="1.75em" height="1.75em"></i-bs>
+          </span>
+        } @else {
+          <span class="badge bg-body position-absolute top-0 start-100 translate-middle rounded-pill p-0">
+            <i-bs name="check-circle-fill" class="text-primary" width="1.75em" height="1.75em"></i-bs>
+          </span>
+        }
      }
-    }
-    <ng-container i18n>System Status</ng-container>
-  </button>
-  <a *pngxIfPermissions="{ action: PermissionAction.View, type: PermissionType.Admin }" class="btn btn-sm btn-primary" href="admin/" target="_blank">
-    <ng-container i18n>Open Django Admin</ng-container>
-    &nbsp;<i-bs name="arrow-up-right"></i-bs>
-  </a>
+      <ng-container i18n>System Status</ng-container>
+    </button>
+    <a class="btn btn-sm btn-primary" href="admin/" target="_blank">
+      <ng-container i18n>Open Django Admin</ng-container>
+      &nbsp;<i-bs name="arrow-up-right"></i-bs>
+    </a>
+  }
 </pngx-page-header>

 <form [formGroup]="settingsForm" (ngSubmit)="saveSettings()">
--- a/src-ui/src/app/components/admin/settings/settings.component.spec.ts
+++ b/src-ui/src/app/components/admin/settings/settings.component.spec.ts
@ -418,6 +418,7 @@ describe('SettingsComponent', () => {
      },
    }
    jest.spyOn(systemStatusService, 'get').mockReturnValue(of(status))
+    jest.spyOn(permissionsService, 'isAdmin').mockReturnValue(true)
    completeSetup()
    expect(component['systemStatus']).toEqual(status) // private
    expect(component.systemStatusHasErrors).toBeTruthy()
--- a/src-ui/src/app/components/admin/settings/settings.component.ts
+++ b/src-ui/src/app/components/admin/settings/settings.component.ts
@ -121,7 +121,7 @@ export class SettingsComponent
  users: User[]
  groups: Group[]

-  private systemStatus: SystemStatus
+  public systemStatus: SystemStatus

  get systemStatusHasErrors(): boolean {
    return (
@ -385,12 +385,7 @@ export class SettingsComponent
      this.settingsForm.patchValue(currentFormValue)
    }

-    if (
-      this.permissionsService.currentUserCan(
-        PermissionAction.View,
-        PermissionType.Admin
-      )
-    ) {
+    if (this.permissionsService.isAdmin()) {
      this.systemStatusService.get().subscribe((status) => {
        this.systemStatus = status
      })
--- a/src-ui/src/app/components/app-frame/app-frame.component.html
+++ b/src-ui/src/app/components/app-frame/app-frame.component.html
@ -267,13 +267,15 @@
                }
              </a>
            </li>
-            <li class="nav-item app-link" *pngxIfPermissions="{ action: PermissionAction.View, type: PermissionType.Admin }">
-              <a class="nav-link" routerLink="logs" routerLinkActive="active" (click)="closeMenu()" ngbPopover="Logs"
-                i18n-ngbPopover [disablePopover]="!slimSidebarEnabled" placement="end" container="body"
-                triggers="mouseenter:mouseleave" popoverClass="popover-slim">
-                <i-bs class="me-1" name="text-left"></i-bs><span>&nbsp;<ng-container i18n>Logs</ng-container></span>
-              </a>
-            </li>
+            @if (permissionsService.isAdmin()) {
+              <li class="nav-item app-link">
+                <a class="nav-link" routerLink="logs" routerLinkActive="active" (click)="closeMenu()" ngbPopover="Logs"
+                  i18n-ngbPopover [disablePopover]="!slimSidebarEnabled" placement="end" container="body"
+                  triggers="mouseenter:mouseleave" popoverClass="popover-slim">
+                  <i-bs class="me-1" name="text-left"></i-bs><span>&nbsp;<ng-container i18n>Logs</ng-container></span>
+                </a>
+              </li>
+            }
            <li class="nav-item mt-2" tourAnchor="tour.outro">
              <a class="px-3 py-2 text-muted small d-flex align-items-center flex-wrap text-decoration-none"
                target="_blank" rel="noopener noreferrer" href="https://docs.paperless-ngx.com" ngbPopover="Documentation"
--- a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.html
+++ b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.html
@ -16,11 +16,15 @@
          <pngx-input-text i18n-title title="First name" formControlName="first_name" [error]="error?.first_name"></pngx-input-text>
          <pngx-input-text i18n-title title="Last name" formControlName="last_name" [error]="error?.first_name"></pngx-input-text>

-          <div class="mb-2">
+          <div class="mb-2 d-flex flex-column">
            <div class="form-check form-switch form-check-inline">
              <input type="checkbox" class="form-check-input" id="is_active" formControlName="is_active">
              <label class="form-check-label" for="is_active" i18n>Active</label>
            </div>
+            <div class="form-check form-switch form-check-inline">
+              <input type="checkbox" class="form-check-input" id="is_staff" formControlName="is_staff">
+              <label class="form-check-label" for="is_staff"><ng-container i18n>Admin</ng-container> <small class="form-text text-muted ms-1" i18n>Access logs, Django backend</small></label>
+            </div>
            <div class="form-check form-switch form-check-inline">
              <input type="checkbox" class="form-check-input" id="is_superuser" formControlName="is_superuser" (change)="onToggleSuperUser()">
              <label class="form-check-label" for="is_superuser"><ng-container i18n>Superuser</ng-container> <small class="form-text text-muted ms-1" i18n>(Grants all permissions and can view objects)</small></label>
--- a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts
+++ b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts
@ -56,6 +56,7 @@ export class UserEditDialogComponent
      first_name: new FormControl(''),
      last_name: new FormControl(''),
      is_active: new FormControl(true),
+      is_staff: new FormControl(true),
      is_superuser: new FormControl(false),
      groups: new FormControl([]),
      user_permissions: new FormControl([]),
--- a/src-ui/src/app/guards/permissions.guard.ts
+++ b/src-ui/src/app/guards/permissions.guard.ts
@ -23,10 +23,12 @@ export class PermissionsGuard {
    state: RouterStateSnapshot
  ): boolean | UrlTree {
    if (
-      !this.permissionsService.currentUserCan(
-        route.data.requiredPermission.action,
-        route.data.requiredPermission.type
-      )
+      (route.data.requireAdmin && !this.permissionsService.isAdmin()) ||
+      (route.data.requiredPermission &&
+        !this.permissionsService.currentUserCan(
+          route.data.requiredPermission.action,
+          route.data.requiredPermission.type
+        ))
    ) {
      // Check if tour is running 1 = TourState.ON
      if (this.tourService.getStatus() !== 1) {
--- a/src-ui/src/app/services/permissions.service.spec.ts
+++ b/src-ui/src/app/services/permissions.service.spec.ts
@ -418,4 +418,25 @@ describe('PermissionsService', () => {
      )
    ).toBeTruthy()
  })
+
+  it('correctly checks admin status', () => {
+    permissionsService.initialize([], {
+      username: 'testuser',
+      last_name: 'User',
+      first_name: 'Test',
+      id: 1,
+      is_staff: true,
+    })
+
+    expect(permissionsService.isAdmin()).toBeTruthy()
+
+    permissionsService.initialize([], {
+      username: 'testuser',
+      last_name: 'User',
+      first_name: 'Test',
+      id: 1,
+    })
+
+    expect(permissionsService.isAdmin()).toBeFalsy()
+  })
 })
--- a/src-ui/src/app/services/permissions.service.ts
+++ b/src-ui/src/app/services/permissions.service.ts
@ -24,7 +24,6 @@ export enum PermissionType {
  MailRule = '%s_mailrule',
  User = '%s_user',
  Group = '%s_group',
-  Admin = '%s_logentry',
  ShareLink = '%s_sharelink',
  CustomField = '%s_customfield',
  Workflow = '%s_workflow',
@ -52,6 +51,10 @@ export class PermissionsService {
    )
  }

+  public isAdmin(): boolean {
+    return this.currentUser?.is_staff
+  }
+
  public currentUserOwnsObject(object: ObjectWithPermissions): boolean {
    return (
      !object ||
--- a/src/documents/permissions.py
+++ b/src/documents/permissions.py
@ -40,7 +40,7 @@ class PaperlessObjectPermissions(DjangoObjectPermissions):

 class PaperlessAdminPermissions(BasePermission):
    def has_permission(self, request, view):
-        return request.user.has_perm("admin.view_logentry")
+        return request.user.is_staff


 def get_groups_with_only_permission(obj, codename):
--- a/src/documents/tests/test_api_permissions.py
+++ b/src/documents/tests/test_api_permissions.py
@ -131,6 +131,7 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
    def test_api_sufficient_permissions(self):
        user = User.objects.create_user(username="test")
        user.user_permissions.add(*Permission.objects.all())
+        user.is_staff = True
        self.client.force_authenticate(user)

        Document.objects.create(title="Test")
--- a/src/documents/tests/test_api_uisettings.py
+++ b/src/documents/tests/test_api_uisettings.py
@ -27,6 +27,7 @@ class TestApiUiSettings(DirectoriesMixin, APITestCase):
            {
                "id": self.test_user.id,
                "username": self.test_user.username,
+                "is_staff": True,
                "is_superuser": True,
                "groups": [],
                "first_name": self.test_user.first_name,
--- a/src/documents/views.py
+++ b/src/documents/views.py
@ -1270,6 +1270,7 @@ class UiSettingsView(GenericAPIView):
        user_resp = {
            "id": user.id,
            "username": user.username,
+            "is_staff": user.is_staff,
            "is_superuser": user.is_superuser,
            "groups": list(user.groups.values_list("id", flat=True)),
        }