Security - authorization mode request cache (#33248) (#33353)

* Changing authorization mode to useRbacForRequest and caching response

* Removing authorization from the mocha server fixture, it's not used

* Switching usages to useRbacForRequest

* Using jest's mocking support instead of hand-rolling one

* Using the request fixture

x-pack/plugins/security/index.js

@@ -140,7 +140,7 @@ export const security = (kibana) => new kibana.Plugin({
       const { callWithRequest, callWithInternalUser } = adminCluster;
       const callCluster = (...args) => callWithRequest(request, ...args);
 
-      if (authorization.mode.useRbac()) {
+      if (authorization.mode.useRbacForRequest(request)) {
         const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser);
         return new savedObjects.SavedObjectsClient(internalRepository);
       }
@@ -150,7 +150,7 @@ export const security = (kibana) => new kibana.Plugin({
     });
 
     savedObjects.addScopedSavedObjectsClientWrapperFactory(Number.MIN_VALUE, ({ client, request }) => {
-      if (authorization.mode.useRbac()) {
+      if (authorization.mode.useRbacForRequest(request)) {
         const { spaces } = server.plugins;
 
         return new SecureSavedObjectsClientWrapper({

x-pack/plugins/security/server/lib/__tests__/__fixtures__/server.js

@@ -37,14 +37,6 @@ export function serverFixture() {
         getUser: stub(),
         authenticate: stub(),
         deauthenticate: stub(),
-        authorization: {
-          mode: {
-            useRbac: stub(),
-          },
-          actions: {
-            login: 'stub-login-action',
-          },
-        },
       },
 
       xpack_main: {

x-pack/plugins/security/server/lib/authorization/mode.js

@@ -7,9 +7,15 @@
 export function authorizationModeFactory(
   xpackInfoFeature,
 ) {
+  const useRbacForRequestCache = new WeakMap();
+
   return {
-    useRbac() {
-      return xpackInfoFeature.getLicenseCheckResults().allowRbac;
+    useRbacForRequest(request) {
+      if (!useRbacForRequestCache.has(request)) {
+        useRbacForRequestCache.set(request, Boolean(xpackInfoFeature.getLicenseCheckResults().allowRbac));
+      }
+
+      return useRbacForRequestCache.get(request);
     },
   };
 }

x-pack/plugins/security/server/lib/authorization/mode.test.js

@@ -5,31 +5,70 @@
  */
 
 import { authorizationModeFactory } from './mode';
+import { requestFixture } from '../__tests__/__fixtures__/request';
+
+class MockXPackInfoFeature {
+  constructor(allowRbac) {
+    this.getLicenseCheckResults.mockReturnValue({ allowRbac });
+  }
+
+  getLicenseCheckResults = jest.fn();
+}
+
+describe(`#useRbacForRequest`, () => {
+  test(`throws an Error if request isn't specified`, async () => {
+    const mockXpackInfoFeature = new MockXPackInfoFeature(false);
+    const mode = authorizationModeFactory(mockXpackInfoFeature);
+
+    expect(() => mode.useRbacForRequest()).toThrowErrorMatchingInlineSnapshot(
+      `"Invalid value used as weak map key"`
+    );
+  });
+
+  test(`throws an Error if request is "null"`, async () => {
+    const mockXpackInfoFeature = new MockXPackInfoFeature(false);
+    const mode = authorizationModeFactory(mockXpackInfoFeature);
+
+    expect(() => mode.useRbacForRequest(null)).toThrowErrorMatchingInlineSnapshot(
+      `"Invalid value used as weak map key"`
+    );
+  });
 
-const createMockXpackInfoFeature = (allowRbac) => {
-  return {
-    getLicenseCheckResults() {
-      return {
-        allowRbac
-      };
-    }
-  };
-};
-
-describe(`#useRbac`, () => {
   test(`returns false if xpackInfoFeature.getLicenseCheckResults().allowRbac is false`, async () => {
-    const mockXpackInfoFeature = createMockXpackInfoFeature(false);
+    const mockXpackInfoFeature = new MockXPackInfoFeature(false);
     const mode = authorizationModeFactory(mockXpackInfoFeature);
+    const request = requestFixture();
 
-    const result = mode.useRbac();
+    const result = mode.useRbacForRequest(request);
     expect(result).toBe(false);
   });
 
+  test(`returns false if xpackInfoFeature.getLicenseCheckResults().allowRbac is initially false, and changes to true`, async () => {
+    const mockXpackInfoFeature = new MockXPackInfoFeature(false);
+    const mode = authorizationModeFactory(mockXpackInfoFeature);
+    const request = requestFixture();
+
+    expect(mode.useRbacForRequest(request)).toBe(false);
+    mockXpackInfoFeature.getLicenseCheckResults.mockReturnValue({ allowRbac: true });
+    expect(mode.useRbacForRequest(request)).toBe(false);
+  });
+
   test(`returns true if xpackInfoFeature.getLicenseCheckResults().allowRbac is true`, async () => {
-    const mockXpackInfoFeature = createMockXpackInfoFeature(true);
+    const mockXpackInfoFeature = new MockXPackInfoFeature(true);
     const mode = authorizationModeFactory(mockXpackInfoFeature);
+    const request = requestFixture();
 
-    const result = mode.useRbac();
+    const result = mode.useRbacForRequest(request);
     expect(result).toBe(true);
   });
+
+  test(`returns true if xpackInfoFeature.getLicenseCheckResults().allowRbac is initially true, and changes to false`, async () => {
+    const mockXpackInfoFeature = new MockXPackInfoFeature(true);
+    const mode = authorizationModeFactory(mockXpackInfoFeature);
+    const request = requestFixture();
+
+    expect(mode.useRbacForRequest(request)).toBe(true);
+    mockXpackInfoFeature.getLicenseCheckResults.mockReturnValue({ allowRbac: false });
+    expect(mode.useRbacForRequest(request)).toBe(true);
+  });
 });

x-pack/plugins/spaces/server/lib/__snapshots__/spaces_client.test.ts.snap

@@ -2,22 +2,22 @@
 
 exports[`#create authorization is null throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
 
-exports[`#create authorization.mode.useRbac returns false throws bad request when we're at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
+exports[`#create authorization.mode.useRbacForRequest returns false throws bad request when we're at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
 
-exports[`#create useRbac is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to create spaces"`;
+exports[`#create useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to create spaces"`;
 
-exports[`#create useRbac is true throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
+exports[`#create useRbacForRequest is true throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
 
 exports[`#delete authorization is null throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
 
-exports[`#delete authorization.mode.useRbac returns false throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
+exports[`#delete authorization.mode.useRbacForRequest returns false throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
 
-exports[`#delete authorization.mode.useRbac returns true throws Boom.forbidden if the user isn't authorized 1`] = `"Unauthorized to delete spaces"`;
+exports[`#delete authorization.mode.useRbacForRequest returns true throws Boom.forbidden if the user isn't authorized 1`] = `"Unauthorized to delete spaces"`;
 
-exports[`#delete authorization.mode.useRbac returns true throws bad request if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
+exports[`#delete authorization.mode.useRbacForRequest returns true throws bad request if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
 
-exports[`#get useRbac is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to get foo-space space"`;
+exports[`#get useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to get foo-space space"`;
 
-exports[`#getAll useRbac is true throws Boom.forbidden when user isn't authorized for any spaces 1`] = `"Forbidden"`;
+exports[`#getAll useRbacForRequest is true throws Boom.forbidden when user isn't authorized for any spaces 1`] = `"Forbidden"`;
 
-exports[`#update useRbac is true throws Boom.forbidden when user isn't authorized at space 1`] = `"Unauthorized to update spaces"`;
+exports[`#update useRbacForRequest is true throws Boom.forbidden when user isn't authorized at space 1`] = `"Unauthorized to update spaces"`;