Adds option to set and use an organization name

[zenmonkeykstop]

Status

Ready for review

Description of Changes

Fixes #1575.

• adds organization_name field to the instance configuration table
• adds option to set said field in the Admin interface
• updates SI and JI templates to use the field value in page titles, logo image alt texts, and SI metadata endpoint
• cleans up some flash message handling duplication in the Admin interface.
• removes a potential 500 error when handling the submission preferences form response.

Testing

dev environment

• run make dev on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page
• clear the organization name field and submit via SET ORGANIZATION NAME
  • verify that an error flash message is displayed with text "This field is required"
• update the organization name to a string with more than 64 characters and submit
  • verify that an error flash message is displayed warning about the character limit
• update the organization name to a string with HTML code (and well under 64 char limit)
  • verify that the page title shows the new organization name with html entities escaped.
• update the organization name to a string with no HTML and less than 64 characters.
  • verify that the page title shows the new organization name
  • inspect the page logo image and verify that its alt text contains the new organization name
  • open the Source Interface homepage and verify that its title is prefixed with the new organization name
  • inspect the SI homepage logo image and verify that its alt text contains the new organization name
  • navigate to the SI /lookup page and verify that its title is prefixed with the new organization name
  • navigate to the SI /metadata endpoint and verify that that the JSON response is valid and contains an organization_name key with value set to the new organization name
• for extra QA points, get creative and try to break page rendering with funky organization names!

staging environment (for apparmor)

• run make staging on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page, and update the organization name to a string with no HTML and less than 64 characters.
• verify that the preferences saved flash message is displayed correctly.

upgrade scenario

• run through the upgrade scenario and verify that the organization name can be set and changed post-upgrade.

Deployment

No special considerations for deployment - there is an added db column which should be handled by migration scripts.

Checklist

If you made changes to the server application code:

• Linting (make lint) and tests (make test) pass in the development container

• I have written a test plan and validated it for this PR

Choose one of the following:

• I have opened a PR in the docs repo for these changes, or will do so later
• I would appreciate help with the documentation
• These changes do not require documentation

[sssoleileraaa]

I focused on reviewing for functionality and making sure I didn't find any bugs. Also here are some followup UX improvements, which I don't believe are blocking, since they have a higher-level scope than the change in this PR:

• The error message shows up below the field and scrolls the user back to the top of the page so that you probably won't see the error message (it shows up for me when I display the page at 67% normal size). This is also how the error for uploading the logo works: "You can only upload PNG image files" shows up but you are scrolled to the top of the page, so it feels as if it has worked, especially when the error is out of scope.

• The config options seem out of order to me. For instance, updating the org name and logo seem to go together and like they belong at the top of the page so that it's ordered least-to-most technically advanced, probably: (1) Organization Name, (2) Logo, (3) Submission Preferences, (4) Alerts

• Another UX improvment that I think could, proabably should, be done in this PR is to explain what setting the org name does. For example, instead of just "Set your organization name" we could say above it something like: "The name of your organization is displayed in the page title and logo alt text for both the Source and Journalist interface."

dev environment

• run make dev on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page
• clear the organization name field and submit via SET ORGANIZATION NAME
  • verify that an error flash message is displayed with text "This field is required"
• update the organization name to a string with more than 64 characters and submit
  • verify that an error flash message is displayed warning about the character limit
• update the organization name to a string with HTML code (and well under 64 char limit)
  • verify that the page title shows the new organization name with html entities escaped.
• update the organization name to a string with no HTML and less than 64 characters.
  • verify that the page title shows the new organization name
  • inspect the page logo image and verify that its alt text contains the new organization name
  • open the Source Interface homepage and verify that its title is prefixed with the new organization name
  • inspect the SI homepage logo image and verify that its alt text contains the new organization name
  • navigate to the SI /lookup page and verify that its title is prefixed with the new organization name
• for extra QA points, get creative and try to break page rendering with funky organization names!
  • i was unsuccessful

staging environment (for apparmor)

• run make staging on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page, and update the organization name to a string with no HTML and less than 64 characters.
• verify that the preferences saved flash message is displayed correctly.

upgrade scenario

• run through the upgrade scenario and verify that the organization name can be set and changed post-upgrade.
  • 🌀 IN PROGRESS

I still have the code review to do...

[sssoleileraaa]

• The error message shows up below the field and scrolls the user back to the top of the page so that you probably won't see the error message (it shows up for me when I display the page at 67% normal size). This is also how the error for uploading the logo works: "You can only upload PNG image files" shows up but you are scrolled to the top of the page, so it feels as if it has worked, especially when the error is out of scope.

This has now been addressed. Just one nit to add more space between the blurb and the text box with the organization name. Also you made me think of an enhancement to make (outside of this PR) to add the current name of the organization as placeholder text.

• The config options seem out of order to me. For instance, updating the org name and logo seem to go together and like they belong at the top of the page so that it's ordered least-to-most technically advanced, probably: (1) Organization Name, (2) Logo, (3) Submission Preferences, (4) Alerts

This has now been addressed.

• Another UX improvment that I think could, proabably should, be done in this PR is to explain what setting the org name does. For example, instead of just "Set your organization name" we could say above it something like: "The name of your organization is displayed in the page title and logo alt text for both the Source and Journalist interface."

This has now been addressed.

---

As of now I just need more time to finish review of code and to rerun the staging tests, so will update soon!

[zenmonkeykstop]

vertical space? I thought I added a <br> in there already so horizontal space should not be an issue

[ninavizz]

Oh my, this feels very important to call-out in the Admin UI, as needing to follow guidelines we lay forth in the docs. Also nervous about this shipping, before we've resolved how to handle this in the Source UI. I feel they should both deploy, together; as folks need to see how it will be surfaced, before entering in their information.

[zenmonkeykstop]

@ninavizz as it stands, the Source Interface impact is in the page titles and logo alt text - the logo alt text is as described in #4280 - these updates would all deploy together but not have any impact unless/until an org chose to change the setting.

[ninavizz]

@zenmonkeykstop Yeah, I feel that even that small a thing wd need user testing. I don't want to ship things to the Source UI, w/o being validated by users, first. And, tbh, near-term planned pipeline fixes to the SI will open up other opportunities to name the org inline with page text, that will improve user cognition of how SecureDrop works, significantly. I'm THRILLED this was done for Aaron Swartz Day, as this has been a biggy comprehension blocker in groking what this SecureDrop thing is, I feel, for a long time. Looking forward to having a synchronous dialog about this, soon!

[sssoleileraaa]

Ran through the test plan again, including the upgrade-scenario and everything works as expected. looks good!

[sssoleileraaa]

oh, i was so involved with testing this (this test plan took a long time since i had to rebuild staging and rerun through the upgrade plan multiple times) i missed the conversation between nina an zenmonkey... reading backsroll and see that we should chat more first. @emkll what do you think, is this an after-standup sort of discusssion?

blocked while we discuss whether or not we want to add this feature to the SI

[eloquence]

Overall I think the UX changes here are pretty minimal and I'm optimistic we can land this with a bit more discussion. I do think the change of the page title and alt text in the Source Interface has some potentially problematic side effects.

• The page title will change to "Org Name | Protecting Journalists and Sources". Typically, if organizations put a motto after their name, it's their own motto, and this could be seen as potentially conflating our branding with theirs. Maybe when the name is set it should simply say "Org Name | SecureDrop".
• The same potential for brand conflation applies when the SecureDrop logo is still set to the default logo. Perhaps the name should only be used here if the logo has in fact been customized? This one's a bit less of an issue.
• I've not observed any other uses of the org name yet, but they'd have to be scrutinized similarly.

Let's discuss a bit more here on the PR & synchronously. @ninavizz, I'm not sure I agree we would need user research to validate this change, but I do think we need to at least resolve some of the potential side effects.

[zenmonkeykstop]

• That title change makes sense. Title is now "SecureDrop | Protecting Journalists and Sources" if the org name is unset or set to SecureDrop, and "Org Name | SecureDrop" otherwise.
• I'm less inclined to agree on the image point. Alt text is an alternative to the image, more likely to be displayed in situations where the logo isn't legible anyway, and the accessibility benefit of setting it is more tangible than the potential downside of branding confusion.
• The only other place it's used in the SI currently is in the metadata endpoint, which IMO is pretty innocuous. It's useful to be able to access it in a structured format and it's nothing you couldn't get by scraping the page itself. (though I realize now I should have added it in the test plan - will do so)

[codecov-io]

Codecov Report

Merging #5629 (83bbe25) into develop (b24cbaa) will increase coverage by 0.07%.
The diff coverage is 80.88%.

@@             Coverage Diff             @@
##           develop    #5629      +/-   ##
===========================================
+ Coverage    85.46%   85.54%   +0.07%     
===========================================
  Files           51       52       +1     
  Lines         3709     3771      +62     
  Branches       464      474      +10     
===========================================
+ Hits          3170     3226      +56     
- Misses         439      440       +1     
- Partials       100      105       +5     

Impacted Files	Coverage Δ
securedrop/source_app/api.py	100.00% <ø> (ø)
.../92fba0be98e9_added_organization_name_field_in_.py	66.66% <66.66%> (ø)
securedrop/models.py	91.66% <77.77%> (-0.55%)	⬇️
securedrop/journalist_app/admin.py	92.92% <80.76%> (+0.25%)	⬆️
securedrop/journalist_app/__init__.py	93.45% <100.00%> (+0.18%)	⬆️
securedrop/journalist_app/forms.py	100.00% <100.00%> (ø)
securedrop/source_app/__init__.py	100.00% <100.00%> (ø)
securedrop/source_app/main.py	91.75% <0.00%> (ø)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b24cbaa...83bbe25. Read the comment docs.

[eloquence]

Chatted a bit about this w/ @emkll and @ninavizz last week. From a UX perspective this is good to land in the current form; we can always iterate further - thanks much for working on it Kev!

[rmol]

The test plan went fine. It is possible to insert control characters in the organization name, and while they do seem to be properly handled in the UI and the metadata response, they can be problematic elsewhere. Querying the instance_config table in a SQLite shell can result in some garbled terminal output. I haven't spotted a way this would be more than annoying, but in addition to escaping the name supplied by the admin, maybe we could add a regular expression validator to the form. Something like "^[-:.\w\s]+$" covers all the current sites in our directory and some random tests of Unicode characters.

I also think we should skip the organization name in the alt text of the site logo. It's misleading if the logo is left at the default. A screenreader will read the organization name in the title of the page, so it's also not strictly necessary.

Testing

dev environment

• run make dev on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page
• clear the organization name field and submit via SET ORGANIZATION NAME
  • verify that an error flash message is displayed with text "This field is required"
• update the organization name to a string with more than 64 characters and submit
  • verify that an error flash message is displayed warning about the character limit
• update the organization name to a string with HTML code (and well under 64 char limit)
  • verify that the page title shows the new organization name with html entities escaped.
• update the organization name to a string with no HTML and less than 64 characters.
  • verify that the page title shows the new organization name
  • inspect the page logo image and verify that its alt text contains the new organization name
  • open the Source Interface homepage and verify that its title is prefixed with the new organization name
  • inspect the SI homepage logo image and verify that its alt text contains the new organization name
  • navigate to the SI /lookup page and verify that its title is prefixed with the new organization name
  • navigate to the SI /metadata endpoint and verify that that the JSON response is valid and contains an organization_name key with value set to the new organization name
• for extra QA points, get creative and try to break page rendering with funky organization names!

staging environment (for apparmor)

• run make staging on this branch
• log in to the JI with an admin account, navigate to the Admin > Instance Config page, and update the organization name to a string with no HTML and less than 64 characters.
• verify that the preferences saved flash message is displayed correctly.

[zenmonkeykstop]

It is possible to test for the existence of a custom logo and only update the alt text then, but it seems like overkill and being way too precious about the brand. (There's nothing to stop someone just uploading the current logo as a custom logo, or (more likely) combining the SD logo with their own. Detecting that would be possible but extremely silly.)

If you mean just leave it blank altogether, looking at w3c guidelines, specifically https://www.w3.org/WAI/tutorials/images/decision-tree/ would seem to suggest that the best thing to do (as it is an image with a link) would be to have text like " Securedrop: Home", to allow screenreaders to describe the behavior of the link. The only situation where we would just leave it blank is in the case where it was purely decorative.

I do like the idea of improving text field sanitisation with a regex to catch non-printing chars - IMO this should be applied to other fields as well, including first/lastnames. I'm inclined to say that it should be a separate PR in that case, as otherwise the testing scope of this one will include all those fields too.

[rmol]

the best thing to do (as it is an image with a link) would be to have text like " Securedrop: Home", to allow screenreaders to describe the behavior of the link

Yes.

I do like the idea of improving text field sanitisation with a regex to catch non-printing chars - IMO this should be applied to other fields as well, including first/lastnames. I'm inclined to say that it should be a separate PR in that case

Yes, good call. I'll put in a separate ticket for that.