Repository navigation
[backend] fix(security): add csrf token management (#1785) - #4956
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 (damgouj)
left a comment
There was a problem hiding this comment.
it doesn't seem to be working with external API calls
|
Gabriel Pezé (@gabriel-peze) do we need to keep this PR alive ? Or can we close it ? |
|
Hello Romuald Lemesle (@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
|
Romuald Lemesle (@RomuDeuxfois) & damgouj (@damgouj) 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 Romuald Lemesle (@RomuDeuxfois), damgouj (@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 Pezé (@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