-
Notifications
You must be signed in to change notification settings - Fork 12k
feat: add Workflow resource to PBAC system with permission enforcement #22845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
17e3735
de134d2
abfa40b
f634378
49c52a9
7e7ce86
11462f2
95348c6
58888f5
21ec5f6
e5c2a71
52a5c70
86741a1
07aec7a
b23799d
1b00abc
be805bd
e80209a
e7523c6
d0b472b
dcd1f60
c9a4662
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { useState } from "react"; | |
|
|
||
| import { getPlaceholderAvatar } from "@calcom/lib/defaultAvatarImage"; | ||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||
| import type { WorkflowPermissions } from "@calcom/lib/server/repository/workflow-permissions"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| import { trpc } from "@calcom/trpc/react"; | ||
| import classNames from "@calcom/ui/classNames"; | ||
| import { ArrowButton } from "@calcom/ui/components/arrow-button"; | ||
|
|
@@ -52,7 +53,8 @@ export type WorkflowType = Workflow & { | |
| }; | ||
| }; | ||
| }[]; | ||
| readOnly?: boolean; | ||
| readOnly?: boolean; // Keep for backward compatibility | ||
| permissions?: WorkflowPermissions; | ||
| isOrg?: boolean; | ||
| }; | ||
| interface Props { | ||
|
|
@@ -136,7 +138,7 @@ export default function WorkflowListPage({ workflows }: Props) { | |
| : "Untitled"} | ||
| </div> | ||
| <div> | ||
| {workflow.readOnly && ( | ||
| {(workflow.permissions?.readOnly ?? workflow.readOnly) && ( | ||
| <Badge variant="gray" className="ml-2 "> | ||
| {t("readonly")} | ||
| </Badge> | ||
|
|
@@ -257,7 +259,9 @@ export default function WorkflowListPage({ workflows }: Props) { | |
| color="secondary" | ||
| variant="icon" | ||
| StartIcon="pencil" | ||
| disabled={workflow.readOnly} | ||
| disabled={ | ||
| workflow.permissions ? !workflow.permissions.canUpdate : workflow.readOnly | ||
| } | ||
| onClick={async () => await router.replace(`/workflows/${workflow.id}`)} | ||
| data-testid="edit-button" | ||
| /> | ||
|
|
@@ -270,40 +274,52 @@ export default function WorkflowListPage({ workflows }: Props) { | |
| }} | ||
| color="secondary" | ||
| variant="icon" | ||
| disabled={workflow.readOnly} | ||
| disabled={ | ||
| workflow.permissions ? !workflow.permissions.canDelete : workflow.readOnly | ||
| } | ||
| StartIcon="trash-2" | ||
| data-testid="delete-button" | ||
| /> | ||
| </Tooltip> | ||
| </ButtonGroup> | ||
| </div> | ||
| {!workflow.readOnly && ( | ||
| {(workflow.permissions?.canUpdate || | ||
| workflow.permissions?.canDelete || | ||
| (!workflow.permissions && !workflow.readOnly)) && ( | ||
|
Comment on lines
+286
to
+288
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify dropdown visibility logic for edge cases. The dropdown visibility condition is complex and should be tested to ensure it behaves correctly in all scenarios:
Let me verify the logic by checking if there are any test cases that cover these scenarios: 🏁 Script executed: #!/bin/bash
# Description: Search for test files that cover the WorkflowListPage dropdown logic
# Find test files related to WorkflowListPage
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | xargs rg -l "WorkflowListPage"
# Search for existing tests that verify dropdown behavior
rg -A 10 -B 5 "dropdown.*visible|visible.*dropdown" --type ts --type tsx
# Look for permission-related test cases
rg -A 10 -B 5 "permissions.*canUpdate|permissions.*canDelete" --type ts --type tsxLength of output: 297 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate all .spec.ts/.spec.tsx/.test.ts/.test.tsx files in the repo
test_files=$(fd --type f --extension ts --extension tsx --regex '.*\.(spec|test)\.(ts|tsx)$')
if [ -z "$test_files" ]; then
echo "No test/spec files found."
exit 0
fi
echo ">> Test files found:"
echo "$test_files"
echo
echo ">> Searching for WorkflowListPage references in test files..."
echo "$test_files" | xargs -r rg -n "WorkflowListPage" || true
echo
echo ">> Searching for dropdown usage in test files..."
echo "$test_files" | xargs -r rg -n "dropdown" || true
echo
echo ">> Searching for permissions checks (canUpdate|canDelete) in test files..."
echo "$test_files" | xargs -r rg -n "permissions.*canUpdate|permissions.*canDelete" || trueLength of output: 149105 Add coverage for WorkflowListPage dropdown logic The conditional around the dropdown in WorkflowListPage isn’t covered by any existing tests. Please add unit tests for the following scenarios against Test cases to add:
🤖 Prompt for AI Agents |
||
| <div className="block sm:hidden"> | ||
| <Dropdown> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button type="button" color="minimal" variant="icon" StartIcon="ellipsis" /> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent> | ||
| <DropdownMenuItem> | ||
| <DropdownItem | ||
| type="button" | ||
| StartIcon="pencil" | ||
| onClick={async () => await router.replace(`/workflows/${workflow.id}`)}> | ||
| {t("edit")} | ||
| </DropdownItem> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem> | ||
| <DropdownItem | ||
| type="button" | ||
| color="destructive" | ||
| StartIcon="trash-2" | ||
| onClick={() => { | ||
| setDeleteDialogOpen(true); | ||
| setwWorkflowToDeleteId(workflow.id); | ||
| }}> | ||
| {t("delete")} | ||
| </DropdownItem> | ||
| </DropdownMenuItem> | ||
| {(workflow.permissions | ||
| ? workflow.permissions.canUpdate | ||
| : !workflow.readOnly) && ( | ||
| <DropdownMenuItem> | ||
| <DropdownItem | ||
| type="button" | ||
| StartIcon="pencil" | ||
| onClick={async () => await router.replace(`/workflows/${workflow.id}`)}> | ||
| {t("edit")} | ||
| </DropdownItem> | ||
| </DropdownMenuItem> | ||
| )} | ||
| {(workflow.permissions | ||
| ? workflow.permissions.canDelete | ||
| : !workflow.readOnly) && ( | ||
| <DropdownMenuItem> | ||
| <DropdownItem | ||
| type="button" | ||
| color="destructive" | ||
| StartIcon="trash-2" | ||
| onClick={() => { | ||
| setDeleteDialogOpen(true); | ||
| setwWorkflowToDeleteId(workflow.id); | ||
| }}> | ||
| {t("delete")} | ||
| </DropdownItem> | ||
| </DropdownMenuItem> | ||
| )} | ||
| </DropdownMenuContent> | ||
| </Dropdown> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,10 @@ function WorkflowsPage({ filteredList }: PageProps) { | |
| disableMobileButton={true} | ||
| onlyShowWithNoTeams={true} | ||
| includeOrg={true} | ||
| withPermission={{ | ||
| permission: "workflow.create", | ||
| fallbackRoles: ["ADMIN", "OWNER"], | ||
| }} | ||
| /> | ||
| ) : null | ||
| }> | ||
|
|
@@ -99,6 +103,10 @@ function WorkflowsPage({ filteredList }: PageProps) { | |
| disableMobileButton={true} | ||
| onlyShowWithTeams={true} | ||
| includeOrg={true} | ||
| withPermission={{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user should still be able to create individual workflows even when the team doesn't allow it |
||
| permission: "workflow.create", | ||
| fallbackRoles: ["ADMIN", "OWNER"], | ||
| }} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This button does't show the teams the user has access to:

This one does:
