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

Fix the bug of capabilities request not supporting carrying authinfo #2014

Merged

Conversation

yubonluo
Copy link
Contributor

Description

Fix the issue of capabilities API not supporting carrying authinfo。

Category

Bug fix

Why these changes are required?

When user call core.capabilities.registerSwitcher to store certain state which is about authinfo, the request need carry the auth information.

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@yubonluo yubonluo changed the title Fix the issue of capabilities API not supporting carrying authinfo Fix the issue of capabilities request not supporting carrying authinfo Jun 19, 2024
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.61%. Comparing base (06b2e96) to head (33421ee).

Current head 33421ee differs from pull request most recent head 50f7d2f

Please upload reports for the commit 50f7d2f to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2014   +/-   ##
=======================================
  Coverage   70.61%   70.61%           
=======================================
  Files          97       97           
  Lines        2600     2600           
  Branches      387      387           
=======================================
  Hits         1836     1836           
  Misses        668      668           
  Partials       96       96           

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

@yubonluo yubonluo changed the title Fix the issue of capabilities request not supporting carrying authinfo Fix the bug of capabilities request not supporting carrying authinfo Jun 19, 2024
@derek-ho
Copy link
Collaborator

Blocked on: #2009 to fix the main CI

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

@yubonluo Thank you for the PR. Would you also add a test to confirm the behavior?

server/auth/types/authentication_type.ts Show resolved Hide resolved
@yubonluo yubonluo force-pushed the capabilities-support-authinfo branch from 826cf0e to 6de5625 Compare June 20, 2024 04:29
@yubonluo
Copy link
Contributor Author

@yubonluo Thank you for the PR. Would you also add a test to confirm the behavior?

@DarshitChanpura Sure, the unit test has been added.

server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved
server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved
@yubonluo
Copy link
Contributor Author

yubonluo commented Jun 20, 2024

@derek-ho @RyanL1997 @DarshitChanpura Cloud you please help me to review and merge this pr? Thanks!

derek-ho
derek-ho previously approved these changes Jun 20, 2024
Copy link
Collaborator

@derek-ho derek-ho left a comment

Choose a reason for hiding this comment

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

LGTM @yubonluo I am not familiar with this API, can you provide some more information on how it is used/how to hit it on my local so I can try to replicate and understand what use case this is solving? Is this related? https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/capabilities/routes/resolve_capabilities.ts#L40

@yubonluo
Copy link
Contributor Author

LGTM @yubonluo I am not familiar with this API, can you provide some more information on how it is used/how to hit it on my local so I can try to replicate and understand what use case this is solving? Is this related? https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/capabilities/routes/resolve_capabilities.ts#L40

@derek-ho Yes, this is related. The capabilities service provide two methods (https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/capabilities/capabilities_service.ts).
You can use it like this:

export class WorkspacePlugin implements Plugin<WorkspacePluginSetup, WorkspacePluginStart> {
    public async setup(core: CoreSetup) {
        core.capabilities.registerProvider(() => ({
            workspaces: {
            enabled: true,
            permissionEnabled: isPermissionControlEnabled,
            isDashboardAdmin: false,
      },
    }));
      core.capabilities.registerSwitcher((request) => {
          const isDashboardAdmin = getWorkspaceState(request).isDashboardAdmin || false;
          return { workspaces: { isDashboardAdmin } };
    });
    }
}

@yubonluo yubonluo force-pushed the capabilities-support-authinfo branch from 50f7d2f to 3bc290f Compare June 21, 2024 01:56
Signed-off-by: yubonluo <[email protected]>
@yubonluo yubonluo force-pushed the capabilities-support-authinfo branch from 3bc290f to c78e1af Compare June 21, 2024 02:18
Signed-off-by: yubonluo <[email protected]>
@yubonluo
Copy link
Contributor Author

@derek-ho Cloud you help me merge this pr? Thanks~

@derek-ho derek-ho added backport 2.x backport to 2.x branch labels Jun 24, 2024
@derek-ho derek-ho merged commit 293490d into opensearch-project:main Jun 24, 2024
19 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 24, 2024
…2014)

* capabilities api support authinfo

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

* optimize the annotation

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

* optimize the code

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

---------

Signed-off-by: yubonluo <[email protected]>
(cherry picked from commit 293490d)
derek-ho pushed a commit that referenced this pull request Jun 24, 2024
…2014) (#2017)

* capabilities api support authinfo

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

* optimize the annotation

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

* optimize the code

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

---------

Signed-off-by: yubonluo <[email protected]>
(cherry picked from commit 293490d)

Co-authored-by: yuboluo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants