Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates v3 warnings with varying messages based on instance configs #5679

Merged
merged 5 commits into from
Jan 8, 2021
+50 −23
Merged

Updates v3 warnings with varying messages based on instance configs #5679

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad1e140
updated v3 warnings with varying CTA based on instance configs
zenmonkeykstop Dec 21, 2020
2f4d8ce
removed unnecessary test
zenmonkeykstop Dec 21, 2020
8f1d356
Updated banner rendering, text based on UX review.
zenmonkeykstop Jan 6, 2021
740994f
further banner text updates
zenmonkeykstop Jan 7, 2021
ed2a20e
Further banner text revisions
zenmonkeykstop Jan 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@
/var/www/securedrop/static/i/delete_gray.png r,
/var/www/securedrop/static/i/delete_red.png r,
/var/www/securedrop/static/i/bang-stop.png r,
/var/www/securedrop/static/i/bang-circle.png r,
/var/www/securedrop/static/i/favicon.png r,
/var/www/securedrop/static/i/font-awesome/black/guard.svg r,
/var/www/securedrop/static/i/font-awesome/black/times.svg r,
Expand Down
14 changes: 11 additions & 3 deletions securedrop/journalist_app/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,13 @@ def create_app(config: 'SDConfig') -> Flask:
app.config['SQLALCHEMY_DATABASE_URI'] = config.DATABASE_URI
db.init_app(app)

v2_enabled = path.exists(path.join(config.SECUREDROP_DATA_ROOT, 'source_v2_url'))
v3_enabled = path.exists(path.join(config.SECUREDROP_DATA_ROOT, 'source_v3_url'))
def _url_exists(u: str) -> bool:
return path.exists(path.join(config.SECUREDROP_DATA_ROOT, u))

v2_enabled = _url_exists('source_v2_url') or ((not _url_exists('source_v2_url'))
and (not _url_exists('source_v3_url')))
v3_enabled = _url_exists('source_v3_url')

app.config.update(V2_ONION_ENABLED=v2_enabled, V3_ONION_ENABLED=v3_enabled)

# TODO: Attaching a Storage dynamically like this disables all type checking (and
Expand Down Expand Up @@ -161,9 +166,12 @@ def setup_g() -> 'Optional[Response]':
else:
g.organization_name = gettext('SecureDrop')

if not app.config['V3_ONION_ENABLED'] or app.config['V2_ONION_ENABLED']:
if app.config['V2_ONION_ENABLED'] and not app.config['V3_ONION_ENABLED']:
g.show_v2_onion_eol_warning = True

if app.config['V2_ONION_ENABLED'] and app.config['V3_ONION_ENABLED']:
g.show_v2_onion_migration_warning = True

if request.path.split('/')[1] == 'api':
pass # We use the @token_required decorator for the API endpoints
else: # We are not using the API
Expand Down
10 changes: 8 additions & 2 deletions securedrop/journalist_templates/base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@

{% if g.user %}
{% if g.show_v2_onion_eol_warning %}
<div id="v2-onion-eol" class="warning-banner">
{{ gettext('<strong>Update Required:</strong> Your SecureDrop servers are still running v2 onion services, which are being phased out for security reasons. In February 2021, v2 onion services will be disabled, and your SecureDrop servers may become unreachable. <a href="//securedrop.org/v2-onion-eol" rel="noreferrer">Learn More</a>') }}
<div id="v2-onion-eol" class="alert-banner">
<img src="{{ url_for('static', filename='i/bang-circle.png') }}" width="20" height="20"> {{ gettext('<strong>Update Required:</strong> Your SecureDrop servers must be updated to use v3 onion services by April 30 to remain live. <a href="//securedrop.org/v2-onion-eol" rel="noreferrer">Learn More</a>') }}
</div>
{% endif %}

{% if g.show_v2_onion_migration_warning %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that v3 has already been configured, it might make sense to make this alert a lower level than alert

<div id="v2-complete-migration" class="alert-banner">
<img src="{{ url_for('static', filename='i/bang-circle.png') }}" width="20" height="20"> {{ gettext('<strong>Update Required:</strong> Steps remain to complete your onion service update. Please notifyyour admin. <a href="//securedrop.org/v2-onion-eol" rel="noreferrer">Learn More</a>') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: notifyyour->notify your

I'm fine with the wording here but I would suggest a slightly tweaked version

Update Completion Required: Steps remain to complete your SecureDrop server's update to v3 onion services.

Rationale:

  • The wording "Update Completion" is from @ninavizz's proposal in Add clearer action reminder in Journalist Interface to enable v3 #5672 and I think makes sense, especially since completing the update may involve landing page changes & multi-stakeholder comms. If it sounds too nonstandard, perhaps "Action Required" would work as well.

  • I think the phrase "v3 onion services" should be included because it very clearly identifies the technical component we're talking about (if I'm an admin, there's at least a chance I've heard of it). The user shouldn't have to click through to "Learn more" to find out that important detail.

  • IMO we can ditch the "Please notify" for brevity; it's essentially implied, not included in the other notice, and the person seeing the notice may well be an admin.

Copy link
Contributor Author

@zenmonkeykstop zenmonkeykstop Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about best of all worlds:
"Action Required Set up v3 onion services before April 30 to keep your SecureDrop servers online. Learn more"
"Action Required Complete the v3 onion services setup before April 30. Learn more"

(This suggestion is based on both your comments above and feedback in #5672 - it's about as terse as I think it can get while still being accurate. I'm dead set against "Update Completion Required," it has a high cognitive load factor compared to just "Update required" IMO and reads like an awkward phishing attempt.)

</div>
{% endif %}

Expand Down
3 changes: 3 additions & 0 deletions securedrop/sass/_base.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
@import modules/warning
// Warning - Warning messages that show up in a banner to the user.
@import modules/warning-banner
// Alert - Urgent messages requiring action that show up in a banner to the user.
@import modules/alert-banner
// Confirm prompt - When deleting something this prompt is shown
@import modules/confirm-prompt
// 'Serious' text - Seems to be unused. Delete?
Expand Down Expand Up @@ -122,6 +124,7 @@
+panel
+warning
+warning-banner
+alert-banner
+confirm-prompt
+serious-text
+code
Expand Down
21 changes: 21 additions & 0 deletions securedrop/sass/modules/_alert-banner.sass
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
=alert-banner
.alert-banner
display: block
background-color: $color_urgent_coral
color: white
padding: 10px 0
emkll marked this conversation as resolved.
Show resolved Hide resolved
width: 100%
text-align: center
font-size: small
box-sizing: border-box
-moz-box-sizing: border-box

img
vertical-align: text-bottom

a
color: white
border-bottom: 1px solid white

.close
cursor: pointer
Binary file added securedrop/static/i/bang-circle.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 6 additions & 18 deletions securedrop/tests/test_journalist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ def test_user_sees_v2_eol_warning_if_only_v2_is_enabled(config, journalist_app,
resp = app.get(url_for('main.index'))

text = resp.data.decode('utf-8')
assert "v2-onion-eol" in text, text
assert 'id="v2-onion-eol"' in text, text
assert 'id="v2-complete-migration"' not in text, text


def test_user_sees_v2_eol_warning_if_both_v2_and_v3_enabled(config, journalist_app, test_journo):
Expand All @@ -91,7 +92,8 @@ def test_user_sees_v2_eol_warning_if_both_v2_and_v3_enabled(config, journalist_a
resp = app.get(url_for('main.index'))

text = resp.data.decode('utf-8')
assert "v2-onion-eol" in text, text
assert 'id="v2-onion-eol"' not in text, text
assert 'id="v2-complete-migration"' in text, text


def test_user_does_not_see_v2_eol_warning_if_only_v3_enabled(config, journalist_app, test_journo):
Expand All @@ -106,22 +108,8 @@ def test_user_does_not_see_v2_eol_warning_if_only_v3_enabled(config, journalist_
resp = app.get(url_for('main.index'))

text = resp.data.decode('utf-8')
assert "v2-onion-eol" not in text, text


def test_user_sees_v2_eol_warning_if_both_urls_do_not_exist(config, journalist_app, test_journo):
journalist_app.config.update(V2_ONION_ENABLED=False, V3_ONION_ENABLED=False)
with journalist_app.test_client() as app:
_login_user(
app,
test_journo['username'],
test_journo['password'],
test_journo['otp_secret'])

resp = app.get(url_for('main.index'))

text = resp.data.decode('utf-8')
assert "v2-onion-eol" in text, text
assert 'id="v2-onion-eol"' not in text, text
assert 'id="v2-complete-migration"' not in text, text


def test_user_with_whitespace_in_username_can_login(journalist_app):
Expand Down