Skip to content

Correct slow log user for RCS 2.0 #130140

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Jul 24, 2025
Merged

Correct slow log user for RCS 2.0 #130140

merged 22 commits into from
Jul 24, 2025

Conversation

gmjehovich
Copy link
Contributor

@gmjehovich gmjehovich commented Jun 26, 2025 •
edited
Loading

Description:
This PR addresses an issue where Elasticsearch slow logs, specifically on the fulfilling cluster during a Cross-Cluster Search (CCS) with RCS 2.0, displayed the authentication details of the cross-cluster API key's creator instead of the original user who initiated the remote search.

Solution Overview:

  • Corrected User Context for CCS: Modified Security.getAuthContextForSlowLog() to extract the originalAuthentication (the Authentication object representing the user on the querying cluster) when processing cross-cluster access requests.
    • Includes user.effective.* fields if the original user was performing a run-as operation on the querying cluster.
    • Includes apikey.id and apikey.name if the original user authenticated via an API key on the querying cluster.

Testing:

  • Added comprehensive unit tests for getAuthContextForSlowLog() in SecurityTests to cover various scenarios for both local and cross-cluster access.
  • See comments for discussion on integration tests (resolved)

Ticket
Original issue is ES-8568 on Jira.

@gmjehovich gmjehovich added Team:Security Meta label for security team :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) >enhancement labels Jun 26, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @gmjehovich, I've created a changelog YAML for you.

@gmjehovich
Copy link
Contributor Author

Discussion on Integration tests:

As I understand, SecuritySlowLogIT is designed for a single-cluster setup, whereas true Cross-Cluster Search (CCS) inherently requires two clusters. I believe we would need to set up a test that spins up multiple clusters.

Are there existing multi-cluster IT frameworks or standard practices within Elasticsearch that could accommodate a true E2E CCS test for this kind of logging behavior?
Or is this generally considered too complex or out of scope, relying instead on the comprehensive unit test coverage for the getAuthContextForSlowLog() logic?

@gmjehovich gmjehovich self-assigned this Jun 26, 2025
@gmjehovich gmjehovich requested a review from n1v0lg June 26, 2025 21:08
@n1v0lg
Copy link
Contributor

n1v0lg commented Jun 27, 2025

As I understand, SecuritySlowLogIT is designed for a single-cluster setup, whereas true Cross-Cluster Search (CCS) inherently requires two clusters. I believe we would need to set up a test that spins up multiple clusters.

@gmjehovich true!

I think an integration test is a good idea. We have AbstractRemoteClusterSecurityTestCase as CCS test infrastructure. I'd model a new REST test case after e.g., RemoteClusterSecurityApiKeyRestIT -- the test method itself will of course be different but the two cluster setup should be fairly similar.

@gmjehovich gmjehovich marked this pull request as ready for review July 10, 2025 18:28
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

n1v0lg
n1v0lg reviewed Jul 17, 2025
View reviewed changes
Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

Nice work, and solid test coverage!

A few suggestions but this is almost ready to ship.

@n1v0lg n1v0lg self-requested a review July 24, 2025 12:41
n1v0lg
n1v0lg approved these changes Jul 24, 2025
View reviewed changes
Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on the test coverage 👍

@gmjehovich gmjehovich merged commit 88f9b54 into elastic:main Jul 24, 2025
39 checks passed
szybia added a commit to szybia/elasticsearch that referenced this pull request Jul 25, 2025
@szybia
Merge remote-tracking branch 'upstream/main' into index-pipeline-trac…
11a2dfe 
…king

* upstream/main: (90 commits)
  Register a blob cache long counter metric for total evicted regions (elastic#131862)
  Move plan attribute resolution to its own component (elastic#131830)
  Make restore support multi-project (elastic#131661)
  Use logically more correct expression (elastic#131869)
  [ES|QL] Change equals and hashcode for ConstantNullBlock (elastic#131817)
  Update `TransportVersion` to support a new model (elastic#131488)
  Correct slow log user for RCS 2.0 (elastic#130140)
  Revert "Remove 8.17 from dev branches"
  Mute org.elasticsearch.compute.aggregation.ValuesBytesRefGroupingAggregatorFunctionTests testSomeFiltered elastic#131878
  Remove 8.17 from dev branches
  Revert "CompressorFactory.compressor (elastic#131655)" (elastic#131866)
  Add fast path for single value in VALUES aggregator (elastic#130510)
  Resolve inference release tests failing due to missing feature flag  (elastic#131841)
  [Docs] Replace placeholder URLs (elastic#131309)
  CompressorFactory.compressor (elastic#131655)
  add availability info for speed loading setting (elastic#131714)
  [Logstash] Move `elastic_integration` plugin usage to ES logstash-bridge. (elastic#131486)
  Migrate x-pack-enrich legacy rest tests to new test framework (elastic#131743)
  Fix plugin example test failures due to deprecation warning (elastic#131819)
  Remove deprecated function isNotNullAndFoldable (elastic#130944)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n1v0lg n1v0lg n1v0lg approved these changes

Assignees

@gmjehovich gmjehovich

Labels
>enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v9.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gmjehovich @elasticsearchmachine @n1v0lg