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

Add realtime compiler dashboard CSRF protection #1454

Merged
merged 17 commits into from
Nov 13, 2023

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Nov 12, 2023

While we make it clear that the realtime compiler is only intended to be used for local development (as it uses the built in PHP server, which is not designed for public use, as per the manual), since it does allow access to the file system I felt it responsible to at least add some CSRF protection to the dashboard forms. Also extracts a base class while I'm at it.

I ran benchmarks, and adding the session has a negligible performance impact of about 2.5 seconds for 10 000 requests, and that's only for the dashboard page.

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #1454 (abca991) into master (ddd0c75) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##              master     #1454     +/-   ##
=============================================
  Coverage     100.00%   100.00%             
- Complexity      1728      3456   +1728     
=============================================
  Files            180       360    +180     
  Lines           4693      9386   +4693     
=============================================
+ Hits            4693      9386   +4693     

see 180 files with indirect coverage changes

@caendesilva caendesilva changed the title Realtime compiler security Add realtime compiler dashboard CSRF protection Nov 12, 2023
@caendesilva caendesilva force-pushed the realtime-compiler-security branch from e8b7ffd to 3f3cc54 Compare November 12, 2023 20:18
@caendesilva caendesilva force-pushed the realtime-compiler-security branch from 400bb2d to f06ff3a Compare November 13, 2023 10:25
This reverts commit 8ee7f57af9d94f9feb00ed4993174954081fc623.
These expire with the session anyways, and expiring them manually means only one dashboard action works per page reload which is not helpful.
We probably don't need this
Makes it more readable but has same semantics
@caendesilva caendesilva marked this pull request as ready for review November 13, 2023 14:13
@caendesilva caendesilva merged commit 7c58d74 into master Nov 13, 2023
19 checks passed
@caendesilva caendesilva deleted the realtime-compiler-security branch November 13, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants