[backend] Fix the file change right #5587

Kedae · 2024-01-18T13:31:15Z

codecov · 2024-01-18T14:10:13Z

Codecov Report

Attention: 39 lines in your changes are missing coverage. Please review.

Comparison is base (6ef714f) 66.12% compared to head (dccdd7d) 66.14%.

Files	Patch %	Lines
...l/src/modules/internal/document/document-domain.ts	65.00%	14 Missing ⚠️
...-platform/opencti-graphql/src/http/httpPlatform.js	0.00%	10 Missing ⚠️
...rm/opencti-graphql/src/resolvers/stixCoreObject.js	20.00%	4 Missing ⚠️
...opencti-graphql/src/resolvers/externalReference.js	25.00%	3 Missing ⚠️
...encti-graphql/src/resolvers/stixCyberObservable.js	25.00%	3 Missing ⚠️
...cti-platform/opencti-graphql/src/resolvers/file.js	33.33%	2 Missing ⚠️
...ql/src/modules/internal/document/document-types.ts	0.00%	1 Missing ⚠️
...ncti-graphql/src/resolvers/stixCoreRelationship.js	50.00%	1 Missing ⚠️
.../opencti-graphql/src/resolvers/stixDomainObject.js	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5587      +/-   ##
==========================================
+ Coverage   66.12%   66.14%   +0.01%     
==========================================
  Files         512      512              
  Lines       60398    60400       +2     
  Branches     4401     4406       +5     
==========================================
+ Hits        39936    39949      +13     
+ Misses      20462    20451      -11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SouadHadjiat · 2024-01-19T16:02:56Z

opencti-platform/opencti-graphql/src/modules/internal/document/document-domain.ts

 // Get Files paginated with auto enrichment
 // Images metadata for users
 // In progress virtual files from export
-export const paginatedForPathsWithEnrichment = async (context: AuthContext, user: AuthUser, paths: string[], opts?: FilesOptions<BasicStoreEntityDocument>) => {
+export const paginatedForPathsWithEnrichment = async (context: AuthContext, user: AuthUser, path: string, entity_id: string, opts?: FilesOptions<BasicStoreEntityDocument>) => {


if it's now singular "path" instead of "paths", maybe we can rename the method too ? paginatedForPathWithEnrichment or paginatedFilesWithEnrichment so it's not dependent on the parameters

SouadHadjiat · 2024-01-19T16:11:20Z

opencti-platform/opencti-graphql/src/modules/internal/document/document-domain.ts

@@ -133,15 +159,14 @@ export const paginatedForPathsWithEnrichment = async (context: AuthContext, user
    orderOptions.orderMode = OrderingMode.Desc;
  }
  const listOptions = { ...opts, ...findOpts, ...orderOptions };
-  const pagination = await listEntitiesPaginated<BasicStoreEntityDocument>(context, user, [ENTITY_TYPE_INTERNAL_FILE], listOptions);
+
+  await checkFileAccess(context, user, entity_id, { entity_id, id: path, filename: '' });


the third parameter of checkFileAccess is scope and here it's set with entity_id, doesn't seem right, no ?

Well, it's the scope for the "UserAction" log, so I think it's a good call to have the entity_id to tell "User A tried to access files of this specific entity and cannot". But I'm open to other thoughts

scopes should be a limited list of operations I think, but I don't know well the activity logging part. I can see this graph in a user activity analytics which is a distribution based on scope :

It might not help to have ids in scope

You're completely right, my bad I'll change that :)

it's even defined in UserActionListener, all event_scope are typed with specific values

SouadHadjiat · 2024-01-19T16:45:47Z

There is an issue here : I can see all analyst workbenches in a report (I should see only the report analyst workbenches, none here)
I can't reproduce the issue on testing platform.

SouadHadjiat · 2024-01-19T16:49:14Z

opencti-platform/opencti-graphql/src/resolvers/stixCoreObject.js

    },
    pendingFiles: (stixCoreObject, { first }, context) => {
-      return paginatedForPathsWithEnrichment(context, context.user, ['import/pending'], { first, entity_id: stixCoreObject.id });
+      return paginatedForPathWithEnrichment(context, context.user, 'import/pending', stixCoreObject.id, { first });


if the entity_id is not part of the options, it won't be used as a filter to list only files linked to this entity_id

SouadHadjiat

Fix analyst workbench list displayed in an entity data tab (should be filtered by entity, it displays all analyst workbenches)

SouadHadjiat · 2024-01-22T08:40:27Z

opencti-platform/opencti-graphql/src/modules/internal/document/document-domain.ts

 // Get Files paginated with auto enrichment
 // Images metadata for users
 // In progress virtual files from export
-export const paginatedForPathsWithEnrichment = async (context: AuthContext, user: AuthUser, paths: string[], opts?: FilesOptions<BasicStoreEntityDocument>) => {
+export const paginatedForPathWithEnrichment = async (context: AuthContext, user: AuthUser, path: string, entity_id: string, opts?: FilesOptions<BasicStoreEntityDocument>) => {


I don't really understand why entity_id has been extracted from opts as a separate parameter knowing that FilesOptions type still has entity_id. Why not keep entity_id inside opts and extract in the method ?

It's a bit of semantic actually. We want to enforce that entity_id is a important prop from this function to get files and not just some stuff you can add in opts.
I understand your concern, we've discussed that with Julien and thougth it would be more explicit that way

SouadHadjiat

Tested locally and it seem to work well ✅

Kedae added the filigran team use to identify PR from the Filigran team label Jan 18, 2024

Kedae requested a review from richard-julien January 18, 2024 13:31

Kedae linked an issue Jan 18, 2024 that may be closed by this pull request

Impossible to see a file uploaded if we are not part of the platform organization #5435

Closed

Kedae force-pushed the issue/5435 branch from a6c8c2f to d2970e5 Compare January 18, 2024 14:37

Kedae removed the request for review from richard-julien January 18, 2024 14:41

SouadHadjiat reviewed Jan 19, 2024

View reviewed changes

Kedae force-pushed the issue/5435 branch from d2970e5 to c5919e1 Compare January 19, 2024 16:26

SouadHadjiat reviewed Jan 19, 2024

View reviewed changes

SouadHadjiat requested changes Jan 19, 2024

View reviewed changes

[backend] Fix the file change right

dccdd7d

Kedae force-pushed the issue/5435 branch from c5919e1 to dccdd7d Compare January 19, 2024 21:14

Kedae requested a review from SouadHadjiat January 19, 2024 21:15

SouadHadjiat reviewed Jan 22, 2024

View reviewed changes

SouadHadjiat approved these changes Jan 22, 2024

View reviewed changes

Kedae merged commit 184a4ca into master Jan 22, 2024
5 checks passed

SamuelHassine deleted the issue/5435 branch February 1, 2024 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[backend] Fix the file change right #5587

[backend] Fix the file change right #5587

Kedae commented Jan 18, 2024

codecov bot commented Jan 18, 2024 •

edited

Loading

SouadHadjiat Jan 19, 2024

SouadHadjiat Jan 19, 2024

Kedae Jan 19, 2024

SouadHadjiat Jan 19, 2024

Kedae Jan 19, 2024

SouadHadjiat Jan 19, 2024

SouadHadjiat commented Jan 19, 2024

SouadHadjiat Jan 19, 2024

SouadHadjiat left a comment

SouadHadjiat Jan 22, 2024

Kedae Jan 22, 2024

SouadHadjiat left a comment

[backend] Fix the file change right #5587

[backend] Fix the file change right #5587

Conversation

Kedae commented Jan 18, 2024

codecov bot commented Jan 18, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SouadHadjiat commented Jan 19, 2024

Choose a reason for hiding this comment

SouadHadjiat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SouadHadjiat left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 18, 2024 •

edited

Loading