opencti icon indicating copy to clipboard operation
opencti copied to clipboard

[backend] check user access rights before deletion and merging (#6421)

Open JeremyCloarec opened this issue 1 year ago • 3 comments

Proposed changes

  • We now check that the user has access to all related relations before deleting

Related issues

  • #6421

Checklist

  • [x] I consider the submitted work as finished
  • [x] I tested the code for its functionality
  • [ ] I wrote test cases for the relevant uses case (coverage and e2e)
  • [ ] I added/update the relevant documentation (either on github or on notion)
  • [ ] Where necessary I refactored code to improve the overall quality

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

JeremyCloarec avatar Apr 25 '24 09:04 JeremyCloarec

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.43%. Comparing base (9b9bc01) to head (74ac904).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6811   +/-   ##
=======================================
  Coverage   67.43%   67.43%           
=======================================
  Files         561      561           
  Lines       68563    68568    +5     
  Branches     5719     5722    +3     
=======================================
+ Hits        46233    46238    +5     
  Misses      22330    22330           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 25 '24 09:04 codecov[bot]

This issue happens not only when deleting an object but also when merging 2 entities.

repro case:

  • Malware X targets City1, relationship marking RED
  • Malware X targets City2, relationship marking GREEN
  • Log in with user only allowed for GREEN, but with capa to merge knowledge
  • Merge City1 into City2

--> Merge is ok, but if you look at Malware X victimology, you see a "Restricted" (the City1 that has been deleted by the merge).

labo-flg avatar Apr 26 '24 07:04 labo-flg

linked to https://github.com/OpenCTI-Platform/opencti/pull/6821

I just make sure we don not crash

labo-flg avatar Apr 26 '24 08:04 labo-flg