Add logout page that directs users to click New Identity button in Tor browser #5116

rocodes · 2020-02-05T17:05:00Z

Add page after logout that directs users to click the New Identity button in Tor browser to complete their session
Amend session timeout message to reflect new "click new identity" CTA
Amend functional test to reflect changes in logout behaviour

Status

Ready for Review

Description of Changes

Closes #4952.

Changes proposed in this pull request:

Additional "logout" route that directs users to click the New Identity button in Tor browser.
Amend functional tests to reflect behaviour of new logout page and modified wording on session timeout.

Testing

Scenario 1: Log in to source interface, perform any actions, click "logout". Ensure you are logged out and directed to the new "click New Identity" CTA page.
Scenario 2: Navigate to $source_interface_url/logout without logging in, ensure you are immediately redirected to the main source interface page
(optional Scenario 3: Log in and wait for session timeout, or run session timeout functional test. Ensure you see the "click New Identity" CTA in the session timeout flashed message).
Visual code review of source_app/main.py, tests/functional/source_nagivation_steps.py, source_templates/logout.html, source_templates/session_timeout.html to ensure changes are satisfactory
Run functional tests

Deployment

no special considerations

Checklist

If you made changes to the server application code:

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

ninavizz · 2020-02-05T18:11:38Z

I can haz screenie (using Nicholas' nomenclature for screenshot)?

rocodes · 2020-02-05T18:20:44Z

Note that it's pretty much just a placeholder @ninavizz pending your suggestions

eloquence · 2020-04-14T22:39:25Z

securedrop/source_templates/session_timeout.html

@@ -3,6 +3,6 @@
    <img src="{{ url_for('static', filename='i/hand_with_fingerprint.png') }}">
  </div>
  <div class="message"><strong>{{ gettext('Important!') }}</strong><br>
-  <p>{{ gettext('Your session timed out due to inactivity. Please login again if you want to continue using SecureDrop, or select "New Identity" from the onion button in the Tor browser\'s toolbar to clear all history of your SecureDrop usage from this device. If you are not using Tor Browser, restart your browser.') }}</p>
+    <p>{{ gettext('You were logged out due to inactivity. Click the <img src={icon} alt="broom icon" width="16" height="16">&nbsp;<strong>New Identity</strong> button in your Tor browser\'s toolbar. This will clear your Tor browser activity data on this device.').format(icon=url_for('static', filename='i/torbroom-black.png'))  }}</p>


"button in your Tor browser's toolbar" -> "button in your Tor browser's toolbar before moving on" per language in freedomofpress/securedrop-ux#94 (comment)

eloquence · 2020-04-14T22:51:43Z

All scenarios look good to me in functional testing. A couple of design notes:

Session timeout scenario

Nina's mockup switches this to a new message bubble style we don't have yet (compare for visual, not wording):

That's a bigger lift and I'm OK with tracking that as a follow-up iteration. @ninavizz I'm curious if there any fixes we can make more quickly that you'd be comfortable with, e.g., removing the hand of god from the message.

Logout scenario

You have some excessive whitespace here below the headline; just removing that <br> may fix:

Compare with the mockup (for visual comparison, not wording):

eloquence · 2020-04-14T22:54:28Z

I'll also note that Nina's design has the "Log in" button back on the Logout page, that's probably something we want to implement, to ensure that a user who accidentally logged out can easily find their way back in (not everyone will understand they can click on the logo).

ninavizz · 2020-04-15T17:22:13Z

I love @eloquence's suggestion to more simply remove the hand-o-god from the existing messaging style, if an iterative scooch towards the new style w/o a full style update, is sought. Getting that broom in there and the visual focus, is my primary priority... since the naming of that button is just not intuitive. Thank you, Ro!

rocodes · 2020-04-16T16:28:18Z

fyi for @ninavizz:

Someone who is more of a css wizard than I am may have opinions about some of this styling. I have noticed the stop-bang doesn't render properly in Chrome (it's squished), but this may not matter since this is a TB-only context (at least in an ideal world)

securedrop/source_templates/logout.html

eloquence · 2020-04-16T20:50:32Z

securedrop/sass/modules/_flash.sass

    margin: 10px
+    padding: 10px 16px


This doesn't quite have the intended effect; it adds a bit more padding than needed (note the amount of whitespace around the icon compared with Nina's mockup), and also changes the styling other flash messages. Instead, I would leave this value at 10px, and add padding-left:16px or similar to the .important specification.

fixed in 0acdd39; was worried about internationalization so I have a rtl version as well.

securedrop/sass/modules/_flash.sass

eloquence · 2020-04-16T21:22:41Z

securedrop/source_templates/flashed.html

@@ -7,6 +7,8 @@
        <img src="{{ url_for('static', filename='i/font-awesome/info-circle-black.png') }}" width="20" height="16">
        {% elif category == 'error' %}
        <img class="pull-left" src="{{ url_for('static', filename='i/font-awesome/exclamation-triangle-black.png') }}" width="20" height="17">
+        {% elif category == 'important' %}
+        <img class="pull-left" src="{{ url_for('static', filename='i/bang-stop.png') }}" width="22" height="22">


Due to the flex layout, I don't believe you need the pull-left here; it works fine without it in my testing.

removed in 0acdd39

…ity button in Tor browser to complete their session, if they were logged in, else redirectst to main source interface page. - Remove logout_flashed message since we redirect to a new page now. - Update functional tests to include _is_on_logout_page method and replace test_logout_flashed_message with test_logout screenshot in testsourcelayout - update flashed message graphic and add login button to successful logout page (may need button alignment tweak) - Include coral-theme broom graphic and move flash-notif styling to _flash.sass - update apparmor profile to include logout.html, new assets

rocodes · 2020-04-16T22:31:48Z

@zenmonkeykstop added apparmor changes and squashed commits.

eloquence · 2020-04-16T23:13:42Z

All good from my end, thanks much for the quick changes @rocodes! Have only looked at the design, HTML/CSS, and basic functioning so far, so still needs a full code review from @zenmonkeykstop.

zenmonkeykstop · 2020-04-17T14:32:17Z

Tested in staging env:

make build-debs
make staging

no 500 errors on new logout page, no errors in source error log or kernel log (apparmor is good)
flash message on first and subsequent submits displays correctly with both rtl and ltr text
Timeout flash message not displayed correctly - aspect ratio on icon is incorrect (see screenshot)

rocodes · 2020-04-17T15:12:19Z

I have seen what @zenmonkeykstop found (squished icon) in Chrome, but not FF. Investigating

zenmonkeykstop · 2020-04-17T16:39:43Z

Confirmed that the icon thing is present on current develop as well. It only happens on first use of the flash message icon - when the image is in local cache, it displays correctly.

zenmonkeykstop

Tests pass, LGTM (only rendering issue found is already present in develop, so unrelated to this PR).

rocodes requested review from heartsucker, kushaldas and redshiftzero as code owners February 5, 2020 17:05

rocodes force-pushed the 4952-logout-new-identity-tor branch from 50d98fa to a461ad2 Compare February 5, 2020 17:07

rocodes force-pushed the 4952-logout-new-identity-tor branch from a461ad2 to 7d24c43 Compare February 5, 2020 18:40

eloquence mentioned this pull request Feb 6, 2020

Update UI/docs consistent with "New Identity" behavior change in Tor Browser 9.0 #4952

Closed

rocodes force-pushed the 4952-logout-new-identity-tor branch 4 times, most recently from 48f77bc to bc91227 Compare April 13, 2020 14:36

eloquence reviewed Apr 14, 2020

View reviewed changes

rocodes force-pushed the 4952-logout-new-identity-tor branch from ad12807 to b53c943 Compare April 16, 2020 16:50

eloquence reviewed Apr 16, 2020

View reviewed changes

securedrop/source_templates/logout.html Show resolved Hide resolved

eloquence reviewed Apr 16, 2020

View reviewed changes

securedrop/sass/modules/_flash.sass Show resolved Hide resolved

eloquence reviewed Apr 16, 2020

View reviewed changes

securedrop/sass/modules/_flash.sass Show resolved Hide resolved

eloquence reviewed Apr 16, 2020

View reviewed changes

rocodes force-pushed the 4952-logout-new-identity-tor branch from b53c943 to 0acdd39 Compare April 16, 2020 22:29

rocodes requested review from conorsch and emkll as code owners April 16, 2020 22:29

zenmonkeykstop approved these changes Apr 17, 2020

View reviewed changes

zenmonkeykstop merged commit bba4bb8 into freedomofpress:develop Apr 17, 2020

rocodes deleted the 4952-logout-new-identity-tor branch April 17, 2020 17:12

zenmonkeykstop mentioned this pull request Apr 29, 2020

Release SecureDrop 1.3.0 #5205

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logout page that directs users to click New Identity button in Tor browser #5116

Add logout page that directs users to click New Identity button in Tor browser #5116

rocodes commented Feb 5, 2020 •

edited

Loading

ninavizz commented Feb 5, 2020

rocodes commented Feb 5, 2020 •

edited

Loading

eloquence Apr 14, 2020

eloquence commented Apr 14, 2020

eloquence commented Apr 14, 2020

ninavizz commented Apr 15, 2020

rocodes commented Apr 16, 2020 •

edited

Loading

eloquence Apr 16, 2020

rocodes Apr 16, 2020

eloquence Apr 16, 2020

rocodes Apr 16, 2020

rocodes commented Apr 16, 2020

eloquence commented Apr 16, 2020 •

edited

Loading

zenmonkeykstop commented Apr 17, 2020

rocodes commented Apr 17, 2020

zenmonkeykstop commented Apr 17, 2020

zenmonkeykstop left a comment

Add logout page that directs users to click New Identity button in Tor browser #5116

Add logout page that directs users to click New Identity button in Tor browser #5116

Conversation

rocodes commented Feb 5, 2020 • edited Loading

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the server application code:

ninavizz commented Feb 5, 2020

rocodes commented Feb 5, 2020 • edited Loading

eloquence Apr 14, 2020

Choose a reason for hiding this comment

eloquence commented Apr 14, 2020

Session timeout scenario

Logout scenario

eloquence commented Apr 14, 2020

ninavizz commented Apr 15, 2020

rocodes commented Apr 16, 2020 • edited Loading

eloquence Apr 16, 2020

Choose a reason for hiding this comment

rocodes Apr 16, 2020

Choose a reason for hiding this comment

eloquence Apr 16, 2020

Choose a reason for hiding this comment

rocodes Apr 16, 2020

Choose a reason for hiding this comment

rocodes commented Apr 16, 2020

eloquence commented Apr 16, 2020 • edited Loading

zenmonkeykstop commented Apr 17, 2020

rocodes commented Apr 17, 2020

zenmonkeykstop commented Apr 17, 2020

zenmonkeykstop left a comment

Choose a reason for hiding this comment

rocodes commented Feb 5, 2020 •

edited

Loading

rocodes commented Feb 5, 2020 •

edited

Loading

rocodes commented Apr 16, 2020 •

edited

Loading

eloquence commented Apr 16, 2020 •

edited

Loading