Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backend] Fix UI issue when changing organization in a user (#7682) #8054

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

CelineSebe
Copy link
Member

@CelineSebe CelineSebe commented Aug 16, 2024

Proposed changes

  • replace return user in assignOrganizationToUser

Related issues

Checklist

  • I consider the submitted work as finished
  • 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

@CelineSebe CelineSebe added the filigran team use to identify PR from the Filigran team label Aug 16, 2024
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.09%. Comparing base (078052e) to head (0434d76).
Report is 52 commits behind head on master.

Files Patch % Lines
...pencti-platform/opencti-graphql/src/domain/user.js 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8054      +/-   ##
==========================================
+ Coverage   67.75%   68.09%   +0.34%     
==========================================
  Files         572      572              
  Lines       71026    71080      +54     
  Branches     6047     6450     +403     
==========================================
+ Hits        48122    48403     +281     
+ Misses      22904    22677     -227     

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

@SouadHadjiat SouadHadjiat self-requested a review August 19, 2024 08:02
@@ -419,6 +419,11 @@ export const assignOrganizationToUser = async (context, user, userId, organizati
if (!isUserHasCapability(user, SETTINGS_SET_ACCESSES) && isUserHasCapability(user, VIRTUAL_ORGANIZATION_ADMIN)) {
throw ForbiddenAccess();
}
const targetUser = await storeLoadById(context, user, userId, ENTITY_TYPE_USER);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a concern about this assignOrganizationToUser, I wonder if we shouldn't use another method, or add more checks when the user is an organization admin (without SETTINGS_SET_ACCESSES).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should at least use findById method instead of storeLoadById which will do the right security checks.
But I also wonder if this method is usefull for an organization admin. To be checked with product team.

@CelineSebe CelineSebe changed the title [backend] Fix UI issue when changing organization in a user (#6782) [backend] Fix UI issue when changing organization in a user (#7682) Aug 19, 2024
@lndrtrbn lndrtrbn merged commit e26d492 into master Aug 20, 2024
5 checks passed
@lndrtrbn lndrtrbn deleted the issue/7682 branch August 20, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing "Organizations" in a user make the page full re-render and close the drawer
3 participants