add workspace attribute check and refactor saved object acl check #93

raintygao · 2023-08-18T03:53:42Z

Description

add workspace attribute check and refactor saved object acl check

Issues Resolved

Screenshot

Testing the changes

Check List

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

codecov-commenter · 2023-08-18T09:52:15Z

Codecov Report

Merging #93 (2398e76) into workspace (a9ffe1b) will increase coverage by 0.00%.
Report is 2 commits behind head on workspace.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           workspace      #93   +/-   ##
==========================================
  Coverage      65.79%   65.79%           
==========================================
  Files           3334     3335    +1     
  Lines          64418    64445   +27     
  Branches       10250    10260   +10     
==========================================
+ Hits           42383    42402   +19     
- Misses         19466    19475    +9     
+ Partials        2569     2568    -1

Flag	Coverage Δ
Linux_1	`34.75% <ø> (+0.02%)`	⬆️
Linux_3	`42.70% <0.00%> (+<0.01%)`	⬆️
Windows_1	`34.77% <ø> (+0.02%)`	⬆️
Windows_2	`54.73% <0.00%> (-0.01%)`	⬇️
Windows_3	`42.70% <0.00%> (-0.01%)`	⬇️
Windows_4	`34.77% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
.../server/saved_objects/permission_control/client.ts	`2.70% <0.00%> (-0.16%)`	⬇️

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

ruanyl · 2023-08-21T03:26:37Z

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

    request: OpenSearchDashboardsRequest,
    permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[]
  ) {
-    if (!workspaceId) {
+    // PermissionMode here is an array which is merged by workspace type required permission and other saved object required permission.


I couldn't get this, could you elaborate a bit?

PermissionMode here is an array which is merged by workspace type required permission and other saved object required permission.

Previously, our validation logic for savedobject was:

if(isWorkspaceTypeSavedObject){ validate(workspaceType, workspaceTypeNeedPermissions) }else{ validate(type, savedObjectNeedPermissions) }

Now, we can merge workspaceTypeNeedPermissions and savedObjectNeedPermissions and unify savedObject permission check code.

validate(type, savedObjectNeedPermissions || workspaceTypeNeedPermissions)

In this way, the verification can be performed, unless there is dirty data which cannot be controlled.

ruanyl · 2023-08-21T03:53:04Z

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

-        if (this.isRelatedToWorkspace(cur.type)) {
-          acc.push(cur.id);
+      for (const object of objects) {
+        const workspacePermitted = await this.validateWorkspaceAttributesPermitted(


I'm a bit concern about the bulk action here, it looks the validation of the objects list is sequential, which means it validates the object one by one, if there was a long object list, I feel this is going to take unexpected long time. cc @wanglam

Maybe we can find some way to do these parallelly, validate 5-10 objects one time. What do you think about that? I think this is out of this PR scope, we can separate one task to optimize it.

The logic here is a little tricky. If we want to bulk update the four objects [a, b, c, d], if a and b only have the operation permission on the workspace attribute, and c and d only have the operation permission on the object, then the update operation can still succeed. This means we have to do validation based on the single object.

From the perspective of experience, we can do several verifications based on single object at the same time in a new task.

Signed-off-by: tygao <tygao@amazon.com>

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

Signed-off-by: tygao <tygao@amazon.com>

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

…ects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

…ects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>

Signed-off-by: tygao <tygao@amazon.com>

…to validateAtLeastOnePermittedWorkspaces Signed-off-by: tygao <tygao@amazon.com>

* feat: add permission check for saved objects Signed-off-by: tygao <tygao@amazon.com> * feat: add attribute check Signed-off-by: tygao <tygao@amazon.com> * merge workspace error fix Signed-off-by: tygao <tygao@amazon.com> * update addToWorkspacesWithPermissionControl Signed-off-by: tygao <tygao@amazon.com> * feat: add workspace num check Signed-off-by: tygao <tygao@amazon.com> * chore: pass right access Signed-off-by: tygao <tygao@amazon.com> * chore: merge validateSingleObjectPermissions Signed-off-by: tygao <tygao@amazon.com> * chore: merge validateSingleObjectPermissions Signed-off-by: tygao <tygao@amazon.com> * chore: update bulkUpdate and update logic Signed-off-by: tygao <tygao@amazon.com> * feat: remove attribute check Signed-off-by: tygao <tygao@amazon.com> * feat: pass validate for config object Signed-off-by: tygao <tygao@amazon.com> * Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com> * Update src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com> * update Signed-off-by: tygao <tygao@amazon.com> * chore: update update and bulkUpdate to reduce calling get function Signed-off-by: tygao <tygao@amazon.com> * fix: change validateUpdateWithWorkspacePermission from validateMulti to validateAtLeastOnePermittedWorkspaces Signed-off-by: tygao <tygao@amazon.com> --------- Signed-off-by: tygao <tygao@amazon.com> Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>

github-actions bot added the valued-contributor label Aug 18, 2023

wanglam reviewed Aug 18, 2023

View reviewed changes

ruanyl reviewed Aug 21, 2023

View reviewed changes

ruanyl mentioned this pull request Aug 22, 2023

fix: incorrect permission modes when checking workspace permissions #98

Merged

8 tasks

raintygao added 9 commits August 22, 2023 15:03

feat: add permission check for saved objects

7ecb30c

Signed-off-by: tygao <tygao@amazon.com>

feat: add attribute check

2f9c173

Signed-off-by: tygao <tygao@amazon.com>

merge workspace error fix

3f9b8e9

Signed-off-by: tygao <tygao@amazon.com>

update addToWorkspacesWithPermissionControl

78477ed

Signed-off-by: tygao <tygao@amazon.com>

feat: add workspace num check

bf64cf0

Signed-off-by: tygao <tygao@amazon.com>

chore: pass right access

677ba1a

Signed-off-by: tygao <tygao@amazon.com>

chore: merge validateSingleObjectPermissions

cfa5cf4

Signed-off-by: tygao <tygao@amazon.com>

chore: merge validateSingleObjectPermissions

43e3713

Signed-off-by: tygao <tygao@amazon.com>

chore: update bulkUpdate and update logic

3c86bcb

Signed-off-by: tygao <tygao@amazon.com>

raintygao force-pushed the feat-object-acl branch from b9fdd4e to 3c86bcb Compare August 22, 2023 08:01

wanglam reviewed Aug 22, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

raintygao added 2 commits August 22, 2023 16:41

feat: remove attribute check

df83981

Signed-off-by: tygao <tygao@amazon.com>

feat: pass validate for config object

81b0a45

Signed-off-by: tygao <tygao@amazon.com>

wanglam approved these changes Aug 23, 2023

View reviewed changes

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

Update src/plugins/workspace/server/saved_objects/workspace_saved_obj…

836c22d

…ects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Outdated Show resolved Hide resolved

ruanyl reviewed Aug 23, 2023

View reviewed changes

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts Show resolved Hide resolved

raintygao and others added 4 commits August 23, 2023 17:27

Update src/plugins/workspace/server/saved_objects/workspace_saved_obj…

7a64af2

…ects_client_wrapper.ts Co-authored-by: Yulong Ruan <ruanyu1@gmail.com>

update

885229f

Signed-off-by: tygao <tygao@amazon.com>

chore: update update and bulkUpdate to reduce calling get function

d2abb9a

Signed-off-by: tygao <tygao@amazon.com>

fix: change validateUpdateWithWorkspacePermission from validateMulti …

2398e76

…to validateAtLeastOnePermittedWorkspaces Signed-off-by: tygao <tygao@amazon.com>

raintygao merged commit d384519 into ruanyl:workspace Aug 25, 2023
44 of 47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add workspace attribute check and refactor saved object acl check #93

add workspace attribute check and refactor saved object acl check #93

raintygao commented Aug 18, 2023

codecov-commenter commented Aug 18, 2023 •

edited

Loading

ruanyl Aug 21, 2023

raintygao Aug 21, 2023 •

edited

Loading

ruanyl Aug 21, 2023

wanglam Aug 21, 2023

raintygao Aug 21, 2023 •

edited

Loading

raintygao Aug 21, 2023 •

edited

Loading

add workspace attribute check and refactor saved object acl check #93

add workspace attribute check and refactor saved object acl check #93

Conversation

raintygao commented Aug 18, 2023

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov-commenter commented Aug 18, 2023 • edited Loading

Codecov Report

ruanyl Aug 21, 2023

Choose a reason for hiding this comment

raintygao Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

ruanyl Aug 21, 2023

Choose a reason for hiding this comment

wanglam Aug 21, 2023

Choose a reason for hiding this comment

raintygao Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

raintygao Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Aug 18, 2023 •

edited

Loading

raintygao Aug 21, 2023 •

edited

Loading

raintygao Aug 21, 2023 •

edited

Loading

raintygao Aug 21, 2023 •

edited

Loading