From 2366f9515d5b6168bec3ba1fd8650557d72042be Mon Sep 17 00:00:00 2001 From: Chirag Chhatrala <60499540+chiragchhatrala@users.noreply.github.com> Date: Mon, 30 Dec 2024 19:05:23 +0530 Subject: [PATCH] Readonly User (#637) * Readonly User * Refactor FormPolicy and TemplatePolicy to centralize write operation logic - Introduced a private method `canPerformWriteOperation` in both FormPolicy and TemplatePolicy to encapsulate the logic for determining if a user can perform write operations on the respective models. - Updated the `update`, `delete`, `restore`, and `forceDelete` methods in FormPolicy to use the new method for improved readability and maintainability. - Simplified the `update` and `delete` methods in TemplatePolicy to leverage the centralized write operation logic. This refactoring enhances code clarity and reduces duplication across policy classes. * Refactor user and workspace permissions handling - Updated FormController to authorize form creation based on workspace context. - Removed the `is_readonly` attribute from UserResource and integrated it into WorkspaceResource for better encapsulation. - Refactored User model to eliminate the `getIsReadonlyAttribute` method, shifting readonly logic to the Workspace model. - Adjusted FormPolicy and TemplatePolicy to utilize workspace readonly checks for user permissions. - Updated various frontend components to reference workspace readonly status instead of user readonly status, enhancing clarity and consistency in permission handling. These changes improve the management of user permissions in relation to workspaces, ensuring a more robust and maintainable authorization system. * Fix isReadonlyUser * fix pint --------- Co-authored-by: Julien Nahum --- .../Http/Controllers/Forms/FormController.php | 3 +-- .../Controllers/WorkspaceUserController.php | 13 ++++++------ api/app/Http/Resources/WorkspaceResource.php | 1 + api/app/Models/User.php | 7 +++++++ api/app/Models/Workspace.php | 8 +++++++ api/app/Policies/FormPolicy.php | 21 +++++++++++++------ api/app/Policies/TemplatePolicy.php | 20 +++++++----------- client/components/open/tables/OpenTable.vue | 7 ++++++- .../pages/admin/AddUserToWorkspace.vue | 3 ++- .../pages/admin/EditWorkSpaceUser.vue | 3 ++- .../pages/auth/components/RegisterForm.vue | 3 +++ .../components/pages/forms/show/ExtraMenu.vue | 14 +++++++++---- .../pages/settings/WorkSpaceUser.vue | 1 + client/pages/forms/[slug]/show.vue | 15 ++++++++----- client/pages/forms/[slug]/show/share.vue | 3 +++ client/pages/home.vue | 3 ++- client/pages/settings.vue | 19 ++++++++++------- client/pages/settings/workspace.vue | 12 +++++------ 18 files changed, 102 insertions(+), 54 deletions(-) diff --git a/api/app/Http/Controllers/Forms/FormController.php b/api/app/Http/Controllers/Forms/FormController.php index 1f99c17e4..ac987b1a9 100644 --- a/api/app/Http/Controllers/Forms/FormController.php +++ b/api/app/Http/Controllers/Forms/FormController.php @@ -108,10 +108,9 @@ public function indexAll() public function store(StoreFormRequest $request) { - $this->authorize('create', Form::class); - $workspace = Workspace::findOrFail($request->get('workspace_id')); $this->authorize('view', $workspace); + $this->authorize('create', [Form::class, $workspace]); $formData = $this->formCleaner ->processRequest($request) diff --git a/api/app/Http/Controllers/WorkspaceUserController.php b/api/app/Http/Controllers/WorkspaceUserController.php index 7848d4ccd..f58269bba 100644 --- a/api/app/Http/Controllers/WorkspaceUserController.php +++ b/api/app/Http/Controllers/WorkspaceUserController.php @@ -31,7 +31,7 @@ public function addUser(Request $request, $workspaceId) $this->validate($request, [ 'email' => 'required|email', - 'role' => 'required|in:admin,user', + 'role' => 'required|in:' . implode(',', User::ROLES), ]); $user = User::where('email', $request->email)->first(); @@ -62,10 +62,11 @@ private function inviteUser(Workspace $workspace, string $email, string $role) { if ( UserInvite::where('email', $email) - ->where('workspace_id', $workspace->id) - ->notExpired() - ->pending() - ->exists()) { + ->where('workspace_id', $workspace->id) + ->notExpired() + ->pending() + ->exists() + ) { return $this->success([ 'message' => 'User has already been invited.' ]); @@ -86,7 +87,7 @@ public function updateUserRole(Request $request, $workspaceId, $userId) $this->authorize('adminAction', $workspace); $this->validate($request, [ - 'role' => 'required|in:admin,user', + 'role' => 'required|in:' . implode(',', User::ROLES), ]); $workspace->users()->sync([ diff --git a/api/app/Http/Resources/WorkspaceResource.php b/api/app/Http/Resources/WorkspaceResource.php index a7c4b0e97..a7168796f 100644 --- a/api/app/Http/Resources/WorkspaceResource.php +++ b/api/app/Http/Resources/WorkspaceResource.php @@ -18,6 +18,7 @@ public function toArray($request) { return array_merge(parent::toArray($request), [ 'max_file_size' => $this->max_file_size / 1000000, + 'is_readonly' => $this->isReadonlyUser($request->user()), ]); } } diff --git a/api/app/Models/User.php b/api/app/Models/User.php index 3180c1c10..547337cf3 100644 --- a/api/app/Models/User.php +++ b/api/app/Models/User.php @@ -21,6 +21,13 @@ class User extends Authenticatable implements JWTSubject public const ROLE_ADMIN = 'admin'; public const ROLE_USER = 'user'; + public const ROLE_READONLY = 'readonly'; + + public const ROLES = [ + self::ROLE_ADMIN, + self::ROLE_USER, + self::ROLE_READONLY, + ]; /** * The attributes that are mass assignable. diff --git a/api/app/Models/Workspace.php b/api/app/Models/Workspace.php index 68a0d8115..274f6b33e 100644 --- a/api/app/Models/Workspace.php +++ b/api/app/Models/Workspace.php @@ -203,4 +203,12 @@ public function forms() { return $this->hasMany(Form::class); } + + public function isReadonlyUser(?User $user) + { + return $user ? $this->users() + ->wherePivot('user_id', $user->id) + ->wherePivot('role', User::ROLE_READONLY) + ->exists() : false; + } } diff --git a/api/app/Policies/FormPolicy.php b/api/app/Policies/FormPolicy.php index e35c9f84f..bdf16ca48 100644 --- a/api/app/Policies/FormPolicy.php +++ b/api/app/Policies/FormPolicy.php @@ -4,6 +4,7 @@ use App\Models\Forms\Form; use App\Models\User; +use App\Models\Workspace; use Illuminate\Auth\Access\HandlesAuthorization; class FormPolicy @@ -35,9 +36,17 @@ public function view(User $user, Form $form) * * @return mixed */ - public function create(User $user) + public function create(User $user, Workspace $workspace) { - return true; + return !$workspace->isReadonlyUser($user); + } + + /** + * Determine whether the user can perform write operations on the model. + */ + private function canPerformWriteOperation(User $user, Form $form): bool + { + return $user->ownsForm($form) && !$form->workspace->isReadonlyUser($user); } /** @@ -47,7 +56,7 @@ public function create(User $user) */ public function update(User $user, Form $form) { - return $user->ownsForm($form); + return $this->canPerformWriteOperation($user, $form); } /** @@ -57,7 +66,7 @@ public function update(User $user, Form $form) */ public function delete(User $user, Form $form) { - return $user->ownsForm($form); + return $this->canPerformWriteOperation($user, $form); } /** @@ -67,7 +76,7 @@ public function delete(User $user, Form $form) */ public function restore(User $user, Form $form) { - return $user->ownsForm($form); + return $this->canPerformWriteOperation($user, $form); } /** @@ -77,6 +86,6 @@ public function restore(User $user, Form $form) */ public function forceDelete(User $user, Form $form) { - return $user->ownsForm($form); + return $this->canPerformWriteOperation($user, $form); } } diff --git a/api/app/Policies/TemplatePolicy.php b/api/app/Policies/TemplatePolicy.php index 896f47b81..b2e5ed305 100644 --- a/api/app/Policies/TemplatePolicy.php +++ b/api/app/Policies/TemplatePolicy.php @@ -12,8 +12,6 @@ class TemplatePolicy /** * Determine whether the user can create models. - * - * @return \Illuminate\Auth\Access\Response|bool */ public function create(User $user) { @@ -21,22 +19,20 @@ public function create(User $user) } /** - * Determine whether the user can update the model. - * - * @return mixed + * Determine whether the user can perform write operations on the model. */ - public function update(User $user, Template $template) + private function canPerformWriteOperation(User $user, Template $template): bool { return $user->admin || $user->template_editor || $template->creator_id === $user->id; } - /** - * Determine whether the user can delete the model. - * - * @return mixed - */ + public function update(User $user, Template $template) + { + return $this->canPerformWriteOperation($user, $template); + } + public function delete(User $user, Template $template) { - return $user->admin || $user->template_editor || $template->creator_id === $user->id; + return $this->canPerformWriteOperation($user, $template); } } diff --git a/client/components/open/tables/OpenTable.vue b/client/components/open/tables/OpenTable.vue index d0687c898..301593bf0 100644 --- a/client/components/open/tables/OpenTable.vue +++ b/client/components/open/tables/OpenTable.vue @@ -29,6 +29,7 @@

@@ -181,6 +182,8 @@ export default { return { workingFormStore, form: storeToRefs(workingFormStore).content, + user: useAuthStore().user, + workspace: useWorkspacesStore().getCurrent, } }, @@ -188,7 +191,6 @@ export default { return { tableHash: null, skip: false, - hasActions: true, internalColumns: [], rafId: null, fieldComponents: { @@ -213,6 +215,9 @@ export default { }, computed: { + hasActions() { + return !this.workspace.is_readonly + }, formData() { return [...this.data].sort((a, b) => new Date(b.created_at) - new Date(a.created_at)) } diff --git a/client/components/pages/admin/AddUserToWorkspace.vue b/client/components/pages/admin/AddUserToWorkspace.vue index b08020620..09424bf35 100644 --- a/client/components/pages/admin/AddUserToWorkspace.vue +++ b/client/components/pages/admin/AddUserToWorkspace.vue @@ -48,7 +48,8 @@ const workspacesStore = useWorkspacesStore() const roleOptions = [ {name: "User", value: "user"}, - {name: "Admin", value: "admin"} + {name: "Admin", value: "admin"}, + {name: "Read Only", value: "readonly"} ] const newUser = ref("") diff --git a/client/components/pages/admin/EditWorkSpaceUser.vue b/client/components/pages/admin/EditWorkSpaceUser.vue index d214ba1fe..145f5bd28 100644 --- a/client/components/pages/admin/EditWorkSpaceUser.vue +++ b/client/components/pages/admin/EditWorkSpaceUser.vue @@ -18,7 +18,8 @@ :label="'New Role for '+props.user.name" :options="[ { name: 'User', value: 'user' }, - { name: 'Admin', value: 'admin' } + { name: 'Admin', value: 'admin' }, + { name: 'Read Only', value: 'readonly' }, ]" option-key="value" display-key="name" diff --git a/client/components/pages/auth/components/RegisterForm.vue b/client/components/pages/auth/components/RegisterForm.vue index a50da576b..a5334ab53 100644 --- a/client/components/pages/auth/components/RegisterForm.vue +++ b/client/components/pages/auth/components/RegisterForm.vue @@ -24,6 +24,7 @@ /> -
+
- + @@ -99,6 +102,7 @@ const authStore = useAuthStore() const formsStore = useFormsStore() const formEndpoint = "/open/forms/{id}" const user = computed(() => authStore.user) +const workspace = computed(() => useWorkspacesStore().getCurrent) const loadingDuplicate = ref(false) const loadingDelete = ref(false) @@ -128,8 +132,9 @@ const items = computed(() => { } }] : [] ], - [ - ...props.isMainPage ? [{ + ...workspace.value.is_readonly ? [] : [ + [ + ...props.isMainPage ? [{ label: 'Edit', icon: 'i-heroicons-pencil-square-20-solid', to: { name: 'forms-slug-edit', params: { slug: props.form.slug } } @@ -166,6 +171,7 @@ const items = computed(() => { class: 'text-red-800 hover:bg-red-50 hover:text-red-600 group', iconClass: 'text-red-900 group-hover:text-red-800' } + ] ] ].filter((group) => group.length > 0) }) diff --git a/client/components/pages/settings/WorkSpaceUser.vue b/client/components/pages/settings/WorkSpaceUser.vue index 1bf8cfbb0..916616b87 100644 --- a/client/components/pages/settings/WorkSpaceUser.vue +++ b/client/components/pages/settings/WorkSpaceUser.vue @@ -5,6 +5,7 @@ Workspace Members
@@ -98,6 +99,7 @@ @@ -253,6 +255,7 @@ const slug = useRoute().params.slug formsStore.startLoading() const form = computed(() => formsStore.getByKey(slug)) +const workspace = computed(() => workspacesStore.getCurrent) const loading = computed(() => formsStore.loading || workspacesStore.loading) const displayClosesDate = computed(() => { @@ -279,11 +282,13 @@ const tabsList = [ route: "forms-slug-show-submissions", params: { 'slug': slug } }, - { - name: "Integrations", - route: "forms-slug-show-integrations", - params: { 'slug': slug } - }, + ...workspace.value.is_readonly ? [] : [ + { + name: "Integrations", + route: "forms-slug-show-integrations", + params: { 'slug': slug } + }, + ], { name: "Analytics", route: "forms-slug-show-stats", diff --git a/client/pages/forms/[slug]/show/share.vue b/client/pages/forms/[slug]/show/share.vue index 8d56f2d89..c4f7651ae 100644 --- a/client/pages/forms/[slug]/show/share.vue +++ b/client/pages/forms/[slug]/show/share.vue @@ -3,6 +3,7 @@
@@ -54,6 +55,8 @@ import RegenerateFormLink from "~/components/pages/forms/show/RegenerateFormLink import AdvancedFormUrlSettings from "~/components/open/forms/components/AdvancedFormUrlSettings.vue" import EmbedFormAsPopupModal from "~/components/pages/forms/show/EmbedFormAsPopupModal.vue" +const workspace = computed(() => useWorkspacesStore().getCurrent) + const props = defineProps({ form: { type: Object, required: true }, }) diff --git a/client/pages/home.vue b/client/pages/home.vue index f0686d169..7dd729446 100644 --- a/client/pages/home.vue +++ b/client/pages/home.vue @@ -8,6 +8,7 @@ Your Forms @@ -86,7 +87,7 @@ again.
authStore.user) +const workspace = computed(() => useWorkspacesStore().getCurrent) const tabsList = computed(() => { const tabs = [ { @@ -66,14 +67,16 @@ const tabsList = computed(() => { name: "Workspace Settings", route: "settings-workspace", }, - { - name: "Access Tokens", - route: "settings-access-tokens", - }, - { - name: "Connections", - route: "settings-connections", - }, + ...workspace.value.is_readonly ? [] : [ + { + name: "Access Tokens", + route: "settings-access-tokens", + }, + { + name: "Connections", + route: "settings-connections", + }, + ], { name: "Password", route: "settings-password", diff --git a/client/pages/settings/workspace.vue b/client/pages/settings/workspace.vue index 75718b25e..3aefbc10b 100644 --- a/client/pages/settings/workspace.vue +++ b/client/pages/settings/workspace.vue @@ -9,8 +9,10 @@ You can switch to another workspace in top left corner of the page.
- - +