Repository navigation
Cases implicit privileges provider - #152714
Conversation
…ana-specific implicit privileges
…les-cluster-test Add javaRestTest for Kibana implicit privileges
Adds an ImplicitPrivilegesProvider for the Kibana Cases-as-data indices (.cases, .cases-activity, .cases-attachments), scoped by both Kibana space (space_id) and owning solution (owner: cases, observability, securitySolution) via the cases:<owner>/getCase actions. Also drops the redundant x-pack-core extendedPlugins entry flagged in outstanding review feedback on elastic#148331, since x-pack-security already extends it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
a596bf5 to
3c961c9
Compare
…ssons to the Cases provider Mirrors three fixes that landed on elastic#148331 this week, applied only to the Cases-specific files (not the shared plugin scaffold): - Fast-path return when the role has no application privileges at all, avoiding an unnecessary scan of stored privileges. - Replace stream().anyMatch() + list allocation with a plain for-loop for the resolved-name matching path (JIT-friendlier). - Hand-roll the DLS queries via XContentBuilder rather than QueryBuilders.termQuery/termsQuery, which always serialize a "boost":1.0 field that would otherwise leak into the query surfaced via GET /_security/role/<name>?include_implicit=true. Deliberately not applied: the KibanaPlugin -> KibanaSecurityPlugin rename and the shared x-pack/qa test tweak, since both are part of elastic#148331's own surface rather than cases-focused code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aPlugin Merging main (which now has elastic#148331, including the KibanaPlugin -> KibanaSecurityPlugin rename) left both classes present: the stale KibanaPlugin.java carried our Cases provider registration, while the new KibanaSecurityPlugin.java (correctly wired into module-info.java, the SecurityExtension SPI file, and build.gradle) only had the alerts provider. Delete the orphan and register KibanaCasesImplicitPrivilegesProvider alongside KibanaAlertsImplicitPrivilegesProvider on the real class. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…loop Was being rebuilt up to three times per role block (once per owner action) even though the underlying privileges array never changes - StringMatcher.of constructs an Automaton for wildcard patterns, so that cost shouldn't be paid three times over. Build it once per block and reuse it across all three action checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Pinging @elastic/es-security (Team:Security) |
testGetPrivilegesForApiKeyWorksIfItDoesNotHaveAssignedPrivileges hardcodes the exact privilege set for a superuser-equivalent API key. elastic#148331 already had to add an entry here for the alerts provider's implicit grant, since a superuser's wildcard application privilege resolves through every registered ImplicitPrivilegesProvider. Adding KibanaCasesImplicitPrivilegesProvider means a second entry now appears - one .cases*/.cases-activity*/.cases-attachments* grant with three per-owner DLS queries (cases, observability, securitySolution), positioned before the alerts entry per the actual observed CI output. Confirmed via repo-wide grep that no other test hardcodes this index pattern, so this is the only assertion affected by the second provider. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…er' into cases-implicit-privileges-provider
Mirrors the alerts provider's rewrite for the new ImplicitPrivilegesProvider signature: getImplicitIndicesPrivileges(Collection<ResolvedApplicationPrivilege>) replaces the old (RoleDescriptor, Collection<ApplicationPrivilegeDescriptor>) pair. CompositeRolesStore now resolves each grant's action automaton once via ApplicationPrivilege.get(...) - covering both the resolved-name and raw-action-pattern paths, plus wildcard application-name expansion - and passes the resolved ApplicationPrivilege + resources to every provider, so providers no longer rebuild a matcher per role block. Collapses collectResourcesByOwner to a single flat loop over the resolved grants, testing privilege.predicate().test(action) per owner action. The dual-track matching, the per-block StringMatcher construction, and the now-redundant empty-applicationPrivileges fast path are all gone - that work now lives upstream in ApplicationPrivilege.get and CompositeRolesStore. Tests updated to build ResolvedApplicationPrivilege fixtures via a resolve() helper (mirroring the alerts test's own helper) that calls ApplicationPrivilege.get(...) exactly as CompositeRolesStore does, so the existing owner-isolation, multi-owner, and wildcard test coverage exercises the real resolution path rather than hand-rolled matching. Verified: 24/24 unit tests, 1/1 IT test, full :x-pack:plugin:kibana:check green, and re-ran ApiKeyRestIT.testGetPrivilegesForApiKeyWorksIfItDoesNotHaveAssignedPrivileges (fixed earlier for the second-provider surface) to confirm the SPI refactor doesn't change the observable query shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ebarlas
left a comment
There was a problem hiding this comment.
The changes look great overall! They exactly mirror #148331, so I don't have much feedback.
One notable difference in docs/changelog/. This PR ought to have a changelog entry as well.
area: Security
issues: []
pr: 152714
summary: Contribute implicit index privileges for Kibana Cases from the `x-pack-kibana`
plugin
type: feature|
|
||
| public class KibanaCasesImplicitPrivilegesProviderTests extends ESTestCase { | ||
|
|
||
| private static final String[] CASES_INDICES = { ".cases*", ".cases-activity*", ".cases-attachments*" }; |
There was a problem hiding this comment.
These index patterns are redundant. .cases* covers both .cases-activity* and .cases-attachments*.
| static final String GET_CASE_ACTION_OBSERVABILITY = "cases:observability/getCase"; | ||
| static final String GET_CASE_ACTION_CASES = "cases:cases/getCase"; | ||
|
|
||
| static final Map<String, String> GET_CASE_ACTIONS_BY_OWNER = Map.of( |
There was a problem hiding this comment.
This is a mapping from action -> owner.
OWNER_BY_GET_CASE_ACTION would be a clearer name
| "privileges": [ | ||
| "read" | ||
| ], | ||
| "query": [ |
There was a problem hiding this comment.
This literal comparison is brittle, because it relies on Set<BytesReference> query ordering in GetUserPrivilegesResponse production code.
Consider using a local test utility to sort before comparing.
@SuppressWarnings("unchecked")
private static void sortIndicesQueryLists(Map<String, Object> privilegesResponse) {
final List<Map<String, Object>> indices = (List<Map<String, Object>>) privilegesResponse.get("indices");
for (Map<String, Object> index : indices) {
final List<String> query = (List<String>) index.get("query");
if (query != null) {
query.sort(null);
}
}
}- Simplify CASES_INDICES to a single ".cases*" pattern - it already covers ".cases-activity*" and ".cases-attachments*" since they share the prefix, so listing all three was redundant. Updated the provider, its unit tests, and the IT test's assertions (which located and verified the implicit grant by matching on ".cases-activity*" in the "names" list, no longer present now that "names" is a single entry). - Rename GET_CASE_ACTIONS_BY_OWNER -> OWNER_BY_GET_CASE_ACTION - the map goes action -> owner, and the old name read the other way around. - Fix ApiKeyRestIT's brittle literal comparison: GetUserPrivilegesResponse assembles the merged "query" list (and, as a superuser role with both providers active now exercises, the outer "indices" list itself) from Sets internally, so neither has a guaranteed order. Normalize both the actual and expected sides to a canonical order before comparing, rather than asserting on either - the reviewer's suggested query-list sort alone wasn't sufficient once the outer list's hash bucketing also shifted after the CASES_INDICES simplification. Verified: 24/24 Cases unit tests, 1/1 Cases IT test, full :x-pack:plugin:kibana:check green, and ApiKeyRestIT passing across 5 randomized iterations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
buildkite benchmark this with es-security-has-privileges-api |
💚 Build Succeeded
This build ran two es-security-has-privileges-api benchmarks to evaluate performance impact of this PR. History |
|
|
||
| Set<String> spaceIds = resources.stream() | ||
| .filter(r -> r.startsWith(RESOURCE_PREFIX)) | ||
| .map(r -> r.substring(RESOURCE_PREFIX.length())) |
There was a problem hiding this comment.
One thing that's worth considering is what the behaviour should be if a role has "resources": ["space:*"] or even space:d*? As it is currently implemented it would interpret everything after space: as literal space ID. Which means that for space:* it would do a literal match on space_id=*. Should wildcards be handled in space IDs?
There was a problem hiding this comment.
Ah, yea, that would be problematic because in this scenario a user wouldn't have access to these indices even though they have access to all of the spaces 🤔 . How would you recommend going about checking this? A special wildcard case?
| * concrete and settled by equality; a residual wildcard (e.g. {@code "kibana-*"} or | ||
| * {@code "*"} with no matching stored descriptor) is matched with an automaton. | ||
| */ | ||
| private static boolean applicationMatchesKibana(String application) { |
There was a problem hiding this comment.
nit: This method is same as in KibanaAlertsImplicitPrivilegesProvider. We should consider creating a shared util class to avoid duplication. The same goes for space ID parsing. It's another good candidate to refactor into its own util method. Can be done in a followup PR.
Summary
Adds a second
ImplicitPrivilegesProviderto thex-pack/plugin/kibanaplugin (introduced in #148331 for Alerting V2), this one for Kibana Cases.When a user holds a Kibana application privilege whose stored definition includes a
cases:<owner>/getCaseaction, this provider implicitly grantsreadon the.cases*,.cases-activity*, and.cases-attachments*index patterns (which match each index literal and any future sibling/reindexed indices owned by Cases), DLS-scoped by theownerfield always and the top-levelspace_idfield for non-wildcard space resources.Unlike Alerting V2, Cases documents carry two independent scoping dimensions — the owning solution (
owner:securitySolution/observability/cases) and the Kibana space (space_id) — and each owner has its own action. So the DLS query for every grant always filters onowner, plusspace_idunless the role holds the wildcard resource (*) for that owner.A role granting
cases:observability/getCaseonspace:marketingcan therefore runFROM .cases-activityin ES|QL and see only Observability-owned case documents whosespace_idequalsmarketing, with no explicit index privilege configuration in the role.Background
Builds on:
kibanax-pack plugin to manage Kibana-specific implicit privileges #148331 - thex-pack/plugin/kibanaplugin, itsSecurityExtensionregistration, and theKibanaAlertsImplicitPrivilegesProviderthis mirrors.ImplicitPrivilegesProviderSPI, Get Roles API surfacing (?include_implicit=true), and wildcard application-name support.The
.cases*indices, their top-levelspace_id/ownerdocument fields, and thecases:<owner>/getCaseactions this provider keys on are all created on the Kibana side by the Cases "as data" V2 work — useful for reviewers to see where the index patterns, document fields, and actions come from:.cases- [Cases][Cases As Data V2] - Introduce Cases As Data V2 kibana#269581.cases-activity- [Cases] Cases analytics v2 - Activity Index (.cases-activity) kibana#275686.cases-attachments- [Final][Cases] Cases analytics v2 - Attachments Index (.cases-attachments) kibana#276117This is the Cases piece of Phase 2 of the As-Data RBAC initiative.
What this adds
KibanaCasesImplicitPrivilegesProvider- theImplicitPrivilegesProviderimplementation for Cases, registered alongside the alerts provider viaKibanaPlugin#getImplicitPrivilegesProviders. Resources are grouped by owner and emitted as one DLS grant per owner:owner-only when the role holds the wildcard resource for that owner,owner+space_idotherwise. Same dual-track matching as the alerts provider — a role block grants an owner's action if either itsprivileges[]names a stored Kibana privilege whose action set matches, or itsprivileges[]are themselves action patterns that match (e.g.cases:securitySolution/*,*) — and application-name matching honors wildcards (kibana-*,*).cases/common/constants,cases/server/cases_analytics_v2/constants.ts, and theauthorization_corecases actions/privileges). Keep them in sync if those change..github/CODEOWNERS- per-file override co-owningKibanaCasesImplicitPrivilegesProvider(and its test) with@elastic/kibana-cases, following the convention feat(kibana,security): introducekibanax-pack plugin to manage Kibana-specific implicit privileges #148331 set for the alerts provider (co-owned with@elastic/response-ops).KibanaCasesImplicitPrivilegesProviderTests) covering the owner/space grouping, the wildcard-resource (all-spaces) path, resolved-name vs raw-pattern matching, non-Kibana-application / wildcard-app-name paths, and the generated DLS query shape.javaRestTestintegration test (KibanaCasesImplicitPrivilegesIT) exercising the end-to-end grant against a running cluster.KibanaAlertsImplicitPrivilegesProvider(e.g. switching fromAutomatonstoStringMatcher) and its changelog entry.