Skip to content

[entityanalytics_okta] Fix permissions enrichment - #20732

Open
chrisberkhout wants to merge 2 commits into
elastic:mainfrom
chrisberkhout:entityanalytics_okta-perms-fix
Open

[entityanalytics_okta] Fix permissions enrichment#20732
chrisberkhout wants to merge 2 commits into
elastic:mainfrom
chrisberkhout:entityanalytics_okta-perms-fix

Conversation

@chrisberkhout

Copy link
Copy Markdown
Contributor

Proposed commit message

[entityanalytics_okta] Fix permissions enrichment

Fixes permissions enrichment problems that were exposed by the switch to
default minimal-state sync.

The two issues are:
- Disagreement about correct `enrich_with` values: `perms` (legacy path)
  vs `permissions` (minimal-state path).
- Role enrichment not implicitly activated by permissions enrichment
  (on the minimal-state path).

The fixed package works with existing beats versions.

Discussion

Problems

Since 3.5.0 (#20212) this package defaults to use_minimal_state: true, so it uses Filebeat's minimal-state provider (beats#50685). That exposed two problems with how the template is rendered with the "Enrich User Permissions" option set:

  1. Wrong value name. The minimal-state provider strictly validates enrich_with and names this enrichment permissions; the template was using the legacy spelling perms. Enabling the option made the input fail permanently with unable to create minimal-state provider "okta": unknown enrich_with value, and the integration went unhealthy. The legacy path never validated enrich_with (unknown values are silently ignored), which is why this only started failing once minimal-state became the default.

  2. The documented roles implication is unfulfilled. The option description promises "Enabling this option implicitly enables role enrichment". The legacy path implements that in code (roles are fetched when either value is present), but the minimal-state provider only fetches permissions inside the roles enrichment — so even a correctly spelled permissions without roles validates cleanly and then silently does nothing.

Changes

Both changes are in entity-analytics.yml.hbs; rendered values are otherwise unchanged.

1. The permissions spelling depends on the implementation used: permissions when use_minimal_state is true, perms when false. The manifest already requires agent.version: ^9.5.0 since 3.5.0, so every supported agent has both paths and honours the flag. A template comment records the condition for collapsing this to a single value later: the legacy path accepting permissions.

2. The template now emits roles whenever enrich_user_perms is set, restoring the documented implication on the minimal-state path. This is a no-op for the legacy path, which would fetch roles anyway.

Policy tests

Three new policy tests assert the rendered enrich_with list:

  1. default renders groups only
  2. permissions with default minimal-state renders groups, roles, permissions and — critically — no perms
  3. permissions with use_minimal_state: false renders groups, roles, perms

Recommended beats follow-up

  • Legacy path (x-pack/filebeat/input/entityanalytics/provider/okta/okta.go): accept permissions as a synonym for perms. This is the condition for removing the template branch added here.
  • Minimal-state path (x-pack/filebeat/input/entityanalytics/provider/okta/minimal.go, backed by elastic/entcollect): accept perms as a synonym, and implement the documented roles implication (or reject permissions without roles instead of silently no-opping). This unbreaks package versions 3.2.0–3.5.0, which send bare perms, on current and future agents.
  • Docs: the Filebeat entity-analytics reference doc should standardize on the permissions spelling rather than perms.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

@chrisberkhout chrisberkhout self-assigned this Aug 14, 2026
@chrisberkhout
chrisberkhout requested a review from a team as a code owner August 14, 2026 15:13
@chrisberkhout chrisberkhout added the bugfix Pull request that fixes a bug issue label Aug 14, 2026
@chrisberkhout
chrisberkhout requested a review from a team as a code owner August 14, 2026 15:13
@chrisberkhout chrisberkhout added Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] Integration:entityanalytics_okta Okta Entity Analytics labels Aug 14, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

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

@chrisberkhout
chrisberkhout requested a review from efd6 August 14, 2026 15:15
@github-actions

Copy link
Copy Markdown
Contributor

✅ Elastic Docs Style Checker (Vale)

No issues found on modified lines!


The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale.

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown
Contributor

✅ All changelog entries have the correct PR link.

- groups
{{#if enrich_user_roles}}
- roles
{{else if enrich_user_perms}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Severity: 🔵 Low confidence: medium path: packages/entityanalytics_okta/data_stream/entity/agent/stream/entity-analytics.yml.hbs:46

The new roles branch is not gated on use_minimal_state even though its comment justifies it purely by minimal-state behaviour, so legacy policies also gain an extra - roles entry; either gate the branch or reword the comment to say the entry is emitted unconditionally.

Details

The comment on lines 47-49 states that the explicit - roles is needed because "the minimal-state path only fetches permissions as part of the 'roles' enrichment", and that "the legacy path infers this from 'perms'". By that reasoning the entry is redundant on the legacy path, yet the branch is not gated on use_minimal_state -- so legacy policies now render groups, roles, perms where they previously rendered groups, perms. That change is visible in the new test-enrich-perms-legacy.expected fixture.

The rendered value is still valid on the legacy path (roles is what enrich_user_roles already emits there), so this is not a functional break -- but it does produce a policy diff for existing legacy configurations, and it leaves the code and its own comment out of step. The adjacent permissions/perms branch on lines 60-64 does gate on use_minimal_state, which makes the inconsistency more noticeable.

Recommendation:

Either gate the implied-roles branch the same way the permissions branch is gated:

{{#if enrich_user_roles}}
  - roles
{{else if enrich_user_perms}}
{{#if use_minimal_state}}
{{!-- The minimal-state path only fetches permissions as part of the "roles"
      enrichment, so role enrichment must be requested explicitly. The legacy
      path infers it from "perms". --}}
  - roles
{{/if}}
{{/if}}

or keep the current unconditional form and reword the comment to say that roles is listed explicitly for both paths (required by minimal-state, harmless and already implied on legacy).


🤖 AI-Generated Review | Vera Review Bot - v0.2.6 | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@vera-review-bot

Copy link
Copy Markdown

Review summary

Issues found across the latest commits 4e9a299 — 1 medium, 1 low
  • 🔵 The new roles branch is not gated on use_minimal_state even though its comment justifies it purely by minimal-state behaviour, so legacy policies also gain an extra - roles entry (link) (Unresolved)

Package-level:

  • 🟡 The bug being fixed lives on the minimal-state path, but every system test config pins use_minimal_state: false, so the fixed path still gets no end-to-end coverage

A new commit triggers another review — at most once every 15 minutes. I skip the PR while it's approved or has merge conflicts.

🤖 AI-Generated Review | Vera Review Bot - v0.2.6 | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

cc @chrisberkhout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug issue Integration:entityanalytics_okta Okta Entity Analytics 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.

1 participant