Onboards security-analytics plugin to centralized resource authz#1735
Draft
DarshitChanpura wants to merge 14 commits into
Draft
Onboards security-analytics plugin to centralized resource authz#1735DarshitChanpura wants to merge 14 commits into
DarshitChanpura wants to merge 14 commits into
Conversation
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Member
Author
|
CI will be unblocked once #1736 is merged and this branch gets those changes |
- 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>
DarshitChanpura
force-pushed
the
onboard-centralized-auth
branch
from
July 20, 2026 21:20
1504e38 to
673de27
Compare
Contributor
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit 2c11cf1.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
Member
Author
|
CI failures are unrelated to this PR — all builds fail with |
- 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>
DarshitChanpura
force-pushed
the
onboard-centralized-auth
branch
from
July 22, 2026 18:53
0d5231e to
33f2410
Compare
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>
DarshitChanpura
force-pushed
the
onboard-centralized-auth
branch
from
July 22, 2026 22:25
fdf4398 to
8466213
Compare
- 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>
- Register .opensearch-sap-detectors-config and .opensearch-sap- correlation-rules-config as system indices so the security plugin grants the plugin subject (PluginClient) access on search - Delete test roles before recreation in IT setup, since createIndexRole helper skips update if role already exists (leftover state from prior JVM would prevent permission updates) Known limitation: sharing record creation on resource create is racy — the security plugin's ResourceIndexListener.postIndex sees null userSubject in some paths because our transport actions stash context without proper restore-around-operation. This affects ~10 of the ResourceSharingIT tests and requires a deeper refactor to match the reporting/AD plugin pattern (stash + restore inline). The core framework integration (SPI, DocRequest, PluginClient, access-level enforcement) is functional and matches the other plugins that have onboarded. Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Registering .opensearch-sap-detectors-config and .opensearch-sap- correlation-rules-config as system indices caused a regression in non-resource-sharing tests: even the admin user was denied access with 'no permissions for []' errors. The security plugin's SystemIndexAccessEvaluator restricts access to registered system indices in ways that broke existing functionality. The plugin subject search access issue (originally motivating this registration) needs a different solution. Also update ResourceSharingIT test setup with expanded permissions and role cleanup for future iterations. 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>
1 task
Member
Author
|
CI blocked by #1749 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements resource-access-control for detector and correlation-rule.
Related Issues
Resolves #1747
Check List
--signoff.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.