Skip to content
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

[Workspace][Feature] Add ACL related functions #5084

Merged
merged 38 commits into from
Mar 5, 2024

Conversation

gaobinlong
Copy link
Contributor

@gaobinlong gaobinlong commented Sep 21, 2023

Description

The main change of this PR are:

  • Add permissions field to the default mapping of the OSD index(.kibana) to support ACL for workspace
  • Update the test snapshot after we change the default mapping of the OSD index
  • Add ACL class and basic functions

Issues Resolved

#5083

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

…ct#146)

* [Workspace] Add acl related functions for workspace

Signed-off-by: gaobinlong <[email protected]>

* Minor change

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: gaobinlong <[email protected]>
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Attention: Patch coverage is 89.89899% with 10 lines in your changes missing coverage. Please review.

Project coverage is 67.13%. Comparing base (5ef10da) to head (b80101a).
Report is 629 commits behind head on main.

Files with missing lines Patch % Lines
...ore/server/saved_objects/permission_control/acl.ts 87.50% 3 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5084      +/-   ##
==========================================
+ Coverage   67.10%   67.13%   +0.03%     
==========================================
  Files        3315     3316       +1     
  Lines       63922    64015      +93     
  Branches    10220    10256      +36     
==========================================
+ Hits        42892    42975      +83     
- Misses      18544    18547       +3     
- Partials     2486     2493       +7     
Flag Coverage Δ
Linux_1 31.63% <0.00%> (-0.01%) ⬇️
Linux_2 55.20% <89.89%> (+0.13%) ⬆️
Linux_3 44.57% <0.00%> (-0.02%) ⬇️
Linux_4 35.16% <0.00%> (-0.02%) ⬇️
Windows_1 31.65% <0.00%> (-0.01%) ⬇️
Windows_2 55.17% <89.89%> (+0.13%) ⬆️
Windows_3 44.59% <0.00%> (-0.02%) ⬇️
Windows_4 35.16% <0.00%> (-0.02%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: gaobinlong <[email protected]>
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening!

IMO, it might be worth to have this one hooked into the saved object functionality for us to see how it is being used and to give suggestions.

I see it on the mappings but I'm not sure how it interacts for existing data barring the mapping addition.

Also, just a heads up but I'm sure you already know. I'm really easily able to pass a compressed file of a 100+ MB files to the saved objects API. But the default size of the saved objects payload is quite large. Some users saved objects are sometimes too large that it is really not as performant. So based off the design proposal seeing how these permission controls are being used might make a big difference for our suggestions and increased latency to a potential slower response.

Would also suggest adding a configuration here that gives cluster admins the ability to disable permission controls? But I'm not sure how that impacts your design proposal but would make me sleep easier at night when we are filtering responses.

Final thing, which I tried seeing in the design, I'm pretty sure the saved objects retrieval is slightly cached and it might refresh at specific times. So I can see it having an impact on design if we want to worry about where to store this information but seems like a difficult issue to solve. For example, what do we plan to do as a user who originally had access, could still get the saved objects. Will they have the ability to have write permissions still in the cache state. They also still have access to the data and resending the request without refreshing the page.

I will paste a links to this on the design doc.

Thanks again!

Flyingliuhub
Flyingliuhub previously approved these changes Mar 2, 2024
Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: gaobinlong <[email protected]>
Copy link
Member

@ruanyl ruanyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job! Appreciate your dedication to refining the code according to the comments and discussions. It looks good to me now. 👍

@ruanyl ruanyl merged commit 54c36fe into opensearch-project:main Mar 5, 2024
67 checks passed
gaobinlong added a commit to gaobinlong/OpenSearch-Dashboards that referenced this pull request Mar 5, 2024
* [Workspace] Add ACL related functions for workspace (opensearch-project#146)

* [Workspace] Add acl related functions for workspace

Signed-off-by: gaobinlong <[email protected]>

* Minor change

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>

* Modify changelog

Signed-off-by: gaobinlong <[email protected]>

* Add more unit test cases

Signed-off-by: gaobinlong <[email protected]>

* Modify test case

Signed-off-by: gaobinlong <[email protected]>

* Some minor change

Signed-off-by: gaobinlong <[email protected]>

* Add more test cases

Signed-off-by: gaobinlong <[email protected]>

* Optimize some code and the comments of the functions

Signed-off-by: gaobinlong <[email protected]>

* Add more comments for some basic functions

Signed-off-by: gaobinlong <[email protected]>

* Export more interfaces

Signed-off-by: gaobinlong <[email protected]>

* consume permissions in repository

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: consume permissions in serializer

Signed-off-by: SuZhou-Joe <[email protected]>

* Add unit tests for consuming permissions in repository

Signed-off-by: gaobinlong <[email protected]>

* Remove double exclamation

Signed-off-by: gaobinlong <[email protected]>

* Rename some variables

Signed-off-by: gaobinlong <[email protected]>

* Remove duplicated semicolon

Signed-off-by: gaobinlong <[email protected]>

* Add permissions field to the mapping only if the permission control is enabled

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

* Add feature flag config to the yml file

Signed-off-by: gaobinlong <[email protected]>

* Make the comment of feature flag more clear

Signed-off-by: gaobinlong <[email protected]>

* Make comment more clear

Signed-off-by: gaobinlong <[email protected]>

* Remove management permission type

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
gaobinlong added a commit to gaobinlong/OpenSearch-Dashboards that referenced this pull request Mar 8, 2024
* [Workspace] Add ACL related functions for workspace (opensearch-project#146)

* [Workspace] Add acl related functions for workspace

Signed-off-by: gaobinlong <[email protected]>

* Minor change

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>

* Modify changelog

Signed-off-by: gaobinlong <[email protected]>

* Add more unit test cases

Signed-off-by: gaobinlong <[email protected]>

* Modify test case

Signed-off-by: gaobinlong <[email protected]>

* Some minor change

Signed-off-by: gaobinlong <[email protected]>

* Add more test cases

Signed-off-by: gaobinlong <[email protected]>

* Optimize some code and the comments of the functions

Signed-off-by: gaobinlong <[email protected]>

* Add more comments for some basic functions

Signed-off-by: gaobinlong <[email protected]>

* Export more interfaces

Signed-off-by: gaobinlong <[email protected]>

* consume permissions in repository

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: consume permissions in serializer

Signed-off-by: SuZhou-Joe <[email protected]>

* Add unit tests for consuming permissions in repository

Signed-off-by: gaobinlong <[email protected]>

* Remove double exclamation

Signed-off-by: gaobinlong <[email protected]>

* Rename some variables

Signed-off-by: gaobinlong <[email protected]>

* Remove duplicated semicolon

Signed-off-by: gaobinlong <[email protected]>

* Add permissions field to the mapping only if the permission control is enabled

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

* Add feature flag config to the yml file

Signed-off-by: gaobinlong <[email protected]>

* Make the comment of feature flag more clear

Signed-off-by: gaobinlong <[email protected]>

* Make comment more clear

Signed-off-by: gaobinlong <[email protected]>

* Remove management permission type

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
gaobinlong added a commit to ruanyl/OpenSearch-Dashboards that referenced this pull request Mar 8, 2024
…) (#287)

* [Workspace] Add ACL related functions for workspace (#146)

* [Workspace] Add acl related functions for workspace



* Minor change



---------



* Modify changelog



* Add more unit test cases



* Modify test case



* Some minor change



* Add more test cases



* Optimize some code and the comments of the functions



* Add more comments for some basic functions



* Export more interfaces



* consume permissions in repository



* feat: consume permissions in serializer



* Add unit tests for consuming permissions in repository



* Remove double exclamation



* Rename some variables



* Remove duplicated semicolon



* Add permissions field to the mapping only if the permission control is enabled



* Fix test failure



* Add feature flag config to the yml file



* Make the comment of feature flag more clear



* Make comment more clear



* Remove management permission type



* Fix test failure



---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
SuZhou-Joe added a commit to ruanyl/OpenSearch-Dashboards that referenced this pull request Mar 11, 2024
* [Workspace][Feature] Add ACL related functions (opensearch-project#5084)

* [Workspace] Add ACL related functions for workspace (#146)

* [Workspace] Add acl related functions for workspace

Signed-off-by: gaobinlong <[email protected]>

* Minor change

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>

* Modify changelog

Signed-off-by: gaobinlong <[email protected]>

* Add more unit test cases

Signed-off-by: gaobinlong <[email protected]>

* Modify test case

Signed-off-by: gaobinlong <[email protected]>

* Some minor change

Signed-off-by: gaobinlong <[email protected]>

* Add more test cases

Signed-off-by: gaobinlong <[email protected]>

* Optimize some code and the comments of the functions

Signed-off-by: gaobinlong <[email protected]>

* Add more comments for some basic functions

Signed-off-by: gaobinlong <[email protected]>

* Export more interfaces

Signed-off-by: gaobinlong <[email protected]>

* consume permissions in repository

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: consume permissions in serializer

Signed-off-by: SuZhou-Joe <[email protected]>

* Add unit tests for consuming permissions in repository

Signed-off-by: gaobinlong <[email protected]>

* Remove double exclamation

Signed-off-by: gaobinlong <[email protected]>

* Rename some variables

Signed-off-by: gaobinlong <[email protected]>

* Remove duplicated semicolon

Signed-off-by: gaobinlong <[email protected]>

* Add permissions field to the mapping only if the permission control is enabled

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

* Add feature flag config to the yml file

Signed-off-by: gaobinlong <[email protected]>

* Make the comment of feature flag more clear

Signed-off-by: gaobinlong <[email protected]>

* Make comment more clear

Signed-off-by: gaobinlong <[email protected]>

* Remove management permission type

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>

* Remove duplicated attribute

Signed-off-by: gaobinlong <[email protected]>

* Enable acl in integration tests for workspace client wrapper

Signed-off-by: gaobinlong <[email protected]>

* Format the code

Signed-off-by: gaobinlong <[email protected]>

* Modify feature flag

Signed-off-by: gaobinlong <[email protected]>

* Modify feature flag to false

Signed-off-by: gaobinlong <[email protected]>

* Modify feature flag to true

Signed-off-by: gaobinlong <[email protected]>

* Enhance the test case

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
SuZhou-Joe added a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Mar 18, 2024
…) (opensearch-project#287)

* [Workspace] Add ACL related functions for workspace (opensearch-project#146)

* [Workspace] Add acl related functions for workspace



* Minor change



---------



* Modify changelog



* Add more unit test cases



* Modify test case



* Some minor change



* Add more test cases



* Optimize some code and the comments of the functions



* Add more comments for some basic functions



* Export more interfaces



* consume permissions in repository



* feat: consume permissions in serializer



* Add unit tests for consuming permissions in repository



* Remove double exclamation



* Rename some variables



* Remove duplicated semicolon



* Add permissions field to the mapping only if the permission control is enabled



* Fix test failure



* Add feature flag config to the yml file



* Make the comment of feature flag more clear



* Make comment more clear



* Remove management permission type



* Fix test failure



---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 11, 2024
* [Workspace] Add ACL related functions for workspace (#146)

* [Workspace] Add acl related functions for workspace

Signed-off-by: gaobinlong <[email protected]>

* Minor change

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>

* Modify changelog

Signed-off-by: gaobinlong <[email protected]>

* Add more unit test cases

Signed-off-by: gaobinlong <[email protected]>

* Modify test case

Signed-off-by: gaobinlong <[email protected]>

* Some minor change

Signed-off-by: gaobinlong <[email protected]>

* Add more test cases

Signed-off-by: gaobinlong <[email protected]>

* Optimize some code and the comments of the functions

Signed-off-by: gaobinlong <[email protected]>

* Add more comments for some basic functions

Signed-off-by: gaobinlong <[email protected]>

* Export more interfaces

Signed-off-by: gaobinlong <[email protected]>

* consume permissions in repository

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: consume permissions in serializer

Signed-off-by: SuZhou-Joe <[email protected]>

* Add unit tests for consuming permissions in repository

Signed-off-by: gaobinlong <[email protected]>

* Remove double exclamation

Signed-off-by: gaobinlong <[email protected]>

* Rename some variables

Signed-off-by: gaobinlong <[email protected]>

* Remove duplicated semicolon

Signed-off-by: gaobinlong <[email protected]>

* Add permissions field to the mapping only if the permission control is enabled

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

* Add feature flag config to the yml file

Signed-off-by: gaobinlong <[email protected]>

* Make the comment of feature flag more clear

Signed-off-by: gaobinlong <[email protected]>

* Make comment more clear

Signed-off-by: gaobinlong <[email protected]>

* Remove management permission type

Signed-off-by: gaobinlong <[email protected]>

* Fix test failure

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
(cherry picked from commit 54c36fe)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ananzh pushed a commit that referenced this pull request Apr 12, 2024
* [Workspace] Add ACL related functions for workspace (#146)

* consume permissions in repository
* feat: consume permissions in serializer
* Add unit tests for consuming permissions in repository
* Remove double exclamation
* Rename some variables
* Remove duplicated semicolon
* Add permissions field to the mapping only if the permission control is enabled
* Fix test failure
* Add feature flag config to the yml file
* Make the comment of feature flag more clear
* Make comment more clear
* Remove management permission type
* Fix test failure

---------

(cherry picked from commit 54c36fe)

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Josh Romero <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Workspace] Define and implement ACL for the saved objects within workspaces