Addressing pr feedback

elastic · May 3, 2021 · 9fae3d9 · 9fae3d9
1 parent fbf5784
commit 9fae3d9
Show file tree

Hide file tree

Showing 21 changed files with 66 additions and 61 deletions.
diff --git a/x-pack/plugins/cases/common/api/cases/case.ts b/x-pack/plugins/cases/common/api/cases/case.ts
@@ -13,7 +13,6 @@ import { CommentResponseRt } from './comment';
 import { CasesStatusResponseRt, CaseStatusRt } from './status';
 import { CaseConnectorRt, ESCaseConnector } from '../connectors';
 import { SubCaseResponseRt } from './sub_case';
-import { OWNER_FIELD } from './constants';
 
 export enum CaseType {
   collection = 'collection',
@@ -39,7 +38,7 @@ const CaseBasicRt = rt.type({
   [caseTypeField]: CaseTypeRt,
   connector: CaseConnectorRt,
   settings: SettingsRt,
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 const CaseExternalServiceBasicRt = rt.type({
@@ -80,7 +79,7 @@ const CasePostRequestNoTypeRt = rt.type({
   title: rt.string,
   connector: CaseConnectorRt,
   settings: SettingsRt,
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 /**
@@ -115,7 +114,7 @@ export const CasesFindRequestRt = rt.partial({
   searchFields: rt.union([rt.array(rt.string), rt.string]),
   sortField: rt.string,
   sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]),
-  [OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
+  owner: rt.union([rt.array(rt.string), rt.string]),
 });
 
 export const CaseResponseRt = rt.intersection([
@@ -177,7 +176,7 @@ export const ExternalServiceResponseRt = rt.intersection([
 ]);
 
 export const AllTagsFindRequestRt = rt.partial({
-  [OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
+  owner: rt.union([rt.array(rt.string), rt.string]),
 });
 
 export const AllReportersFindRequestRt = AllTagsFindRequestRt;

diff --git a/x-pack/plugins/cases/common/api/cases/comment.ts b/x-pack/plugins/cases/common/api/cases/comment.ts
@@ -6,7 +6,6 @@
  */
 
 import * as rt from 'io-ts';
-import { OWNER_FIELD } from './constants';
 import { SavedObjectFindOptionsRt } from '../saved_object';
 
 import { UserRT } from '../user';
@@ -29,7 +28,7 @@ export const CommentAttributesBasicRt = rt.type({
   ]),
   created_at: rt.string,
   created_by: UserRT,
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
   pushed_at: rt.union([rt.string, rt.null]),
   pushed_by: rt.union([UserRT, rt.null]),
   updated_at: rt.union([rt.string, rt.null]),
@@ -45,7 +44,7 @@ export enum CommentType {
 export const ContextTypeUserRt = rt.type({
   comment: rt.string,
   type: rt.literal(CommentType.user),
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 /**
@@ -61,7 +60,7 @@ export const AlertCommentRequestRt = rt.type({
     id: rt.union([rt.string, rt.null]),
     name: rt.union([rt.string, rt.null]),
   }),
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 const AttributesTypeUserRt = rt.intersection([ContextTypeUserRt, CommentAttributesBasicRt]);

diff --git a/x-pack/plugins/cases/common/api/cases/configure.ts b/x-pack/plugins/cases/common/api/cases/configure.ts
@@ -18,7 +18,7 @@ const ClosureTypeRT = rt.union([rt.literal('close-by-user'), rt.literal('close-b
 const CasesConfigureBasicRt = rt.type({
   connector: CaseConnectorRt,
   closure_type: ClosureTypeRT,
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 const CasesConfigureBasicWithoutOwnerRt = rt.type(
@@ -48,12 +48,12 @@ export const CaseConfigureResponseRt = rt.intersection([
     id: rt.string,
     version: rt.string,
     error: rt.union([rt.string, rt.null]),
-    [OWNER_FIELD]: rt.string,
+    owner: rt.string,
   }),
 ]);
 
 export const GetConfigureFindRequestRt = rt.partial({
-  [OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
+  owner: rt.union([rt.array(rt.string), rt.string]),
 });
 
 export const CaseConfigureRequestParamsRt = rt.type({

diff --git a/x-pack/plugins/cases/common/api/cases/sub_case.ts b/x-pack/plugins/cases/common/api/cases/sub_case.ts
@@ -10,7 +10,6 @@ import * as rt from 'io-ts';
 import { NumberFromString } from '../saved_object';
 import { UserRT } from '../user';
 import { CommentResponseRt } from './comment';
-import { OWNER_FIELD } from './constants';
 import { CasesStatusResponseRt } from './status';
 import { CaseStatusRt } from './status';
 
@@ -27,7 +26,7 @@ export const SubCaseAttributesRt = rt.intersection([
     created_by: rt.union([UserRT, rt.null]),
     updated_at: rt.union([rt.string, rt.null]),
     updated_by: rt.union([UserRT, rt.null]),
-    [OWNER_FIELD]: rt.string,
+    owner: rt.string,
   }),
 ]);
 

diff --git a/x-pack/plugins/cases/common/api/cases/user_actions.ts b/x-pack/plugins/cases/common/api/cases/user_actions.ts
@@ -42,7 +42,7 @@ const CaseUserActionBasicRT = rt.type({
   action_by: UserRT,
   new_value: rt.union([rt.string, rt.null]),
   old_value: rt.union([rt.string, rt.null]),
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 const CaseUserActionResponseRT = rt.intersection([

diff --git a/x-pack/plugins/cases/common/api/connectors/mappings.ts b/x-pack/plugins/cases/common/api/connectors/mappings.ts
@@ -6,7 +6,6 @@
  */
 
 import * as rt from 'io-ts';
-import { OWNER_FIELD } from '../cases/constants';
 
 const ActionTypeRT = rt.union([
   rt.literal('append'),
@@ -32,7 +31,7 @@ export const ConnectorMappingsAttributesRT = rt.type({
 
 export const ConnectorMappingsRt = rt.type({
   mappings: rt.array(ConnectorMappingsAttributesRT),
-  [OWNER_FIELD]: rt.string,
+  owner: rt.string,
 });
 
 export type ConnectorMappingsAttributes = rt.TypeOf<typeof ConnectorMappingsAttributesRT>;

diff --git a/x-pack/plugins/cases/server/authorization/utils.ts b/x-pack/plugins/cases/server/authorization/utils.ts
@@ -13,7 +13,7 @@ export const getOwnersFilter = (savedObjectType: string, owners: string[]): Kuer
   return nodeBuilder.or(
     owners.reduce<KueryNode[]>((query, owner) => {
       ensureFieldIsSafeForQuery(OWNER_FIELD, owner);
-      query.push(nodeBuilder.is(`${savedObjectType}.attributes.owner`, owner));
+      query.push(nodeBuilder.is(`${savedObjectType}.attributes.${OWNER_FIELD}`, owner));
       return query;
     }, [])
   );

diff --git a/x-pack/plugins/cases/server/client/cases/push.ts b/x-pack/plugins/cases/server/client/cases/push.ts
@@ -90,7 +90,7 @@ export const push = async (
     // We need to change the logic when we support subcases
     if (theCase?.status === CaseStatuses.closed) {
       throw Boom.conflict(
-        `This case ${theCase.title} is closed. You can not pushed if the case is closed.`
+        `The ${theCase.title} case is closed. Pushing a closed case is not allowed.`
       );
     }
 

diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts
@@ -358,16 +358,11 @@ function partitionPatchRequest(
   conflictedCases: CasePatchRequest[];
   casesToAuthorize: Array<SavedObject<ESCaseAttributes>>;
 } {
-  const reqCasesMap = patchReqCases.reduce((acc, req) => {
-    acc.set(req.id, req);
-    return acc;
-  }, new Map<string, CasePatchRequest>());
-
   const nonExistingCases: CasePatchRequest[] = [];
   const conflictedCases: CasePatchRequest[] = [];
   const casesToAuthorize: Array<SavedObject<ESCaseAttributes>> = [];
 
-  for (const reqCase of reqCasesMap.values()) {
+  for (const reqCase of patchReqCases) {
     const foundCase = casesMap.get(reqCase.id);
 
     if (!foundCase || foundCase.error) {

diff --git a/x-pack/test/case_api_integration/common/lib/authentication/index.ts b/x-pack/test/case_api_integration/common/lib/authentication/index.ts
@@ -7,7 +7,7 @@
 
 import { FtrProviderContext as CommonFtrProviderContext } from '../../../common/ftr_provider_context';
 import { Role, User, UserInfo } from './types';
-import { users } from './users';
+import { superUser, users } from './users';
 import { roles } from './roles';
 import { spaces } from './spaces';
 
@@ -90,3 +90,5 @@ export const deleteSpacesAndUsers = async (getService: CommonFtrProviderContext[
   await deleteSpaces(getService);
   await deleteUsersAndRoles(getService);
 };
+
+export const superUserSpace1Auth = { user: superUser, space: 'space1' };
diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts
@@ -39,6 +39,8 @@ import {
   superUser,
 } from '../../../../common/lib/authentication/users';
 
+import { superUserSpace1Auth } from '../../../../common/lib/authentication';
+
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext): void => {
   const supertestWithoutAuth = getService('supertestWithoutAuth');
@@ -238,14 +240,14 @@ export default ({ getService }: FtrProviderContext): void => {
           supertest: supertestWithoutAuth,
           caseId: caseSec.id,
           expectedHttpCode: 200,
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
         });
 
         await getCase({
           supertest: supertestWithoutAuth,
           caseId: caseObs.id,
           expectedHttpCode: 200,
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
         });
       });
 

diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts
@@ -58,6 +58,7 @@ import {
   secOnlyRead,
   superUser,
 } from '../../../../common/lib/authentication/users';
+import { superUserSpace1Auth } from '../../../../common/lib/authentication';
 
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext): void => {
@@ -1169,7 +1170,7 @@ export default ({ getService }: FtrProviderContext): void => {
           expectedHttpCode: 403,
         });
 
-        const resp = await findCases({ supertest, auth: { user: superUser, space: 'space1' } });
+        const resp = await findCases({ supertest, auth: superUserSpace1Auth });
         expect(resp.cases.length).to.eql(3);
         // the update should have failed and none of the title should have been changed
         expect(resp.cases[0].title).to.eql(postCaseReq.title);

diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/status/get_status.ts
@@ -25,6 +25,7 @@ import {
   secOnlyRead,
   superUser,
 } from '../../../../../common/lib/authentication/users';
+import { superUserSpace1Auth } from '../../../../../common/lib/authentication';
 
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext): void => {
@@ -93,13 +94,13 @@ export default ({ getService }: FtrProviderContext): void => {
             supertestWithoutAuth,
             getPostCaseRequest({ owner: 'observabilityFixture' }),
             200,
-            { user: superUser, space: 'space1' }
+            superUserSpace1Auth
           ),
           createCase(
             supertestWithoutAuth,
             getPostCaseRequest({ owner: 'observabilityFixture' }),
             200,
-            { user: superUser, space: 'space1' }
+            superUserSpace1Auth
           ),
         ]);
 
@@ -124,7 +125,7 @@ export default ({ getService }: FtrProviderContext): void => {
               },
             ],
           },
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
         });
 
         for (const scenario of [

diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts
@@ -33,6 +33,7 @@ import {
   secOnlyRead,
   superUser,
 } from '../../../../common/lib/authentication/users';
+import { superUserSpace1Auth } from '../../../../common/lib/authentication';
 
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext): void => {
@@ -277,14 +278,14 @@ export default ({ getService }: FtrProviderContext): void => {
             supertestWithoutAuth,
             getPostCaseRequest({ owner: 'securitySolutionFixture' }),
             200,
-            { user: superUser, space: 'space1' }
+            superUserSpace1Auth
           );
 
           const commentResp = await createComment({
             supertest: supertestWithoutAuth,
             caseId: postedCase.id,
             params: postCommentUserReq,
-            auth: { user: superUser, space: 'space1' },
+            auth: superUserSpace1Auth,
           });
 
           await deleteComment({
@@ -309,14 +310,14 @@ export default ({ getService }: FtrProviderContext): void => {
           supertestWithoutAuth,
           getPostCaseRequest({ owner: 'securitySolutionFixture' }),
           200,
-          { user: superUser, space: 'space1' }
+          superUserSpace1Auth
         );
 
         const commentResp = await createComment({
           supertest: supertestWithoutAuth,
           caseId: postedCase.id,
           params: postCommentUserReq,
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
         });
 
         await deleteComment({

diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/find_comments.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/find_comments.ts
@@ -40,6 +40,7 @@ import {
   globalRead,
   obsSecRead,
 } from '../../../../common/lib/authentication/users';
+import { superUserSpace1Auth } from '../../../../common/lib/authentication';
 
 // eslint-disable-next-line import/no-default-export
 export default ({ getService }: FtrProviderContext): void => {
@@ -312,12 +313,12 @@ export default ({ getService }: FtrProviderContext): void => {
           supertestWithoutAuth,
           getPostCaseRequest({ owner: 'observabilityFixture' }),
           200,
-          { user: superUser, space: 'space1' }
+          superUserSpace1Auth
         );
 
         await createComment({
           supertest: supertestWithoutAuth,
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
           params: { ...postCommentUserReq, owner: 'observabilityFixture' },
           caseId: obsCase.id,
         });
@@ -340,12 +341,12 @@ export default ({ getService }: FtrProviderContext): void => {
           supertestWithoutAuth,
           getPostCaseRequest({ owner: 'observabilityFixture' }),
           200,
-          { user: superUser, space: 'space1' }
+          superUserSpace1Auth
         );
 
         await createComment({
           supertest: supertestWithoutAuth,
-          auth: { user: superUser, space: 'space1' },
+          auth: superUserSpace1Auth,
           params: { ...postCommentUserReq, owner: 'observabilityFixture' },
           caseId: obsCase.id,
         });