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

Support retrieval of all API keys #46887

Closed
cjcenizal opened this issue Sep 19, 2019 · 3 comments · Fixed by #47274
Closed

Support retrieval of all API keys #46887

cjcenizal opened this issue Sep 19, 2019 · 3 comments · Fixed by #47274
Assignees
Labels
>enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.5.0

Comments

@cjcenizal
Copy link
Contributor

Currently, GET _security/api_key requires you to provide some identifying information with which to find and retrieve keys. Can we support the retrieval of all API keys by default, in the absence of id, name, realm_name, and username?

CC @bizybot @tvernum @bytebilly @bmcconaghy

@cjcenizal cjcenizal added >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Sep 19, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@bytebilly
Copy link
Contributor

@cjcenizal yes this makes sense to me.
Currently, a possible workaround to retrieve the entire list is to call the existing API with all the realms that are configured in the system. I'm not sure if there is a way to obtain the list of realms, but this would not require any backend change in the API code.

A couple of thoughts we could consider as part of this discussion:

  1. add the "owner" of the key (principal+realm?) to the response for each entry
  2. make it work with the manage_own_api_key, transparently filtering results based on permissions
  3. paginate results, if we feel this list could be too long for a single response

@tvernum
Copy link
Contributor

tvernum commented Sep 24, 2019

@bizybot will sort this out. It was an oversight due to making the GET and DELETE APIs support the same set of parameters, and intentionally not wanting an DELETE-everything API.

  1. add the "owner" of the key (principal+realm?) to the response for each entry

We do that already. If you look at the GET response in the example section of the docs, it has username and realm.

  1. make it work with the manage_own_api_key, transparently filtering results based on permissions

It is intentional that we don't so this. We essentially have 2 APIs (though for reasons of history, it's 1 API with an optional parameter).
If you specify owner=true it automatically filters to only show your API keys (if you have access to API keys)
if you specify owner=false (or don't specify it at all, because false is the default), it attempts to show API keys for all owners (if you have access to do so).

We try very hard to have APIs that do what they say, and give you errors if you are not permitted to do so, rather than having APIs that significantly change behaviour depending on your access level.
for the GET API that's not so bad, but for the DELETE API, having it delete your keys if you had manage_own_api_key but delete everyone's keys if you have manage_api_key is the sort of unexpected behaviour we don't want.

  1. paginate results, if we feel this list could be too long for a single response

This is a problem today (and not just for API keys, it's also true for listing users).
Interally we do a scroll and collect all the results within the GET API. At some point that will exceed a reasonable usage of memory, and we ought not to do that, but exposing scroll-ids over the security index is a problem (technically we have protections there, but I have a strong preference not to rely on them as our only line of defense).
When we introduce "system indices" and switch the security index across it might be safe enough to hand out an opaque scroll id over than index that isn't accessible via the search APIs.

bizybot pushed a commit to bizybot/elasticsearch that referenced this issue Sep 30, 2019
This commit adds support to retrieve all API keys if the
authenticated user is authorized to do so.
This removes the restriction of specifying one of the
parameters (like id, name, username and/or realm name)
when `owner` is set to `false`.

Closes elastic#46887
bizybot added a commit that referenced this issue Oct 7, 2019
This commit adds support to retrieve all API keys if the authenticated
user is authorized to do so.
This removes the restriction of specifying one of the
parameters (like id, name, username and/or realm name)
when the `owner` is set to `false`.

Closes #46887
bizybot added a commit to bizybot/elasticsearch that referenced this issue Oct 7, 2019
…7274)

This commit adds support to retrieve all API keys if the authenticated
user is authorized to do so.
This removes the restriction of specifying one of the
parameters (like id, name, username and/or realm name)
when the `owner` is set to `false`.

Closes elastic#46887
bizybot added a commit that referenced this issue Oct 7, 2019
…47641)

This commit adds support to retrieve all API keys if the authenticated
user is authorized to do so.
This removes the restriction of specifying one of the
parameters (like id, name, username and/or realm name)
when the `owner` is set to `false`.

Closes #46887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants