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

[Fleet] Document behavior of /api/fleet/enrollment_api_keys #155550

Closed
andrewkroh opened this issue Apr 21, 2023 · 7 comments · Fixed by #191585
Closed

[Fleet] Document behavior of /api/fleet/enrollment_api_keys #155550

andrewkroh opened this issue Apr 21, 2023 · 7 comments · Fixed by #191585
Assignees
Labels
documentation Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@andrewkroh
Copy link
Member

While using the Fleet enrollment_api_keys API I had some usage observations and questions that I'm hoping can be documented/answered within the openapi spec.

Creating Enrollment API key requires unique name

POST kbn:/api/fleet/enrollment_api_keys
{
  "name": "My Token",
  "policy_id": "elastic-agent-managed-ep"
}

The limitation that name must be unique from all existing enrollment api keys should be documented. IMO that limitation does seem arbitrary given that enrollment token are identified by a UUID generated at creation time. But if this is documented, then callers can be aware of this and build in workarounds like appending random suffixes.

Deleting Enrollment API key does not delete, it revokes

DELETE kbn:/api/fleet/enrollment_api_keys/{id}

This API does not delete the resource (it's more of a revocation). It changes the active attribute to false. Can the API documentation be update to explain this behavior?

If this operation does not actually delete the enrollment api key, then how can that be accomplished? GET /api/fleet/enrollment_api_keys/{id} continues to return the resource. Do these age out after some period? Should users be worried about creating too many if these cannot be deleted?

I don't really mind the fact that it is not deleted, but it is limiting that users cannot create a new enrollment token using the same name because another one exists. If that limitation were removed this would be less of an issue.

@andrewkroh andrewkroh added Team:Fleet Team label for Observability Data Collection Fleet team Question Ticket having question for Dev team labels Apr 21, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jen-huang jen-huang added documentation and removed Question Ticket having question for Dev team labels May 2, 2023
@criamico
Copy link
Contributor

It would be great to document also the behaviour of GET /api/fleet/enrollment_api_keys endpoint. It was asked by some users to clarify when an api key gets invalidated and what's the expected behaviour.

@criamico
Copy link
Contributor

I also filed a bug related to this area, it might be useful to take a look at it: #190708

@jillguyonnet
Copy link
Contributor

I've opened #191585 to improve the OpenAPI spec. As detailed in the PR description, I think it would be beneficial to add some clarification to the docs as well. Our OpenAPI spec only gives so much context, so it would probably be a good complement.

Regarding what happens to revoked/invalidated tokens, the Security API doc mentions that they are automatically deleted after the retention period, which is an Elasticsearch setting. I've suggested clarifying this in the Fleet docs.

Regarding the constraint that enrollment token names must be unique, I agree that it seems strange. I found the original issue that put this constraint in place, but it's still unclear to me why the name itself would have to be unique, given that the UUID is appended (see #85118 (comment)). @nchaulet would you happen to remember this?

@jillguyonnet
Copy link
Contributor

Followup issue to update Fleet docs: elastic/ingest-docs#1270

@nchaulet
Copy link
Member

Regarding the constraint that enrollment token names must be unique, I agree that it seems strange. I found the #85118 that put this constraint in place, but it's still unclear to me why the name itself would have to be unique, given that the UUID is appended (see #85118 (comment)). @nchaulet would you happen to remember this?

Not sure to totally remember I think it's probably was implemented before adding a uuid we can probably remove that verification.

For the delete operation I think we could probably introduce a new option (could be usefull for agents too) ?deleteDocument=true (open to better naming here) to have an hard delete when needed and not a soft delete

@jillguyonnet
Copy link
Contributor

Thanks for your reply @nchaulet

1. Regarding the existing constraint: the doc has been updated to reflect this. However, it's unnecessary perhaps it could be captured elsewhere.

2. Also good point regarding the delete operation. If I'm not mistaken, the documents are never removed from .fleet-enrollment-api-keys in case of soft deletion. Looking at the service code, documents are only deleted when the forceDelete parameter is true, which if I'm not mistaken only happens when resetting preconfigured data:

if (forceDelete) {
await esClient.delete({
index: ENROLLMENT_API_KEYS_INDEX,
id,
refresh: 'wait_for',
});
} else {
await esClient.update({
index: ENROLLMENT_API_KEYS_INDEX,
id,
body: {
doc: {
active: false,
},
},
refresh: 'wait_for',
});
}

So maybe my previous comment was misleading:

Regarding what happens to revoked/invalidated tokens, the Security API doc mentions that they are automatically deleted after the retention period, which is an Elasticsearch setting.

Which I assume means the documents are deleted from the security index (see also https://support.elastic.dev/knowledge/view/18faf2df). I've been trying to prove this locally by running ES with -E xpack.security.authc.api_key.delete.retention_period=1m but I'm not seeing any documents removed, so I'm not sure.

cc @kpollich for these two questions.

jillguyonnet added a commit that referenced this issue Aug 30, 2024
## Summary

Closes #191719

Attempting to generate a Fleet enrollment token with a name that ends
with `!` produces a malformed ES query which causes `POST
agents/enrollment_api_keys` to fail with 500.

This PR adds a narrow fix by escaping question marks (which is a
`query_string` [special
character](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#query-string-syntax)).

Note: this query probably wouldn't be necessary if we removed the
constraint of unique name, as discussed in
#155550.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants