From fb33255bd0550aa0d2f0a6ab2b6b82d32ee5d30e Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 16 Feb 2017 13:33:32 +0000
Subject: [PATCH] Show a more useful message if you get signed out
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

> Users that allow their session to expire, or access a bookmarked link
> are told they need to "Sign in to access this page" - we should
> explain that it's because they've been away a while, so that they
> understand why they're being asked to log in again.

– https://www.pivotaltracker.com/story/show/140016919

The message we were showing before (Please log in to access this page is
the default message from Flask Login).

In order to stop this flash message from appearing, we need to override
the default handler for when a user is unauthorised. We’re overriding it
with the same behaviour, minus the flash message.

If you navigate deliberately to the sign in page it’s unchanged.

Content is Sheryll-approved.
---
 app/main/views/sign_in.py                   | 10 ++++++-
 app/templates/views/signin.html             | 14 +++++++--
 tests/app/main/views/test_accept_invite.py  | 33 +++++++++++++--------
 tests/app/main/views/test_platform_admin.py |  2 +-
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py
index 1ad7a2521..6d4a53996 100644
--- a/app/main/views/sign_in.py
+++ b/app/main/views/sign_in.py
@@ -16,6 +16,7 @@ from flask_login import (
 )
 
 from app import (
+    login_manager,
     user_api_client,
     service_api_client,
     invite_api_client
@@ -75,7 +76,14 @@ def sign_in():
             ).format(password_reset=url_for('.forgot_password'))
         ))
 
-    return render_template('views/signin.html', form=form)
+    return render_template('views/signin.html', form=form, again=bool(request.args.get('next')))
+
+
+@login_manager.unauthorized_handler
+def sign_in_again():
+    return redirect(
+        url_for('main.sign_in', next=request.path)
+    )
 
 
 def _get_and_verify_user(user, password):
diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html
index 5b2ab7168..5e7ab5c8f 100644
--- a/app/templates/views/signin.html
+++ b/app/templates/views/signin.html
@@ -10,9 +10,19 @@
 
 <div class="grid-row">
   <div class="column-two-thirds">
-    <h1 class="heading-large">Sign in</h1>
 
-    <p>If you do not have an account, you can <a href="register">create one now</a>.</p>
+    {% if again %}
+      <h1 class="heading-large">You need to sign in again</h1>
+      <p>
+        We sign you out if you haven’t used Notify for a while.
+      </p>
+    {% else %}
+      <h1 class="heading-large">Sign in</h1>
+      <p>
+        If you do not have an account, you can
+        <a href="{{ url_for('.register') }}">create one now</a>.
+      </p>
+    {% endif %}
 
     <form method="post" autocomplete="nope">
       {{ textbox(form.email_address) }}
diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py
index 1e6e1d9be..1a0102384 100644
--- a/tests/app/main/views/test_accept_invite.py
+++ b/tests/app/main/views/test_accept_invite.py
@@ -75,10 +75,13 @@ def test_if_existing_user_accepts_twice_they_redirect_to_sign_in(
     response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True)
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert page.h1.string.strip() == 'Sign in'
-    flash_banners = page.find_all('div', class_='banner-default')
-    assert len(flash_banners) == 1
-    assert flash_banners[0].text.strip() == 'Please log in to access this page.'
+    assert (
+        page.h1.string,
+        page.select('main p')[0].text.strip(),
+    ) == (
+        'You need to sign in again',
+        'We sign you out if you haven’t used Notify for a while.',
+    )
 
 
 def test_existing_user_of_service_get_redirected_to_signin(
@@ -98,10 +101,13 @@ def test_existing_user_of_service_get_redirected_to_signin(
     response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True)
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert page.h1.string.strip() == 'Sign in'
-    flash_banners = page.find_all('div', class_='banner-default')
-    assert len(flash_banners) == 1
-    assert flash_banners[0].text.strip() == 'Please log in to access this page.'
+    assert (
+        page.h1.string,
+        page.select('main p')[0].text.strip(),
+    ) == (
+        'You need to sign in again',
+        'We sign you out if you haven’t used Notify for a while.',
+    )
     assert mock_accept_invite.call_count == 1
 
 
@@ -130,10 +136,13 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(
 
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert page.h1.string.strip() == 'Sign in'
-    flash_banners = page.find_all('div', class_='banner-default')
-    assert len(flash_banners) == 1
-    assert flash_banners[0].text.strip() == 'Please log in to access this page.'
+    assert (
+        page.h1.string,
+        page.select('main p')[0].text.strip(),
+    ) == (
+        'You need to sign in again',
+        'We sign you out if you haven’t used Notify for a while.',
+    )
 
 
 def test_new_user_accept_invite_calls_api_and_redirects_to_registration(
diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py
index 195a1ecab..a27baf0d5 100644
--- a/tests/app/main/views/test_platform_admin.py
+++ b/tests/app/main/views/test_platform_admin.py
@@ -15,7 +15,7 @@ def test_should_redirect_if_not_logged_in(
 ):
     response = client.get(url_for('main.platform_admin'))
     assert response.status_code == 302
-    assert url_for('main.index', _external=True) in response.location
+    assert response.location == url_for('main.sign_in', next=url_for('main.platform_admin'), _external=True)
 
 
 def test_should_403_if_not_platform_admin(