From c05f9875b2229a376dd8afce47a507f7471ccfa2 Mon Sep 17 00:00:00 2001 From: Chirag Chhatrala Date: Thu, 19 Dec 2024 15:34:57 +0530 Subject: [PATCH] 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. --- api/app/Http/Controllers/Forms/FormController.php | 3 +-- api/app/Http/Resources/UserResource.php | 1 - api/app/Http/Resources/WorkspaceResource.php | 1 + api/app/Models/User.php | 5 ----- api/app/Models/Workspace.php | 10 +++++++++- api/app/Policies/FormPolicy.php | 7 ++++--- api/app/Policies/TemplatePolicy.php | 4 ++-- client/components/open/tables/OpenTable.vue | 3 ++- client/components/pages/forms/show/ExtraMenu.vue | 3 ++- client/components/pages/settings/WorkSpaceUser.vue | 1 + client/pages/forms/[slug]/show.vue | 9 ++++----- client/pages/forms/[slug]/show/share.vue | 5 ++--- client/pages/home.vue | 6 ++---- client/pages/settings.vue | 11 ++++++----- client/pages/settings/workspace.vue | 12 +++++------- 15 files changed, 41 insertions(+), 40 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/Resources/UserResource.php b/api/app/Http/Resources/UserResource.php index a64d93649..a126648ac 100644 --- a/api/app/Http/Resources/UserResource.php +++ b/api/app/Http/Resources/UserResource.php @@ -19,7 +19,6 @@ public function toArray($request) 'has_enterprise_subscription' => $this->has_enterprise_subscription, 'admin' => $this->admin, 'moderator' => $this->moderator, - 'is_readonly' => $this->is_readonly, 'template_editor' => $this->template_editor, 'has_customer_id' => $this->has_customer_id, 'has_forms' => $this->has_forms, 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 f1058defb..547337cf3 100644 --- a/api/app/Models/User.php +++ b/api/app/Models/User.php @@ -128,11 +128,6 @@ public function getModeratorAttribute() return in_array($this->email, config('opnform.moderator_emails')) || $this->admin; } - public function getIsReadonlyAttribute() - { - return $this->workspaces()->where('role', self::ROLE_READONLY)->exists(); - } - public function getTemplateEditorAttribute() { return $this->admin || in_array($this->email, config('opnform.template_editor_emails')); diff --git a/api/app/Models/Workspace.php b/api/app/Models/Workspace.php index 68a0d8115..acdc1463b 100644 --- a/api/app/Models/Workspace.php +++ b/api/app/Models/Workspace.php @@ -196,11 +196,19 @@ public function owners() public function billingOwners(): Collection { - return $this->owners->filter(fn ($owner) => $owner->is_subscribed); + return $this->owners->filter(fn($owner) => $owner->is_subscribed); } public function forms() { return $this->hasMany(Form::class); } + + public function isReadonlyUser(User $user) + { + return $this->users() + ->wherePivot('user_id', $user->id) + ->wherePivot('role', User::ROLE_READONLY) + ->exists(); + } } diff --git a/api/app/Policies/FormPolicy.php b/api/app/Policies/FormPolicy.php index ea3947376..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,9 @@ public function view(User $user, Form $form) * * @return mixed */ - public function create(User $user) + public function create(User $user, Workspace $workspace) { - return !$user->is_readonly; + return !$workspace->isReadonlyUser($user); } /** @@ -45,7 +46,7 @@ public function create(User $user) */ private function canPerformWriteOperation(User $user, Form $form): bool { - return $user->ownsForm($form) && !$user->is_readonly; + return $user->ownsForm($form) && !$form->workspace->isReadonlyUser($user); } /** diff --git a/api/app/Policies/TemplatePolicy.php b/api/app/Policies/TemplatePolicy.php index 93ba2e9b0..b2e5ed305 100644 --- a/api/app/Policies/TemplatePolicy.php +++ b/api/app/Policies/TemplatePolicy.php @@ -15,7 +15,7 @@ class TemplatePolicy */ public function create(User $user) { - return $user !== null && !$user->is_readonly; + return $user !== null; } /** @@ -23,7 +23,7 @@ public function create(User $user) */ private function canPerformWriteOperation(User $user, Template $template): bool { - return ($user->admin || $user->template_editor || $template->creator_id === $user->id) && !$user->is_readonly; + return $user->admin || $user->template_editor || $template->creator_id === $user->id; } public function update(User $user, Template $template) diff --git a/client/components/open/tables/OpenTable.vue b/client/components/open/tables/OpenTable.vue index a3e26d3bd..301593bf0 100644 --- a/client/components/open/tables/OpenTable.vue +++ b/client/components/open/tables/OpenTable.vue @@ -183,6 +183,7 @@ export default { workingFormStore, form: storeToRefs(workingFormStore).content, user: useAuthStore().user, + workspace: useWorkspacesStore().getCurrent, } }, @@ -215,7 +216,7 @@ export default { computed: { hasActions() { - return !this.user.is_readonly + 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/forms/show/ExtraMenu.vue b/client/components/pages/forms/show/ExtraMenu.vue index 69b141d9d..280ff9b94 100644 --- a/client/components/pages/forms/show/ExtraMenu.vue +++ b/client/components/pages/forms/show/ExtraMenu.vue @@ -102,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) @@ -131,7 +132,7 @@ const items = computed(() => { } }] : [] ], - ...user.value.is_readonly ? [] : [ + ...workspace.value.is_readonly ? [] : [ [ ...props.isMainPage ? [{ label: 'Edit', 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
@@ -99,7 +99,7 @@ @@ -246,8 +246,6 @@ useOpnSeoMeta({ title: "Home", }) -const authStore = useAuthStore() -const user = computed(() => authStore.user) const route = useRoute() const formsStore = useFormsStore() const workingFormStore = useWorkingFormStore() @@ -257,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(() => { @@ -283,7 +282,7 @@ const tabsList = [ route: "forms-slug-show-submissions", params: { 'slug': slug } }, - ...user.value.is_readonly ? [] : [ + ...workspace.value.is_readonly ? [] : [ { name: "Integrations", route: "forms-slug-show-integrations", diff --git a/client/pages/forms/[slug]/show/share.vue b/client/pages/forms/[slug]/show/share.vue index 5a8a51b79..c4f7651ae 100644 --- a/client/pages/forms/[slug]/show/share.vue +++ b/client/pages/forms/[slug]/show/share.vue @@ -3,7 +3,7 @@
@@ -55,8 +55,7 @@ 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 authStore = useAuthStore() -const user = computed(() => authStore.user) +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 76ca2595f..7dd729446 100644 --- a/client/pages/home.vue +++ b/client/pages/home.vue @@ -8,7 +8,7 @@ Your Forms @@ -87,7 +87,7 @@ again.
authStore.user) const subscriptionModalStore = useSubscriptionModalStore() const formsStore = useFormsStore() const workspacesStore = useWorkspacesStore() diff --git a/client/pages/settings.vue b/client/pages/settings.vue index 5af8f4317..7b373f6bd 100644 --- a/client/pages/settings.vue +++ b/client/pages/settings.vue @@ -56,17 +56,18 @@ useOpnSeoMeta({ const authStore = useAuthStore() const user = computed(() => authStore.user) +const workspace = computed(() => useWorkspacesStore().getCurrent) const tabsList = computed(() => { const tabs = [ { name: "Profile", route: "settings-profile", }, - ...user.value.is_readonly ? [] : [ - { - name: "Workspace Settings", - route: "settings-workspace", - }, + { + name: "Workspace Settings", + route: "settings-workspace", + }, + ...workspace.value.is_readonly ? [] : [ { name: "Access Tokens", route: "settings-access-tokens", 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.
- - +