Skip to content

[Response Ops][Task Manager] Update functions do not handle API key invalidation#249109

Merged
doakalexi merged 13 commits intoelastic:mainfrom
doakalexi:alerting/244918
Jan 29, 2026
Merged

[Response Ops][Task Manager] Update functions do not handle API key invalidation#249109
doakalexi merged 13 commits intoelastic:mainfrom
doakalexi:alerting/244918

Conversation

@doakalexi
Copy link
Copy Markdown
Contributor

@doakalexi doakalexi commented Jan 14, 2026

Resolves #244918
Resolves #247560. I fixed a skipped test, bc I needed to write tests in that area.

Summary

This PR updates the bulk update function in to regenerate the API key on task update. There may be cases where we don't want to regenerate the API key, so I added an flag that can be passed in the with the request to indicate whether or not we want regenerate the API key.

I made some changes to bulk update function to handle the following:

  • If a task with an API key is updated without a request, we cannot correctly get the encrypted saved objects client to use for update and so we throw an error.
  • If a task with an API key is updated with a request and the regenerate flag is set to true, we invalidate the old API key and create a new API key using the new request.
  • If a task without an API key is updated with a request, no changes are made bc we don't want to add an API key to this task
  • If a task without an API key is updated without a request, no changes are made.

Note: I didn't update the update or bulkPartialUpdate functions because they are only used internally by task manager. Pls let me know if I'm misunderstanding, and I can add them there too.

Checklist

To verify

  1. Update scheduled_reports_service.ts here (on line 496) with
await this.taskManager.bulkUpdateSchedules([id], schedule, {
        request: this.request,
        regenerateApiKey: true,
      });
  1. Start Kibana
  2. Go to discover and create a scheduled report and let it run
  3. Go to dev tools and run the following command. Take note of the API key
POST .kibana_task_manager/_search
{
    "query": {
        "term": {
          "task.taskType": {
            "value": "report:execute-scheduled"
          }
        }
    }
}
  1. Go to the reporting page in stack management and edit the schedule date to be a couple minutes in the future
  2. Repeat step 4 and verify that the API key has been updated.
  3. Verify that after the update the scheduled report runs as expected.

@doakalexi doakalexi changed the title Adding logic to regenerate api keys on task update [Response Ops][Task Manager] Update functions do not handle API key invalidation Jan 15, 2026
@doakalexi doakalexi added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v9.4.0 labels Jan 15, 2026
);
});

test('uses regular repository when no request is provided even if docs have apiKey and userScope', async () => {
Copy link
Copy Markdown
Contributor Author

@doakalexi doakalexi Jan 15, 2026

Choose a reason for hiding this comment

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

I updated this test to throw an error, but the changes make it seem like I changed the other tests too.

@doakalexi
Copy link
Copy Markdown
Contributor Author

/oblt-deploy

@doakalexi doakalexi marked this pull request as ready for review January 21, 2026 15:15
@doakalexi doakalexi requested a review from a team as a code owner January 21, 2026 15:15
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@pmuellr
Copy link
Copy Markdown
Contributor

pmuellr commented Jan 22, 2026

from #244918 (comment) :

if a user updates the search query, that should result in an API key change.

Note that search query, for a scheduled report, isn't stored in the task at all, so it makes sense to parameterize the regenerateApiKey flag - the caller is the only one that can determine if the API key should be updated. However, it's set for all the elements passed in. I don't think this is a problem today, but if some code did need to do some bulk updates with some regen'd and some not, they'd have to make two separate bulk calls. So, not a problem, something to consider for the future.

Likewise, if any doc in an update with fail with one of the new constraints (has an API key, request is null), ALL of the updates will fail. Again, we may want to revisit this later, but I think it's fine for now, as it seems unlikely folks would be mixing and matching tasks with and without API keys.

Copy link
Copy Markdown
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, except for the timing on marking API keys invalid. Looks like it's too early to me, an error after processing those could potential cancel the update, but also invalidate the keys that are still in the tasks?

if (todayMonth === runAtMonth) {
expect(runAtDay >= todayDay + 2).to.be(true);
} else if (todayMonth < runAtMonth) {
} else if (todayMonth < runAtMonth || todayYear < runAtYear) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I notice this failed on Dec 29 - makes sense, fails starting on Dec 29, starts working again Jan 1 :-)

): Promise<BulkUpdateResult[]> {
const soClientToUpdate = this.getSoClientForUpdate(docs, options);
const apiKeyAndUserScopeMap =
(await this.regenerateApiKeyFromRequest(docs, options)) || new Map();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We mark invalidated API keys in this new function, but it seems too early, if an error occurs somewhere after here and we throw an error instead of doing the update. Seems like we should only invalidate the keys if the SO bulk update succeeds.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh okay that makes sense! I will update this, thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in this commit, 48d89c0

@elasticmachine
Copy link
Copy Markdown
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #39 / Cloud Security Posture Security Alerts Page - Graph visualization expanded flyout - show alert details
  • [job] [logs] FTR Configs #39 / Cloud Security Posture Test adding Cloud Security Posture Integrations CSPM AWS CIS_AWS Organization Manual Direct Access CIS_AWS Organization Manual Direct Access Workflow

History

Copy link
Copy Markdown
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, left a code style comment


// If a task with an API key is updated with a request
if (hasEncryptedFields && options?.request && options?.regenerateApiKey) {
const docsWithApiKeys: ConcreteTaskInstance[] = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could move these two lines up, to be in top-level function scope. And then also move the return { ... } down, replacing the current return null;. Then you wouldn't need the conditional assignment at the call-site.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in this commit, fb51512 and this one 0b83b97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thx

@doakalexi doakalexi merged commit 96115da into elastic:main Jan 29, 2026
16 checks passed
mbondyra added a commit to mbondyra/kibana that referenced this pull request Jan 30, 2026
…iew_cps

* commit '5f7fec57cb01883038810bd735a0666683b49904': (116 commits)
  [Security Solution][Attacks/Alerts][Setup and miscellaneous] Advanced setting to control feature visibility (elastic#250157) (elastic#250830)
  Fix synthtrace `fetch` usage (elastic#250950)
  [APM] Add Nodes and Edges components and selection logic (elastic#250937)
  [Docs] Update alerting-settings.md and add serverless value for one parameter (elastic#250842)
  [Agent Builder] filestore: initial implementation (elastic#250043)
  [CPS] Support CPS in Vega ESQL (elastic#250693)
  Adjustments to cascade document esql helpers (elastic#250560)
  [Security Solutions] Trial Companion - adds ai chat and elastic agent detectors (elastic#250908)
  [Obs Presentation] Code Scanning Alert Fixes (elastic#250858)
  [performance] add return and refresh render scenarios to dashboard journeys (elastic#250939)
  skip failing test suite (elastic#245458)
  Add Cloud Forwarder onboarding tile to O11y Solution (elastic#250325)
  [Traces] Remove APM unified trace waterall embeddable registration (elastic#250808)
  [Discover] [Metrics] Fix: metrics grid titles do not update on order change (elastic#250963)
  [a11y] Fix Eui modal title annoucment (elastic#250459)
  [Cloud Security] [Fleet] Add cloud connector access scope for input or package level credential definitions (elastic#250280)
  [WorkplaceAI] SharePoint Online stack connector (elastic#248737)
  [Response Ops][Task Manager] Update functions do not handle API key invalidation (elastic#249109)
  [Osquery] Remove @kbn/timelines-plugin dependency from osquery plugin (elastic#250055)
  [One Discover][Logs UX] Update OpenTelemetry Semantic Conventions (elastic#250346)
  ...
hannahbrooks pushed a commit to hannahbrooks/kibana that referenced this pull request Jan 30, 2026
…nvalidation (elastic#249109)

Resolves elastic#244918
Resolves elastic#247560. I fixed a
skipped test, bc I needed to write tests in that area.

## Summary
This PR updates the bulk update function in to regenerate the API key on
task update. There may be cases where we don't want to regenerate the
API key, so I added an flag that can be passed in the with the request
to indicate whether or not we want regenerate the API key.

 I made some changes to bulk update function to handle the following:
- If a task with an API key is updated without a request, we cannot
correctly get the encrypted saved objects client to use for update and
so we throw an error.
- If a task with an API key is updated with a request and the regenerate
flag is set to true, we invalidate the old API key and create a new API
key using the new request.
- If a task without an API key is updated with a request, no changes are
made bc we don't want to add an API key to this task
- If a task without an API key is updated without a request, no changes
are made.

Note: I didn't update the `update` or `bulkPartialUpdate` functions
because they are only used internally by task manager. Pls let me know
if I'm misunderstanding, and I can add them there too.

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### To verify

1. Update `scheduled_reports_service.ts`
[here](https://github.com/elastic/kibana/blob/804c2bbd2aedd81fd708db7435492d9701ef90c5/x-pack/platform/plugins/private/reporting/server/services/scheduled_reports/scheduled_reports_service.ts#L496)
(on line 496) with
```
await this.taskManager.bulkUpdateSchedules([id], schedule, {
        request: this.request,
        regenerateApiKey: true,
      });
```

3. Start Kibana
4. Go to discover and create a scheduled report and let it run
5. Go to dev tools and run the following command. Take note of the API
key
```
POST .kibana_task_manager/_search
{
    "query": {
        "term": {
          "task.taskType": {
            "value": "report:execute-scheduled"
          }
        }
    }
}
```
6. Go to the reporting page in stack management and edit the schedule
date to be a couple minutes in the future
7. Repeat step 4 and verify that the API key has been updated.
8. Verify that after the update the scheduled report runs as expected.
adcoelho added a commit that referenced this pull request Feb 19, 2026
…reports run schedule (#253767)

Closes #245086

## Summary

As a follow-up to the changes in #249109 I am updating the call to
bulkUpdateSchedules to regenerate the API key.

I used #244918 as a reference
where the description says:

> If a task with an API key is updated with a request, we **should**
invalidate the old API key and create a new API key using the new
request.

## Testing

1. Create a scheduled report which should run soon
2. Verify that created scheduled report ran successfully in the
Reporting > Exports tab
3. Update the report to run soon again via Reporting > Schedules or via
API
4. Confirm that the edited scheduled report ran successfully
ersin-erdal pushed a commit to ersin-erdal/kibana that referenced this pull request Feb 19, 2026
…reports run schedule (elastic#253767)

Closes elastic#245086

## Summary

As a follow-up to the changes in elastic#249109 I am updating the call to
bulkUpdateSchedules to regenerate the API key.

I used elastic#244918 as a reference
where the description says:

> If a task with an API key is updated with a request, we **should**
invalidate the old API key and create a new API key using the new
request.

## Testing

1. Create a scheduled report which should run soon
2. Verify that created scheduled report ran successfully in the
Reporting > Exports tab
3. Update the report to run soon again via Reporting > Schedules or via
API
4. Confirm that the edited scheduled report ran successfully
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v9.4.0

Projects

None yet

3 participants