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

Security improvements #1227

Merged
merged 2 commits into from
Jul 15, 2022
Merged

Security improvements #1227

merged 2 commits into from
Jul 15, 2022

Conversation

mher
Copy link
Owner

@mher mher commented Jul 9, 2022

  • Limit auth option to simple regex
  • By default set random value for cookie_secret option
  • Warn about running without authentication
  • Set CORS headers only if authentication is not configured

@tprynn
Copy link

tprynn commented Jul 10, 2022

Hey, checking in on this, sorry about any delay in response - been busy since I just started a new job.

cookie_secret isn't an effective prevention for CSRF attacks. The essential innovation / interesting piece of a CSRF attack is that it abuses how web browsers handle cross-site requests. Basically, browsers will automatically send the user's cookie with most types of cross-site requests. Thus, cookies alone can't prevent a CSRF attack. There are a number of options to prevent CSRF but unfortunately none of them can realistically be implemented without making a breaking change to at least some portion of existing clients.

At line 21 of flower/views/auth.py, I'd recommend using re.escape rather than the manual escaping you're doing now, and replacing the .* wildcard with the character class [A-Za-z0-9!#$%&'*+/=?^_`{|}~.\-]+.

@mher mher merged commit cd6a3c9 into master Jul 15, 2022
DiegoVallely pushed a commit to metocean/flower that referenced this pull request Aug 16, 2023
* Security improvements

* Update pattern escape

Co-authored-by: Mher Movsisyan <mher@users.noreply.github.com>
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.

None yet

2 participants