Name of CSRF cookie is hard-coded.

Hey folks,

I've run into a problem with CSRF while using Dash with Django. The gist of it is that the route handler in our setup used for processing the _dash-update-component call requires CSRF (not something we can turn off). This is all fine, however, api.js in dash-renderer
hardcodes the name of the cookie to read the token from

'X-CSRFToken': cookie.parse(document.cookie)._csrf_token

This means that we have to either change our cookie names across the project, or simply not use dash. I'd rather use it cause it's a great product, but I can't change our cookie name across the board.

Would it be possible to make this value configurable?

Thanks!

I believe that we aren't actually using this token anymore, as we removed CSRF protection in #146. This logic wasn't removed in the dash front-end, and so I believe we are safe to remove it.

Looks like this is causing some other folks issues as well, like refinery-platform/heatmap-scatter-dash#53

@chriddyp

@chriddyp
I saw #141 where you guys removed CSRF protection.
I understand the reasoning for removing your own implementation, however, some folks won't be able to remove CSRF protection from those routes due to their own security concerns.

I would propose a different logic instead. If the CSRF cookie is present, it should be delivered to the backend using the X-CSRF header the same way it is right now. If it is not, the header should not be sent at all to prevent from confusing backends like Django. Finally, the name of the cookie should be configurable to address the first case.

This logic allows for backends to choose to either enable to disable CSRF protection on their end, and does not require CSRF implementation in Dash, which covers both use cases.

Thoughts?

I'd also like to point out the logic mentioned here: https://security.stackexchange.com/questions/23371/csrf-protection-with-custom-headers-and-without-validating-token/58308#58308
is not future proof. If the underlying conditions for CSRF headers change in the future, some backends will find themselves open to CSRF attacks, possibly without their knowledge.

some folks won't be able to remove CSRF protection from those routes due to their own security concerns.

I'd also like to point out the logic mentioned here: https://security.stackexchange.com/questions/23371/csrf-protection-with-custom-headers-and-without-validating-token/58308#58308
is not future proof. If the underlying conditions for CSRF headers change in the future,

I would argue that this behaviour:

Simply put, an attacker can coerce a victims browser to make the following types of requests:

All GET requests

POST requests with bodies of type application/x-www-form-urlencoded, multipart/form-data and text/plain
An attacker can not:

Coerce the browser to post other content types, such as application/json

is fundamental to the web and will not change in the future as that would have many legacy security implications.

However, in your case, it seems like more of an issue that while using Dash with Django, there are other routes that need CSRF protection (e.g. multipart/form-data POSTs) and therefore CSRF needs to be turned on for all routes or none of them.

I would accept a PR that keeps the token but makes it configurable. We can make this a dash config variable. I'll follow up with a comment on how this code works.

Here is how the config logic works in Dash:

Dash backend exposes them through app.config:

dash/dash/dash.py

Lines 48 to 52 in 9a4c50c

    
           self.config = _AttributeDict({ 
        
               'suppress_callback_exceptions': False, 
        
               'routes_pathname_prefix': url_base_pathname, 
        
               'requests_pathname_prefix': url_base_pathname 
        
           })

Dash backend templates them into HTML:

dash/dash/dash.py

Line 293 in 9a4c50c

config = self._generate_config_html()
Dash frontend reads the JSON config from the HTML DOM and puts it into the standard redux store:
https://github.com/plotly/dash-renderer/blob/f6fc7a1b41d8b3f885a14b621ebcab3c9a1a746e/src/reducers/config.js#L6
https://github.com/plotly/dash-renderer/blob/f6fc7a1b41d8b3f885a14b621ebcab3c9a1a746e/src/reducers/reducer.js#L19
Dash frontend can access the variables through the standard redux store throughout the code:
https://github.com/plotly/dash-renderer/blob/c07c5875088c4f7f502afddf6b56f3ac8a038c02/src/actions/api.js#L36

We're merging dash-renderer into the dash repo. Moving this issue to the dash repo in case anyone is interested in following up. As mentioned we'd happily accept a PR to make the cookie and header configurable.

feat: disable cookie/localstorage access under restricted sandboxes (p…

Issue 256, 257, 608 - Add conditional styling capabilities (plotly#729)

feat: disable cookie/localstorage access under restricted sandboxes (#…

Issue 256, 257, 608 - Add conditional styling capabilities (#729)

@gvwilson

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

Navigation Menu

Repository navigation

Name of CSRF cookie is hard-coded. #729

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	self.config = _AttributeDict({
	'suppress_callback_exceptions': False,
	'routes_pathname_prefix': url_base_pathname,
	'requests_pathname_prefix': url_base_pathname
	})

Name of CSRF cookie is hard-coded. #729

Description

Activity

chriddyp commented on Jan 18, 2018

chriddyp commented on Jan 18, 2018

ckm2k1 commented on Jan 20, 2018

ckm2k1 commented on Jan 20, 2018

chriddyp commented on Jan 22, 2018

chriddyp commented on Jan 22, 2018

alexcjohnson commented on May 22, 2019

gvwilson commented on Jun 3, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions