Merge pull request #3284 from alphagov/fix-sign-out-bug

Check if user is authenticated before signing out

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'))

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

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,