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/frontend] Allow security admin to renew users token (#6643) #8667

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

aHenryJard
Copy link
Member

@aHenryJard aHenryJard commented Oct 15, 2024

Proposed changes

  • Add test coverage on existing renew query
  • Add frontend action in the user detail screen + popup to confirm before token revoke.
  • Add an audit log on token renewal
  • Add required capa SETTINGS_SETACCESSES both at UI level and graphql level (with auth) to prevent organization admin with this capa to renew other users token.
  • Forbid renewal for admin token defined in configuration (either yaml or env variables)

Refactors and improvements:

  • move user-test to typescript

Capture d'écran 2024-10-15 101159

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

@github-actions github-actions bot added the filigran team use to identify PR from the Filigran team label Oct 15, 2024
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 66.30%. Comparing base (b04b787) to head (0905367).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...pencti-platform/opencti-graphql/src/domain/user.js 90.90% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8667      +/-   ##
==========================================
+ Coverage   66.28%   66.30%   +0.02%     
==========================================
  Files         597      597              
  Lines       60929    60949      +20     
  Branches     6254     6263       +9     
==========================================
+ Hits        40385    40412      +27     
+ Misses      20544    20537       -7     

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

@aHenryJard aHenryJard marked this pull request as ready for review October 16, 2024 06:47
Copy link
Member

@labo-flg labo-flg left a comment

Choose a reason for hiding this comment

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

tested ok

@aHenryJard aHenryJard marked this pull request as draft October 16, 2024 10:28
@aHenryJard aHenryJard marked this pull request as ready for review October 17, 2024 10:03
@aHenryJard aHenryJard marked this pull request as draft October 18, 2024 05:21
@aHenryJard
Copy link
Member Author

Change required: yaml admin token should not be renewable that way.

@aHenryJard aHenryJard marked this pull request as ready for review October 21, 2024 08:13
@aHenryJard aHenryJard merged commit c7a29d4 into master Oct 21, 2024
6 checks passed
@aHenryJard aHenryJard deleted the issue/6643-revoke-token branch October 21, 2024 09:09
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.

Leaked Tokens are not revokable
3 participants