[Task Manager] Use strategy pattern for API key management

[ersin-erdal]

Summary

Refactors the Task Manager API key handling to use a strategy pattern, replacing scattered conditional logic with two concrete strategy implementations.

Key design rule: Grant and Usage are independent

• Grant UIAM: whenever security.authc.apiKeys.uiam is available (proactively store both keys so switching is seamless)
• Use UIAM: only when config.api_key_type === 'uiam'

Strategy Pattern

• ApiKeyStrategy interface defines grant, use, invalidation, and marking behaviors
• EsApiKeyStrategy (default): grants only ES keys, uses ES keys for fake requests
• EsAndUiamApiKeyStrategy: grants both ES + UIAM keys, uses UIAM for fake requests when configured (with ES fallback for pre-existing tasks)
• Factory function createApiKeyStrategy selects based on UIAM availability

API Key Storage

• ES API keys: persisted as encoded id:key pair (base64), used as ApiKey <base64> in headers
• UIAM API keys: persisted as raw essu_* credential, used as ApiKey essu_* in headers
• Both apiKey and uiamApiKey are encrypted via ESO on the task SO
• uiamApiKeyId stored in userScope for invalidation tracking

Changes

• New files: api_key_strategy.ts, es_api_key_strategy.ts, es_and_uiam_api_key_strategy.ts, create_api_key_strategy.ts
• task.ts: Added uiamApiKey to TaskInstance/SerializedConcreteTaskInstance, uiamApiKeyId to TaskUserScope
• task_store.ts: Uses ApiKeyStrategy for granting, invalidation marking, and decrypts both apiKey and uiamApiKey
• task_runner.ts: Uses strategy.getApiKeyForFakeRequest() for key selection
• plugin.ts: Creates strategy, adds registerUiamApiKeyInvalidateFn to start contract
• task_manager_dependencies: Registers uiamApiKey for ESO encryption, wires registerUiamApiKeyInvalidateFn
• invalidate_api_keys_task.ts: Passes invalidateUiamApiKeyFn through to runInvalidate
• SO model version 9 with uiamApiKeyId mapping

To verify:

• Add below configs to your serverless.dev.yml
  xpack.task_manager.api_key_type: 'uiam'
xpack.task_manager.invalidate_api_key_task.removalDelay: '10s'
xpack.task_manager.invalidate_api_key_task.interval: '1m'
  so we can see the invalidation task works without a problem.

• Run Kibana and Elasticsearch with the below commands:
  yarn es serverless --projectType observability --uiam
yarn start --serverless oblt --uiam --run-examples

• Login with the system_indices_superuser user.

• Go to: http://localhost:5601/app/triggersActionsUiExample/task_manager_with_api_key

• Create task 1 and task 2

• Use the below query to check if the task has both ES and UIAM API Keys

GET /.kibana_task_manager_*/_search
{
  "query": { 
    "bool": { 
      "must": [
        { "match": { "task.taskType":   "taskWithApiKey" }}
      ]
    }
  }
}

• Remove one of the tasks, and check if both ES and UIAM API Keys to be queued to be invalidated.

GET /.kibana_task_manager_*/_search
{
  "query": {
    "match": {
      "type": {
        "query": "api_key_to_invalidate"
      }
    }
  }
}

• Wait for a minute and do the same as above, the API Keys should be invalidated and removed from the index.
  There shouldn't be any error on the console.

• Add the task you removed again and do the same tests with Remove All Tasks which uses bulkDelete.

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[darnautov]

is it okay to write an actual api key to a regular SO here?

[ersin-erdal]

Good catch, no it is not, converted it to an encrypted field.

[ersin-erdal]

@jeramysoucy Can you review this part please?
api_key_to_invalidate is an existing SO type, now we are adding a new encrypted field, uiamApiKey.
Do we need an intermediate release?

[jeramysoucy]

I don't think this has come up before, but it is a similar case to adding an encrypted attribute to an existing ESO.

Is uiamApiKey a new attribute for this SO type? Or has it already been in use as an non-encrypted field? Is this attribute expected to be populated already in the current version of serverless Kibana?

The concern here would be that the unencrypted value that is already stored in objects will not be decryptable in the new version of Kibana. I think we can solve this scenario with the createModelVersion wrapper. However, the scenario where the previous version of Kibana expects the value to not be encrypted might be problematic.

Could you provide some additional info?

• Is the attribute already in use?
• If so, what is the life cycle for the ESO? Is it short-lived or persisted over a long duration?

Related, I see you're also adding an encrypted attribute to the task ESO. I have the same concerns/questions for that as well.

[ersin-erdal]

Is the attribute already in use?

No it is not, hasn't been used before.

Is this attribute expected to be populated already in the current version of serverless Kibana?

No, it will be populated with this PR

Related, I see you're also adding an encrypted attribute to the task ESO. I have the same concerns/questions for that as well.

It is the same, new field as encrypted, hasn't been used before.

[jeramysoucy]

Thanks! In this case we should be ok. The old version of Kibana will not attempt to decrypt or utilize the encrypted value.

[doakalexi]

LGTM! Tested locally and works as expected

[doakalexi]

I am not sure if this is required, but I was looking at some other routes in ftr_apis and they use requiredPrivileges: ['ftrApis']. Not sure if this is something we should use too?

[ersin-erdal]

switched to it, looks working

[macroscopeapp]

Scout Test Review

🔴 Blocker: .meta manifest is incomplete

Rule: Suite wiring/discovery — new specs must be picked up by Playwright config and manifests.

File: x-pack/platform/plugins/shared/task_manager/test/scout/.meta/api/parallel.json

The .meta/api/parallel.json manifest only includes the 2 tests from task_api_keys.spec.ts but is missing all 7 tests from task_schedule_and_delete.spec.ts. This auto-generated file is used for CI test planning and lane distribution — missing entries mean those tests won't be tracked properly.

Fix: Regenerate the manifest by running:

node scripts/scout update-test-config-manifests

---

🟡 Major: Missing task cleanup in task_api_keys.spec.ts

Rule: Keep cleanup in hooks

File: x-pack/platform/plugins/shared/task_manager/test/scout/api/parallel_tests/task_api_keys.spec.ts

The beforeAll creates a task, and the second test deletes it as part of its assertion logic. If that test fails before the delete call, the task is orphaned — there's no safety-net cleanup in afterAll. The afterAll only cleans api_key_to_invalidate SOs but not the task itself.

Suggested fix: Add task cleanup to afterAll:

apiTest.afterAll(async ({ apiClient, kbnClient, samlAuth }) => {
  // Clean up the task if it still exists
  if (createdTaskId) {
    const { cookieHeader } = await samlAuth.asInteractiveUser('admin');
    await apiClient
      .delete(`internal/task_manager/tasks/${createdTaskId}`, {
        headers: { ...COMMON_HEADERS, ...cookieHeader },
      })
      .catch(() => {});
  }
  await kbnClient.savedObjects.clean({ types: ['api_key_to_invalidate'] });
});

---

🟡 Major: Unused local fixture — spec files import from @kbn/scout instead of local fixtures/

Rule: Reuse-first — use the fixture infrastructure you've set up.

Files:

• x-pack/platform/plugins/shared/task_manager/test/scout/api/fixtures/index.ts (defines local apiTest)
• x-pack/platform/plugins/shared/task_manager/test/scout/api/parallel_tests/task_api_keys.spec.ts:8 (imports from @kbn/scout)
• x-pack/platform/plugins/shared/task_manager/test/scout/api/parallel_tests/task_schedule_and_delete.spec.ts:8 (imports from @kbn/scout)

The fixtures/index.ts defines a custom apiTest extension but neither spec file uses it — both import apiTest directly from @kbn/scout. Either:

• Use the local fixture in your spec files (change import { apiTest, tags } from '@kbn/scout' → import { apiTest } from '../fixtures' + import { tags } from '@kbn/scout'), or
• Remove the unused fixture file if you don't need custom extensions.

Dead fixture files add confusion for future contributors.

---

🔵 Minor: COMMON_HEADERS should use as const for type safety

Rule: Use constants for shared test values

File: x-pack/platform/plugins/shared/task_manager/test/scout/api/fixtures/constants.ts:8

The best practices example shows COMMON_HEADERS declared with as const. The current definition omits it.

 export const COMMON_HEADERS = {
   'kbn-xsrf': 'some-xsrf-token',
   'x-elastic-internal-origin': 'kibana',
   'Content-Type': 'application/json;charset=UTF-8',
-};
+} as const;

---

✅ Positive: Good practices observed

• Deployment tags are applied consistently on all suites (tags.serverless.observability.complete).
• Descriptive test names clearly describe expected behavior (e.g., "when task is removed, apiKey and uiamApiKey are queued for invalidation").
• RBAC / permission testing is well done: the task_schedule_and_delete.spec.ts suite includes 403 assertions for viewer role on both schedule and delete endpoints.
• Constants are extracted into a shared constants.ts — good reuse pattern.
• Cleanup in hooks is properly implemented in task_schedule_and_delete.spec.ts afterAll.
• Response body validation goes beyond status codes (e.g., checking taskType, enabled, schedule.interval, deleted fields).

^{Posted via Macroscope — Scout Test Review}

[csr]

Scout Test Review

Please temporarily discard the Scout Test Review comment. We're whipping up a fix to make this more relevant. The blocker "Blocker: .meta manifest is incomplete" isn't something we encourage necessarily (no harm in regenerating config manifest files, but this isn't a blocker at all).

[macroscopeapp]

Scout Test Review

✅ Previous blocker resolved

The .meta/api/parallel.json manifest now includes all 9 tests from both spec files. Nice fix!

---

🔵 Minor

1. Reuse credentials — cache samlAuth sessions in beforeAll

Rule: Validate endpoints with apiClient / API auth best practices

File: x-pack/platform/plugins/shared/task_manager/test/scout/api/parallel_tests/task_schedule_and_delete.spec.ts

Every test in this suite calls samlAuth.asInteractiveUser('admin') (or 'viewer'), creating a fresh SAML session per test. The Scout docs recommend generating credentials once in beforeAll and reusing them across tests in the suite.

Suggested change:

let adminCookie: Record<string, string>;
let viewerCookie: Record<string, string>;

apiTest.beforeAll(async ({ samlAuth }) => {
  adminCookie = (await samlAuth.asInteractiveUser('admin')).cookieHeader;
  viewerCookie = (await samlAuth.asInteractiveUser('viewer')).cookieHeader;
});

// Then in each test:
headers: { ...COMMON_HEADERS, ...adminCookie }

The same applies to task_api_keys.spec.ts where the second test re-creates the admin session even though beforeAll already created one.

---

2. Validate the response body on error paths (not just status)

Rule: Don't just verify the status code—validate the response body

Files:

• task_schedule_and_delete.spec.ts:84 — "returns 400 when taskType is missing" only asserts toHaveStatusCode(400)
• task_schedule_and_delete.spec.ts:145 — "returns 404 for a non-existent task" only asserts toHaveStatusCode(404)

Consider adding a light body assertion (e.g. expect(response.body).toHaveProperty('message')) to confirm the error response contract and catch regressions where the status code is correct but the error message is wrong or missing.

---

⚪ Nit

Unused testData re-export

File: x-pack/platform/plugins/shared/task_manager/test/scout/api/fixtures/index.ts:12

export * as testData from './constants';

Neither spec file imports testData — they import directly from '../fixtures/constants'. Consider either using the testData namespace consistently (as seen in other Scout test suites like osquery) or removing the unused re-export.

---

👍 What this PR does well

• Proper deployment tags on both suites (tags.serverless.observability.complete)
• Good test names that read as behavior descriptions
• RBAC coverage: both suites test the forbidden path (viewer → 403), following the minimal permissions guidance
• Resilient cleanup in afterAll with .catch(() => {}) and kbnClient.savedObjects.clean()
• Constants extracted to a shared constants.ts with clear comments explaining TEST_TASK_TYPE selection
• Proper fixture extension pattern via baseApiTest.extend()
• Good response body validation in happy-path tests (checking taskType, enabled, schedule, deleted, and the raw ES document fields)

^{Posted via Macroscope — Scout Test Review}

[macroscopeapp]

Scout Test Review

⚪ Nit: Duplicate fixture files across test directories

Rule: Use constants for shared test values

Files:

• x-pack/platform/plugins/shared/task_manager/test/scout/api/fixtures/constants.ts
• x-pack/platform/plugins/shared/task_manager/test/scout_task_manager_uiam/api/fixtures/constants.ts
• (Same for index.ts in both directories)

These are byte-for-byte identical. Since both test suites belong to the same plugin, consider extracting shared constants (e.g. COMMON_HEADERS, TEST_TASK_TYPE) to a common location like test/scout/shared/constants.ts and importing from there. This avoids drift if one file is updated but not the other.

That said, the duplication is small and both directories are self-contained — so this is low priority.

---

👍 Good config set separation

Nice work moving task_api_keys.spec.ts into a dedicated scout_task_manager_uiam config set with its own server config that enables --xpack.task_manager.grant_uiam_api_keys=true. This avoids polluting the default base config for all Scout suites and follows the feedback from the Scout team. The CI config entry (task_manager in .buildkite/scout_ci_config.yml) should pick up both directories, keeping things wired correctly.

^{Posted via Macroscope — Scout Test Review}

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: ca13796

Failed CI Steps

• FTR Configs #164
• FTR Configs #181
• FTR Configs #181

Test Failures

• [job] [logs] FTR Configs #181 / Entity Analytics - Risk Score Maintainer @ess @serverless @serverlessQA Risk Score Maintainer Task Lifecycle with maintainer test logs data @skipInServerlessMKI resumes producing additional scores after stop and restart when triggered
• [job] [logs] FTR Configs #181 / Entity Analytics - Risk Score Maintainer @ess @serverless @serverlessQA Risk Score Maintainer Task Lifecycle with maintainer test logs data @skipInServerlessMKI resumes producing additional scores after stop and restart when triggered
• [job] [logs] FTR Configs #164 / task_manager migrations 8.5.0 migrates active tasks to set enabled to true

Metrics [docs]

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
taskManager	12	13	+1

Unknown metric groups

API count

id	before	after	diff
taskManager	130	131	+1

History

• 💔 Build #429936 failed f5cb90c
• 💔 Build #429733 failed 5dfc7a6
• 💔 Build #428992 failed d230160
• 💔 Build #428873 failed 969f3f8
• 💔 Build #428614 failed 156635c

cc @ersin-erdal

[macroscopeapp]

Scout Test Review

Found 1 new issue (minor). See inline comment for details.

Previous feedback (duplicate fixtures nit, credential caching minor) still applies — no code changes observed for those, so not re-posting.

This review is experimental. Share your feedback in the #appex-qa channel.

^{Posted via Macroscope — Scout Test Review}

[macroscopeapp]

Minor issue in the UIAM API keys test: fragile absolute count assertion.

^{Posted via Macroscope — Scout Test Review}

[dmlemeshko]

src/platform/packages/shared/kbn-scout/src/servers/configs/config_sets/task_manager_uiam/serverless/observability_complete.serverless.config.ts LGTM

[macroscopeapp]

Scout Test Review

Re-flagging 1 issue (minor) — the file moved from parallel_tests/ to tests/, making the prior inline comment orphaned. See inline comment for details.

Previous feedback (duplicate fixtures nit, credential caching minor) still applies — no substantive code changes observed for those, so not re-posting.

This review is experimental. Share your feedback in the #appex-qa channel.

^{Posted via Macroscope — Scout Test Review}

[macroscopeapp]

🔵 Minor — Keep test suites independent

(Re-flagging — file moved from parallel_tests/ to tests/, orphaning the previous comment.)

Asserting an absolute count of 0 before the delete is fragile: if a prior run's afterAll cleanup didn't execute (runner crash, timeout, etc.), leftover api_key_to_invalidate SOs will cause a misleading failure.

Consider using a relative count so the test is resilient to leftover state:

Suggested change

	const { saved_objects: pendingBefore } = await kbnClient.savedObjects.find({
	type: 'api_key_to_invalidate',
	});
	expect(pendingBefore).toHaveLength(0);
	const { saved_objects: pendingBefore } = await kbnClient.savedObjects.find({
	type: 'api_key_to_invalidate',
	});
	const countBefore = pendingBefore.length;

Then assert the delta at the end:

      expect(pendingAfter).toHaveLength(countBefore + 2);

^{Posted via Macroscope — Scout Test Review}

[macroscopeapp]

🟢 Low server/task_store.ts:909

In _bulkRemove, unwrap(taskInstance) returns either a ConcreteTaskInstance or an error object { type; id; error }, but the result is immediately cast to ConcreteTaskInstance without checking isOk(taskInstance) first. When a task retrieval fails, the error object lacks apiKey/uiamApiKey/userScope, so docHasEncryptedApiKey returns false and the API key is never marked for invalidation — leaving orphaned keys in the system.

🤖 Copy this AI Prompt to have your agent fix this:

In file x-pack/platform/plugins/shared/task_manager/server/task_store.ts around line 909:

In `_bulkRemove`, `unwrap(taskInstance)` returns either a `ConcreteTaskInstance` or an error object `{ type; id; error }`, but the result is immediately cast to `ConcreteTaskInstance` without checking `isOk(taskInstance)` first. When a task retrieval fails, the error object lacks `apiKey`/`uiamApiKey`/`userScope`, so `docHasEncryptedApiKey` returns `false` and the API key is never marked for invalidation — leaving orphaned keys in the system.

Evidence trail:
1. x-pack/platform/plugins/shared/task_manager/server/task_store.ts lines 132-134: `BulkGetResult` type definition showing error type as `{ type: string; id: string; error: SavedObjectError }`
2. x-pack/platform/plugins/shared/task_manager/server/task_store.ts lines 976-1003: `_bulkGet` returns `asErr({ id, type, error })` for failed retrievals
3. x-pack/platform/plugins/shared/task_manager/server/lib/result_type.ts line 77: `unwrap` returns `result.error` (not `result.value`) when result is Err
4. x-pack/platform/plugins/shared/task_manager/server/task_store.ts lines 912-917: `unwrap(taskInstance) as ConcreteTaskInstance` without `isOk` check
5. x-pack/platform/plugins/shared/task_manager/server/task_store.ts lines 1336-1339: `docHasEncryptedApiKey` checks `apiKey`/`uiamApiKey`/`userScope` which are undefined on error objects
6. x-pack/platform/plugins/shared/task_manager/server/task_store.ts lines 927-928: `bulkDelete` uses original `taskIds`, not filtered by retrieval success