Skip to content

More fixes after trying the simplified api#17695

Merged
jsoriano merged 4 commits intoelastic:mainfrom
jsoriano:more-fixes-after-simplified-api
Mar 11, 2026
Merged

More fixes after trying the simplified api#17695
jsoriano merged 4 commits intoelastic:mainfrom
jsoriano:more-fixes-after-simplified-api

Conversation

@jsoriano
Copy link
Copy Markdown
Member

@jsoriano jsoriano commented Mar 6, 2026

Follow up to #17656, more issues found on tests after trying to use the simplified API in elastic/elastic-package#3307.

In m365_defender and microsoft_defender_cloud the mock configuration is fixed for the correct refresh token url.
Variables were not correctly set with previous implementation of elastic-package and it looks like the wrong refresh was never tried, so it was silently ignored.

The test in ti_abusetech was actually missing a required variable.

@jsoriano jsoriano self-assigned this Mar 6, 2026
@jsoriano jsoriano requested a review from a team as a code owner March 6, 2026 11:08
@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

🚀 Benchmarks report

Package m365_defender 👍(4) 💚(2) 💔(2)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
incident 1100.11 867.08 -233.03 (-21.18%) 💔
vulnerability 12195.12 8058.02 -4137.1 (-33.92%) 💔

To see the full report comment with /test benchmark fullreport

Copy link
Copy Markdown
Collaborator

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewkroh andrewkroh added Integration:ti_abusech abuse.ch Integration:m365_defender Microsoft Defender XDR Integration:microsoft_defender_cloud Microsoft Defender for Cloud Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Mar 6, 2026
@elasticmachine
Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The mock config changes for OAuth2 v2.0 refresh token endpoint and the ti_abusech auth_key addition look correct.

One additional file may need the same fix:

packages/m365_defender/_dev/deploy/docker/vulnerability-http-mock-config.yml (lines 12-22) still uses the old /tenant_id/oauth2/token path and includes the Authorization: Basic header for the refresh token rule, which appears to be the same pattern being fixed in the alert and incident mock configs. Consider whether this file should also be updated for consistency.


Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

cc @jsoriano

@jsoriano
Copy link
Copy Markdown
Member Author

jsoriano commented Mar 9, 2026

@claude

packages/m365_defender/_dev/deploy/docker/vulnerability-http-mock-config.yml (lines 12-22) still uses the old /tenant_id/oauth2/token path and includes the Authorization: Basic header for the refresh token rule, which appears to be the same pattern being fixed in the alert and incident mock configs. Consider whether this file should also be updated for consistency.

You are right that this config looks also incorrect, but it is not failing at the moment with main or with the changes in elastic/elastic-package#3307, so I would leave it as is by now.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Claude finished @jsoriano's task in 23s —— View job


Acknowledging Your Response

  • Understand the context
  • Review the response and provide acknowledgment

Understood! That's a reasonable approach. I can confirm looking at vulnerability-http-mock-config.yml that lines 12-22 do indeed have the inconsistent pattern (using /tenant_id/oauth2/token with Authorization: Basic header), but since the tests are passing both on main and with the elastic-package changes, it makes sense to keep this PR focused on the failing cases only.

If the vulnerability tests start failing in the future due to this inconsistency, it can be addressed in a separate PR at that time.



Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@jsoriano jsoriano merged commit c8b7740 into elastic:main Mar 11, 2026
9 checks passed
@jsoriano jsoriano deleted the more-fixes-after-simplified-api branch March 11, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-plz-triage Integration:m365_defender Microsoft Defender XDR Integration:microsoft_defender_cloud Microsoft Defender for Cloud Integration:ti_abusech abuse.ch Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants