Flask-Middleware / flask-security

Quick and simple security for Flask applications
MIT License
622 stars 154 forks source link

/login JSON api accepts CSRF token through the body #943

Closed lilz-egoto closed 3 months ago

lilz-egoto commented 4 months ago

When using the JSON api and sending POST /login with a JSON body, adding csrf_token as a third key in the body lets you successfully login. Is this as intended?

image

Headers for the same request:

image

The security related settings I have are:

app.config["SECURITY_CSRF_COOKIE_NAME"] = "CSRF-Cookie"
app.config["SECURITY_CSRF_HEADER"] = "CSRF-Header"
app.config["SECURITY_CSRF_PROTECT_MECHANISMS"] = ["session"]
app.config["WTF_CSRF_TIME_LIMIT"] = 900
app.config["WTF_CSRF_CHECK_DEFAULT"] = False
app.config["SECURITY_CSRF_IGNORE_UNAUTH_ENDPOINTS"] = False

flask_wtf.CSRFProtect(app)

In the CSRF Pro tips section of the documentation it is stated that, If you enable CSRFProtect(app) and you want to send request data as JSON, then you must include the CSRF token in the header but it seems we have bypassed this protection.

lilz-egoto commented 4 months ago

Ok well I see now that most liekly has to do with flask-wtf: https://flask-wtf.readthedocs.io/en/0.15.x/config/#configuration

Specifically: WTF_CSRF_FIELD_NAME | Name of the form field and session key that holds the CSRF token. Default is csrf_token. but this should apply only to form based requests no?

jwag956 commented 3 months ago

This is all way more complex than I wish due to all the various config parameters. The TL;DR is that with the recent PR - this actually won't work anymore which is fine. There wasn't any protection bypassed - before this recent change if the first CSRF check failed (the one our decorator does) fails - it would pass through to the form where another CSRF check was done. Whew!

It is worth pointing out that regardless of whether the client sends JSON or forms - both are converted (by Flask) into a form - which is then validated (where csrf is checked).

lilz-egoto commented 2 months ago

Thank you, I have verified the updated functionality on 5.4.3