Deleting no longer used privileges

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

@@ -51,12 +51,12 @@ export async function registerPrivilegesWithCluster(server) {
     });
   };
 
-  const shouldRemovePrivileges = (existingPrivileges, expectedPrivileges) => {
+  const getPrivilegesToDelete = (existingPrivileges, expectedPrivileges) => {
     if (isEmpty(existingPrivileges)) {
-      return false;
+      return [];
     }
 
-    return difference(Object.keys(existingPrivileges[application]), Object.keys(expectedPrivileges[application])).length > 0;
+    return difference(Object.keys(existingPrivileges[application]), Object.keys(expectedPrivileges[application]));
   };
 
   const privilegeMap = buildPrivilegeMap(savedObjectTypes, actions);
@@ -75,18 +75,19 @@ export async function registerPrivilegesWithCluster(server) {
       return;
     }
 
-    // The ES privileges POST endpoint only allows us to add new privileges, or update specified privileges; it doesn't
-    // remove unspecified privileges. We don't currently have a need to remove privileges, as this would be a
-    // backwards compatibility issue, and we'd have to figure out how to migrate roles, so we're throwing an Error if we
-    // unintentionally get ourselves in this position.
-    if (shouldRemovePrivileges(existingPrivileges, expectedPrivileges)) {
-      throw new Error(`Privileges are missing and can't be removed, currently.`);
+    const privilegesToDelete = getPrivilegesToDelete(existingPrivileges, expectedPrivileges);
+    for (const privilegeToDelete of privilegesToDelete) {
+      server.log(['security', 'debug'], `Deleting Kibana Privilege ${privilegeToDelete} from Elasticearch for ${application}`);
+      await callCluster('shield.deletePrivilege', {
+        application,
+        privilege: privilegeToDelete
+      });
     }
 
-    server.log(['security', 'debug'], `Updated Kibana Privileges with Elasticearch for ${application}`);
     await callCluster('shield.postPrivileges', {
       body: expectedPrivileges
     });
+    server.log(['security', 'debug'], `Updated Kibana Privileges with Elasticearch for ${application}`);
   } catch (err) {
     server.log(['security', 'error'], `Error registering Kibana Privileges with Elasticsearch for ${application}: ${err.message}`);
     throw err;

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

@@ -67,9 +67,9 @@ const registerPrivilegesWithClusterTest = (description, {
   };
 
   const createExpectUpdatedPrivileges = (mockServer, mockCallWithInternalUser, error) => {
-    return (postPrivilegesBody) => {
+    return (postPrivilegesBody, deletedPrivileges = []) => {
       expect(error).toBeUndefined();
-      expect(mockCallWithInternalUser).toHaveBeenCalledTimes(2);
+      expect(mockCallWithInternalUser).toHaveBeenCalledTimes(2 + deletedPrivileges.length);
       expect(mockCallWithInternalUser).toHaveBeenCalledWith('shield.getPrivilege', {
         privilege: application,
       });
@@ -79,7 +79,19 @@ const registerPrivilegesWithClusterTest = (description, {
           body: postPrivilegesBody,
         }
       );
-
+      for (const deletedPrivilege of deletedPrivileges) {
+        expect(mockServer.log).toHaveBeenCalledWith(
+          ['security', 'debug'],
+          `Deleting Kibana Privilege ${deletedPrivilege} from Elasticearch for ${application}`
+        );
+        expect(mockCallWithInternalUser).toHaveBeenCalledWith(
+          'shield.deletePrivilege',
+          {
+            application,
+            privilege: deletedPrivilege
+          }
+        );
+      }
       expect(mockServer.log).toHaveBeenCalledWith(
         ['security', 'debug'],
         `Registering Kibana Privileges with Elasticsearch for ${application}`
@@ -208,7 +220,7 @@ registerPrivilegesWithClusterTest(`inserts privileges when we don't have any exi
   }
 });
 
-registerPrivilegesWithClusterTest(`throws error when we should be removing privilege`, {
+registerPrivilegesWithClusterTest(`deletes no-longer specified privileges`, {
   privilegeMap: {
     global: {
       foo: ['action:foo'],
@@ -236,11 +248,32 @@ registerPrivilegesWithClusterTest(`throws error when we should be removing privi
         name: 'space_bar',
         actions: ['action:not-bar'],
         metadata: {},
+      },
+      space_baz: {
+        application,
+        name: 'space_baz',
+        actions: ['action:not-baz'],
+        metadata: {},
       }
     }
   },
-  assert: ({ expectErrorThrown }) => {
-    expectErrorThrown(`Privileges are missing and can't be removed, currently.`);
+  assert: ({ expectUpdatedPrivileges }) => {
+    expectUpdatedPrivileges({
+      [application]: {
+        foo: {
+          application,
+          name: 'foo',
+          actions: ['action:foo'],
+          metadata: {},
+        },
+        space_bar: {
+          application,
+          name: 'space_bar',
+          actions: ['action:bar'],
+          metadata: {},
+        }
+      }
+    }, [ 'quz', 'space_baz' ]);
   }
 });
 

x-pack/server/lib/esjs_shield_plugin.js

@@ -390,6 +390,23 @@
       }]
     });
 
+    shield.deletePrivilege = ca({
+      method: 'DELETE',
+      urls: [{
+        fmt: '/_xpack/security/privilege/<%=application%>/<%=privilege%>',
+        req: {
+          application: {
+            type: 'string',
+            required: true
+          },
+          privilege: {
+            type: 'string',
+            required: true
+          }
+        }
+      }]
+    });
+
     shield.postPrivileges = ca({
       method: 'POST',
       needBody: true,