From 7c1867fde660d6896178ed03fb875aa6f673b649 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 27 Apr 2016 16:39:17 +0100 Subject: [PATCH 1/2] Record login including remembered user login events to the api based of flask login signals. --- app/__init__.py | 13 +++++ app/event_handlers.py | 52 +++++++++++++++++++ app/main/views/sign_out.py | 12 +++-- app/notify_client/events_api_client.py | 21 ++++++++ tests/__init__.py | 1 + .../main/notify_client/test_events_client.py | 17 ++++++ tests/app/main/views/test_two_factor.py | 7 ++- tests/app/test_event_handlers.py | 29 +++++++++++ tests/conftest.py | 9 ++++ 9 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 app/event_handlers.py create mode 100644 app/notify_client/events_api_client.py create mode 100644 tests/app/main/notify_client/test_events_client.py create mode 100644 tests/app/test_event_handlers.py diff --git a/app/__init__.py b/app/__init__.py index 773bc01b5..0d6ce6154 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -38,6 +38,7 @@ from app.notify_client.statistics_api_client import StatisticsApiClient from app.notify_client.status_api_client import StatusApiClient from app.notify_client.template_statistics_api_client import TemplateStatisticsApiClient from app.notify_client.user_api_client import UserApiClient +from app.notify_client.events_api_client import EventsApiClient login_manager = LoginManager() csrf = CsrfProtect() @@ -51,6 +52,7 @@ status_api_client = StatusApiClient() invite_api_client = InviteApiClient() statistics_api_client = StatisticsApiClient() template_statistics_client = TemplateStatisticsApiClient() +events_api_client = EventsApiClient() asset_fingerprinter = AssetFingerprinter() # The current service attached to the request stack. @@ -75,6 +77,7 @@ def create_app(): invite_api_client.init_app(application) statistics_api_client.init_app(application) template_statistics_client.init_app(application) + events_api_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' @@ -109,6 +112,8 @@ def create_app(): application.context_processor(_attach_current_service) register_errorhandlers(application) + setup_event_handlers() + return application @@ -272,3 +277,11 @@ def register_errorhandlers(application): if current_app.config.get('DEBUG', None): raise error return _error_response(500) + + +def setup_event_handlers(): + from flask.ext.login import user_logged_in, user_login_confirmed + from app.event_handlers import on_user_logged_in, on_user_login_confirmed + + user_logged_in.connect(on_user_logged_in) + user_login_confirmed.connect(on_user_login_confirmed) diff --git a/app/event_handlers.py b/app/event_handlers.py new file mode 100644 index 000000000..90c9e78fe --- /dev/null +++ b/app/event_handlers.py @@ -0,0 +1,52 @@ +from app import events_api_client +from flask_login import current_user + + +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: + event_data = _construct_event_data(request) + if kwargs.get('user'): + event_data['user_id'] = kwargs.get('user').id + if not kwargs.get('event_type'): + return + events_api_client.create_event(kwargs['event_type'], event_data) + except Exception as e: + sender.logger.error('Error creating event') + sender.logger.error(e) + + +def _construct_event_data(request): + return {'ip_address': _get_remote_addr(request), + 'browser_fingerprint': _get_browser_fingerprint(request)} + + +# This might not be totally correct depending on proxy setup +def _get_remote_addr(request): + if request.headers.getlist("X-Forwarded-For"): + return request.headers.getlist("X-Forwarded-For")[0] + else: + return request.remote_addr + + +def _get_browser_fingerprint(request): + browser = request.user_agent.browser + version = request.user_agent.version + platform = request.user_agent.platform + user_agent_string = request.user_agent.string + # at some point this may be hashed? + finger_print = {'browser': browser, + 'platform': platform, + 'version': version, + 'user_agent_string': user_agent_string} + + return finger_print diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index cbe2738e3..4a76f567e 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -1,12 +1,16 @@ -from flask import (render_template, redirect, url_for) -from flask import session -from flask_login import (login_required, logout_user) +from flask import ( + redirect, + url_for, + session +) + +from flask.ext.login import logout_user from app.main import main @main.route('/sign-out', methods=(['GET'])) def sign_out(): - session.clear() logout_user() + session.clear() return redirect(url_for('main.sign_in')) diff --git a/app/notify_client/events_api_client.py b/app/notify_client/events_api_client.py new file mode 100644 index 000000000..af487419a --- /dev/null +++ b/app/notify_client/events_api_client.py @@ -0,0 +1,21 @@ +from notifications_python_client.base import BaseAPIClient + + +class EventsApiClient(BaseAPIClient): + def __init__(self, base_url=None, client_id=None, secret=None): + super(self.__class__, self).__init__(base_url=base_url or 'base_url', + client_id=client_id or 'client_id', + secret=secret or 'secret') + + def init_app(self, app): + self.base_url = app.config['API_HOST_NAME'] + self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] + self.secret = app.config['ADMIN_CLIENT_SECRET'] + + def create_event(self, event_type, event_data): + data = { + 'event_type': event_type, + 'data': event_data + } + resp = self.post(url='/events', data=data) + return resp['data'] diff --git a/tests/__init__.py b/tests/__init__.py index 502c52f3e..f3adcf85d 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -14,6 +14,7 @@ class TestClient(FlaskClient): session['_fresh'] = True if mocker: mocker.patch('app.user_api_client.get_user', return_value=user) + mocker.patch('app.events_api_client.create_event') if mocker and service: mocker.patch('app.service_api_client.get_service', return_value={'data': service}) login_user(user, remember=True) diff --git a/tests/app/main/notify_client/test_events_client.py b/tests/app/main/notify_client/test_events_client.py new file mode 100644 index 000000000..6868bdc3d --- /dev/null +++ b/tests/app/main/notify_client/test_events_client.py @@ -0,0 +1,17 @@ +from app.notify_client.events_api_client import EventsApiClient + + +def test_events_client_calls_correct_api_endpoint(mocker): + + expected_url = '/events' + event_type = 'anything' + event_data = {'does_not': 'matter'} + expected_data = {'event_type': event_type, 'data': event_data} + + client = EventsApiClient() + + mock_post = mocker.patch('app.notify_client.events_api_client.EventsApiClient.post') + + client.create_event(event_type, event_data) + + mock_post.assert_called_once_with(url=expected_url, data=expected_data) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index cad3e0882..3e18f99f4 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,6 +1,8 @@ from flask import url_for from tests.conftest import SERVICE_ONE_ID +from unittest.mock import ANY + def test_should_render_two_factor_page(app_, api_user_active, @@ -23,7 +25,8 @@ def test_should_login_user_and_redirect_to_service_dashboard(app_, mock_get_user, mock_get_user_by_email, mock_check_verify_code, - mock_get_services_with_one_service): + mock_get_services_with_one_service, + mock_events): with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: @@ -39,6 +42,8 @@ def test_should_login_user_and_redirect_to_service_dashboard(app_, _external=True ) + mock_events.assert_called_with('sucessful_login', ANY) + def test_should_login_user_and_should_redirect_to_next_url(app_, api_user_active, diff --git a/tests/app/test_event_handlers.py b/tests/app/test_event_handlers.py new file mode 100644 index 000000000..30a4859f6 --- /dev/null +++ b/tests/app/test_event_handlers.py @@ -0,0 +1,29 @@ +from unittest.mock import ANY + +from app.event_handlers import ( + on_user_logged_in, + on_user_login_confirmed +) + + +def test_on_user_logged_in_calls_events_api(app_, api_user_active, mock_events): + + with app_.test_request_context(): + on_user_logged_in(app_, api_user_active) + mock_events.assert_called_with('sucessful_login', + {'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)}) diff --git a/tests/conftest.py b/tests/conftest.py index 3a12d25e7..ce0037e68 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -846,3 +846,12 @@ def mock_get_template_statistics(mocker, service_one, fake_uuid): return mocker.patch( 'app.template_statistics_client.get_template_statistics_for_service', side_effect=_get_stats) + + +@pytest.fixture(scope='function') +def mock_events(mocker): + + def _create_event(event_type, event_data): + return {'some': 'data'} + + return mocker.patch('app.events_api_client.create_event', side_effect=_create_event) From 7b611faee958f6191cef543161ffcd536124f09d Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 27 Apr 2016 16:45:50 +0100 Subject: [PATCH 2/2] Missing requirement --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 6c761f771..63ca047ad 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,7 @@ credstash==1.8.0 boto3==1.3.0 Pygments==2.0.2 py-gfm==0.1.2 +blinker==1.4 git+https://github.com/alphagov/notifications-python-client.git@0.5.0#egg=notifications-python-client==0.5.0