[Fleet] Enforce superuser role for all fleet APIs (#85136)

2020-12-08 17:21:45 -05:00 · 2020-12-08 17:21:45 -05:00 · 943bce1512
parent d44fa13227
commit 943bce1512
7 changed files with 187 additions and 135 deletions
--- a/x-pack/plugins/fleet/server/plugin.ts
+++ b/x-pack/plugins/fleet/server/plugin.ts
@ -44,7 +44,8 @@ import {
  registerDataStreamRoutes,
  registerAgentPolicyRoutes,
  registerSetupRoutes,
-  registerAgentRoutes,
+  registerAgentAPIRoutes,
+  registerElasticAgentRoutes,
  registerEnrollmentApiKeyRoutes,
  registerInstallScriptRoutes,
  registerOutputRoutes,
@ -73,6 +74,7 @@ import { CloudSetup } from '../../cloud/server';
 import { agentCheckinState } from './services/agents/checkin/state';
 import { registerFleetUsageCollector } from './collectors/register';
 import { getInstallation } from './services/epm/packages';
+import { makeRouterEnforcingSuperuser } from './routes/security';

 export interface FleetSetupDeps {
  licensing: LicensingPluginSetup;
@ -213,6 +215,7 @@ export class FleetPlugin
    }

    const router = core.http.createRouter();
+
    const config = await this.config$.pipe(first()).toPromise();

    // Register usage collection
@ -220,16 +223,17 @@ export class FleetPlugin

    // Always register app routes for permissions checking
    registerAppRoutes(router);
-
+    // For all the routes we enforce the user to have role superuser
+    const routerSuperuserOnly = makeRouterEnforcingSuperuser(router);
    // Register rest of routes only if security is enabled
    if (this.security) {
-      registerSetupRoutes(router, config);
-      registerAgentPolicyRoutes(router);
-      registerPackagePolicyRoutes(router);
-      registerOutputRoutes(router);
-      registerSettingsRoutes(router);
-      registerDataStreamRoutes(router);
-      registerEPMRoutes(router);
+      registerSetupRoutes(routerSuperuserOnly, config);
+      registerAgentPolicyRoutes(routerSuperuserOnly);
+      registerPackagePolicyRoutes(routerSuperuserOnly);
+      registerOutputRoutes(routerSuperuserOnly);
+      registerSettingsRoutes(routerSuperuserOnly);
+      registerDataStreamRoutes(routerSuperuserOnly);
+      registerEPMRoutes(routerSuperuserOnly);

      // Conditional config routes
      if (config.agents.enabled) {
@ -245,12 +249,14 @@ export class FleetPlugin
          // we currently only use this global interceptor if fleet is enabled
          // since it would run this func on *every* req (other plugins, CSS, etc)
          registerLimitedConcurrencyRoutes(core, config);
-          registerAgentRoutes(router, config);
-          registerEnrollmentApiKeyRoutes(router);
+          registerAgentAPIRoutes(routerSuperuserOnly, config);
+          registerEnrollmentApiKeyRoutes(routerSuperuserOnly);
          registerInstallScriptRoutes({
-            router,
+            router: routerSuperuserOnly,
            basePath: core.http.basePath,
          });
+          // Do not enforce superuser role for Elastic Agent routes
+          registerElasticAgentRoutes(router, config);
        }
      }
    }
--- a/x-pack/plugins/fleet/server/routes/agent/index.ts
+++ b/x-pack/plugins/fleet/server/routes/agent/index.ts
@ -81,7 +81,7 @@ function makeValidator(jsonSchema: any) {
  };
 }

-export const registerRoutes = (router: IRouter, config: FleetConfigType) => {
+export const registerAPIRoutes = (router: IRouter, config: FleetConfigType) => {
  // Get one
  router.get(
    {
@ -119,6 +119,96 @@ export const registerRoutes = (router: IRouter, config: FleetConfigType) => {
    getAgentsHandler
  );

+  // Agent actions
+  router.post(
+    {
+      path: AGENT_API_ROUTES.ACTIONS_PATTERN,
+      validate: PostNewAgentActionRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postNewAgentActionHandlerBuilder({
+      getAgent: AgentService.getAgent,
+      createAgentAction: AgentService.createAgentAction,
+    })
+  );
+
+  router.post(
+    {
+      path: AGENT_API_ROUTES.UNENROLL_PATTERN,
+      validate: PostAgentUnenrollRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postAgentUnenrollHandler
+  );
+
+  router.put(
+    {
+      path: AGENT_API_ROUTES.REASSIGN_PATTERN,
+      validate: PutAgentReassignRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    putAgentsReassignHandler
+  );
+
+  // Get agent events
+  router.get(
+    {
+      path: AGENT_API_ROUTES.EVENTS_PATTERN,
+      validate: GetOneAgentEventsRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-read`] },
+    },
+    getAgentEventsHandler
+  );
+
+  // Get agent status for policy
+  router.get(
+    {
+      path: AGENT_API_ROUTES.STATUS_PATTERN,
+      validate: GetAgentStatusRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-read`] },
+    },
+    getAgentStatusForAgentPolicyHandler
+  );
+  // upgrade agent
+  router.post(
+    {
+      path: AGENT_API_ROUTES.UPGRADE_PATTERN,
+      validate: PostAgentUpgradeRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postAgentUpgradeHandler
+  );
+  // bulk upgrade
+  router.post(
+    {
+      path: AGENT_API_ROUTES.BULK_UPGRADE_PATTERN,
+      validate: PostBulkAgentUpgradeRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postBulkAgentsUpgradeHandler
+  );
+  // Bulk reassign
+  router.post(
+    {
+      path: AGENT_API_ROUTES.BULK_REASSIGN_PATTERN,
+      validate: PostBulkAgentReassignRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postBulkAgentsReassignHandler
+  );
+
+  // Bulk unenroll
+  router.post(
+    {
+      path: AGENT_API_ROUTES.BULK_UNENROLL_PATTERN,
+      validate: PostBulkAgentUnenrollRequestSchema,
+      options: { tags: [`access:${PLUGIN_ID}-all`] },
+    },
+    postBulkAgentsUnenrollHandler
+  );
+};
+
+export const registerElasticAgentRoutes = (router: IRouter, config: FleetConfigType) => {
  const pollingRequestTimeout = config.agents.pollingRequestTimeout;
  // Agent checkin
  router.post(
@ -226,92 +316,4 @@ export const registerRoutes = (router: IRouter, config: FleetConfigType) => {
      saveAgentEvents: AgentService.saveAgentEvents,
    })
  );
-
-  // Agent actions
-  router.post(
-    {
-      path: AGENT_API_ROUTES.ACTIONS_PATTERN,
-      validate: PostNewAgentActionRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postNewAgentActionHandlerBuilder({
-      getAgent: AgentService.getAgent,
-      createAgentAction: AgentService.createAgentAction,
-    })
-  );
-
-  router.post(
-    {
-      path: AGENT_API_ROUTES.UNENROLL_PATTERN,
-      validate: PostAgentUnenrollRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postAgentUnenrollHandler
-  );
-
-  router.put(
-    {
-      path: AGENT_API_ROUTES.REASSIGN_PATTERN,
-      validate: PutAgentReassignRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    putAgentsReassignHandler
-  );
-
-  // Get agent events
-  router.get(
-    {
-      path: AGENT_API_ROUTES.EVENTS_PATTERN,
-      validate: GetOneAgentEventsRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-read`] },
-    },
-    getAgentEventsHandler
-  );
-
-  // Get agent status for policy
-  router.get(
-    {
-      path: AGENT_API_ROUTES.STATUS_PATTERN,
-      validate: GetAgentStatusRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-read`] },
-    },
-    getAgentStatusForAgentPolicyHandler
-  );
-  // upgrade agent
-  router.post(
-    {
-      path: AGENT_API_ROUTES.UPGRADE_PATTERN,
-      validate: PostAgentUpgradeRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postAgentUpgradeHandler
-  );
-  // bulk upgrade
-  router.post(
-    {
-      path: AGENT_API_ROUTES.BULK_UPGRADE_PATTERN,
-      validate: PostBulkAgentUpgradeRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postBulkAgentsUpgradeHandler
-  );
-  // Bulk reassign
-  router.post(
-    {
-      path: AGENT_API_ROUTES.BULK_REASSIGN_PATTERN,
-      validate: PostBulkAgentReassignRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postBulkAgentsReassignHandler
-  );
-
-  // Bulk unenroll
-  router.post(
-    {
-      path: AGENT_API_ROUTES.BULK_UNENROLL_PATTERN,
-      validate: PostBulkAgentUnenrollRequestSchema,
-      options: { tags: [`access:${PLUGIN_ID}-all`] },
-    },
-    postBulkAgentsUnenrollHandler
-  );
 };
--- a/x-pack/plugins/fleet/server/routes/index.ts
+++ b/x-pack/plugins/fleet/server/routes/index.ts
@ -8,7 +8,10 @@ export { registerRoutes as registerPackagePolicyRoutes } from './package_policy'
 export { registerRoutes as registerDataStreamRoutes } from './data_streams';
 export { registerRoutes as registerEPMRoutes } from './epm';
 export { registerRoutes as registerSetupRoutes } from './setup';
-export { registerRoutes as registerAgentRoutes } from './agent';
+export {
+  registerAPIRoutes as registerAgentAPIRoutes,
+  registerElasticAgentRoutes as registerElasticAgentRoutes,
+} from './agent';
 export { registerRoutes as registerEnrollmentApiKeyRoutes } from './enrollment_api_key';
 export { registerRoutes as registerInstallScriptRoutes } from './install_script';
 export { registerRoutes as registerOutputRoutes } from './output';
--- a/x-pack/plugins/fleet/server/routes/security.ts
+++ b/x-pack/plugins/fleet/server/routes/security.ts
@ -0,0 +1,43 @@
+/*
+ * 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 { IRouter, RequestHandler } from 'src/core/server';
+import { appContextService } from '../services';
+
+export function enforceSuperUser<T1, T2, T3>(
+  handler: RequestHandler<T1, T2, T3>
+): RequestHandler<T1, T2, T3> {
+  return function enforceSuperHandler(context, req, res) {
+    const security = appContextService.getSecurity();
+    const user = security.authc.getCurrentUser(req);
+    if (!user) {
+      return res.unauthorized();
+    }
+
+    const userRoles = user.roles || [];
+    if (!userRoles.includes('superuser')) {
+      return res.forbidden({
+        body: {
+          message: 'Access to Fleet API require the superuser role.',
+        },
+      });
+    }
+    return handler(context, req, res);
+  };
+}
+
+export function makeRouterEnforcingSuperuser(router: IRouter): IRouter {
+  return {
+    get: (options, handler) => router.get(options, enforceSuperUser(handler)),
+    delete: (options, handler) => router.delete(options, enforceSuperUser(handler)),
+    post: (options, handler) => router.post(options, enforceSuperUser(handler)),
+    put: (options, handler) => router.put(options, enforceSuperUser(handler)),
+    patch: (options, handler) => router.patch(options, enforceSuperUser(handler)),
+    handleLegacyErrors: (handler) => router.handleLegacyErrors(handler),
+    getRoutes: () => router.getRoutes(),
+    routerPath: router.routerPath,
+  };
+}
--- a/x-pack/test/fleet_api_integration/apis/agents/acks.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/acks.ts
@ -15,7 +15,8 @@ export default function (providerContext: FtrProviderContext) {
  const esClient = getService('es');
  const kibanaServer = getService('kibanaServer');

-  const supertest = getSupertestWithoutAuth(providerContext);
+  const supertestWithoutAuth = getSupertestWithoutAuth(providerContext);
+  const supertest = getService('supertest');
  let apiKey: { id: string; api_key: string };

  describe('fleet_agents_acks', () => {
@ -49,7 +50,7 @@ export default function (providerContext: FtrProviderContext) {
    });

    it('should return a 401 if this a not a valid acks access', async () => {
-      await supertest
+      await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set('Authorization', 'ApiKey NOT_A_VALID_TOKEN')
@ -60,7 +61,7 @@ export default function (providerContext: FtrProviderContext) {
    });

    it('should return a 200 if this a valid acks request', async () => {
-      const { body: apiResponse } = await supertest
+      const { body: apiResponse } = await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set(
@ -91,13 +92,10 @@ export default function (providerContext: FtrProviderContext) {
        })
        .expect(200);
      expect(apiResponse.action).to.be('acks');
+
      const { body: eventResponse } = await supertest
        .get(`/api/fleet/agents/agent1/events`)
        .set('kbn-xsrf', 'xx')
-        .set(
-          'Authorization',
-          `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}`
-        )
        .expect(200);
      const expectedEvents = eventResponse.list.filter(
        (item: Record<string, string>) =>
@ -120,7 +118,7 @@ export default function (providerContext: FtrProviderContext) {
    });

    it('should return a 400 when request event list contains event for another agent id', async () => {
-      const { body: apiResponse } = await supertest
+      const { body: apiResponse } = await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set(
@ -147,7 +145,7 @@ export default function (providerContext: FtrProviderContext) {
    });

    it('should return a 400 when request event list contains action that does not belong to agent current actions', async () => {
-      const { body: apiResponse } = await supertest
+      const { body: apiResponse } = await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set(
@ -181,7 +179,7 @@ export default function (providerContext: FtrProviderContext) {
    });

    it('should return a 400 when request event list contains action types that are not allowed for acknowledgement', async () => {
-      const { body: apiResponse } = await supertest
+      const { body: apiResponse } = await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set(
@ -211,10 +209,6 @@ export default function (providerContext: FtrProviderContext) {
      const { body: actionRes } = await supertest
        .post(`/api/fleet/agents/agent1/actions`)
        .set('kbn-xsrf', 'xx')
-        .set(
-          'Authorization',
-          `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}`
-        )
        .send({
          action: {
            type: 'UPGRADE',
@ -223,7 +217,7 @@ export default function (providerContext: FtrProviderContext) {
        })
        .expect(200);
      const actionId = actionRes.item.id;
-      await supertest
+      await supertestWithoutAuth
        .post(`/api/fleet/agents/agent1/acks`)
        .set('kbn-xsrf', 'xx')
        .set(
--- a/x-pack/test/fleet_api_integration/apis/agents/delete.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/delete.ts
@ -11,7 +11,9 @@ export default function ({ getService }: FtrProviderContext) {
  const esArchiver = getService('esArchiver');
  const supertest = getService('supertestWithoutAuth');
  const security = getService('security');
-  const users: { [rollName: string]: { username: string; password: string; permissions?: any } } = {
+  const users: {
+    [roleName: string]: { username: string; password: string; permissions?: any; roles?: string[] };
+  } = {
    fleet_user: {
      permissions: {
        feature: {
@ -23,6 +25,7 @@ export default function ({ getService }: FtrProviderContext) {
      password: 'changeme',
    },
    fleet_admin: {
+      roles: ['superuser'],
      permissions: {
        feature: {
          fleet: ['all'],
@ -48,7 +51,7 @@ export default function ({ getService }: FtrProviderContext) {
          // Import a repository first
          await security.user.create(user.username, {
            password: user.password,
-            roles: [roleName],
+            roles: [roleName, ...(user.roles || [])],
            full_name: user.username,
          });
        }
--- a/x-pack/test/fleet_api_integration/apis/agents/list.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/list.ts
@ -10,7 +10,8 @@ import { FtrProviderContext } from '../../../api_integration/ftr_provider_contex

 export default function ({ getService }: FtrProviderContext) {
  const esArchiver = getService('esArchiver');
-  const supertest = getService('supertestWithoutAuth');
+  const supertestWithoutAuth = getService('supertestWithoutAuth');
+  const supertest = getService('supertest');
  const security = getService('security');
  const users: { [rollName: string]: { username: string; password: string; permissions?: any } } = {
    kibana_basic_user: {
@ -72,40 +73,40 @@ export default function ({ getService }: FtrProviderContext) {
      await esArchiver.unload('fleet/agents');
    });

-    it('should return the list of agents when requesting as a user with fleet write permissions', async () => {
-      const { body: apiResponse } = await supertest
+    it('should return a 403 if a user without the superuser role try to access the APU', async () => {
+      await supertestWithoutAuth
        .get(`/api/fleet/agents`)
        .auth(users.fleet_admin.username, users.fleet_admin.password)
-        .expect(200);
+        .expect(403);
+    });
+    it('should not return the list of agents when requesting as a user without fleet permissions', async () => {
+      await supertestWithoutAuth
+        .get(`/api/fleet/agents`)
+        .auth(users.kibana_basic_user.username, users.kibana_basic_user.password)
+        .expect(403);
+    });
+
+    it('should return the list of agents when requesting as a user with fleet write permissions', async () => {
+      const { body: apiResponse } = await supertest.get(`/api/fleet/agents`).expect(200);

      expect(apiResponse).to.have.keys('page', 'total', 'list');
      expect(apiResponse.total).to.eql(4);
    });
    it('should return the list of agents when requesting as a user with fleet read permissions', async () => {
-      const { body: apiResponse } = await supertest
-        .get(`/api/fleet/agents`)
-        .auth(users.fleet_user.username, users.fleet_user.password)
-        .expect(200);
+      const { body: apiResponse } = await supertest.get(`/api/fleet/agents`).expect(200);
      expect(apiResponse).to.have.keys('page', 'total', 'list');
      expect(apiResponse.total).to.eql(4);
    });
-    it('should not return the list of agents when requesting as a user without fleet permissions', async () => {
-      await supertest
-        .get(`/api/fleet/agents`)
-        .auth(users.kibana_basic_user.username, users.kibana_basic_user.password)
-        .expect(403);
-    });
+
    it('should return a 400 when given an invalid "kuery" value', async () => {
      await supertest
        .get(`/api/fleet/agents?kuery=m`) // missing saved object type
-        .auth(users.fleet_user.username, users.fleet_user.password)
        .expect(400);
    });
    it('should accept a valid "kuery" value', async () => {
      const filter = encodeURIComponent('fleet-agents.shared_id : "agent2_filebeat"');
      const { body: apiResponse } = await supertest
        .get(`/api/fleet/agents?kuery=${filter}`)
-        .auth(users.fleet_user.username, users.fleet_user.password)
        .expect(200);

      expect(apiResponse.total).to.eql(1);