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.

Sign up for GitHub

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

Jump to bottom

Adding optional Description field to Roles APIs #182039

Merged
merged 8 commits into from May 1, 2024
Merged

Adding optional Description field to Roles APIs #182039

merged 8 commits into from May 1, 2024

Conversation

kc13greiner
Copy link
Contributor

@kc13greiner kc13greiner commented Apr 29, 2024 •
edited

Summary

In preparation for the KB UI and ES API to accept description for Roles, Kibana get, getAll, and put Roles routes should handle a description.

Testing

Start KB/ES locally

In Dev Tools PUT role:

PUT kbn:api/security/role/mytestrole
{
  "description": "This is a test role",
  "metadata": {
    "version": 1
  },
  "elasticsearch": {
    "cluster": [ ],
    "indices": [ ]
  },
  "kibana": [
    {
      "base": [ ],
      "feature": {
       "discover": [ "all" ],
        "visualize": [ "all" ],
        "dashboard": [ "all" ],
        "dev_tools": [ "read" ],
        "advancedSettings": [ "read" ],
        "indexPatterns": [ "read" ],
        "graph": [ "all" ],
        "apm": [ "read" ],
        "maps": [ "read" ],
        "canvas": [ "read" ],
        "infrastructure": [ "all" ],
        "logs": [ "all" ],
        "uptime": [ "all"  ]
      },
      "spaces": [ "*" ]
    }
  ]
}

This will fail since ES doesn't accept descriptions yet

Pull the ES Role Description PR elastic/elasticsearch#107088

and run yarn es source and yarn start

Rerun the PUT above, receive 204!

Check the role description with a GET:

GET kbn:api/security/role/mytestrole
Screenshot 2024-04-30 at 9 43 32 PM

It has a limit of 2048 per the requirements:
Screenshot 2024-04-30 at 9 45 13 PM

@kc13greiner kc13greiner added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Apr 29, 2024
@kc13greiner kc13greiner self-assigned this Apr 29, 2024
@kc13greiner
Copy link
Contributor Author

/ci

@kc13greiner
Copy link
Contributor Author

/ci

@kc13greiner
Copy link
Contributor Author

/ci

@kc13greiner kc13greiner marked this pull request as ready for review May 1, 2024 01:46
@kc13greiner kc13greiner requested a review from a team as a code owner May 1, 2024 01:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

elena-shostak
elena-shostak approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@elena-shostak elena-shostak left a comment

Choose a reason for hiding this comment

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

LGTM in advance 👍

Could you please also update the get_all_by_space.test.ts test for new added route, where we get all routes for a given space?

@kc13greiner
Copy link
Contributor Author

LGTM in advance 👍

Could you please also update the get_all_by_space.test.ts test for new added route, where we get all routes for a given space?

@elena-shostak Definitely! Added in latest commit: 6c812ba

@kc13greiner
Copy link
Contributor Author

/ci

@kc13greiner
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/security-plugin-types-common 36 37 +1
security 200 201 +1
total +2
Unknown metric groups

API count

id before after diff
@kbn/security-plugin-types-common 83 84 +1
security 406 407 +1
total +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kc13greiner

@kc13greiner kc13greiner merged commit 565a447 into elastic:main May 1, 2024
17 checks passed
@kc13greiner kc13greiner deleted the feature/roles_description_api_changes branch May 1, 2024 17:20
TinLe added a commit to TinLe/kibana that referenced this pull request May 1, 2024
@TinLe
Merge branch 'master' into dependabot/npm_and_yarn/nodemailer-6.9.9
56f9aa4 
* master: (1654 commits)
  Bump ejs from 3.1.9 to 3.1.10
  Don't render exceptions flyout if data is loading (elastic#181588)
  Enable value list modal (elastic#181593)
  skip flaky suite (elastic#181777)
  skip failing test suite (elastic#182263)
  [Mappings Editor] Disable _source field in serverless (elastic#181712)
  [data.search] Fix unhandled promise rejections (elastic#181785)
  [Fleet] Fix logic for detecting first time Elastic Agent users (elastic#182214)
  [ML] Decouple data_visualizer from MapEmbeddable (elastic#181928)
  [ES|QL] Sorting accepts expressions (elastic#181916)
  [ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (elastic#182176)
  Adding optional Description field to Roles APIs (elastic#182039)
  Upgrade Markdown-it to 14.1.0 (elastic#182244)
  Bump xml-crypto from 5.0.0 to 6.0.0
  [DOCS] Fix docs and screenshots for rule creation changes (elastic#181925)
  Update dependency elastic-apm-node to ^4.5.3 (main) (elastic#182236)
  [Obs AI Assistant] register alert details context in observability plugin (elastic#181501)
  Add `@typescript-eslint/no-floating-promises` (elastic#181456)
  [Playground] Propagate Error message into FE (elastic#182201)
  [ES|QL] Rename the setting to a more generic one and move to the general section (elastic#182074)
  ...
yuliacech pushed a commit to yuliacech/kibana that referenced this pull request May 3, 2024
@kc13greiner @yuliacech
Adding optional Description field to Roles APIs (elastic#182039)
5a1e912 
## Summary

In preparation for the KB UI and ES API to accept `description` for
Roles, Kibana `get`, `getAll`, and `put` Roles routes should handle a
description.

## Testing

Start KB/ES locally

In Dev Tools PUT role:
```
PUT kbn:api/security/role/mytestrole
{
  "description": "This is a test role",
  "metadata": {
    "version": 1
  },
  "elasticsearch": {
    "cluster": [ ],
    "indices": [ ]
  },
  "kibana": [
    {
      "base": [ ],
      "feature": {
       "discover": [ "all" ],
        "visualize": [ "all" ],
        "dashboard": [ "all" ],
        "dev_tools": [ "read" ],
        "advancedSettings": [ "read" ],
        "indexPatterns": [ "read" ],
        "graph": [ "all" ],
        "apm": [ "read" ],
        "maps": [ "read" ],
        "canvas": [ "read" ],
        "infrastructure": [ "all" ],
        "logs": [ "all" ],
        "uptime": [ "all"  ]
      },
      "spaces": [ "*" ]
    }
  ]
}
```

This will fail since ES doesn't accept `descriptions` yet

Pull the ES Role Description PR
elastic/elasticsearch#107088

and run `yarn es source` and `yarn start`

Rerun the PUT above, receive 204!

Check the role `description` with a GET:

```
GET kbn:api/security/role/mytestrole
```

<img width="1712" alt="Screenshot 2024-04-30 at 9 43 32 PM"
src="https://github.com/elastic/kibana/assets/21210601/20394086-f223-4be8-8660-eb8d3930116c">


It has a limit of 2048 per the requirements:
<img width="2248" alt="Screenshot 2024-04-30 at 9 45 13 PM"
src="https://github.com/elastic/kibana/assets/21210601/7dc97a8e-1246-49e4-954e-885be433c7c7">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elena-shostak elena-shostak elena-shostak approved these changes

Assignees

@kc13greiner kc13greiner

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kc13greiner @elasticmachine @kibana-ci @elena-shostak @kibanamachine