feat: optimize test cases

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
wanglam · Mar 5, 2024 · 60b3b02 · 60b3b02
1 parent f16f1cc
commit 60b3b02
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 19 deletions.
diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts
@@ -8,6 +8,7 @@ import { httpServerMock, httpServiceMock } from '../../../core/server/mocks';
 import { generateRandomId, getPrincipalsFromRequest } from './utils';
 
 describe('workspace utils', () => {
+  const mockAuth = httpServiceMock.createAuth();
   it('should generate id with the specified size', () => {
     expect(generateRandomId(6)).toHaveLength(6);
   });
@@ -23,13 +24,19 @@ describe('workspace utils', () => {
 
   it('should return empty map when request do not have authentication', () => {
     const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
-    const result = getPrincipalsFromRequest(mockRequest);
+    mockAuth.get.mockReturnValueOnce({
+      status: AuthStatus.unknown,
+      state: {
+        user_name: 'bar',
+        backend_roles: ['foo'],
+      },
+    });
+    const result = getPrincipalsFromRequest(mockRequest, mockAuth);
     expect(result).toEqual({});
   });
 
   it('should return normally when request has authentication', () => {
     const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
-    const mockAuth = httpServiceMock.createAuth();
     mockAuth.get.mockReturnValueOnce({
       status: AuthStatus.authenticated,
       state: {
@@ -42,15 +49,24 @@ describe('workspace utils', () => {
     expect(result.groups).toEqual(['foo']);
   });
 
-  it('should return a fake user when there is auth field but no backend_roles or user name', () => {
+  it('should throw error when request is not authenticated', () => {
     const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
-    const mockAuth = httpServiceMock.createAuth();
     mockAuth.get.mockReturnValueOnce({
       status: AuthStatus.unauthenticated,
       state: {},
     });
-    const result = getPrincipalsFromRequest(mockRequest, mockAuth);
-    expect(result.users?.[0].startsWith('_user_fake_')).toEqual(true);
-    expect(result.groups).toEqual(undefined);
+    expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow('NOT_AUTHORIZED');
+  });
+
+  it('should throw error when authentication status is not expected', () => {
+    const mockRequest = httpServerMock.createOpenSearchDashboardsRequest();
+    mockAuth.get.mockReturnValueOnce({
+      // @ts-ignore
+      status: 'foo',
+      state: {},
+    });
+    expect(() => getPrincipalsFromRequest(mockRequest, mockAuth)).toThrow(
+      'UNEXPECTED_AUTHORIZATION_STATUS'
+    );
   });
 });
diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts
@@ -33,19 +33,20 @@ export const getPrincipalsFromRequest = (
     return payload;
   }
 
-  if (authInfoResp?.status === AuthStatus.unauthenticated) {
-    /**
-     * use a fake user that won't be granted permission explicitly when authenticated error.
-     */
-    payload[PrincipalType.Users] = [`_user_fake_${Date.now()}_`];
+  if (authInfoResp?.status === AuthStatus.authenticated) {
+    const authInfo = authInfoResp?.state as AuthInfo | null;
+    if (authInfo?.backend_roles) {
+      payload[PrincipalType.Groups] = authInfo.backend_roles;
+    }
+    if (authInfo?.user_name) {
+      payload[PrincipalType.Users] = [authInfo.user_name];
+    }
     return payload;
   }
-  const authInfo = authInfoResp?.state as AuthInfo | null;
-  if (authInfo?.backend_roles) {
-    payload[PrincipalType.Groups] = authInfo.backend_roles;
-  }
-  if (authInfo?.user_name) {
-    payload[PrincipalType.Users] = [authInfo.user_name];
+
+  if (authInfoResp?.status === AuthStatus.unauthenticated) {
+    throw new Error('NOT_AUTHORIZED');
   }
-  return payload;
+
+  throw new Error('UNEXPECTED_AUTHORIZATION_STATUS');
 };