Skip to content

[backend] fix(security): add csrf token management (#1785)#4956

Merged
gabriel-peze merged 20 commits intomasterfrom
issue/1785
Apr 23, 2026
Merged

[backend] fix(security): add csrf token management (#1785)#4956
gabriel-peze merged 20 commits intomasterfrom
issue/1785

Conversation

@gabriel-peze
Copy link
Copy Markdown
Contributor

Proposed changes

  • Add CSRF token management

Testing Instructions

  1. Verify all platform work as usual

Related issues

@gabriel-peze gabriel-peze self-assigned this Feb 16, 2026
@gabriel-peze gabriel-peze added the filigran team use to identify PR from the Filigran team label Feb 16, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 16, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 58.62%. Comparing base (5d7ddd7) to head (a42e0e9).
⚠️ Report is 32 commits behind head on master.

Files with missing lines Patch % Lines
...main/java/io/openaev/config/AppSecurityConfig.java 90.90% 0 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread openaev-api/src/main/java/io/openaev/config/AppSecurityConfig.java Outdated
Comment thread openaev-api/src/main/java/io/openaev/config/AppSecurityConfig.java Outdated
Copy link
Copy Markdown
Member

@damgouj damgouj left a comment

Choose a reason for hiding this comment

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

it doesn't seem to be working with external API calls

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@RomuDeuxfois
Copy link
Copy Markdown
Member

@gabriel-peze do we need to keep this PR alive ? Or can we close it ?

@gabriel-peze
Copy link
Copy Markdown
Contributor Author

Hello @RomuDeuxfois
Yes, i need to realise a huge work on it, to list all routes, and define wich one is called by a technical element (collector, injector, agent, etc) to disable csrf on them, and let the csrf process only on the routes used by frontend.

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.

@gabriel-peze gabriel-peze force-pushed the issue/1785 branch 3 times, most recently from 339942b to 24b5622 Compare March 25, 2026 16:50
@gabriel-peze
Copy link
Copy Markdown
Contributor Author

@RomuDeuxfois & @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.

@gabriel-peze gabriel-peze added the do not merge Do not merge this PR until this tag will be removed label Apr 13, 2026
@gabriel-peze
Copy link
Copy Markdown
Contributor Author

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.
Antoine send a message to Nicolas to tackle it.

But the code is ready to be reviewed if you want to, until i'll be allowed to merge it.

@gabriel-peze
Copy link
Copy Markdown
Contributor Author

Need to wait for this issue to allow this PR to be merged.
Note : Do not forget about the documentation, to add a breaking change with the version.

@damgouj damgouj requested review from damgouj and removed request for damgouj April 23, 2026 13:28
@MarineLeM MarineLeM self-requested a review April 23, 2026 13:34
@MarineLeM
Copy link
Copy Markdown
Contributor

Test ok with this issue

@antoinemzs antoinemzs removed the do not merge Do not merge this PR until this tag will be removed label Apr 23, 2026
@antoinemzs
Copy link
Copy Markdown
Member

Good job @gabriel-peze, we should be testing this in integration as soon as possible

@gabriel-peze
Copy link
Copy Markdown
Contributor Author

Thanks @antoinemzs, i was waiting to create the doc pr before merging :)
I can merge this PR now

@gabriel-peze gabriel-peze merged commit dd437fb into master Apr 23, 2026
14 checks passed
@gabriel-peze gabriel-peze deleted the issue/1785 branch April 23, 2026 15:44
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.

[security] Cross-Site Request Forgery (CSRF)

6 participants