Onboards security-analytics plugin to centralized resource authz by DarshitChanpura · Pull Request #1735 · opensearch-project/security-analytics

DarshitChanpura · 2026-06-20T07:11:26Z

Description

Implements resource-access-control for detector and correlation-rule.

Related Issues

Resolves #1747

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

DarshitChanpura · 2026-06-20T07:28:20Z

CI will be unblocked once #1736 is merged and this branch gets those changes

github-actions · 2026-07-20T21:21:04Z

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 9fa2af3.

⛔ Hard block: Issues at High severity or above will block this PR from merging.

Path	Line	Severity	Description
build.gradle	244	high	New compile-only dependency 'opensearch-security-spi' added. Per mandatory supply-chain rule, all dependency additions must be flagged for maintainer verification regardless of apparent legitimacy.
build.gradle	116	high	New optional plugin dependency 'opensearch-security' added via extendedPlugins. Build/plugin dependency changes require maintainer verification per mandatory supply-chain flagging rule.
src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java	1740	medium	Internal security plugin header '_opendistro_security_authenticated_user' is captured from the original request context and re-injected into a stashed thread context. This carries authenticated-user identity across a security boundary (stashed context normally clears transient identity to prevent privilege escalation). Code comment justifies this as needed for resource-ownership recording, but the pattern enables cross-thread authentication context propagation.
src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCorrelationRuleAction.java	163	medium	Same pattern as TransportIndexDetectorAction: '_opendistro_security_authenticated_user' persistent header captured pre-stash and re-injected post-stash into the write thread context. Manipulates internal security plugin auth headers across thread-context boundaries.
src/main/java/org/opensearch/securityanalytics/transport/TransportGetDetectorAction.java	78	low	Backend role validation and per-resource user permission checks are conditionally skipped when ResourceSharingUtils.shouldUseResourceAuthz() returns true. If the feature flag or the ResourceSharingClient are misconfigured or compromised, both authorization layers are silently bypassed. Same pattern applies in TransportIndexDetectorAction and TransportSearchDetectorAction.

The table above displays the top 10 most important findings.

Total: 5 | Critical: 0 | High: 2 | Medium: 2 | Low: 1

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

DarshitChanpura · 2026-07-20T21:33:56Z

CI failures are unrelated to this PR — all builds fail with Could not find org.opensearch.plugin:alerting:3.8.0.0-SNAPSHOT. The alerting plugin snapshot hasn't been published to Maven yet after the version bump to 3.8.0. Will resolve once the upstream artifact is available.

DarshitChanpura · 2026-07-24T04:58:08Z

CI blocked by #1749

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

- Add all_shared_principals keyword field to detectors and correlation-rules index mappings for DLS-based filtering - Rename resource-action-groups.yml to resource-access-levels.yml to match security plugin's ResourceAccessLevelHelper (PR #6039) - Configure protected_types setting in test cluster so resource sharing enforcement activates for detector and correlation-rule Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

- Request classes (Get/Delete/IndexDetector, Index/DeleteCorrelationRule) implement DocRequest so security plugin intercepts and enforces document-level access control automatically - Plugin implements IdentityAwarePlugin and creates a PluginClient (FilterClient with PluginSubject) for DLS-filtered search operations - Transport actions conditionally skip legacy backend-role checks when resource-sharing is enabled via ResourceSharingUtils.shouldUseResourceAuthz - Search actions use PluginClient when feature is enabled so DLS filters results by all_shared_principals field - Add integration tests covering detector and correlation-rule CRUD and search with resource sharing enabled Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Add extensive test scenarios: - Full access-level progression (read-only -> read-write -> full-access) - Read-only user cannot update or delete - Read-write user can update but not delete or share further - Full-access user can share further and delete - Updates by shared user are visible to owner - Non-owner with full-access can delete the resource - Multiple users shared at different levels simultaneously - Owner always retains access through sharing/revocation changes - Owner update does not break shared users' access - Correlation rule update with read-write access - Revocation removes access - Legacy behavior preserved when feature is disabled Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The static protected_types cluster setting in build.gradle caused all existing integration tests to fail with 403 because the security plugin's ResourceAccessEvaluator enforced resource-sharing for every request implementing DocRequest, but existing tests don't set up sharing records. Fix: remove protected_types from the static cluster config and instead enable it dynamically via the cluster settings API within ResourceSharingIT setup/teardown. This ensures resource-sharing enforcement only applies when our resource-sharing tests run. Also fix index name collision: replace randomIndex() (which always returns "windows") with unique per-test index names. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

randomDetector() hardcodes 'windows' as its input index. Tests must create that exact index name. Use a create-if-not-exists helper to handle multiple tests sharing the same JVM. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The resource-sharing API uses a single endpoint: - PUT /_plugins/_security/api/resource/share (with resource_id and resource_type in the body) - PATCH /_plugins/_security/api/resource/share (for revoke, with resource_id and resource_type in the body) Previously the tests used a path-based URL format that doesn't exist. Also add wait for sharing record creation after resource creation (ResourceIndexListener creates it asynchronously). Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The resource-sharing share API requires the caller to be the resource owner (or have share permission on the resource). Previously the tests used admin client() which doesn't own the resources. Changes: - shareResource/revokeResource now accept a RestClient parameter - All calls pass ownerClient (the resource creator) - Add indices:admin/delete and alerting cluster perms to test roles - Remove unused shareResourceAsUser (merged into shareResource) Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

- Use persistent (not transient) cluster settings for protected_types to survive across test class boundaries - Simplify cluster permissions to use wildcards matching the security plugin's built-in security_analytics_full_access pattern - Remove cluster:admin/security/resource/share from role (the share API authorization is handled by resource ownership, not cluster permissions) - Fix index permissions to use broader patterns matching system indices Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

- Detector.docParse() now skips extra top-level fields (like all_shared_principals injected by the security plugin) instead of failing with parsing_exception on END_OBJECT check - Restore cluster:admin/security/resource/share permission to test role (required by security plugin to authorize share API calls) - Add .opensearch-sap-* and .opendistro-alerting-* to test role's index patterns so system indices are accessible Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Use the pre-defined security_analytics_full_access and alerting_full_access roles for cluster/index permissions. These grant the plugin-level trust needed for alerting/notification system-index writes during detector creation. Add one custom role for the extras (resource/share, settings/update) that aren't in the pre-defined roles. This matches the working pattern in SecureDetectorRestApiIT which successfully creates detectors as a non-admin user. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

When resource-sharing is enabled, the security plugin's ResourceIndexListener.postIndex records resource ownership by reading the authenticated user from the persistent thread-context header at the time the resource doc is written. Detector creation hands off to the alerting plugin's thread pools (monitor/workflow creation) before the detector doc is written, and those threads do not carry the security plugin's authenticated-user marker -- so postIndex saw a null user and no ownership record was created, which then caused the resource share API to return 403 for the owner. Capture the authenticated-user marker before the context is stashed and re-inject it on the write thread (only if absent) for both the detector and correlation-rule index writes. The context otherwise stays stashed, so the writes remain trusted system-index operations. This is a no-op when resource-sharing is disabled (marker is null). Also make ResourceSharingIT robust to the framework's eventual consistency (ownership record and all_shared_principals DLS field are populated asynchronously): - poll the share API and search hit counts until they converge - wait for owner visibility after create before asserting - send a valid body on update (PUT) forbidden-checks so requests reach the authorization layer instead of failing body parsing with 400 - align delete expectations with resource-access-levels.yml (read_write grants delete; sharing requires full_access) - fix multi-user role-mapping JSON and create roles fresh Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Configure plugins.security.experimental.resource_sharing.protected_types at node startup (alongside the resource_sharing.enabled flag) instead of toggling it dynamically per-test via the cluster settings API. With protected_types set from cluster bootstrap, the resource-sharing indexing listener and DLS filter are active from the first request, which removes a settings-propagation race that made ResourceSharingIT flaky under slower CI timing. Removes the now-unused dynamic enable/disable helpers from the IT. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The security-enabled CI job runs the full integTest suite (--tests '*IT') against a single cluster. With protected_types set from cluster boot, every detector/correlation-rule operation is routed through resource authorization and DLS. Legacy ITs act as the plain admin client and get filtered/denied by that, so running them against the protected cluster produced ~100 spurious 403s and DLS count mismatches. Filter the integTest task by the resource_sharing.enabled flag: run only ResourceSharingIT when enabled, and exclude it otherwise. This mirrors the reporting plugin's approach and gives the resource-sharing tests an isolated, uncontended cluster (which also removes cross-test user/role churn that caused share 403s and visibility timeouts in ResourceSharingIT itself). Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

…races The security-enabled resource-sharing CI job intermittently failed with three distinct races, all rooted in the fact that the security plugin applies changes asynchronously: 1. createUser/mapUsersToRole trigger an async security-config reload. If the test body ran before the reload settled, requests failed with 401 (user not yet loaded) or a spurious 403 (role mapping not yet applied) - most visibly as a persistent 403 on cluster:admin/security/resource/share for the owner. Fix: gate setup on each client being able to authenticate and reach the SA authorization layer before the test proceeds (waitForUserReady). 2. The ResourceIndexListener records resource ownership fire-and-forget after the resource doc is indexed. Sharing before that record is durable made the share API treat the owner as a non-owner (403). Fix: wait for the owner to be able to GET their own resource before sharing, in every detector test. 3. Ownership/DLS record propagation on a saturated single-node cluster can take tens of seconds. Fix: raise the eventual-consistency poll budget to 120s and poll every 500ms (a tighter loop adds request load that slows the very propagation being awaited). Verified locally with the exact CI command (./gradlew integTest -Dsecurity=true -Dhttps=true -Dresource_sharing.enabled=true --tests '*IT'): 11/11 pass with zero first-attempt failures (no reliance on the Gradle retry mechanism). Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The DocRequest.type() implementations and the ResourceProvider.resourceType() methods hardcoded the "detector"/"correlation-rule" strings, duplicating the constants already defined in ResourceSharingUtils. Reference those constants directly so the resource-type identifiers have a single source of truth, mirroring the anomaly-detection plugin's use of shared *_RESOURCE_TYPE constants. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

…-auth

The RS(21) CI job still flaked because the test waited on eventually-consistent proxies rather than the exact async artifacts it depended on. Replace those waits with direct signals: - waitForOwnershipRecord: poll the security plugin's "<index>-sharing" index directly (as super-admin, bypassing DLS) via GET-by-_id (the sharing record's document _id is the resource id). This targets the fire-and-forget ownership record itself instead of a detector GET, which additionally waits on DLS principal population. Treat 404 (not yet written) and 503 (sharing-index shard still RECOVERING right after creation) as transient and keep polling. - waitForRoleEffective: gate setup on the /_plugins/_security/authinfo API reporting OWNER_ROLE in the user's effective roles, rather than probing a detector search. The search is satisfied by security_analytics_full_access and returned before OWNER_ROLE - the last mapping written, granting the resource-share permission - was applied, causing spurious share 403s. - Both search-filtering tests now wait for every resource's ownership record to be durable before asserting search counts, so DLS visibility is triggered for all resources rather than racing the last write (expected:2 but was:1). Verified locally with the exact CI command (./gradlew integTest -Dsecurity=true -Dhttps=true -Dresource_sharing.enabled=true --tests '*IT'): 11/11 pass with zero first-attempt failures. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

The onboarding change dropped the `if (securityEnabled)` guard around the opensearch-security opensearchPlugin dependency, so Gradle resolved and downloaded the security zip even for non-security builds. The zip is only ever consumed inside the securityEnabled block that installs it into the test cluster, so restore the guard to match upstream and avoid the needless resolution. The resource-sharing SPI (compileOnly) is a separate dependency and is unaffected. Verified: `dependencies --configuration opensearchPlugin` reports "No dependencies" without -Dsecurity=true and resolves opensearch-security with it. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

getInstance() used unsynchronized lazy initialization, so two threads racing on the first access could each create a separate accessor; the instance that did not receive setResourceSharingClient() would leave shouldUseResourceAuthz() permanently false and silently fall back to legacy authorization. Use double-checked locking with a volatile instance, and mark the client field volatile so a client assigned on one thread is visible to reader threads. This matches the reporting plugin's accessor implementation. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura · 2026-07-30T01:08:10Z

Code-Diff-Analyzer findings — response

I reviewed all 6 findings from the analyzer report. One was a real (low-severity) fix and has been addressed; the rest are correct-by-design or a policy flag on a legitimate first-party dependency. Details below.

#	Finding	Severity	Disposition
1	New `opensearch-security-spi` compile-only dependency	high	Needs maintainer bypass — see below
5	`ResourceSharingClientAccessor` singleton data race	low	Fixed in `d32faa45`
2	Auth-user header re-injection (`TransportIndexDetectorAction`)	medium	Correct & safe — no change
3	Auth-user header re-injection (`TransportIndexCorrelationRuleAction`)	medium	Correct & safe — no change
4	`TransportGetDetectorAction` skips legacy checks when RS active	medium	Intended design — no change
6	`Detector.docParse` skips unknown top-level fields	low	Necessary & acceptable — no change

#1 (high) — opensearch-security-spi dependency: This is the analyzer's mandatory "flag all dependency changes for provenance review" policy, not a code defect. org.opensearch:opensearch-security-spi is a first-party OpenSearch artifact pinned to the same ${opensearch_build} version as every other core dependency, and it is the foundational SPI this entire PR is built on. This cannot be resolved by a code change — it requires a maintainer to verify provenance and apply the skip-diff-analyzer label, then re-run.

#5 (low) — accessor data race [FIXED]: getInstance() used unsynchronized lazy init; racing threads could create competing instances, one of which would never receive setResourceSharingClient() and leave shouldUseResourceAuthz() permanently false. Fixed with double-checked locking + volatile, matching the reporting plugin's accessor.

#2 / #3 (medium) — auth-user header re-injection: The guard only re-injects the persistent authenticated-user marker that the original request already carried (captured from the caller's context), and only when it is absent, on an already-stashed context. Detector/correlation-rule creation hands off to alerting-plugin threads (monitor/workflow creation) that don't carry this marker; without re-injection, ResourceIndexListener.postIndex cannot record resource ownership. It restores the true caller's identity that was lost across the thread hand-off — it cannot supply an arbitrary or elevated principal, and the write stays a trusted system-index op (no transient user).

#4 (medium) — GetDetector authorization: When resource-sharing is active, authorization is moved to the security plugin's ResourceAccessEvaluator (via the DocRequest interface) plus DLS filtering — the legacy backend-role/ownership checks are replaced, not removed. When the resource-sharing client is null/misconfigured, shouldUseResourceAuthz() returns false and the legacy checks fully apply (fail-safe). This mirrors the reporting and anomaly-detection onboardings.

#6 (low) — docParse skipping unknown fields: Required so the parser tolerates the all_shared_principals field the security plugin injects into stored detector documents. This is the standard forward-compatible-parsing tradeoff.

ResourceSharingIT exercises the security plugin's resource-sharing subsystem, whose config-reload, ownership-record, and DLS-principal propagation are eventually consistent and can lag under load on the single-node CI cluster. The test already gates on readiness signals (authinfo role effectiveness, direct ownership-record lookup, per-resource DLS gating) to minimise this, but a residual retry budget is still required, as in the other resource-sharing onboarded plugins. The resource-sharing matrix job runs only ResourceSharingIT (see the resource_sharing.enabled filter), on its own dedicated cluster, so raising maxRetries to 5 for that job does not mask flakiness in any other test. The normal suite stays at 3. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura force-pushed the onboard-centralized-auth branch from 1504e38 to 673de27 Compare July 20, 2026 21:20

DarshitChanpura closed this Jul 21, 2026

DarshitChanpura reopened this Jul 21, 2026

DarshitChanpura force-pushed the onboard-centralized-auth branch from 0d5231e to 33f2410 Compare July 22, 2026 18:53

DarshitChanpura closed this Jul 22, 2026

DarshitChanpura reopened this Jul 22, 2026

DarshitChanpura force-pushed the onboard-centralized-auth branch from fdf4398 to 8466213 Compare July 22, 2026 22:25

DarshitChanpura mentioned this pull request Jul 23, 2026

Register detector and correlation-rule indices as system indices #1749

Merged

1 task

DarshitChanpura added 11 commits July 27, 2026 14:06

Onboards security-analytics plugin to centralized resource authz

9a28d67

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura force-pushed the onboard-centralized-auth branch from 2c11cf1 to 635f32b Compare July 27, 2026 21:11

DarshitChanpura added 5 commits July 27, 2026 18:09

DarshitChanpura closed this Jul 28, 2026

DarshitChanpura reopened this Jul 28, 2026

DarshitChanpura added 2 commits July 29, 2026 16:12

Merge remote-tracking branch 'upstream/main' into onboard-centralized…

18ffcc1

…-auth

DarshitChanpura marked this pull request as ready for review July 30, 2026 00:40

DarshitChanpura requested review from AWSHurneyt, KashKondaka, amsiglan, eirsep, engechas, getsaurabh02, goyamegh, lezzago, manaswini1920, nishtham-amazon, riysaxen-amzn, toepkerd and vikhy-aws as code owners July 30, 2026 00:40

DarshitChanpura added 2 commits July 29, 2026 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Onboards security-analytics plugin to centralized resource authz - #1735

Onboards security-analytics plugin to centralized resource authz#1735
DarshitChanpura wants to merge 21 commits into
opensearch-project:mainfrom
DarshitChanpura:onboard-centralized-auth

DarshitChanpura commented Jun 20, 2026 •

edited

Loading

Uh oh!

DarshitChanpura commented Jun 20, 2026

Uh oh!

github-actions Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

DarshitChanpura commented Jul 20, 2026

Uh oh!

DarshitChanpura commented Jul 24, 2026

Uh oh!

DarshitChanpura commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

DarshitChanpura commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

DarshitChanpura commented Jun 20, 2026

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Analyzer ❗

Uh oh!

DarshitChanpura commented Jul 20, 2026

Uh oh!

DarshitChanpura commented Jul 24, 2026

Uh oh!

DarshitChanpura commented Jul 30, 2026

Code-Diff-Analyzer findings — response

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

DarshitChanpura commented Jun 20, 2026 •

edited

Loading

github-actions Bot commented Jul 20, 2026 •

edited

Loading