Allow the admin to set custom text in donate box.

[fgregg]

Current the donate content is hardcoded

• https://github.com/datamade/bga-payroll/blob/78aea94ef7c71db923d41a66684c323ee022682d/templates/jinja2/index.html#L15-L43

Set up form in admin interface for authorized users to change copy and to toggle whether it displays.

[smcalilly]

@fgregg Were you expecting that this would need a new model, or is there something built into the admin interface for this? Or would we need to add wagtail to do CMS, so we can have rich text with links?

Also, what all pieces of the text need to be in the form, and what can be hardcoded? Is it okay to hardcode the salutation and the closing, while having a form for the body text? If we hardcode the salutation and the closing, then we could simply add a new model for the body text, with a flag to toggle whether or not it's published. This would be a simple form in the admin interface.

Or do we need to have separate forms for the different pieces of the content, so they could style and add links? That would be best done by wagtail.

[fgregg]

@hancush had ideas about how to implement this. Would you share that context.

[hancush]

@smcalilly You are on the right track! We had to walk back from a CMS due to the cost, so we should go with as simple an implementation here as possible.

what all pieces of the text need to be in the form, and what can be hardcoded? Is it okay to hardcode the salutation and the closing, while having a form for the body text?

This is a great question. Let's start with hard-coded salutation and donate button, with editable message text.

we could simply add a new model for the body text, with a flag to toggle whether or not it's published. This would be a simple form in the admin interface.

We don't need to retain past versions of the text, so a model isn't strictly necessary.

Some important context: We use django-extra-settings to control whether the fundraising banner is shown. (See PR where it was added for more info: https://github.com/datamade/bga-payroll/pull/493.) One key factor is that changing settings automatically clears the homepage from the cache, so the changes go live instantly.

With all of this in mind, I would recommend adding an additional setting that accepts and renders an HTML string to populate the body text in the fundraising banner.

What do you think?

[smcalilly]

@hancush Re: django-extra-settings, nice! I was wondering where this came from. I'll get to work on implementing that and let you know if I have anymore questions.

[smcalilly]

@hancush Would it be okay to use MarkupSafe for safely using the html string in the jinja template? Based on this stackoverflow answer.

[hancush]

@smcalilly What are the advantages of MarkupSafe vs. the safe filter?

[smcalilly]

@hancush From my understanding, the safe filter isn't secure since we're using untrusted user data (the html submitted via the admin interface). Using the Markup class is supposed to sanitize the string (i think?). I found that the jinja docs mention this class, so that dependency already exists in the codebase. I've tested both ways, and both ways work. Which would you prefer?

[hancush]

My reading is that they are equivalent. Good thought on not escaping user data. I generally consider this more of a concern when rendering data provided by site users, e.g., if we had a form where external users could input HTML, we would not want to render out that HTML because it could be malicious. I worry less that BGA will add malicious code to their own site. Are there other risks I'm missing?

[smcalilly]

Yikes! I can pass through and execute javascript using either method. I was under the assumption that the Markup class would make that more safe...

From what I know, it's just generally a good security practice to never trust a user. But I agree that BGA wouldn't add malicious code. The threat would be stolen admin credentials, in which case there are probably bigger concerns? @hancush

[hancush]

I'd say so, @smcalilly. With that said, I wouldn't be opposed if you wanted to spike using a sanitization library like bleach. In addition to the security angle, it also seems to correct markup errors, like unclosed tags, which could be quite useful. We should probably whitelist at least the tags we're already using in the template, in addition to their defaults.

[smcalilly]

bleach is cool and easy to use. I whitelisted the html tags that exist for this message, and tested that it wouldn't execute any JavaScript. I'll submit a PR. @hancush thanks!