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

[Data Source]Add data source permission wrapper and dataSourceAdmin role #7959

Merged
merged 17 commits into from
Sep 5, 2024

Conversation

yubonluo
Copy link
Contributor

@yubonluo yubonluo commented Sep 2, 2024

Description

Adding a permission wrapper to control the permissions of data source to create/update/delete according the data_source_management.manageableBy.
Adding a dataSourceAdmin role to have the all permissions to access data sources.

Issues Resolved

#8009

Screenshot

Data source admin can operate data source

data_source_management.manageableBy = "none"
data_source_management.dataSourceAdmin.groups: ["admin"]
image

If data source is managed by none, any user(Except for dataSourceAdmin) can not operate data source

data_source_management.manageableBy: "none"
data_source_management.dataSourceAdmin.groups: []
image

If data source is managed by all, any user can operate data source

data_source_management.manageableBy = "all"
image

If data source is managed by dashboard admin, only admin user can operate data source

data_source_management.manageableBy: "dashboard_admin"
opensearchDashboards.dashboardAdmin.users: ["admin"]

user is not dashboard admin

image

uses is dashboard admin

image

Testing the changes

Changelog

  • feat: [Data source] Add data source permission wrapper and dataSourceAdmin role

Check List

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

Copy link
Contributor

github-actions bot commented Sep 2, 2024

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

Copy link

codecov bot commented Sep 2, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 60.54%. Comparing base (7dedc58) to head (38383bc).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
..._objects/workspace_saved_objects_client_wrapper.ts 68.75% 2 Missing and 3 partials ⚠️
src/core/server/utils/auth_info.ts 86.66% 0 Missing and 2 partials ⚠️
...d_objects/data_source_premission_client_wrapper.ts 97.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7959      +/-   ##
==========================================
- Coverage   61.09%   60.54%   -0.55%     
==========================================
  Files        3691     3730      +39     
  Lines       87310    88357    +1047     
  Branches    13433    13681     +248     
==========================================
+ Hits        53340    53497     +157     
- Misses      30722    31602     +880     
- Partials     3248     3258      +10     
Flag Coverage Δ
Linux_1 28.71% <42.85%> (-0.61%) ⬇️
Linux_2 56.26% <88.23%> (+0.01%) ⬆️
Linux_3 37.45% <73.43%> (-0.48%) ⬇️
Linux_4 29.60% <21.05%> (+<0.01%) ⬆️
Windows_1 28.73% <42.85%> (-0.61%) ⬇️
Windows_2 56.21% <88.23%> (+0.01%) ⬆️
Windows_3 37.45% <73.43%> (-0.49%) ⬇️
Windows_4 29.60% <21.05%> (+0.01%) ⬆️

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.

@SuZhou-Joe
Copy link
Member

It is recommended to add integration test for new introduced saved objects wrapper, we can add that in a following PR, as well as to cover some missing branches in workspace_saved_objects_wrapper.ts.

@SuZhou-Joe
Copy link
Member

And thanks for the quick update on the comment, the PR looks amazing to me. We finally have API level access control on data sources.

Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[email protected]>
Signed-off-by: yubonluo <[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.

Nice refactor!

@ruanyl ruanyl added the multiple datasource multiple datasource project label Sep 5, 2024
@SuZhou-Joe
Copy link
Member

Run cypress tests (osd:ciGroup6) should be irrelevant to the code changes.

@SuZhou-Joe SuZhou-Joe merged commit bc49b8c into opensearch-project:main Sep 5, 2024
72 of 73 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 5, 2024
…ole (#7959)

* Add data source permission wrapper

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

* Changeset file for PR #7959 created/updated

* optimize the config schema

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

* optimize the code

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

* optimize the code

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

* add some coments and optimize the logic

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

* optimize the code

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

* add unit tests

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

* fix test error

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

* optimize the code

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

* optimize the code

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

* Move some logic to workspace wrapper

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

* delete useless code

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

* delete useless code

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

---------

Signed-off-by: yubonluo <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit bc49b8c)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe pushed a commit that referenced this pull request Sep 5, 2024
…ole (#7959) (#8023)

* Add data source permission wrapper



* Changeset file for PR #7959 created/updated

* optimize the config schema



* optimize the code



* optimize the code



* add some coments and optimize the logic



* optimize the code



* add unit tests



* fix test error



* optimize the code



* optimize the code



* Move some logic to workspace wrapper



* delete useless code



* delete useless code



---------



(cherry picked from commit bc49b8c)

Signed-off-by: yubonluo <[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: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 6, 2024
…ole (#7959) (#8023)

* Add data source permission wrapper

* Changeset file for PR #7959 created/updated

* optimize the config schema

* optimize the code

* optimize the code

* add some coments and optimize the logic

* optimize the code

* add unit tests

* fix test error

* optimize the code

* optimize the code

* Move some logic to workspace wrapper

* delete useless code

* delete useless code

---------

(cherry picked from commit bc49b8c)

Signed-off-by: yubonluo <[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: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit 260b094)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe pushed a commit that referenced this pull request Sep 6, 2024
…ole (#7959) (#8023) (#8042)

* Add data source permission wrapper

* Changeset file for PR #7959 created/updated

* optimize the config schema

* optimize the code

* optimize the code

* add some coments and optimize the logic

* optimize the code

* add unit tests

* fix test error

* optimize the code

* optimize the code

* Move some logic to workspace wrapper

* delete useless code

* delete useless code

---------

(cherry picked from commit bc49b8c)





(cherry picked from commit 260b094)

Signed-off-by: yubonluo <[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: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
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.

3 participants