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

[BUG] [MD] Update credentials that attached to datasource doesn't give the correct client. #2182

Closed
zhongnansu opened this issue Aug 23, 2022 · 3 comments
Assignees
Labels
bug Something isn't working multiple datasource multiple datasource project

Comments

@zhongnansu
Copy link
Member

zhongnansu commented Aug 23, 2022

Describe the bug

Update credentials that attached to datasource doesn't give the correct client.

To Reproduce
Steps to reproduce the behavior:

  1. create 2 credentials. cred-1(right username/password) and cred-2(in-correct username/password)
  2. create a data source ds-1 attached to cred-1
  3. create index-pattern ip-1 that attached to ds-1
  4. Use Discover to search on ip-1
  5. Able to see data from the ds-1
  6. update ds-1 to use cred-2, and save
  7. Use Discover to search on ip-1
  8. Still seeing data from ds-1, which is not supposed to happen, because cred-2 is not even a correct crendential for ds-1

Expected behavior

@zhongnansu zhongnansu added bug Something isn't working multiple datasource multiple datasource project labels Aug 23, 2022
@zhongnansu zhongnansu self-assigned this Aug 23, 2022
@zhongnansu zhongnansu changed the title [BUG] Update credentials that attached to datasource doesn't give the correct client. [BUG] [MD] Update credentials that attached to datasource doesn't give the correct client. Aug 23, 2022
@zhongnansu
Copy link
Member Author

zhongnansu commented Aug 23, 2022

  • Originally I was thinking this issue is caused by the way we implement data source pooling(the LRU cache), but after double check, there's nothing wrong with that. We are only caching the root client, which is always a Client with no-auth. And for each time it needs a client, we always retrieve credential and create a child client to return, and we don't cache this child client.

  • After further looking into the issue. I found the issue is caused by the way we spawn child client with auth option. After first time we spawn a child of a root client with auth, it will be "locked". Afterwards, any getBasicAuthClient of the same endpoint won't overwrite the auth info of it.

    const getBasicAuthClient = (
    rootClient: Client,
    credential: UsernamePasswordTypedContent
    ): Client => {
    const { username, password } = credential;
    return rootClient.child({
    auth: {
    username,
    password,
    },

  • I doubt that opensearch-js client implementation locked this info on purpose, so I kept looking into the Client() implementation in that library and found it's preventing the override of some options including auth
    https://github.com/opensearch-project/opensearch-js/blob/34433e8f589007472b809df3333214d889021d16/index.js#L256-L273

  • This also explains why the existing cluster_client.ts -> getScopedClient is passing in a scoped header, not explicit auth

    asScoped(request: ScopeableRequest) {
    const scopedHeaders = this.getScopedHeaders(request);
    const scopedClient = this.rootScopedClient.child({
    headers: scopedHeaders,
    });
    return new ScopedClusterClient(this.asInternalUser, scopedClient);
    }

Solution

Along with passing in auth while spawning child, we can pass in a header: {authorization: null}, and the Client library has logic to build a auth header every-time based on the auth option, only when header.authorization == null

rootClient.child({
    auth: {
      username,
      password,
    },
    headers: { authorization: null },
  });

fyi @zengyan-amazon

@zengyan-amazon
Copy link
Member

thanks for the investigation, let's implement the fix

@zhongnansu
Copy link
Member Author

PR merged, resolving

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working multiple datasource multiple datasource project
Projects
None yet
Development

No branches or pull requests

2 participants