From 8a19d9496a1fa9f4323804dc9a51c0b45a226293 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Tue, 4 Feb 2020 12:16:58 +0000
Subject: [PATCH] Check if user is authenticated before signing out, this will
 prevent a 500 if the user is an AnonymousUser.

I ended up creating a new test user and logged_in_client, which isn't really great. But I tried adding a current_session_id to the active user in the test, but that broke all other tests.
I tried setting current_session_id in all the users being tested but that didn't work either. I'd like to come back to fixing the tests and reducing the number of conftest methods in another PR. For now this fixes the bug.
---
 app/main/views/sign_out.py            |  4 ++-
 tests/app/main/views/test_sign_out.py | 19 +++++++++++--
 tests/conftest.py                     | 39 +++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py
index 65631348c..38c84a0ef 100644
--- a/app/main/views/sign_out.py
+++ b/app/main/views/sign_out.py
@@ -6,5 +6,7 @@ from app.main import main
 
 @main.route('/sign-out', methods=(['GET']))
 def sign_out():
-    current_user.sign_out()
+    # An AnonymousUser does not have an id
+    if current_user.is_authenticated:
+        current_user.sign_out()
     return redirect(url_for('main.index'))
diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py
index d48affb7e..12e93f33b 100644
--- a/tests/app/main/views/test_sign_out.py
+++ b/tests/app/main/views/test_sign_out.py
@@ -1,16 +1,19 @@
+import flask
 from flask import url_for
 
 from tests.conftest import SERVICE_ONE_ID
 
 
 def test_render_sign_out_redirects_to_sign_in(
-    logged_in_client
+    logged_in_client_with_session
 ):
-    response = logged_in_client.get(
+    assert flask.session
+    response = logged_in_client_with_session.get(
         url_for('main.sign_out'))
     assert response.status_code == 302
     assert response.location == url_for(
         'main.index', _external=True)
+    assert not flask.session
 
 
 def test_sign_out_user(
@@ -46,3 +49,15 @@ def test_sign_out_user(
     )
     with client_request.session_transaction() as session:
         assert session.get('user_id') is None
+
+
+def test_sign_out_of_two_sessions(
+    logged_in_client_with_session
+):
+    logged_in_client_with_session.get(
+        url_for('main.sign_out'))
+    assert not flask.session
+    response = logged_in_client_with_session.get(
+        url_for('main.sign_out'))
+
+    assert response.status_code == 302
diff --git a/tests/conftest.py b/tests/conftest.py
index 27b42cc32..11538dde2 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1184,6 +1184,33 @@ def active_user_with_permissions(fake_uuid):
     return user_data
 
 
+@pytest.fixture(scope='function')
+def active_user_with_session(fake_uuid):
+    user_data = {'id': fake_uuid,
+                 'name': 'Test User',
+                 'password': 'somepassword',
+                 'password_changed_at': str(datetime.utcnow()),
+                 'email_address': 'test@user.gov.uk',
+                 'mobile_number': '07700 900762',
+                 'state': 'active',
+                 'failed_login_count': 0,
+                 'permissions': {SERVICE_ONE_ID: ['send_texts',
+                                                  'send_emails',
+                                                  'send_letters',
+                                                  'manage_users',
+                                                  'manage_templates',
+                                                  'manage_settings',
+                                                  'manage_api_keys',
+                                                  'view_activity']},
+                 'platform_admin': False,
+                 'auth_type': 'sms_auth',
+                 'organisations': [ORGANISATION_ID],
+                 'services': [SERVICE_ONE_ID],
+                 'current_session_id': fake_uuid,
+                 }
+    return user_data
+
+
 @pytest.fixture(scope='function')
 def active_user_with_permission_to_two_services(fake_uuid):
 
@@ -2619,6 +2646,18 @@ def logged_in_client(
     yield client
 
 
+@pytest.fixture(scope='function')
+def logged_in_client_with_session(
+    client,
+    active_user_with_session,
+    mocker,
+    service_one,
+    mock_login
+):
+    client.login(active_user_with_session, mocker, service_one)
+    yield client
+
+
 @pytest.fixture(scope='function')
 def platform_admin_client(
     client,