[Upgrade Assistant] Add support for API keys when reindexing

[alisonelizabeth]

Fixes #72014

Upon receiving a request to reindex, Upgrade Assistant stores the requester's credentials in memory and uses those credentials when updating saved objects and issuing requests for other reindexing-related operations. In a token-based authentication mechanism, these tokens have an expiration time that could expire before reindexing work completes causing the task to stall.

As a partial fix, this PR implements granting a temporary API key that is stored in memory that is then used for issuing requests as advised via #72014 (comment). Once the reindex operation is completed, the API key is invalidated. Note: this implementation is only possible if API keys are enabled. If API keys are not enabled, we will have to continue using the current approach.

How to test

Unzip 6.8.16-data.zip. This contains indices created on 6.8 to trigger the reindex action in UA.

1. Test reindexing with API keys enabled. To enable API keys, start ES as follows:

   yarn es snapshot -E xpack.security.authc.api_key.enabled=true -E path.data=/path/to/6.8.16-data
   

   • Navigate to UA --> Elasticsearch deprecations
   • Select an index that needs to be reindexed, and start the reindexing process
   • Once started, you can verify an API key was created by navigating to Security --> API keys
   • Verify the reindex process completed successfully
   • Once the reindex process is complete, the API key should no longer appear under Security --> API keys

2. Test reindexing with API keys disabled. Restart ES:

   yarn es snapshot -E path.data=/path/to/6.8.16-data
   

   • Navigate to UA --> Elasticsearch deprecations
   • Select an index that needs to be reindexed, and start the reindexing process
   • Verify the reindex process completed successfully

[elasticmachine]

Pinging @elastic/kibana-stack-management (Team:Stack Management)

[jportner]

I tested this as described, and also with the Security plugin disabled in ES.
Everything worked as expected! 🎉

I also double checked to make sure we aren't passing all client request headers to ES -- this plugin already uses the scoped cluster client provided by Core, which is the correct approach.

I have just a couple of nits below, great job on this.

[jportner]

There are two edge cases that come to mind:

1. When the user is already authenticated with an API key, they cannot create another API key (related: Ability to clone granted API keys elasticsearch#59304)
2. When the user is impersonating another user (with run as), they cannot create an API key

So I think that we probably need some additional logic to handle those edge cases, e.g., catch an error and fall back to just using the given credentials.

[alisonelizabeth]

Thanks for the background! I've added a try/catch, which falls back to the user's credentials.

[jportner]

The only downside of this is that it's an in-memory map, so if Kibana restarts for some reason before the reindex is finished, that API key won't ever get deleted.
These API keys use a consistent naming convention, and it doesn't constitute a vulnerability of any sort, so that is acceptable in my book, just wanted to point it out.

[jportner]

Optional nit: given the chance that an API key might not get deleted, maybe it's worth adding a bit of metadata? Something like this:

diff --git a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/credential_store.ts b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/credential_store.ts
index 7f3f4f2b307..41eb7a5afa7 100644
--- a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/credential_store.ts
+++ b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/credential_store.ts
@@ -59,6 +59,10 @@ export const credentialStoreFactory = (logger: Logger): CredentialStore => {
     const apiKeyResult = await security?.authc.apiKeys.grantAsInternalUser(request, {
       name: `ua_reindex_${reindexOpId}`,
       role_descriptors: {},
+      metadata: {
+        description:
+          'Created by the Upgrade Assistant for a reindex operation; this can be safely deleted after Kibana is upgraded.',
+      },
     });
 
     if (apiKeyResult) {

[alisonelizabeth]

Great point! Done.

[jportner]

LGTM!

[cjcenizal]

Tested locally, code LGTM! Thanks for adding tests. Had some nitpicky suggestions, nothing major.

@alisonelizabeth One thing I was missing was a way to reproduce the original problem raised by #72014, so I could verify that these changes address it. Do you know if there's a way to do that?

[cjcenizal]

Might be more succinct to express not.toBeDefined() as toBeUndefined()

[cjcenizal]

Nit: I would find this code easier to digest if we extracted getHash, getApiKey, and invalidateApiKey from the factory function's scope. This would make credentialStoreFactory smaller, so I'd have less information to keep in my head as I read the function. Extracting out the helpers also makes it easier for me to identify their dependencies.

// Generates a stable hash for the reindex operation's current state.
const getHash = (reindexOp: ReindexSavedObject) =>
  createHash('sha256')
    .update(stringify({ id: reindexOp.id, ...reindexOp.attributes }))
    .digest('base64');

const getApiKey = async ({
  request,
  security,
  reindexOpId,
  apiKeysMap,
}: {
  request: KibanaRequest;
  security?: SecurityPluginStart;
  reindexOpId: string;
  apiKeysMap: Map<string, string>;
}): Promise<string | undefined> => {
  const apiKeyResult = await security?.authc.apiKeys.grantAsInternalUser(request, {
    name: `ua_reindex_${reindexOpId}`,
    role_descriptors: {},
  });

  if (apiKeyResult) {
    const { api_key: apiKey, id } = apiKeyResult;
    // Store each API key per reindex operation so that we can later invalidate it when the reindex operation is complete
    apiKeysMap.set(reindexOpId, id);
    // Returns the base64 encoding of `id:api_key`
    // This can be used when sending a request with an "Authorization: ApiKey xxx" header
    return Buffer.from(`${id}:${apiKey}`).toString('base64');
  }
};

const invalidateApiKey = async ({
  apiKeyId,
  security,
  log,
}: {
  apiKeyId: string;
  security?: SecurityPluginStart;
  log: Logger;
}) => {
  try {
    await security?.authc.apiKeys.invalidateAsInternalUser({ ids: [apiKeyId] });
  } catch (error) {
    // Swallow error if there's a problem invalidating API key
    log.debug(`Error invalidating API key for id ${apiKeyId}: ${error.message}`);
  }
};

export const credentialStoreFactory = (logger: Logger): CredentialStore => {
  /* ... */
};

[cjcenizal]

Nit: we could simplify this a bit my moving logic into a condition that API keys are enabled:

      if (areApiKeysEnabled) {
        const apiKey = await getApiKey({ request, security!, reindexOpId: reindexOp.id, apiKeysMap });

        if (apiKey) {
          credMap.set(getHash(reindexOp), {
            ...request.headers,
            authorization: `ApiKey ${apiKey}`,
          });
          return;
        }
      }

      // Set the requestor's credentials in memory if apiKeys are not enabled
      credMap.set(getHash(reindexOp), request.headers);

Once we're inside the condition we know that security is defined, so we can also change the getApiKey signature to expect it, which also makes it simpler:

const getApiKey = async ({
  request,
  security,
  reindexOpId,
  apiKeysMap,
}: {
  request: KibanaRequest;
  security: SecurityPluginStart; // <-- No longer possibly undefined
  reindexOpId: string;
  apiKeysMap: Map<string, string>;
}): Promise<string | undefined> => {
  /* ... */
};

[cjcenizal]

Nittiest nit-pick ever: can we split this comment across two lines? As it reads, it sounds like line 138's comment means that we'll re-associate credentials if an API key isn't being used.

      // If the reindex operation is completed...
      if (reindexOp.attributes.status === ReindexStatus.completed) {
        // ...and an API key is being used, invalidate it
        const apiKeyId = apiKeysMap.get(reindexOp.id);
        if (apiKeyId) {
          await invalidateApiKey({ apiKeyId, security, log });
          return;
        }
      }

[cjcenizal]

Do we also want to delete the API key from apiKeysMap?

apiKeysMap.delete(reindexOp.id);

[alisonelizabeth]

Thanks for the review @cjcenizal! I addressed your feedback.

@alisonelizabeth One thing I was missing was a way to reproduce the original problem raised by #72014, so I could verify that these changes address it. Do you know if there's a way to do that?

It would require configuring SAML, which I believe is dependent on having an Identity Provider. I'm reaching out to the security team to see what options we might have.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 01cca25
• Storybooks Preview
• Documentation Changes

Metrics [docs]

✅ unchanged

History

• 💚 Build #153063 succeeded b005da7
• 💚 Build #152886 succeeded 1b2bd05
• 💚 Build #152419 succeeded 2d26060
• 💚 Build #152329 succeeded c03bbb9
• 💔 Build #152307 failed db9d5a6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[alisonelizabeth]

I opened #112154 to document testing instructions.

I reproduced the behavior on 7.x by setting a short token auth timeout (-E xpack.security.authc.token.timeout=5s), then verified reindexing is working as expected with my changes.