Address more PR review feedback

elastic · Sep 10, 2020 · 70793b0 · 70793b0
1 parent 71bf105
commit 70793b0
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 11 deletions.
diff --git a/...crypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts b/...crypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts
@@ -555,12 +555,18 @@ describe('#bulkUpdate', () => {
   });
 
   describe('namespace', () => {
-    const doTest = async (
-      optionsNamespace: string | undefined,
-      expectOptionsNamespaceInDescriptor: boolean,
-      objectNamespace: string | undefined,
-      expectObjectNamespaceInDescriptor: boolean
-    ) => {
+    interface TestParams {
+      optionsNamespace: string | undefined;
+      objectNamespace: string | undefined;
+      expectOptionsNamespaceInDescriptor: boolean;
+      expectObjectNamespaceInDescriptor: boolean;
+    }
+    const doTest = async ({
+      optionsNamespace,
+      objectNamespace,
+      expectOptionsNamespaceInDescriptor,
+      expectObjectNamespaceInDescriptor,
+    }: TestParams) => {
       const docs = [
         {
           id: 'some-id',
@@ -623,7 +629,6 @@ describe('#bulkUpdate', () => {
             },
             version: 'some-version',
             namespace: objectNamespace,
-
             references: undefined,
           },
         ],
@@ -632,24 +637,44 @@ describe('#bulkUpdate', () => {
     };
 
     it('does not use options `namespace` or object `namespace` to encrypt attributes if neither are specified', async () => {
-      await doTest(undefined, false, undefined, false);
+      await doTest({
+        optionsNamespace: undefined,
+        objectNamespace: undefined,
+        expectOptionsNamespaceInDescriptor: false,
+        expectObjectNamespaceInDescriptor: false,
+      });
     });
 
     describe('with a single-namespace type', () => {
       it('uses options `namespace` to encrypt attributes if it is specified and object `namespace` is not', async () => {
-        await doTest('some-namespace', true, undefined, false);
+        await doTest({
+          optionsNamespace: 'some-namespace',
+          objectNamespace: undefined,
+          expectOptionsNamespaceInDescriptor: true,
+          expectObjectNamespaceInDescriptor: false,
+        });
       });
 
       it('uses object `namespace` to encrypt attributes if it is specified', async () => {
         // object namespace supersedes options namespace
-        await doTest('some-namespace', false, 'another-namespace', true);
+        await doTest({
+          optionsNamespace: 'some-namespace',
+          objectNamespace: 'another-namespace',
+          expectOptionsNamespaceInDescriptor: false,
+          expectObjectNamespaceInDescriptor: true,
+        });
       });
     });
 
     describe('with a non-single-namespace type', () => {
       it('does not use object `namespace` or options `namespace` to encrypt attributes if it is specified', async () => {
         mockBaseTypeRegistry.isSingleNamespace.mockReturnValue(false);
-        await doTest('some-namespace', false, 'another-namespace', false);
+        await doTest({
+          optionsNamespace: 'some-namespace',
+          objectNamespace: 'another-namespace',
+          expectOptionsNamespaceInDescriptor: false,
+          expectObjectNamespaceInDescriptor: false,
+        });
       });
     });
   });

diff --git a/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts b/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
@@ -200,6 +200,8 @@ export class SecureSavedObjectsClientWrapper implements SavedObjectsClientContra
     options: SavedObjectsBaseOptions = {}
   ) {
     const objectNamespaces = objects
+      // The repository treats an `undefined` object namespace is treated as the absence of a namespace, falling back to options.namespace;
+      // in this case, filter it out here so we don't accidentally check for privileges in the Default space when we shouldn't be doing so.
       .filter(({ namespace }) => namespace !== undefined)
       .map(({ namespace }) => namespace!);
     const namespaces = [options?.namespace, ...objectNamespaces];