[Obs AI Assistant] Address PR comments (elastic#204884)

viduni94 · Jan 2, 2025 · df3715d · df3715d
1 parent a08f6a2
commit df3715d
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 151 deletions.
diff --git a/...less/api_integration/test_suites/observability/ai_assistant/common/forbidden_api_error.ts b/...less/api_integration/test_suites/observability/ai_assistant/common/forbidden_api_error.ts
diff --git a/...on/test_suites/observability/ai_assistant/common/observability_ai_assistant_api_client.ts b/...on/test_suites/observability/ai_assistant/common/observability_ai_assistant_api_client.ts
@@ -216,8 +216,8 @@ export async function getObservabilityAIAssistantApiClientService({
   // unauthorized user
   const supertestUnauthorizedWithCookieCredentials: SupertestWithRoleScope =
     await roleScopedSupertest.getSupertestWithRoleScope('viewer', {
-      useCookieHeader: true,
-      withInternalHeaders: false, // No internal headers for unauthorized users
+      useCookieHeader: false,
+      withInternalHeaders: true,
     });
 
   return {

diff --git a/...serverless/api_integration/test_suites/observability/ai_assistant/tests/chat/chat.spec.ts b/...serverless/api_integration/test_suites/observability/ai_assistant/tests/chat/chat.spec.ts
@@ -16,7 +16,6 @@ import { SupertestWithRoleScope } from '@kbn/test-suites-xpack/api_integration/d
 import { FtrProviderContext } from '../../common/ftr_provider_context';
 import { createProxyActionConnector, deleteActionConnector } from '../../common/action_connectors';
 import type { InternalRequestHeader, RoleCredentials } from '../../../../../../shared/services';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 export default function ApiTest({ getService }: FtrProviderContext) {
   const supertestWithoutAuth = getService('supertestWithoutAuth');
@@ -174,8 +173,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
     describe('security roles and access privileges', () => {
       it('should deny access for users without the ai_assistant privilege', async () => {
-        try {
-          await observabilityAIAssistantAPIClient.slsUnauthorized({
+        await observabilityAIAssistantAPIClient
+          .slsUnauthorized({
             endpoint: `POST ${CHAT_API_URL}`,
             params: {
               body: {
@@ -186,11 +185,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
                 scopes: ['all'],
               },
             },
-          });
-          throw new ForbiddenApiError('Expected slsUnauthorized() to throw a 403 Forbidden error');
-        } catch (e) {
-          expect(e.status).to.be(403);
-        }
+          })
+          .expect(403);
       });
     });
   });

diff --git a/...ss/api_integration/test_suites/observability/ai_assistant/tests/complete/complete.spec.ts b/...ss/api_integration/test_suites/observability/ai_assistant/tests/complete/complete.spec.ts
@@ -34,7 +34,6 @@ import {
 } from '../conversations/helpers';
 import { createProxyActionConnector, deleteActionConnector } from '../../common/action_connectors';
 import type { InternalRequestHeader, RoleCredentials } from '../../../../../../shared/services';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 export default function ApiTest({ getService }: FtrProviderContext) {
   const supertestWithoutAuth = getService('supertestWithoutAuth');
@@ -551,8 +550,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
     describe('security roles and access privileges', () => {
       it('should deny access for users without the ai_assistant privilege', async () => {
-        try {
-          await observabilityAIAssistantAPIClient.slsUnauthorized({
+        await observabilityAIAssistantAPIClient
+          .slsUnauthorized({
             endpoint: 'POST /internal/observability_ai_assistant/chat/complete',
             params: {
               body: {
@@ -563,11 +562,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
                 scopes: ['all'],
               },
             },
-          });
-          throw new ForbiddenApiError('Expected slsUnauthorized() to throw a 403 Forbidden error');
-        } catch (e) {
-          expect(e.status).to.be(403);
-        }
+          })
+          .expect(403);
       });
     });
   });

diff --git a/...pi_integration/test_suites/observability/ai_assistant/tests/connectors/connectors.spec.ts b/...pi_integration/test_suites/observability/ai_assistant/tests/connectors/connectors.spec.ts
@@ -13,7 +13,6 @@ import type {
   RoleCredentials,
   SupertestWithoutAuthProviderType,
 } from '../../../../../../shared/services';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 const CONNECTOR_API_URL = '/internal/observability_ai_assistant/connectors';
 
@@ -89,14 +88,11 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
     describe('security roles and access privileges', () => {
       it('should deny access for users without the ai_assistant privilege', async () => {
-        try {
-          await observabilityAIAssistantAPIClient.slsUnauthorized({
+        await observabilityAIAssistantAPIClient
+          .slsUnauthorized({
             endpoint: `GET ${CONNECTOR_API_URL}`,
-          });
-          throw new ForbiddenApiError('Expected slsUnauthorized() to throw a 403 Forbidden error');
-        } catch (e) {
-          expect(e.status).to.be(403);
-        }
+          })
+          .expect(403);
       });
     });
   });

diff --git a/...egration/test_suites/observability/ai_assistant/tests/conversations/conversations.spec.ts b/...egration/test_suites/observability/ai_assistant/tests/conversations/conversations.spec.ts
@@ -14,7 +14,6 @@ import {
 } from '@kbn/observability-ai-assistant-plugin/common/types';
 import type { FtrProviderContext } from '../../common/ftr_provider_context';
 import type { SupertestReturnType } from '../../common/observability_ai_assistant_api_client';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 export default function ApiTest({ getService }: FtrProviderContext) {
   const observabilityAIAssistantAPIClient = getService('observabilityAIAssistantAPIClient');
@@ -287,39 +286,29 @@ export default function ApiTest({ getService }: FtrProviderContext) {
         });
 
         it('POST /internal/observability_ai_assistant/conversation', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'POST /internal/observability_ai_assistant/conversation',
               params: {
                 body: {
                   conversation: conversationCreate,
                 },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('POST /internal/observability_ai_assistant/conversations', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'POST /internal/observability_ai_assistant/conversations',
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('PUT /internal/observability_ai_assistant/conversation/{conversationId}', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'PUT /internal/observability_ai_assistant/conversation/{conversationId}',
               params: {
                 path: {
@@ -331,49 +320,34 @@ export default function ApiTest({ getService }: FtrProviderContext) {
                   }),
                 },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('GET /internal/observability_ai_assistant/conversation/{conversationId}', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'GET /internal/observability_ai_assistant/conversation/{conversationId}',
               params: {
                 path: {
                   conversationId: createResponse.body.conversation.id,
                 },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('DELETE /internal/observability_ai_assistant/conversation/{conversationId}', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'DELETE /internal/observability_ai_assistant/conversation/{conversationId}',
               params: {
                 path: {
                   conversationId: createResponse.body.conversation.id,
                 },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
       });
     });

diff --git a/...ration/test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base.spec.ts b/...ration/test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base.spec.ts
@@ -15,7 +15,6 @@ import {
 } from '@kbn/test-suites-xpack/observability_ai_assistant_api_integration/tests/knowledge_base/helpers';
 import { type KnowledgeBaseEntry } from '@kbn/observability-ai-assistant-plugin/common';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 export default function ApiTest({ getService }: FtrProviderContext) {
   const ml = getService('ml');
@@ -217,8 +216,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
     describe('security roles and access privileges', () => {
       describe('should deny access for users without the ai_assistant privilege', () => {
         it('POST /internal/observability_ai_assistant/kb/entries/save', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'POST /internal/observability_ai_assistant/kb/entries/save',
               params: {
                 body: {
@@ -227,45 +226,30 @@ export default function ApiTest({ getService }: FtrProviderContext) {
                   text: 'My content',
                 },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected unauthorizedUser() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('GET /internal/observability_ai_assistant/kb/entries', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'GET /internal/observability_ai_assistant/kb/entries',
               params: {
                 query: { query: '', sortBy: 'title', sortDirection: 'asc' },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
 
         it('DELETE /internal/observability_ai_assistant/kb/entries/{entryId}', async () => {
-          try {
-            await observabilityAIAssistantAPIClient.slsUnauthorized({
+          await observabilityAIAssistantAPIClient
+            .slsUnauthorized({
               endpoint: 'DELETE /internal/observability_ai_assistant/kb/entries/{entryId}',
               params: {
                 path: { entryId: 'my-doc-id-1' },
               },
-            });
-            throw new ForbiddenApiError(
-              'Expected slsUnauthorized() to throw a 403 Forbidden error'
-            );
-          } catch (e) {
-            expect(e.status).to.be(403);
-          }
+            })
+            .expect(403);
         });
       });
     });

diff --git a/.../test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_setup.spec.ts b/.../test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_setup.spec.ts
@@ -14,7 +14,6 @@ import {
 } from '@kbn/test-suites-xpack/observability_ai_assistant_api_integration/tests/knowledge_base/helpers';
 
 import { FtrProviderContext } from '../../common/ftr_provider_context';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 
 export const KNOWLEDGE_BASE_SETUP_API_URL = '/internal/observability_ai_assistant/kb/setup';
 
@@ -72,19 +71,16 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
     describe('security roles and access privileges', () => {
       it('should deny access for users without the ai_assistant privilege', async () => {
-        try {
-          await observabilityAIAssistantAPIClient.slsUnauthorized({
+        await observabilityAIAssistantAPIClient
+          .slsUnauthorized({
             endpoint: `POST ${KNOWLEDGE_BASE_SETUP_API_URL}`,
             params: {
               query: {
                 model_id: TINY_ELSER.id,
               },
             },
-          });
-          throw new ForbiddenApiError('Expected slsUnauthorized() to throw a 403 Forbidden error');
-        } catch (e) {
-          expect(e.status).to.be(403);
-        }
+          })
+          .expect(403);
       });
     });
   });

diff --git a/...test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_status.spec.ts b/...test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_status.spec.ts
@@ -14,7 +14,6 @@ import {
 } from '@kbn/test-suites-xpack/observability_ai_assistant_api_integration/tests/knowledge_base/helpers';
 import { AI_ASSISTANT_KB_INFERENCE_ID } from '@kbn/observability-ai-assistant-plugin/server/service/inference_endpoint';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
-import { ForbiddenApiError } from '../../common/forbidden_api_error';
 import { KNOWLEDGE_BASE_SETUP_API_URL } from './knowledge_base_setup.spec';
 
 const KNOWLEDGE_BASE_STATUS_API_URL = '/internal/observability_ai_assistant/kb/status';
@@ -73,14 +72,11 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
     describe('security roles and access privileges', () => {
       it('should deny access for users without the ai_assistant privilege', async () => {
-        try {
-          await observabilityAIAssistantAPIClient.slsUnauthorized({
+        await observabilityAIAssistantAPIClient
+          .slsUnauthorized({
             endpoint: `GET ${KNOWLEDGE_BASE_STATUS_API_URL}`,
-          });
-          throw new ForbiddenApiError('Expected unauthorizedUser() to throw a 403 Forbidden error');
-        } catch (e) {
-          expect(e.status).to.be(403);
-        }
+          })
+          .expect(403);
       });
     });
   });