Logging legacy fallback deprecation warning on login

x-pack/plugins/security/index.js

@@ -17,11 +17,10 @@ import { authenticateFactory } from './server/lib/auth_redirect';
 import { checkLicense } from './server/lib/check_license';
 import { initAuthenticator } from './server/lib/authentication/authenticator';
 import { initPrivilegesApi } from './server/routes/api/v1/privileges';
-import { checkPrivilegesWithRequestFactory } from './server/lib/authorization/check_privileges';
 import { SecurityAuditLogger } from './server/lib/audit_logger';
 import { AuditLogger } from '../../server/lib/audit_logger';
 import { SecureSavedObjectsClient } from './server/lib/saved_objects_client/secure_saved_objects_client';
-import { registerPrivilegesWithCluster } from './server/lib/privileges';
+import { initAuthorizationService, registerPrivilegesWithCluster } from './server/lib/authorization';
 import { watchStatusAndLicenseToInitialize } from './server/lib/watch_status_and_license_to_initialize';
 
 export const security = (kibana) => new kibana.Plugin({
@@ -114,9 +113,11 @@ export const security = (kibana) => new kibana.Plugin({
     server.auth.strategy('session', 'login', 'required');
 
     const auditLogger = new SecurityAuditLogger(server.config(), new AuditLogger(server, 'security'));
-    const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(server);
-    const { savedObjects } = server;
 
+    // exposes server.plugins.security.authorization
+    initAuthorizationService(server);
+
+    const { savedObjects } = server;
     savedObjects.setScopedSavedObjectsClientFactory(({
       request,
     }) => {
@@ -130,7 +131,8 @@ export const security = (kibana) => new kibana.Plugin({
         return new savedObjects.SavedObjectsClient(callWithRequestRepository);
       }
 
-      const checkPrivileges = checkPrivilegesWithRequest(request);
+      const { authorization } = server.plugins.security;
+      const checkPrivileges = authorization.checkPrivilegesWithRequest(request);
       const internalRepository = savedObjects.getSavedObjectsRepository(callWithInternalUser);
 
       return new SecureSavedObjectsClient({
@@ -140,6 +142,7 @@ export const security = (kibana) => new kibana.Plugin({
         checkPrivileges,
         auditLogger,
         savedObjectTypes: savedObjects.types,
+        actions: authorization.actions,
       });
     });
 

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

@@ -35,7 +35,13 @@ export function serverFixture() {
       security: {
         getUser: stub(),
         authenticate: stub(),
-        deauthenticate: stub()
+        deauthenticate: stub(),
+        authorization: {
+          checkPrivilegesWithRequest: stub(),
+          actions: {
+            login: 'stub-login-action',
+          },
+        },
       },
 
       xpack_main: {

x-pack/plugins/security/server/lib/authorization/__snapshots__/actions.test.js.snap

@@ -0,0 +1,25 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`#getSavedObjectAction() action of "" throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() action of {} throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() action of 1 throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() action of null throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() action of true throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() action of undefined throws error 1`] = `"action is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of "" throws error 1`] = `"type is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of {} throws error 1`] = `"type is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of 1 throws error 1`] = `"type is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of null throws error 1`] = `"type is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of true throws error 1`] = `"type is required and must be a string"`;
+
+exports[`#getSavedObjectAction() type of undefined throws error 1`] = `"type is required and must be a string"`;

x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.js.snap

@@ -1,3 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`throws error if missing version privilege and has login privilege 1`] = `"Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user."`;
+exports[`with index privileges throws error if missing version privilege and has login privilege 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`;
+
+exports[`with no index privileges throws error if missing version privilege and has login privilege 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`;

x-pack/plugins/security/server/lib/authorization/__snapshots__/init.test.js.snap

@@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`deep freezes exposed service 1`] = `"Cannot delete property 'checkPrivilegesWithRequest' of #<Object>"`;
+
+exports[`deep freezes exposed service 2`] = `"Cannot add property foo, object is not extensible"`;
+
+exports[`deep freezes exposed service 3`] = `"Cannot assign to read only property 'login' of object '#<Object>'"`;

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

@@ -0,0 +1,26 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+import { isString } from 'lodash';
+
+export function actionsFactory(config) {
+  const kibanaVersion = config.get('pkg.version');
+
+  return {
+    getSavedObjectAction(type, action) {
+      if (!type || !isString(type)) {
+        throw new Error('type is required and must be a string');
+      }
+
+      if (!action || !isString(action)) {
+        throw new Error('action is required and must be a string');
+      }
+
+      return `action:saved_objects/${type}/${action}`;
+    },
+    login: `action:login`,
+    version: `version:${kibanaVersion}`,
+  };
+}

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

@@ -0,0 +1,69 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { actionsFactory } from './actions';
+
+const createMockConfig = (settings = {}) => {
+  const mockConfig = {
+    get: jest.fn()
+  };
+
+  mockConfig.get.mockImplementation(key => settings[key]);
+
+  return mockConfig;
+};
+
+describe('#login', () => {
+  test('returns action:login', () => {
+    const mockConfig = createMockConfig();
+
+    const actions = actionsFactory(mockConfig);
+
+    expect(actions.login).toEqual('action:login');
+  });
+});
+
+describe('#version', () => {
+  test(`returns version:\${config.get('pkg.version')}`, () => {
+    const version = 'mock-version';
+    const mockConfig = createMockConfig({ 'pkg.version': version });
+
+    const actions = actionsFactory(mockConfig);
+
+    expect(actions.version).toEqual(`version:${version}`);
+  });
+});
+
+describe('#getSavedObjectAction()', () => {
+  test('uses type and action to build action', () => {
+    const mockConfig = createMockConfig();
+    const actions = actionsFactory(mockConfig);
+    const type = 'saved-object-type';
+    const action = 'saved-object-action';
+
+    const result = actions.getSavedObjectAction(type, action);
+
+    expect(result).toEqual(`action:saved_objects/${type}/${action}`);
+  });
+
+  [null, undefined, '', 1, true, {}].forEach(type => {
+    test(`type of ${JSON.stringify(type)} throws error`, () => {
+      const mockConfig = createMockConfig();
+      const actions = actionsFactory(mockConfig);
+
+      expect(() => actions.getSavedObjectAction(type, 'saved-object-action')).toThrowErrorMatchingSnapshot();
+    });
+  });
+
+  [null, undefined, '', 1, true, {}].forEach(action => {
+    test(`action of ${JSON.stringify(action)} throws error`, () => {
+      const mockConfig = createMockConfig();
+      const actions = actionsFactory(mockConfig);
+
+      expect(() => actions.getSavedObjectAction('saved-object-type', action)).toThrowErrorMatchingSnapshot();
+    });
+  });
+});

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

@@ -4,103 +4,82 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { getClient } from '../../../../../server/lib/get_client_shield';
+import { uniq } from 'lodash';
 import { ALL_RESOURCE } from '../../../common/constants';
-import { getVersionAction, getLoginAction } from '../privileges';
 
 export const CHECK_PRIVILEGES_RESULT = {
-  UNAUTHORIZED: Symbol(),
-  AUTHORIZED: Symbol(),
-  LEGACY: Symbol(),
+  UNAUTHORIZED: Symbol('Unauthorized'),
+  AUTHORIZED: Symbol('Authorized'),
+  LEGACY: Symbol('Legacy'),
 };
 
-export function checkPrivilegesWithRequestFactory(server) {
-  const callWithRequest = getClient(server).callWithRequest;
+export function checkPrivilegesWithRequestFactory(shieldClient, config, actions) {
+  const { callWithRequest } = shieldClient;
 
-  const config = server.config();
-  const kibanaVersion = config.get('pkg.version');
   const application = config.get('xpack.security.rbac.application');
   const kibanaIndex = config.get('kibana.index');
 
-  const loginAction = getLoginAction();
-  const versionAction = getVersionAction(kibanaVersion);
+  const hasIncompatibileVersion = (applicationPrivilegesResponse) => {
+    return !applicationPrivilegesResponse[actions.version] && applicationPrivilegesResponse[actions.login];
+  };
 
-  return function checkPrivilegesWithRequest(request) {
+  const hasAllApplicationPrivileges = (applicationPrivilegesResponse) => {
+    return Object.values(applicationPrivilegesResponse).every(val => val === true);
+  };
 
-    const checkApplicationPrivileges = async (privileges) => {
-      const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', {
-        body: {
-          applications: [{
-            application,
-            resources: [ALL_RESOURCE],
-            privileges
-          }]
-        }
-      });
+  const hasNoApplicationPrivileges = (applicationPrivilegesResponse) => {
+    return Object.values(applicationPrivilegesResponse).every(val => val === false);
+  };
 
-      const hasPrivileges = privilegeCheck.application[application][ALL_RESOURCE];
+  const hasLegacyPrivileges = (indexPrivilegesResponse) => {
+    return Object.values(indexPrivilegesResponse).includes(true);
+  };
 
-      // We include the login action in all privileges, so the existence of it and not the version privilege
-      // lets us know that we're running in an incorrect configuration. Without the login privilege check, we wouldn't
-      // know whether the user just wasn't authorized for this instance of Kibana in general
-      if (!hasPrivileges[getVersionAction(kibanaVersion)] && hasPrivileges[getLoginAction()]) {
-        throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.');
-      }
+  const determineResult = (applicationPrivilegesResponse, indexPrivilegesResponse) => {
+    if (hasAllApplicationPrivileges(applicationPrivilegesResponse)) {
+      return CHECK_PRIVILEGES_RESULT.AUTHORIZED;
+    }
 
-      return {
-        username: privilegeCheck.username,
-        hasAllRequested: privilegeCheck.has_all_requested,
-        privileges: hasPrivileges
-      };
-    };
+    if (hasNoApplicationPrivileges(applicationPrivilegesResponse) && hasLegacyPrivileges(indexPrivilegesResponse)) {
+      return CHECK_PRIVILEGES_RESULT.LEGACY;
+    }
 
-    const hasPrivilegesOnKibanaIndex = async () => {
-      const privilegeCheck = await callWithRequest(request, 'shield.hasPrivileges', {
+    return CHECK_PRIVILEGES_RESULT.UNAUTHORIZED;
+  };
+
+  return function checkPrivilegesWithRequest(request) {
+
+    return async function checkPrivileges(privileges) {
+      const allApplicationPrivileges = uniq([actions.version, actions.login, ...privileges]);
+      const hasPrivilegesResponse = await callWithRequest(request, 'shield.hasPrivileges', {
         body: {
+          applications: [{
+            application,
+            resources: [ALL_RESOURCE],
+            privileges: allApplicationPrivileges
+          }],
           index: [{
             names: [kibanaIndex],
             privileges: ['create', 'delete', 'read', 'view_index_metadata']
-          }]
+          }],
         }
       });
 
-      return Object.values(privilegeCheck.index[kibanaIndex]).includes(true);
-    };
-
-    return async function checkPrivileges(privileges) {
-      const allPrivileges = [versionAction, loginAction, ...privileges];
-      const applicationPrivilegesCheck = await checkApplicationPrivileges(allPrivileges);
-
-      const username = applicationPrivilegesCheck.username;
-
-      // We don't want to expose the version privilege to consumers, as it's an implementation detail only to detect version mismatch
-      const missing = Object.keys(applicationPrivilegesCheck.privileges)
-        .filter(p => !applicationPrivilegesCheck.privileges[p])
-        .filter(p => p !== versionAction);
-
-      if (applicationPrivilegesCheck.hasAllRequested) {
-        return {
-          result: CHECK_PRIVILEGES_RESULT.AUTHORIZED,
-          username,
-          missing,
-        };
-      }
+      const applicationPrivilegesResponse = hasPrivilegesResponse.application[application][ALL_RESOURCE];
+      const indexPrivilegesResponse =  hasPrivilegesResponse.index[kibanaIndex];
 
-      if (!applicationPrivilegesCheck.privileges[loginAction] && await hasPrivilegesOnKibanaIndex()) {
-        const msg = 'Relying on index privileges is deprecated and will be removed in Kibana 7.0';
-        server.log(['warning', 'deprecated', 'security'], msg);
-
-        return {
-          result: CHECK_PRIVILEGES_RESULT.LEGACY,
-          username,
-          missing,
-        };
+      if (hasIncompatibileVersion(applicationPrivilegesResponse)) {
+        throw new Error('Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.');
       }
 
       return {
-        result: CHECK_PRIVILEGES_RESULT.UNAUTHORIZED,
-        username,
-        missing,
+        result: determineResult(applicationPrivilegesResponse, indexPrivilegesResponse),
+        username: hasPrivilegesResponse.username,
+
+        // we only return missing privileges that they're specifically checking for
+        missing: Object.keys(applicationPrivilegesResponse)
+          .filter(privilege => privileges.includes(privilege))
+          .filter(privilege => !applicationPrivilegesResponse[privilege])
       };
     };
   };