Removes unnecessary modifications required to solve this bug

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
opensearch-project · Apr 11, 2024 · d756b6c · d756b6c
1 parent 77608a0
commit d756b6c
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 111 deletions.
diff --git a/server/auth/types/authentication_type.ts b/server/auth/types/authentication_type.ts
@@ -31,7 +31,7 @@ import { SecuritySessionCookie } from '../../session/security_cookie';
 import { SecurityClient } from '../../backend/opensearch_security_client';
 import { resolveTenant, isValidTenant } from '../../multitenancy/tenant_resolver';
 import { UnauthenticatedError } from '../../errors';
-import { AuthType, GLOBAL_TENANT_SYMBOL } from '../../../common';
+import { GLOBAL_TENANT_SYMBOL } from '../../../common';
 
 export interface IAuthenticationType {
   type: string;
@@ -119,7 +119,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
       try {
         const additionalAuthHeader = await this.getAdditionalAuthHeader(request);
         Object.assign(authHeaders, additionalAuthHeader);
-        authInfo = await this.securityClient.authinfo(request, '', additionalAuthHeader);
+        authInfo = await this.securityClient.authinfo(request, additionalAuthHeader);
         cookie = this.getCookie(request, authInfo);
 
         // set tenant from cookie if exist
@@ -210,8 +210,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
       }
     }
     if (!authInfo) {
-      const authRequestType = cookie.isAnonymousAuth ? AuthType.ANONYMOUS : cookie.authType;
-      authInfo = await this.securityClient.authinfo(request, authRequestType, authHeaders);
+      authInfo = await this.securityClient.authinfo(request, authHeaders);
     }
     authState.authInfo = authInfo;
 
@@ -244,10 +243,9 @@ export abstract class AuthenticationType implements IAuthenticationType {
     authHeader: any,
     authInfo: any
   ): Promise<string | undefined> {
-    const authType = cookie.isAnonymousAuth ? AuthType.ANONYMOUS : cookie.authType;
     if (!authInfo) {
       try {
-        authInfo = await this.securityClient.authinfo(request, authType, authHeader);
+        authInfo = await this.securityClient.authinfo(request, authHeader);
       } catch (error: any) {
         throw new UnauthenticatedError(error);
       }

diff --git a/server/auth/types/basic/basic_auth.ts b/server/auth/types/basic/basic_auth.ts
@@ -30,7 +30,12 @@ import { BasicAuthRoutes } from './routes';
 import { AuthenticationType } from '../authentication_type';
 import { LOGIN_PAGE_URI } from '../../../../common';
 import { composeNextUrlQueryParam } from '../../../utils/next_url';
-import { AUTH_HEADER_NAME, AuthType, OPENDISTRO_SECURITY_ANONYMOUS } from '../../../../common';
+import {
+  ANONYMOUS_AUTH_LOGIN,
+  AUTH_HEADER_NAME,
+  AuthType,
+  OPENDISTRO_SECURITY_ANONYMOUS,
+} from '../../../../common';
 
 export class BasicAuthentication extends AuthenticationType {
   public readonly type: string = AuthType.BASIC;
@@ -111,13 +116,21 @@ export class BasicAuthentication extends AuthenticationType {
         request,
         this.coreSetup.http.basePath.serverBasePath
       );
-
-      const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`;
-      return response.redirected({
-        headers: {
-          location: `${redirectLocation}`,
-        },
-      });
+      if (this.config.auth.anonymous_auth_enabled) {
+        const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${ANONYMOUS_AUTH_LOGIN}?${nextUrlParam}`;
+        return response.redirected({
+          headers: {
+            location: `${redirectLocation}`,
+          },
+        });
+      } else {
+        const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`;
+        return response.redirected({
+          headers: {
+            location: `${redirectLocation}`,
+          },
+        });
+      }
     } else {
       return response.unauthorized({
         body: `Authentication required`,

diff --git a/server/auth/types/basic/routes.ts b/server/auth/types/basic/routes.ts
@@ -186,11 +186,7 @@ export class BasicAuthRoutes {
           }
           context.security_plugin.logger.info('The Redirect Path is ' + redirectUrl);
           try {
-            user = await this.securityClient.authenticateWithHeaders(
-              request,
-              AuthType.ANONYMOUS,
-              {}
-            );
+            user = await this.securityClient.authenticateWithHeaders(request, {});
           } catch (error) {
             context.security_plugin.logger.error(
               `Failed authentication: ${error}. Redirecting to Login Page`

diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts
@@ -187,8 +187,7 @@ export class OpenIdAuthRoutes {
           const user = await this.securityClient.authenticateWithHeader(
             request,
             this.openIdAuthConfig.authHeaderName as string,
-            `Bearer ${tokenResponse.idToken}`,
-            AuthType.OPEN_ID
+            `Bearer ${tokenResponse.idToken}`
           );
 
           // set to cookie

diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts
@@ -219,8 +219,7 @@ export class SamlAuthRoutes {
           const user = await this.securityClient.authenticateWithHeader(
             request,
             'authorization',
-            credentials.authorization,
-            AuthType.SAML
+            credentials.authorization
           );
 
           let expiryTime = Date.now() + this.config.session.ttl;

diff --git a/server/backend/opensearch_security_client.ts b/server/backend/opensearch_security_client.ts
@@ -53,7 +53,6 @@ export class SecurityClient {
     request: OpenSearchDashboardsRequest,
     headerName: string,
     headerValue: string,
-    authRequestType: string,
     whitelistedHeadersAndValues: any = {},
     additionalAuthHeaders: any = {}
   ): Promise<User> {
@@ -72,7 +71,6 @@ export class SecurityClient {
       const esResponse = await this.esClient
         .asScoped(request)
         .callAsCurrentUser('opensearch_security.authinfo', {
-          [AUTH_TYPE_PARAM]: authRequestType,
           headers,
         });
       return {
@@ -90,14 +88,12 @@ export class SecurityClient {
 
   public async authenticateWithHeaders(
     request: OpenSearchDashboardsRequest,
-    authRequestType: string,
     additionalAuthHeaders: any = {}
   ): Promise<User> {
     try {
       const esResponse = await this.esClient
         .asScoped(request)
         .callAsCurrentUser('opensearch_security.authinfo', {
-          [AUTH_TYPE_PARAM]: authRequestType,
           headers: additionalAuthHeaders,
         });
       return {
@@ -112,16 +108,11 @@ export class SecurityClient {
     }
   }
 
-  public async authinfo(
-    request: OpenSearchDashboardsRequest,
-    authRequestType: string = '',
-    headers: any = {}
-  ) {
+  public async authinfo(request: OpenSearchDashboardsRequest, headers: any = {}) {
     try {
       return await this.esClient
         .asScoped(request)
         .callAsCurrentUser('opensearch_security.authinfo', {
-          [AUTH_TYPE_PARAM]: authRequestType,
           headers,
         });
     } catch (error: any) {

diff --git a/server/backend/opensearch_security_plugin.ts b/server/backend/opensearch_security_plugin.ts
@@ -13,9 +13,6 @@
  *   permissions and limitations under the License.
  */
 
-import { AUTH_TYPE_PARAM } from '../../common';
-import { addQueryParamsToURLIfAny } from './utils';
-
 // eslint-disable-next-line import/no-default-export
 export default function (Client: any, config: any, components: any) {
   const ca = components.clientAction.factory;
@@ -29,14 +26,7 @@ export default function (Client: any, config: any, components: any) {
    */
   Client.prototype.opensearch_security.prototype.authinfo = ca({
     url: {
-      fmt: `/_plugins/_security/authinfo`,
-      opt: {
-        [AUTH_TYPE_PARAM]: {
-          type: 'string',
-          required: false,
-        },
-      },
-      template: (params) => addQueryParamsToURLIfAny(params, '/_plugins/_security/authinfo'),
+      fmt: '/_plugins/_security/authinfo',
     },
   });
 

diff --git a/server/backend/test/utils.test.ts b/server/backend/test/utils.test.ts
diff --git a/server/backend/utils.ts b/server/backend/utils.ts
diff --git a/server/readonly/readonly_service.ts b/server/readonly/readonly_service.ts
@@ -24,7 +24,6 @@ import {
   isPrivateTenant,
   LOGIN_PAGE_URI,
   CUSTOM_ERROR_PAGE_URI,
-  AuthType,
 } from '../../common';
 import { SecurityClient } from '../backend/opensearch_security_client';
 import { IAuthenticationType, OpenSearchAuthInfo } from '../auth/types/authentication_type';
@@ -105,12 +104,7 @@ export class ReadonlyService extends BaseReadonlyService {
         return false;
       }
 
-      const authType = cookie
-        ? cookie.isAnonymousAuth
-          ? AuthType.ANONYMOUS
-          : cookie.authType
-        : '';
-      const authInfo = await this.securityClient.authinfo(request, authType, headers);
+      const authInfo = await this.securityClient.authinfo(request, headers);
 
       if (!authInfo.user_requested_tenant && cookie) {
         authInfo.user_requested_tenant = cookie.tenant;

diff --git a/test/jest_integration/basic_auth.test.ts b/test/jest_integration/basic_auth.test.ts
@@ -227,11 +227,16 @@ describe('start OpenSearch Dashboards server', () => {
       .unset(AUTHORIZATION_HEADER_NAME);
 
     expect(response.status).toEqual(302);
-    expect(response.header.location).toEqual('/app/login?nextUrl=%2Fapp%2Fhome');
+    expect(response.header.location).toEqual('/auth/anonymous?nextUrl=%2Fapp%2Fhome');
 
     const response2 = await osdTestServer.request.get(root, response.header.location);
 
-    expect(response2.status).toEqual(200);
+    expect(response2.status).toEqual(302);
+    expect(response2.header.location).toEqual('/app/login?nextUrl=%2Fapp%2Fhome');
+
+    const response3 = await osdTestServer.request.get(root, response2.header.location);
+
+    expect(response3.status).toEqual(200);
   });
 
   it('redirect for home follows login for anonymous auth disabled', async () => {
@@ -260,10 +265,14 @@ describe('start OpenSearch Dashboards server', () => {
       .unset(AUTHORIZATION_HEADER_NAME);
 
     expect(response.status).toEqual(302);
-    expect(response.header.location).toEqual(expectedPath);
 
     const response2 = await osdTestServer.request.get(root, response.header.location);
 
-    expect(response2.status).toEqual(200);
+    expect(response2.status).toEqual(302);
+    expect(response2.header.location).toEqual(expectedPath);
+
+    const response3 = await osdTestServer.request.get(root, response2.header.location);
+
+    expect(response3.status).toEqual(200);
   });
 });