Ensure that security is enabled before doing user authentication chec…

…ks (elastic#70127) (elastic#70227) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
MadameSheema · Jul 2, 2020 · 682011b · 682011b
1 parent b2bd4fe
commit 682011b
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 4 deletions.
diff --git a/x-pack/plugins/reporting/server/routes/generation.test.ts b/x-pack/plugins/reporting/server/routes/generation.test.ts
@@ -57,6 +57,9 @@ describe('POST /api/reporting/generate', () => {
         },
       },
       security: {
+        license: {
+          isEnabled: () => true,
+        },
         authc: {
           getCurrentUser: () => ({
             id: '123',

diff --git a/x-pack/plugins/reporting/server/routes/jobs.test.ts b/x-pack/plugins/reporting/server/routes/jobs.test.ts
@@ -47,6 +47,9 @@ describe('GET /api/reporting/jobs/download', () => {
         legacy: { client: { callAsInternalUser: jest.fn() } },
       },
       security: {
+        license: {
+          isEnabled: () => true,
+        },
         authc: {
           getCurrentUser: () => ({
             id: '123',
@@ -113,6 +116,9 @@ describe('GET /api/reporting/jobs/download', () => {
       // @ts-ignore
       ...core.pluginSetupDeps,
       security: {
+        license: {
+          isEnabled: () => true,
+        },
         authc: {
           getCurrentUser: () => undefined,
         },
@@ -136,6 +142,9 @@ describe('GET /api/reporting/jobs/download', () => {
       // @ts-ignore
       ...core.pluginSetupDeps,
       security: {
+        license: {
+          isEnabled: () => true,
+        },
         authc: {
           getCurrentUser: () => ({
             id: '123',

diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts
@@ -46,7 +46,7 @@ describe('authorized_user_pre_routing', function () {
     mockCore = await createMockReportingCore(mockReportingConfig);
   });
 
-  it('should return from handler with null user when security is disabled', async function () {
+  it('should return from handler with a "null" user when security plugin is not found', async function () {
     mockCore.getPluginSetupDeps = () =>
       (({
         // @ts-ignore
@@ -66,12 +66,37 @@ describe('authorized_user_pre_routing', function () {
     expect(handlerCalled).toBe(true);
   });
 
-  it('should return with 401 when security is enabled but no authenticated user', async function () {
+  it('should return from handler with a "null" user when security is disabled', async function () {
     mockCore.getPluginSetupDeps = () =>
       (({
         // @ts-ignore
         ...mockCore.pluginSetupDeps,
         security: {
+          license: {
+            isEnabled: () => false,
+          },
+        }, // disable security
+      } as unknown) as ReportingInternalSetup);
+    const authorizedUserPreRouting = authorizedUserPreRoutingFactory(mockCore);
+    const mockResponseFactory = httpServerMock.createResponseFactory() as KibanaResponseFactory;
+
+    let handlerCalled = false;
+    authorizedUserPreRouting((user: unknown) => {
+      expect(user).toBe(null); // verify the user is a null value
+      handlerCalled = true;
+      return Promise.resolve({ status: 200, options: {} });
+    })(getMockContext(), getMockRequest(), mockResponseFactory);
+
+    expect(handlerCalled).toBe(true);
+  });
+
+  it('should return with 401 when security is enabled and the request is unauthenticated', async function () {
+    mockCore.getPluginSetupDeps = () =>
+      (({
+        // @ts-ignore
+        ...mockCore.pluginSetupDeps,
+        security: {
+          license: { isEnabled: () => true },
           authc: { getCurrentUser: () => null },
         },
       } as unknown) as ReportingInternalSetup);
@@ -87,12 +112,13 @@ describe('authorized_user_pre_routing', function () {
     });
   });
 
-  it(`should return with 403 when security is enabled but user doesn't have allowed role`, async function () {
+  it(`should return with 403 when security is enabled but user doesn't have the allowed role`, async function () {
     mockCore.getPluginSetupDeps = () =>
       (({
         // @ts-ignore
         ...mockCore.pluginSetupDeps,
         security: {
+          license: { isEnabled: () => true },
           authc: { getCurrentUser: () => ({ username: 'friendlyuser', roles: ['cowboy'] }) },
         },
       } as unknown) as ReportingInternalSetup);
@@ -113,6 +139,7 @@ describe('authorized_user_pre_routing', function () {
         // @ts-ignore
         ...mockCore.pluginSetupDeps,
         security: {
+          license: { isEnabled: () => true },
           authc: {
             getCurrentUser: () => ({ username: 'friendlyuser', roles: ['reporting_user'] }),
           },

diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts
@@ -24,7 +24,7 @@ export const authorizedUserPreRoutingFactory = function authorizedUserPreRouting
   return <P, Q, B>(handler: RequestHandlerUser): RequestHandler<P, Q, B, RouteMethod> => {
     return (context, req, res) => {
       let user: ReportingUser = null;
-      if (setupDeps.security) {
+      if (setupDeps.security && setupDeps.security.license.isEnabled()) {
         // find the authenticated user, or null if security is not enabled
         user = getUser(req);
         if (!user) {