From f812f2af4d81a8ae83e743ea2a80e4cacbd3cc94 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 13 Apr 2024 17:35:34 -0700 Subject: [PATCH] Fix: remove admin.logentry perm, use admin (staff) status (#6380) --- docs/usage.md | 6 ++- src-ui/src/app/app-routing.module.ts | 5 +-- .../admin/settings/settings.component.html | 43 ++++++++++--------- .../admin/settings/settings.component.spec.ts | 1 + .../admin/settings/settings.component.ts | 9 +--- .../app-frame/app-frame.component.html | 16 ++++--- .../user-edit-dialog.component.html | 6 ++- .../user-edit-dialog.component.ts | 1 + src-ui/src/app/guards/permissions.guard.ts | 10 +++-- .../app/services/permissions.service.spec.ts | 21 +++++++++ .../src/app/services/permissions.service.ts | 5 ++- src/documents/permissions.py | 2 +- src/documents/tests/test_api_permissions.py | 1 + src/documents/tests/test_api_uisettings.py | 1 + src/documents/views.py | 1 + 15 files changed, 81 insertions(+), 47 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index d77b3b2a6..7cedb976a 100644 --- 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. | diff --git a/src-ui/src/app/app-routing.module.ts b/src-ui/src/app/app-routing.module.ts index 3eebd31bd..12b412f67 100644 --- 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 diff --git a/src-ui/src/app/components/admin/settings/settings.component.html b/src-ui/src/app/components/admin/settings/settings.component.html index 42147a9b8..0fc744edb 100644 --- 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 @@ - - - Open Django Admin -   - + System Status + + + Open Django Admin +   + + }
diff --git a/src-ui/src/app/components/admin/settings/settings.component.spec.ts b/src-ui/src/app/components/admin/settings/settings.component.spec.ts index 6110f7d1d..d53f57b69 100644 --- 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() diff --git a/src-ui/src/app/components/admin/settings/settings.component.ts b/src-ui/src/app/components/admin/settings/settings.component.ts index f04af2f9d..33f6949a1 100644 --- 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 }) diff --git a/src-ui/src/app/components/app-frame/app-frame.component.html b/src-ui/src/app/components/app-frame/app-frame.component.html index b79f99cc0..bdc8d08f2 100644 --- 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 @@ } -
  • - -  Logs - -
    • + @if (permissionsService.isAdmin()) { +
  • + +  Logs + +
    • + }
  • -
    +
    +
    + + +
    diff --git 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 index ebbf0766e..baadfa541 100644 --- 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([]), diff --git a/src-ui/src/app/guards/permissions.guard.ts b/src-ui/src/app/guards/permissions.guard.ts index 77bf6071a..402cc00f5 100644 --- 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) { diff --git a/src-ui/src/app/services/permissions.service.spec.ts b/src-ui/src/app/services/permissions.service.spec.ts index 61e7c9978..f4e01945e 100644 --- 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() + }) }) diff --git a/src-ui/src/app/services/permissions.service.ts b/src-ui/src/app/services/permissions.service.ts index 29b0c1a22..0648f461f 100644 --- 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 || diff --git a/src/documents/permissions.py b/src/documents/permissions.py index bdd6fd555..d16d7aa1c 100644 --- 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): diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 92e47a1ed..d7131b834 100644 --- 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") diff --git a/src/documents/tests/test_api_uisettings.py b/src/documents/tests/test_api_uisettings.py index cb85de91a..2cb6af6f2 100644 --- 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, diff --git a/src/documents/views.py b/src/documents/views.py index 655108f05..5841649d0 100644 --- 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)), }