[backend] fix(security): add csrf token management (#1785)#4956
[backend] fix(security): add csrf token management (#1785)#4956gabriel-peze merged 20 commits intomasterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4956 +/- ##
============================================
+ Coverage 58.60% 58.62% +0.01%
- Complexity 4863 4873 +10
============================================
Files 1030 1030
Lines 30427 30437 +10
Branches 2254 2257 +3
============================================
+ Hits 17832 17843 +11
+ Misses 11545 11544 -1
Partials 1050 1050 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3a44585 to
b010f19
Compare
b010f19 to
c8b6ab6
Compare
damgouj
left a comment
There was a problem hiding this comment.
it doesn't seem to be working with external API calls
|
@gabriel-peze do we need to keep this PR alive ? Or can we close it ? |
|
Hello @RomuDeuxfois Not doing this will force to apply CSRF process on all the elements, instead of applying it smoothly by applying it first on frontend calls (which is already managed). If i push like that my code, frontend will work, but all technical elements will get a 401 on each api calls. I will work on it next week, when i will be on run week. |
339942b to
24b5622
Compare
|
I've made a check on injectors, collectors, implant, connectors, etc, and it brings me a lot of url where we need to disable csrf, even some when i wasn't expecting them, and i keep it large by disabling the whole API route once i got one to avoid side effects at maximum. Even if i succeed to test a lot, i didn't succeed to tests all the injectors, collectors and connectors due to configuration, and because these changes can break a lot of process, i think it will need a huge testing phase before merging it :/ How can we handle it ? I think we should take a little time to talk about it. |
24b5622 to
2c4a155
Compare
2c4a155 to
f205241
Compare
|
Hey @RomuDeuxfois, @damgouj and @antoinemzs I've tagged this PR as "Do not merge" since there is a need for injectors and collectors to be reviewed before merging it. But the code is ready to be reviewed if you want to, until i'll be allowed to merge it. |
|
Need to wait for this issue to allow this PR to be merged. |
|
Test ok with this issue |
|
Good job @gabriel-peze, we should be testing this in integration as soon as possible |
|
Thanks @antoinemzs, i was waiting to create the doc pr before merging :) |
Proposed changes
Testing Instructions
Related issues