Merge pull request #1153 from alphagov/forget-me

Remove remember me cookie

app/__init__.py

@@ -471,8 +471,7 @@ def register_errorhandlers(application):
 
 
 def setup_event_handlers():
-    from flask.ext.login import user_logged_in, user_login_confirmed
+    from flask.ext.login import user_logged_in
-    from app.event_handlers import on_user_logged_in, on_user_login_confirmed
+    from app.event_handlers import on_user_logged_in
 
     user_logged_in.connect(on_user_logged_in)
-    user_login_confirmed.connect(on_user_login_confirmed)

app/config.py

@@ -39,10 +39,6 @@ class Config(object):
     NOTIFY_LOG_LEVEL = 'DEBUG'
     NOTIFY_LOG_PATH = '/var/log/notify/application.log'
     PERMANENT_SESSION_LIFETIME = 20 * 60 * 60  # 20 hours
-    REMEMBER_COOKIE_DURATION = timedelta(days=1)
-    REMEMBER_COOKIE_HTTPONLY = True
-    REMEMBER_COOKIE_NAME = 'notify_admin_remember_me'
-    REMEMBER_COOKIE_SECURE = True
     SEND_FILE_MAX_AGE_DEFAULT = 365 * 24 * 60 * 60  # 1 year
     SESSION_COOKIE_HTTPONLY = True
     SESSION_COOKIE_NAME = 'notify_admin_session'
@@ -82,7 +78,6 @@ class Config(object):
 
 class Development(Config):
     DEBUG = True
-    REMEMBER_COOKIE_SECURE = False
     SESSION_COOKIE_SECURE = False
     WTF_CSRF_ENABLED = False
     SESSION_PROTECTION = None

app/event_handlers.py

@@ -6,11 +6,6 @@ def on_user_logged_in(sender, user):
     _send_event(sender, event_type='sucessful_login', user=user)
 
 
-# should change the event type? This is a remember me login.
-def on_user_login_confirmed(sender):
-    _send_event(sender, event_type='sucessful_login_remember_me', user=current_user)
-
-
 def _send_event(sender, **kwargs):
     from flask import request
     try:

app/main/views/sign_in.py

@@ -11,7 +11,6 @@ from flask import (
 
 from flask_login import (
     current_user,
-    login_fresh,
     confirm_login
 )
 
@@ -49,19 +48,6 @@ def sign_in():
             else:
                 invite_api_client.accept_invite(invited_user['service'], invited_user['id'])
         if user:
-            # Remember me login
-            if not login_fresh() and \
-               not current_user.is_anonymous and \
-               current_user.id == user.id and \
-               user.is_active:
-
-                confirm_login()
-                services = service_api_client.get_active_services({'user_id': str(user.id)}).get('data', [])
-                if (len(services) == 1):
-                    return redirect(url_for('main.service_dashboard', service_id=services[0]['id']))
-                else:
-                    return redirect(url_for('main.choose_service'))
-
             session['user_details'] = {"email": user.email_address, "id": user.id}
             if user.is_active:
                 user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)

app/main/views/two_factor.py

@@ -33,7 +33,7 @@ def two_factor():
             if 'password' in session['user_details']:
                 user = user_api_client.update_password(user.id, password=session['user_details']['password'])
             activated_user = user_api_client.activate_user(user)
-            login_user(activated_user, remember=True)
+            login_user(activated_user)
         finally:
             del session['user_details']
 

app/notify_client/models.py

@@ -1,4 +1,4 @@
-from flask_login import UserMixin, AnonymousUserMixin, login_fresh
+from flask_login import UserMixin, AnonymousUserMixin
 from flask import session
 
 
@@ -30,7 +30,6 @@ class User(UserMixin):
     @property
     def is_authenticated(self):
         return (
-            login_fresh() and
             not self.logged_in_elsewhere() and
             super(User, self).is_authenticated
         )

app/templates/views/cookies.html

@@ -40,21 +40,9 @@
                   Used to keep you logged in
                 </td>
                 <td>
-                  1 hour
+                  20 hours
                 </td>
             </tr>
-            <tr>
-                <td>
-                  notify_admin_remember_me
-                </td>
-                <td width="50%">
-                  Used to let you sign in again on the same day, without requiring 2-factor authentication
-                </td>
-                <td>
-                  24 hours
-                </td>
-            </tr>
-
         </tbody>
     </table>
 

scripts/run_tests.sh

@@ -34,5 +34,5 @@ npm test
 display_result $? 2 "Front end code style check"
 
 ## Code coverage
-py.test -n2 --cov=app --cov-report=term-missing tests/ --junitxml=test_results.xml
+py.test -n2 --cov=app --cov-report=term-missing tests/ --junitxml=test_results.xml --strict
 display_result $? 3 "Code coverage"

tests/__init__.py

@@ -13,7 +13,6 @@ class TestClient(FlaskClient):
         # Skipping authentication here and just log them in
         with self.session_transaction() as session:
             session['user_id'] = user.id
-            session['_fresh'] = True
         if mocker:
             mocker.patch('app.user_api_client.get_user', return_value=user)
             mocker.patch('app.events_api_client.create_event')
@@ -21,10 +20,7 @@ class TestClient(FlaskClient):
             with self.session_transaction() as session:
                 session['service_id'] = service['id']
             mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-        login_user(user, remember=True)
+        login_user(user)
-
-    def login_fresh(self):
-        return True
 
     def logout(self, user):
         self.get(url_for("main.logout"))

tests/app/main/views/test_sign_in.py

@@ -144,45 +144,3 @@ def test_should_attempt_redirect_when_user_is_pending(
             'password': 'val1dPassw0rd!'})
     assert response.location == url_for('main.resend_email_verification', _external=True)
     assert response.status_code == 302
-
-
-def test_not_fresh_session_login_redirects_to_dashboard(
-    logged_in_client,
-    api_user_active,
-    mock_login,
-    mock_get_user_by_email,
-    mock_verify_password,
-    mock_get_services_with_one_service,
-):
-    with logged_in_client.session_transaction() as session:
-        assert session['_fresh']
-        session['_fresh'] = False
-    # This should skip the two factor
-    response = logged_in_client.post(
-        url_for('main.sign_in'), data={
-            'email_address': api_user_active.email_address,
-            'password': 'val1dPassw0rd!'})
-    assert response.status_code == 302
-    service_dct = mock_get_services_with_one_service(api_user_active.id)['data'][0]
-    assert response.location == url_for(
-        'main.service_dashboard', service_id=service_dct['id'], _external=True)
-
-
-def test_not_fresh_session_login_redirects_to_choose_service(
-    logged_in_client,
-    api_user_active,
-    mock_login,
-    mock_get_user_by_email,
-    mock_verify_password,
-    mock_get_services,
-):
-    with logged_in_client.session_transaction() as session:
-        assert session['_fresh']
-        session['_fresh'] = False
-    # This should skip the two factor
-    response = logged_in_client.post(
-        url_for('main.sign_in'), data={
-            'email_address': api_user_active.email_address,
-            'password': 'val1dPassw0rd!'})
-    assert response.status_code == 302
-    assert response.location == url_for('main.choose_service', _external=True)

tests/app/main/views/test_two_factor.py

@@ -141,23 +141,6 @@ def test_should_login_user_when_multiple_valid_codes_exist(
     assert response.status_code == 302
 
 
-def test_remember_me_set(
-    client,
-    api_user_active,
-    mock_get_user,
-    mock_get_user_by_email,
-    mock_check_verify_code,
-    mock_get_services_with_one_service,
-):
-    with client.session_transaction() as session:
-        session['user_details'] = {
-            'id': api_user_active.id,
-            'email': api_user_active.email_address}
-    response = client.post(url_for('main.two_factor'),
-                           data={'sms_code': '23456', 'remember_me': True})
-    assert response.status_code == 302
-
-
 def test_two_factor_should_set_password_when_new_password_exists_in_session(
     client,
     api_user_active,

tests/app/test_event_handlers.py

@@ -1,9 +1,6 @@
 from unittest.mock import ANY
 
-from app.event_handlers import (
+from app.event_handlers import on_user_logged_in
-    on_user_logged_in,
-    on_user_login_confirmed
-)
 
 
 def test_on_user_logged_in_calls_events_api(app_, api_user_active, mock_events):
@@ -14,16 +11,3 @@ def test_on_user_logged_in_calls_events_api(app_, api_user_active, mock_events):
                                        {'browser_fingerprint':
                                         {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''},
                                         'ip_address': ANY, 'user_id': str(api_user_active.id)})
-
-
-def test_on_user_login_confirmed_in_calls_events_api(app_, api_user_active, mock_events):
-
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(api_user_active)  # user must have been logged in already.
-
-            on_user_login_confirmed(app_)
-            mock_events.assert_called_with('sucessful_login_remember_me',
-                                           {'browser_fingerprint':
-                                            {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''},
-                                            'ip_address': ANY, 'user_id': str(api_user_active.id)})