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

Add API Keys app to Management > Security. #45740

Merged
merged 20 commits into from
Oct 14, 2019

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented Sep 14, 2019

Release note

The API Keys app allows users to view and invalidate their own API keys and administrators to view and invalidate all users' API keys.

Details

Loading state

image

API keys not enabled state

image

Loading error state

image

Not allowed state

If the user doesn't have manage_security, manage_api_key, and/or manage_own_api_key permissions, they'll be disallowed from using this app.

image

Admin view

If the user has manage_security or manage_api_key privileges, they have the ability to see and invalidate all users' API keys, as well as see which API key was created by which user in which realm.

image

Empty prompt:

image

Non-admin view

If the user only has the manage_own_api_key privilege then they'll see a list of only their own keys. User and Realm is excluded from the table because this information would be redundant.

image

Empty prompt:

image

To test

  1. Run Elasticsearch with a flag enabled API keys:
yarn es snapshot --license=trial -E xpack.security.authc.api_key.enabled=true
  1. Create an API key in Console:
POST /_security/api_key
{
  "name": "my-api-key",
  "expiration": "1d"
}
  1. Verify that it shows up in the table with the expected information.

To test with limited privileges

  1. Create a new role, and assign it the privilege you want to test (manage_security, manage_api_key, and/or manage_own_api_key).
  2. Create a new user, with this new role and kibana_user.
  3. Log out and log back in as this user.

@cjcenizal cjcenizal added release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Users/Roles/API Keys v8.0.0 v7.5.0 labels Sep 14, 2019
@cjcenizal cjcenizal requested a review from a team as a code owner September 14, 2019 01:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security

@cjcenizal
Copy link
Contributor Author

CC @jethr0null @bytebilly @roncohen

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bytebilly
Copy link
Contributor

@cjcenizal I like it!

This allows users to manage their own keys, that is great, but if I get it correctly it doesn't allow "admins" to manage the entire list of keys (e.g., application keys for APM).
Since this scenario is where the discussion started, I want to be sure that we are all aware of that.

I'm not sure if we need to expose the ID field, since we already have the Name field and it should be unique per-user (see elastic/elasticsearch#46646). Probably the ID field is not really interesting for users, and I feel it could be entirely transparent to the UI.

What do you think?

@legrego
Copy link
Member

legrego commented Sep 16, 2019

This allows users to manage their own keys, that is great, but if I get it correctly it doesn't allow "admins" to manage the entire list of keys (e.g., application keys for APM).

I haven't pulled this PR down to look yet, but if this is the case, maybe API key management would make more sense as part of the user profile page, rather than under the security management interface?

@bytebilly
Copy link
Contributor

I haven't pulled this PR down to look yet, but if this is the case, maybe API key management would make more sense as part of the user profile page, rather than under the security management interface?

It makes sense, as this is normally the place where you manage your own API keys in other software (GitHub, GitLab, etc).

I'm still wondering how the "admin" management works. If we want to reuse the same panel for both "admin" and "own" flows, probably the user profile doesn't fit very well since it's not the place I'd go, as an "admin", to manage the keys for the entire instance. Having two places where the same component is reused makes sense, anyway, from a logical point of view.

@legrego
Copy link
Member

legrego commented Sep 16, 2019

I'm still wondering how the "admin" management works. If we want to reuse the same panel for both "admin" and "own" flows, probably the user profile doesn't fit very well since it's not the place I'd go, as an "admin", to manage the keys for the entire instance.

I agree, the management interface to administer other user's keys wouldn't make sense in the user profile. I would expect that to live in the Management -> Security section. It's similar to how user management works today: Users can access their own account via the user profile page, and change their password from there if necessary. Administrators can go to the Management -> Security -> Users view to manage other user accounts.

@cjcenizal
Copy link
Contributor Author

cjcenizal commented Sep 17, 2019

This allows users to manage their own keys, that is great, but if I get it correctly it doesn't allow "admins" to manage the entire list of keys

Yes, this is correct. It seems like the current API doesn't expose an easy way of retrieving all existing keys. Do you know who I could ping about that / would you mind following up on that for me?

Probably the ID field is not really interesting for users, and I feel it could be entirely transparent to the UI.

Works for me. I will move this detail into a separate detail panel and out of the table.

I haven't pulled this PR down to look yet, but if this is the case, maybe API key management would make more sense as part of the user profile page, rather than under the security management interface?

I think this makes a lot of sense. I'll make this change, thanks for the suggestion!

@bytebilly
Copy link
Contributor

Do you know who I could ping about that / would you mind following up on that for me?

I'll bring this question to the team, and we'll take the opportunity to talk about the future of API keys API (inception intended 🙂).
Updates here, as soon as possible.

@bmcconaghy
Copy link
Contributor

Playing around with the get API keys API, looks like there’s not currently a way to list all API keys. If you know a specific username and you’re an admin, you can list their keys. Wildcards don’t work for username, and if you specify only a realm_name, you get nothing back. The alerting project was hoping for a screen where an admin could go in and disable all API keys for a given user, say they’ve left the company, or have had their access revoked or changed. CJ had a good idea where you could select another user and then be shown all their keys. Verified that I can see other user's keys with cluster "all" privilege.

@bytebilly
Copy link
Contributor

bytebilly commented Sep 20, 2019

@cjcenizal based on discussions we had, we should carefully consider the use case that the feature is trying to address (limited scope service accounts), and defer the personal access token flow when/if we'll move in that direction.

I feel the UX for this is essential to guarantee that the current API keys are used properly and not misunderstood in scope.

Discussion about getting more information in the response is in elastic/elasticsearch#46887.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cjcenizal cjcenizal changed the title Add API Keys app to Management > Security. [skip-ci] [WIP] Add API Keys app to Management > Security. Oct 4, 2019
@cjcenizal cjcenizal force-pushed the api-key-list branch 3 times, most recently from 318b75a to fe9a04a Compare October 7, 2019 22:49
@cjcenizal
Copy link
Contributor Author

@kobelb @gchaps @alisonelizabeth Thanks for your feedback! I've incorporated the changes I think are appropriate for this stage. Brandon, could you please re-review and approve if you're OK with merging this?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cjcenizal
Copy link
Contributor Author

Retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cjcenizal cjcenizal merged commit e8df2fa into elastic:master Oct 14, 2019
@cjcenizal cjcenizal deleted the api-key-list branch October 14, 2019 22:37
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

cjcenizal added a commit that referenced this pull request Oct 15, 2019
* Add API Keys app to Management > Security.
- For admins, list all API Keys created by the user: Name, Date Created, Expiration Date, Status, User, and Realm.
- For non-admins, list own API keys: Name, Date Created, Expiration Date, and Status.
- Surface admin status above table.
- Ability to search by Name and Revoke (invalidate) API keys, and filter by User and Realm.
- Surface feedback when API keys are disabled on Elasticsearch or when user lacks required permissions.
* Add `SectionLoading` component to `es_ui_shared` plugin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Users/Roles/API Keys release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants