qld-gov-au / ckanext-csrf-filter

A CKAN extension to add protection against Cross-Site Request Forgery attacks
GNU Affero General Public License v3.0
0 stars 6 forks source link

Allow Response Streams #37

Closed JVickery-TBS closed 1 year ago

JVickery-TBS commented 1 year ago

feat(blueprint): allow response streams;

The set_csrf_token will always try to insert the hidden token field into forms from the response data. However, this breaks response streams. As by default all HTML templates are NOT streamed in CKAN, I think it is safe to just not do the insertion of the hidden token field for generator responses. This mainly affects big data being streamed to a user, like a large file download.

It is possible for developers to setup template streams for super, super large HTML pages. But I think this would be such a niche case that we do not need to support here? People shouldn't have such large HTML pages that they would need to stream them to the user.

duttonw commented 1 year ago

does this plugin have an option for manual injection on templates as it might be the only way to cover this side issue.

JVickery-TBS commented 1 year ago

@duttonw no feature currently for manual injection. We could just add a template helper in here which would allow for that such as h.csrf_token_field()

JVickery-TBS commented 1 year ago

Just working on the csrf_token_field helper. Trying to figure out if I can get the current response object inside of a template helper function or not. Otherwise, I do not think it is possible to make a template helper and maintain the functionality of the get_response_token method. So may have to leave out a template helper

JVickery-TBS commented 1 year ago

@duttonw @ThrawnCA okay I think the helper works, at least from my testing it a bunch locally. Seems like using flask.Response is fine in this scenario.

I have added some more comments regarding using the template helper with streaming templates.