Skip to content
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.

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

[9.0] [Hardening] Kibana Feature API Privileges Names (#208067) #209315

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 9.0:

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine requested review from a team as code owners February 3, 2025 14:31
@kibanamachine kibanamachine requested review from a team as code owners February 3, 2025 14:31
@kibanamachine kibanamachine enabled auto-merge (squash) February 3, 2025 14:31
@botelastic botelastic bot added the Team:Obs AI Assistant Observability AI Assistant label Feb 3, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

## Summary

As part of our effort to harden API action definitions and enforce
standards this PR adds an utility `ApiPrivileges` class.
It is supposed to be used for both feature registration and API route
definition to construct the privilege name.
```ts
plugins.features.registerKibanaFeature({
  privileges: {
    all: {
      app: [...],
      catalogue: [...],
      api: [ApiPrivileges.manage('subject_name')],
      ...
    },
    read: {
      ...
      api: [ApiPrivileges.read('subject_name')],
      ...
    },
  },
})
....

// route definition
router.get(
  {
    path: 'api_path',
    security: {
      authz: {
        requiredPrivileges: [ApiPrivileges.manage('subject_name')],
      },
    },
  },
  async (ctx, req, res) => {}
);
```

`require_kibana_feature_privileges_naming` eslint rule has been added to
show warning if the API privilege name doesn't satisfy the naming
convention.

### Naming convention

- API privilege should start with valid `ApiOperation`: `manage`,
`read`, `update`, `delete`, `create`
- API privilege should use `_` as separator

❌ `read-entity-a`
❌ `delete_entity-a`
❌ `entity_manage`
✅ `read_entity_a`
✅ `delete_entity_a`
✅ `manage_entity`

> [!IMPORTANT]
> Serverless ZDT update scenario:
>
> - version N has an endpoint protected with the `old_privilege_read`.
> - version N+1 has the same endpoint protected with a new
`read_privilege`.
>
> There might be a short period between the time the UI pod N+1 passes
SO migrations and updates privileges and the time it's marked as
ready-to-handle-requests by k8s, and when UI pod N is terminated.
>
> After discussion with @legrego and @azasypkin we decided to ignore it
due to the perceived risk-to-cost ratio:
> 1. The time window users might be affected is very narrow because we
register privileges late in the Kibana startup flow (e.g., after SO
migrations).
> 2. The transient 403 errors users might get won't result in session
termination and shouldn't lead to data loss.
> 3. The roll-out will be performed in batches over the course of
multiple weeks and implemented by different teams. This means the impact
per release shouldn't be significant.

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

__Relates: https://github.com/elastic/kibana/issues/198716__

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 504510b)
@kibanamachine kibanamachine force-pushed the backport/9.0/pr-208067 branch from 7c320b9 to 079167d Compare February 3, 2025 14:38
@kibanamachine kibanamachine merged commit 15348c0 into elastic:9.0 Feb 3, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Team:Obs AI Assistant Observability AI Assistant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants