[Refactor-1160] Different Values Pointing to Basic Auth, Need to Unify (

#1619) * finished ticket Signed-off-by: Prabhas Kurapati <[email protected]> * fixed invalid auth type error Signed-off-by: Prabhas Kurapati <[email protected]> * fixed unit tests Signed-off-by: Prabhas Kurapati <[email protected]> * made requested changes Signed-off-by: Prabhas Kurapati <[email protected]> * updated saml to AuthType.SAML + fixed basicauth test Signed-off-by: Prabhas Kurapati <[email protected]> --------- Signed-off-by: Prabhas Kurapati <[email protected]>
opensearch-project · Nov 13, 2023 · 2131598 · 2131598
1 parent d14bb68
commit 2131598
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 51 deletions.
diff --git a/public/apps/account/test/log-out-button.test.tsx b/public/apps/account/test/log-out-button.test.tsx
@@ -17,19 +17,13 @@ import { shallow } from 'enzyme';
 import React from 'react';
 import { LogoutButton } from '../log-out-button';
 import { logout } from '../utils';
+import { AuthType } from '../../../../common';
 
 jest.mock('../utils', () => ({
   logout: jest.fn(),
 }));
 
 describe('Account menu - Log out button', () => {
-  enum authType {
-    OpenId = 'openid',
-    SAML = 'saml',
-    Proxy = 'proxy',
-    Basic = 'basicauth',
-  }
-
   const mockHttpStart = {
     basePath: {
       serverBasePath: '',
@@ -39,42 +33,42 @@ describe('Account menu - Log out button', () => {
   describe('renders', () => {
     it('renders when auth type is MultiAuth: openid', () => {
       const component = shallow(
-        <LogoutButton authType={authType.OpenId} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.OPEN_ID} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });
 
     it('renders when auth type is MultiAuth: saml', () => {
       const component = shallow(
-        <LogoutButton authType={authType.SAML} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.SAML} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });
 
     it('renders when auth type is MultiAuth: basicauth', () => {
       const component = shallow(
-        <LogoutButton authType={authType.Basic} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.BASIC} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });
 
     it('renders when auth type is OpenId', () => {
       const component = shallow(
-        <LogoutButton authType={authType.OpenId} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.OPEN_ID} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });
 
     it('renders when auth type is SAML', () => {
       const component = shallow(
-        <LogoutButton authType={authType.SAML} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.SAML} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });
 
     it('renders when auth type is Proxy', () => {
       const component = shallow(
-        <LogoutButton authType={authType.Proxy} http={mockHttpStart} divider={mockDivider} />
+        <LogoutButton authType={AuthType.PROXY} http={mockHttpStart} divider={mockDivider} />
       );
       expect(component).toMatchSnapshot();
     });

diff --git a/public/apps/customerror/custom-error.tsx b/public/apps/customerror/custom-error.tsx
@@ -20,6 +20,7 @@ import ReactDOM from 'react-dom';
 import { Router, Route } from 'react-router-dom';
 import { ERROR_MISSING_ROLE_PATH } from '../../../common';
 import { ClientConfigType } from '../../types';
+import { AuthType } from '../../../common';
 import './_index.scss';
 import { logout } from '../account/utils';
 
@@ -28,7 +29,7 @@ interface CustomErrorDeps {
   subtitle: string;
   http: CoreStart['http'];
   chrome: CoreStart['chrome'];
-  config: ClientConfigType['ui']['basicauth']['login'];
+  config: ClientConfigType['ui'][AuthType.BASIC]['login'];
 }
 
 export function CustomErrorPage(props: CustomErrorDeps) {

diff --git a/public/apps/login/test/login-page.test.tsx b/public/apps/login/test/login-page.test.tsx
@@ -20,6 +20,7 @@ import { LoginPage, extractNextUrlFromWindowLocation } from '../login-page';
 import { validateCurrentPassword } from '../../../utils/login-utils';
 import { API_AUTH_LOGOUT } from '../../../../common';
 import { chromeServiceMock } from '../../../../../../src/core/public/mocks';
+import { AuthType } from '../../../../common';
 
 jest.mock('../../../utils/login-utils', () => ({
   validateCurrentPassword: jest.fn(),
@@ -101,7 +102,7 @@ describe('Login page', () => {
       const config: ClientConfigType = {
         ui: configUI,
         auth: {
-          type: ['basicauth'],
+          type: [AuthType.BASIC],
           logout_url: API_AUTH_LOGOUT,
         },
       };
@@ -115,7 +116,7 @@ describe('Login page', () => {
       const config: ClientConfigType = {
         ui: configUI,
         auth: {
-          type: 'basicauth',
+          type: AuthType.BASIC,
           logout_url: API_AUTH_LOGOUT,
         },
       };
@@ -129,7 +130,7 @@ describe('Login page', () => {
       const config: ClientConfigType = {
         ui: configUI,
         auth: {
-          type: ['basicauth', 'openid', 'saml'],
+          type: [AuthType.BASIC, 'openid', AuthType.SAML],
           logout_url: API_AUTH_LOGOUT,
         },
       };
@@ -173,7 +174,7 @@ describe('Login page', () => {
     const config: ClientConfigType = {
       ui: configUiDefault,
       auth: {
-        type: 'basicauth',
+        type: AuthType.BASIC,
       },
     };
     beforeEach(() => {
@@ -207,7 +208,7 @@ describe('Login page', () => {
     const config: ClientConfigType = {
       ui: configUiDefault,
       auth: {
-        type: 'basicauth',
+        type: AuthType.BASIC,
       },
     };
     beforeEach(() => {

diff --git a/server/auth/auth_handler_factory.test.ts b/server/auth/auth_handler_factory.test.ts
@@ -23,13 +23,17 @@ import {
 import { SecurityPluginConfigType } from '..';
 import { SecuritySessionCookie } from '../session/security_cookie';
 import { getAuthenticationHandler } from './auth_handler_factory';
+import { AuthType } from '../../common';
+
+const mockBasicAuthType = AuthType.BASIC;
+const mockSAMLAuthType = AuthType.SAML;
 
 jest.mock('./types', () => {
   return {
     BasicAuthentication: jest.fn().mockImplementation(() => {
       return {
         authHandler: () => {},
-        type: 'basicauth',
+        type: mockBasicAuthType,
         init: () => {},
       };
     }),
@@ -57,14 +61,14 @@ jest.mock('./types', () => {
     SamlAuthentication: jest.fn().mockImplementation(() => {
       return {
         authHandler: () => {},
-        type: 'saml',
+        type: mockSAMLAuthType,
         init: () => {},
       };
     }),
     MultipleAuthentication: jest.fn().mockImplementation(() => {
       return {
         authHandler: () => {},
-        type: ['openid', 'saml', 'basiauth'],
+        type: ['openid', mockSAMLAuthType, mockBasicAuthType],
         init: () => {},
       };
     }),
@@ -83,54 +87,54 @@ describe('test authentication factory', () => {
 
   test('get basic auth: string array', async () => {
     const auth = await getAuthenticationHandler(
-      ['basicauth'],
+      [AuthType.BASIC],
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('basicauth');
+    expect(auth.type).toEqual(AuthType.BASIC);
   });
 
   test('get basic auth: string', async () => {
     const auth = await getAuthenticationHandler(
-      'basicauth',
+      AuthType.BASIC,
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('basicauth');
+    expect(auth.type).toEqual(AuthType.BASIC);
   });
 
-  test('get basic auth with empty auth type: string array', async () => {
+  test('get basic auth with empty auth type: string', async () => {
     const auth = await getAuthenticationHandler(
-      [''],
+      '',
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('basicauth');
+    expect(auth.type).toEqual(AuthType.BASIC);
   });
 
-  test('get basic auth with empty auth type: string', async () => {
+  test('get basic auth with empty auth type: string array', async () => {
     const auth = await getAuthenticationHandler(
-      '',
+      [''],
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('basicauth');
+    expect(auth.type).toEqual(AuthType.BASIC);
   });
 
   test('get jwt auth: string array', async () => {
@@ -213,28 +217,28 @@ describe('test authentication factory', () => {
 
   test('get saml auth: string array', async () => {
     const auth = await getAuthenticationHandler(
-      ['saml'],
+      [AuthType.SAML],
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('saml');
+    expect(auth.type).toEqual(AuthType.SAML);
   });
 
   test('get saml auth: string', async () => {
     const auth = await getAuthenticationHandler(
-      'saml',
+      AuthType.SAML,
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual('saml');
+    expect(auth.type).toEqual(AuthType.SAML);
   });
 
   test('multiple_auth_enabled is on, get multi auth', async () => {
@@ -244,15 +248,15 @@ describe('test authentication factory', () => {
       },
     };
     const auth = await getAuthenticationHandler(
-      ['openid', 'saml', 'basiauth'],
+      ['openid', AuthType.SAML, AuthType.BASIC],
       router,
       config,
       core,
       esClient,
       sessionStorageFactory,
       logger
     );
-    expect(auth.type).toEqual(['openid', 'saml', 'basiauth']);
+    expect(auth.type).toEqual(['openid', AuthType.SAML, AuthType.BASIC]);
   });
 
   test('multiple_auth_enabled is off, get multi auth', async () => {
@@ -263,7 +267,7 @@ describe('test authentication factory', () => {
     };
     try {
       await getAuthenticationHandler(
-        ['openid', 'saml', 'basiauth'],
+        ['openid', AuthType.SAML, AuthType.BASIC],
         router,
         config,
         core,

diff --git a/server/auth/types/basic/routes.ts b/server/auth/types/basic/routes.ts
@@ -30,6 +30,7 @@ import {
 } from '../../../../common';
 import { resolveTenant } from '../../../multitenancy/tenant_resolver';
 import { encodeUriQuery } from '../../../../../../src/plugins/opensearch_dashboards_utils/common/url/encode_uri_query';
+import { AuthType } from '../../../../common';
 
 export class BasicAuthRoutes {
   constructor(
@@ -112,7 +113,7 @@ export class BasicAuthRoutes {
           credentials: {
             authHeaderValue: `Basic ${encodedCredentials}`,
           },
-          authType: 'basicauth',
+          authType: AuthType.BASIC,
           isAnonymousAuth: false,
           expiryTime: Date.now() + this.config.session.ttl,
         };
@@ -202,7 +203,7 @@ export class BasicAuthRoutes {
           this.sessionStorageFactory.asScoped(request).clear();
           const sessionStorage: SecuritySessionCookie = {
             username: user.username,
-            authType: 'basicauth',
+            authType: AuthType.BASIC,
             isAnonymousAuth: true,
             expiryTime: Date.now() + this.config.session.ttl,
           };

diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts
@@ -45,7 +45,7 @@ import {
 export class SamlAuthentication extends AuthenticationType {
   public static readonly AUTH_HEADER_NAME = 'authorization';
 
-  public readonly type: string = 'saml';
+  public readonly type: string = AuthType.SAML;
 
   constructor(
     config: SecurityPluginConfigType,

diff --git a/server/index.ts b/server/index.ts
@@ -16,14 +16,15 @@
 import { schema, TypeOf } from '@osd/config-schema';
 import { PluginInitializerContext, PluginConfigDescriptor } from '../../../src/core/server';
 import { SecurityPlugin } from './plugin';
+import { AuthType } from '../common';
 
 const validateAuthType = (value: string[]) => {
   const supportedAuthTypes = [
     '',
-    'basicauth',
+    AuthType.BASIC,
     'jwt',
     'openid',
-    'saml',
+    AuthType.SAML,
     'proxy',
     'kerberos',
     'proxycache',
@@ -88,7 +89,7 @@ export const configSchema = schema.object({
 
             if (value.length > 1) {
               const includeBasicAuth = value.find((element) => {
-                return element.toLowerCase() === 'basicauth';
+                return element.toLowerCase() === AuthType.BASIC;
               });
 
               if (!includeBasicAuth) {

diff --git a/server/routes/auth_type_routes.ts b/server/routes/auth_type_routes.ts
@@ -15,7 +15,7 @@
 
 import { IRouter } from 'opensearch-dashboards/server';
 import { SecurityPluginConfigType } from '..';
-
+import { AuthType } from '../../common';
 export function defineAuthTypeRoutes(router: IRouter, config: SecurityPluginConfigType) {
   /**
    * Auth type API that returns current auth type configured on OpenSearchDashboards Server.
@@ -30,7 +30,7 @@ export function defineAuthTypeRoutes(router: IRouter, config: SecurityPluginConf
   router.get(
     { path: '/api/authtype', validate: false, options: { authRequired: false } },
     async (context, request, response) => {
-      const authType = config.auth.type || 'basicauth';
+      const authType = config.auth.type || AuthType.BASIC;
       return response.ok({
         body: {
           authtype: authType,