Skip to content

API specification changes for custom roles #2491

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

Merged
merged 22 commits into from
May 7, 2024

Conversation

n1v0lg
Copy link
Contributor

@n1v0lg n1v0lg commented Apr 8, 2024

With changes for Serverless custom roles, several roles-related APIs have stricter request validation. This PR codifies these requirements into our ES API specification.

Notes & questions to reviewers:

  • The changes to the Elasticsearch APIs are still behind a feature flag (not a proper one, but rather a setting). I should only merge this spec PR once the feature flag is removed, i.e., once the APIs are truly public, correct?
  • The change introduced quite a few validation failures. I believe they are related to the changes in this PR, but I'm unable to run local validation to debug these, due to local setup issues (note my message in the clients channel -- not linking to avoid a private link in a public repo).

Closes: ES-7833

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 9, 2024

@elasticmachine update branch

@n1v0lg n1v0lg marked this pull request as ready for review April 11, 2024 08:09
@n1v0lg n1v0lg requested review from a team and pquentin April 11, 2024 08:28
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! I left one comment about MONITOR_TEXT_STRUCTURE.

I checked with @Anaethelion that the validation did not change locally without the enum changes, so the failures are not related to this PR.

Would you mind opening a follow-up pull request to add the missing ClusterPrivilege (like NONE or MANAGE_BEHAVIORAL_ANALYTICS) and IndexPrivilege (like MANAGE_DATA_STREAM_LIFECYCLE or CROSS_CLUSTER_REPLICATION) values? I suppose it can go in this one too, if you prefer.

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Apr 12, 2024

Would you mind opening a follow-up pull request to add the missing ClusterPrivilege (like NONE or MANAGE_BEHAVIORAL_ANALYTICS) and IndexPrivilege (like MANAGE_DATA_STREAM_LIFECYCLE or CROSS_CLUSTER_REPLICATION) values? I suppose it can go in this one too, if you prefer.

For sure. I'll make it a separate PR because we can make these changes now already, whereas the current PR will have to wait until the Serverless-side API changes are ready to be made public.

Copy link
Contributor

github-actions bot commented May 7, 2024

Following you can find the validation results for the APIs you have changed.

API Status Request Response
security.activate_user_profile 🔴 8/9 8/9
security.authenticate 🔴 28/29 28/29
security.bulk_update_api_keys 🟠 Missing type Missing type
security.change_password 🔴 8/9 8/9
security.clear_api_key_cache 🔴 12/13 12/13
security.clear_cached_privileges 🔴 2/3 2/3
security.clear_cached_realms 🔴 0/1 0/1
security.clear_cached_roles 🔴 1/2 1/2
security.clear_cached_service_tokens 🔴 3/4 3/4
security.create_api_key 🔴 56/57 47/48
security.create_cross_cluster_api_key 🟠 Missing type Missing type
security.create_service_token 🔴 2/3 2/3
security.delete_privileges 🔴 5/6 5/6
security.delete_role_mapping 🔴 8/9 8/9
security.delete_role 🔴 7/8 7/8
security.delete_service_token Missing test Missing test
security.delete_user 🔴 8/9 8/9
security.disable_user_profile 🔴 0/1 0/1
security.disable_user 🔴 2/3 2/3
security.enable_user_profile 🔴 0/1 0/1
security.enable_user 🔴 3/4 3/4
security.enroll_kibana Missing test Missing test
security.enroll_node Missing test Missing test
security.get_api_key 🔴 36/37 13/37
security.get_builtin_privileges 🔴 1/2 1/2
security.get_privileges 🔴 11/12 11/12
security.get_role_mapping 🔴 17/18 9/18
security.get_role 🔴 20/21 19/21
security.get_service_accounts Missing test Missing test
security.get_service_credentials 🔴 0/1 0/1
security.get_settings 🟠 Missing type Missing type
security.get_token 🔴 24/25 23/24
security.get_user_privileges 🔴 7/8 6/8
security.get_user_profile 🔴 7/8 7/8
security.get_user 🔴 24/25 24/25
security.grant_api_key 🔴 6/7 6/7
security.has_privileges_user_profile 🔴 2/3 2/3
security.has_privileges 🔴 23/24 23/24
security.invalidate_api_key 🔴 11/12 11/12
security.invalidate_token 🔴 10/11 10/11
security.oidc_authenticate 🟠 Missing type Missing type
security.oidc_logout 🟠 Missing type Missing type
security.oidc_prepare_authentication 🟠 Missing type Missing type
security.put_privileges 🔴 9/10 9/10
security.put_role_mapping 🔴 1/11 10/11
security.put_role 🔴 37/39 37/38
security.put_user 🔴 48/49 47/48
security.query_api_keys 🔴 12/14 0/14
security.saml_authenticate Missing test Missing test
security.saml_complete_logout Missing test Missing test
security.saml_invalidate Missing test Missing test
security.saml_logout Missing test Missing test
security.saml_prepare_authentication Missing test Missing test
security.saml_service_provider_metadata Missing test Missing test
security.suggest_user_profiles 🔴 0/1 0/1
security.update_api_key 🔴 4/5 4/5
security.update_cross_cluster_api_key 🟠 Missing type Missing type
security.update_settings 🟠 Missing type Missing type
security.update_user_profile_data 🔴 0/1 0/1

You can validate these APIs yourself by using the make validate target.

@pquentin pquentin merged commit 2731ca8 into main May 7, 2024
6 checks passed
@pquentin pquentin deleted the custom-roles-api-spec-changes branch May 7, 2024 13:38
n1v0lg added a commit that referenced this pull request May 7, 2024
n1v0lg added a commit that referenced this pull request May 7, 2024
n1v0lg added a commit that referenced this pull request Jun 10, 2024
Brings back #2491 with a few corrected annotations. 

The changes included in this PR are ready to be published: the privileges pertain to the API key APIs which are already public in Serverless. However, the visibility of the Create or Update Roles API (as well as other Role-related APIs) remains private until the feature is ready. I will open a separate PR for those when the time has come.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants