Start addressing PR feedback

elastic · May 11, 2021 · 202485e · 202485e
1 parent 00f374a
commit 202485e
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 11 deletions.
diff --git a/x-pack/plugins/file_upload/server/capabilities.test.ts b/x-pack/plugins/file_upload/server/capabilities.test.ts
@@ -133,7 +133,9 @@ describe('setupCapabilities', () => {
           `);
 
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
+    expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledWith(request);
     expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledTimes(1);
+    expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledWith(request);
   });
 
   it('registers a capabilities switcher that enables capabilities for privileged users', async () => {
@@ -176,13 +178,19 @@ describe('setupCapabilities', () => {
           `);
 
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
+    expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledWith(request);
     expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledTimes(1);
+    expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledWith(request);
   });
 
-  it('registers a capabilities switcher that skips privilege check for anonymous routes', async () => {
+  it('registers a capabilities switcher that disables capabilities for unauthenticated requests', async () => {
     const coreSetup = coreMock.createSetup();
     const security = securityMock.createStart();
     security.authz.mode.useRbacForRequest.mockReturnValue(true);
+    const mockCheckPrivileges = jest
+      .fn()
+      .mockRejectedValue(new Error('this should not have been called'));
+    security.authz.checkPrivilegesDynamicallyWithRequest.mockReturnValue(mockCheckPrivileges);
     coreSetup.getStartServices.mockResolvedValue([
       (undefined as unknown) as CoreStart,
       { security },
@@ -202,20 +210,17 @@ describe('setupCapabilities', () => {
       },
     } as Capabilities;
 
-    const request = httpServerMock.createKibanaRequest({ routeAuthRequired: false });
+    const request = httpServerMock.createKibanaRequest({ auth: { isAuthenticated: false } });
 
     await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
             Object {
-              "catalogue": Object {},
               "fileUpload": Object {
-                "show": true,
+                "show": false,
               },
-              "management": Object {},
-              "navLinks": Object {},
             }
           `);
 
-    expect(security.authz.mode.useRbacForRequest).not.toHaveBeenCalled();
+    expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
     expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
   });
 
@@ -256,6 +261,7 @@ describe('setupCapabilities', () => {
           `);
 
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
+    expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledWith(request);
     expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
   });
 });
diff --git a/x-pack/plugins/file_upload/server/capabilities.ts b/x-pack/plugins/file_upload/server/capabilities.ts
@@ -10,7 +10,7 @@ import { checkFileUploadPrivileges } from './check_privileges';
 import { StartDeps } from './types';
 
 export const setupCapabilities = (
-  core: Pick<CoreSetup<StartDeps, unknown>, 'capabilities' | 'getStartServices'>
+  core: Pick<CoreSetup<StartDeps>, 'capabilities' | 'getStartServices'>
 ) => {
   core.capabilities.registerProvider(() => {
     return {
@@ -21,8 +21,7 @@ export const setupCapabilities = (
   });
 
   core.capabilities.registerSwitcher(async (request, capabilities, useDefaultCapabilities) => {
-    const isAnonymousRequest = !request.route.options.authRequired;
-    if (useDefaultCapabilities || isAnonymousRequest) {
+    if (useDefaultCapabilities) {
       return capabilities;
     }
     const [, { security }] = await core.getStartServices();

diff --git a/x-pack/plugins/file_upload/server/check_privileges.ts b/x-pack/plugins/file_upload/server/check_privileges.ts
@@ -32,6 +32,10 @@ export const checkFileUploadPrivileges = async ({
     return { hasImportPermission: true };
   }
 
+  if (!request.auth.isAuthenticated) {
+    return { hasImportPermission: false };
+  }
+
   const checkPrivilegesPayload: CheckPrivilegesPayload = {
     elasticsearch: {
       cluster: checkHasManagePipeline ? ['manage_pipeline'] : [],

diff --git a/x-pack/plugins/security/server/index.ts b/x-pack/plugins/security/server/index.ts
@@ -26,7 +26,8 @@ export type {
   InvalidateAPIKeyResult,
   GrantAPIKeyResult,
 } from './authentication';
-export type { CheckPrivilegesPayload, AuthorizationServiceSetup } from './authorization';
+export type { CheckPrivilegesPayload } from './authorization';
+export type AuthorizationServiceSetup = SecurityPluginStart['authz'];
 export { LegacyAuditLogger, AuditLogger, AuditEvent } from './audit';
 export type { SecurityPluginSetup, SecurityPluginStart };
 export type { AuthenticatedUser } from '../common/model';