[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

nchaulet · 2024-08-19T19:20:42Z

Summary

Related to #184864

To avoid being in a state where user cannot uninstall their agent, that PR change the way we handle uninstall token for space awareness:

The uninstalltoken saved object stay space agnostic, but we had a namespaces attributes
We enforce the space check in the uninstall token service (the scope is relatively safe as their is only two public API reaching for those saved object)

Tests

Manually tests, you can enable space awareness by adding the flag to your config

xpack.fleet.enableExperimental: ['useSpaceAwareness']

Then using the API to opt-in for the feature

curl -u elastic:changeme -XPOST "http://localhost:5601/internal/fleet/enable_space_awareness" -H "kbn-xsrf: reporting" -H 'elastic-api-version: 1'

Then you can test creating policies in different space, create uninstall tokens only accessible in their respective space.

obltmachine · 2024-08-19T19:21:08Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

…te --fix'

nchaulet · 2024-08-19T20:14:01Z

/ci

… src/core/server/integration_tests/ci_checks'

…haulet/kibana into feature-uninstall-tokens-space-aware

nchaulet · 2024-08-20T12:23:21Z

/ci

elasticmachine · 2024-08-20T13:24:31Z

Pinging @elastic/fleet (Team:Fleet)

jeramysoucy · 2024-08-21T10:13:36Z

x-pack/plugins/fleet/server/saved_objects/index.ts

@@ -986,7 +986,7 @@ export const getSavedObjectTypes = (
      name: MESSAGE_SIGNING_KEYS_SAVED_OBJECT_TYPE,
      indexPattern: INGEST_SAVED_OBJECT_INDEX,
      hidden: true,
-      namespaceType: useSpaceAwareness ? 'single' : 'agnostic',
+      namespaceType: 'agnostic',


Is the useSpaceAwareness experimental feature potentially being used anywhere of consequence (by customers, internal deployments, etc)?

Single space ESOs use the object's namespace when constructing AAD. Any instances of Kibana where useSpaceAwareness is active, that then upgrade to a version with this change, would cause existing message signing keys and uninstall token objects to be undecryptable.

No useSpaceAwareness is not used in any long standing deployment, the feature is still in development, we used that feature flag more as an integration branch for now than a real feature flag

jeramysoucy · 2024-08-21T10:37:29Z

x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts

-    const tokenObjects = await this.getDecryptedTokenObjects({ filter });
+    const useSpaceAwareness = this.isScoped && (await isSpaceAwarenessEnabled());
+    const namespaceFilter = useSpaceAwareness
+      ? getNamespaceFiltering(this.soClient.getCurrentNamespace() ?? DEFAULT_SPACE_ID)


On line 676, the SO client is being initialized without the spaces extension. The return value for the call to getCurrentNamespace with no parameter will always be undefined. What is the intended behavior here?

We use that non space aware so client only when our service is not scoped (this.isScoped = false) (for background task, fleet setup) and if the service is not scoped we are not doing filtering see the line above. This is covered by integration test, but I could add some unit tests to make that behaviour more obvious.

Ahh...thank you! I missed that.

criamico · 2024-08-21T13:59:48Z

x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.test.ts

@@ -9,7 +9,11 @@ import { createHash } from 'crypto';

 import type { KibanaRequest } from '@kbn/core-http-server';

-import type { SavedObjectsClientContract } from '@kbn/core/server';
+import {


Could you add some more tests here? It would be great to cover some of the functionalities added below. It will give us more confidence, for instance in the event of the feature flag clean up.

Just added some in 6aa8e8b

criamico · 2024-08-21T14:46:13Z

x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts

@@ -61,6 +62,14 @@ interface UninstallTokenSOAggregation {
  by_policy_id: AggregationsMultiBucketAggregateBase<UninstallTokenSOAggregationBucket>;
 }

+function getNamespaceFiltering(namespace: string) {
+  if (namespace === DEFAULT_NAMESPACE_STRING) {
+    return `(${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.attributes.namespaces:default) or (not ${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.attributes.namespaces:*)`;


Just a clarification, is the condition after the or needed to filter out those having namespace = *? I'm not totally sure about this condition, what is going to match.

if we want the token in the default space, we need to filter by object either not having namespaces set or where namespaces include `default, does it make sense to you?

isn't going to match also everything else that has any namespace set because of the wildcard *? But maybe I'm misunderstanding how this works.

the condition is not namespaces:* easy to miss the not here

criamico

LGTM 🚢

nchaulet · 2024-08-21T16:59:28Z

@elasticmachine merge upstream

nchaulet · 2024-08-23T15:09:17Z

@elasticmachine merge upstream

nchaulet · 2024-08-26T12:24:54Z

@elasticmachine merge upstream

kibana-ci · 2024-08-26T14:06:00Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 51677c2

Failed CI Steps

Metrics [docs]

✅ unchanged

History

💛 Build #229498 was flaky 7c536ef
💚 Build #228941 succeeded bc5bb99
💚 Build #228888 succeeded 6aa8e8b
💛 Build #228559 was flaky d4e5963
💔 Build #228401 failed 16c54e3
💔 Build #228390 failed a8fd6f3

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

cc @nchaulet

[Fleet] Uninstalltoken saved object namespace agnostic and space aware

a8fd6f3

nchaulet added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team labels Aug 19, 2024

[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…

16c54e3

…te --fix'

kibanamachine and others added 3 commits August 19, 2024 20:50

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

6e36819

… src/core/server/integration_tests/ci_checks'

fix tests

87e52c6

Merge brancth 'feature-uninstall-tokens-space-aware' of github.com:nc…

d4e5963

…haulet/kibana into feature-uninstall-tokens-space-aware

nchaulet self-assigned this Aug 20, 2024

nchaulet marked this pull request as ready for review August 20, 2024 13:24

nchaulet requested review from a team as code owners August 20, 2024 13:24

jeramysoucy self-requested a review August 21, 2024 10:01

jeramysoucy reviewed Aug 21, 2024

View reviewed changes

jeramysoucy approved these changes Aug 21, 2024

View reviewed changes

criamico reviewed Aug 21, 2024

View reviewed changes

Add more unit tests

6aa8e8b

criamico reviewed Aug 21, 2024

View reviewed changes

criamico approved these changes Aug 21, 2024

View reviewed changes

Merge branch 'main' into feature-uninstall-tokens-space-aware

bc5bb99

nchaulet requested a review from pgayvallet August 23, 2024 14:13

Merge branch 'main' into feature-uninstall-tokens-space-aware

7c536ef

Merge branch 'main' into feature-uninstall-tokens-space-aware

51677c2

pgayvallet approved these changes Aug 28, 2024

View reviewed changes

nchaulet merged commit 2c50c45 into elastic:main Aug 28, 2024
25 checks passed

nchaulet deleted the feature-uninstall-tokens-space-aware branch August 28, 2024 12:20

kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Aug 28, 2024

jen-huang mentioned this pull request Sep 3, 2024

[Fleet] Cannot view Fleet uninstall tokens #191919

Closed

nchaulet mentioned this pull request Sep 9, 2024

[Fleet] Investigate side-effect of a space deletion #184864

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

nchaulet commented Aug 19, 2024 •

edited

Loading

obltmachine commented Aug 19, 2024

nchaulet commented Aug 19, 2024

nchaulet commented Aug 20, 2024

elasticmachine commented Aug 20, 2024

jeramysoucy Aug 21, 2024

nchaulet Aug 21, 2024

jeramysoucy Aug 21, 2024

nchaulet Aug 21, 2024

jeramysoucy Aug 21, 2024

criamico Aug 21, 2024

nchaulet Aug 21, 2024

criamico Aug 21, 2024

nchaulet Aug 21, 2024

criamico Aug 21, 2024

nchaulet Aug 21, 2024

criamico left a comment

nchaulet commented Aug 21, 2024

nchaulet commented Aug 23, 2024

nchaulet commented Aug 26, 2024

kibana-ci commented Aug 26, 2024

[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

[Fleet] Uninstalltoken saved object namespace agnostic and space aware #190741

Conversation

nchaulet commented Aug 19, 2024 • edited Loading

Summary

Tests

obltmachine commented Aug 19, 2024

🤖 GitHub comments

nchaulet commented Aug 19, 2024

nchaulet commented Aug 20, 2024

elasticmachine commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

criamico left a comment

Choose a reason for hiding this comment

nchaulet commented Aug 21, 2024

nchaulet commented Aug 23, 2024

nchaulet commented Aug 26, 2024

kibana-ci commented Aug 26, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

nchaulet commented Aug 19, 2024 •

edited

Loading