From 3261a46c3d3b94577fa5809851e7ca4f3737623b Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 25 Feb 2021 15:33:19 +0100 Subject: [PATCH 2/3] statistics: allow filtering by users OU (#49670) --- src/authentic2/api_views.py | 29 ++++++++++---- src/authentic2/apps/journal/models.py | 2 +- src/authentic2/journal_event_types.py | 16 +++++--- tests/test_api.py | 55 +++++++++++++++++++++------ tests/test_journal.py | 22 +++++++++-- 5 files changed, 95 insertions(+), 29 deletions(-) diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py index 8510bcaf..131733f2 100644 --- a/src/authentic2/api_views.py +++ b/src/authentic2/api_views.py @@ -1125,7 +1125,9 @@ class StatisticsSerializer(serializers.Serializer): time_interval = serializers.ChoiceField(choices=TIME_INTERVAL_CHOICES, default='month') service = ServiceOUField(child=serializers.SlugField(max_length=256), required=False) - ou = serializers.SlugField(required=False, allow_blank=False, max_length=256) + services_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256) + users_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256) + ou = serializers.SlugField(required=False, allow_blank=False, max_length=256) # legacy start = serializers.DateTimeField(required=False, input_formats=['iso-8601', '%Y-%m-%d']) end = serializers.DateTimeField(required=False, input_formats=['iso-8601', '%Y-%m-%d']) @@ -1171,8 +1173,14 @@ class StatisticsAPI(ViewSet): for action in self.get_extra_actions(): url = self.reverse_action(action.url_name) filters = common_filters.copy() - if 'ou' in action.filters: - filters.append({'id': 'ou', 'label': _('Organizational Unit'), 'options': ous}) + if 'services_ou' in action.filters: + filters.append( + {'id': 'services_ou', 'label': _('Services organizational unit'), 'options': ous} + ) + if 'users_ou' in action.filters: + filters.append( + {'id': 'users_ou', 'label': _('Users organizational unit'), 'options': ous} + ) if 'service' in action.filters: filters.append({'id': 'service', 'label': _('Service'), 'options': services}) data = { @@ -1206,19 +1214,24 @@ class StatisticsAPI(ViewSet): } allowed_filters = getattr(self, self.action).filters - service, ou = data.get('service'), data.get('ou') + service = data.get('service') + services_ou = data.get('services_ou') or data.get('ou') # legacy 'ou' filter + users_ou = data.get('users_ou') + if service and 'service' in allowed_filters: service_slug, ou_slug = service kwargs['service'] = get_object_or_404(Service, slug=service_slug, ou__slug=ou_slug) - elif ou and 'ou' in allowed_filters: - kwargs['ou'] = get_object_or_404(get_ou_model(), slug=ou) + if services_ou and 'services_ou' in allowed_filters: + kwargs['services_ou'] = get_object_or_404(get_ou_model(), slug=services_ou) + if users_ou and 'users_ou' in allowed_filters: + kwargs['users_ou'] = get_object_or_404(get_ou_model(), slug=users_ou) return Response({ 'data': getattr(klass, method)(**kwargs), 'err': 0, }) - @stat(name=_('Login count by authentication type'), filters=('ou', 'service')) + @stat(name=_('Login count by authentication type'), filters=('services_ou', 'users_ou', 'service')) def login(self, request): return self.get_statistics(request, UserLogin, 'get_method_statistics') @@ -1230,7 +1243,7 @@ class StatisticsAPI(ViewSet): def service_ou_login(self, request): return self.get_statistics(request, UserLogin, 'get_service_ou_statistics') - @stat(name=_('Registration count by type'), filters=('ou', 'service')) + @stat(name=_('Registration count by type'), filters=('services_ou', 'users_ou', 'service')) def registration(self, request): return self.get_statistics(request, UserRegistration, 'get_method_statistics') diff --git a/src/authentic2/apps/journal/models.py b/src/authentic2/apps/journal/models.py index 1136c2b5..8ac87486 100644 --- a/src/authentic2/apps/journal/models.py +++ b/src/authentic2/apps/journal/models.py @@ -143,7 +143,7 @@ class EventTypeDefinition(metaclass=EventTypeDefinitionMeta): values.append(group_by_field) if which_references: - qs = qs.which_references(which_references) + qs = qs.filter(which_references) if group_by_references: values.append('reference_ids') diff --git a/src/authentic2/journal_event_types.py b/src/authentic2/journal_event_types.py index f38456c1..5f87b7f9 100644 --- a/src/authentic2/journal_event_types.py +++ b/src/authentic2/journal_event_types.py @@ -15,10 +15,11 @@ # along with this program. If not, see . from django.contrib.contenttypes.models import ContentType +from django.db.models import Q from django.utils.translation import ugettext_lazy as _ from authentic2.custom_user.models import get_attributes_map -from authentic2.apps.journal.models import EventTypeDefinition, n_2_pairing_rev +from authentic2.apps.journal.models import EventTypeDefinition, n_2_pairing_rev, EventQuerySet from authentic2.apps.journal.utils import form_to_old_new, Statistics from authentic2.custom_user.models import User @@ -53,12 +54,17 @@ class EventTypeWithHow(EventTypeWithService): super().record(user=user, session=session, service=service, data={'how': how}) @classmethod - def get_method_statistics(cls, group_by_time, service=None, ou=None, start=None, end=None): - if ou: - service = Service.objects.filter(ou=ou) + def get_method_statistics(cls, group_by_time, service=None, services_ou=None, users_ou=None, start=None, end=None): + which_references = Q() + if service: + which_references &= EventQuerySet._which_references_query(service) + if services_ou: + which_references &= EventQuerySet._which_references_query(Service.objects.filter(ou=services_ou)) + if users_ou: + which_references &= EventQuerySet._which_references_query(User.objects.filter(ou=users_ou)) qs = cls.get_statistics( - group_by_time=group_by_time, group_by_field='how', which_references=service, start=start, end=end + group_by_time=group_by_time, group_by_field='how', which_references=which_references, start=start, end=end ) stats = Statistics(qs, time_interval=group_by_time) diff --git a/tests/test_api.py b/tests/test_api.py index cf276c6e..7f1267dd 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -2052,8 +2052,13 @@ def test_api_statistics_list(app, admin): "default": "month", }, { - 'id': 'ou', - 'label': 'Organizational Unit', + 'id': 'services_ou', + 'label': 'Services organizational unit', + 'options': [{'id': 'default', 'label': 'Default organizational unit'}], + }, + { + 'id': 'users_ou', + 'label': 'Users organizational unit', 'options': [{'id': 'default', 'label': 'Default organizational unit'}], }, {'id': 'service', 'label': 'Service', 'options': []}, @@ -2081,8 +2086,8 @@ def test_api_statistics_list(app, admin): service = Service.objects.create(name='Service1', slug='service1', ou=get_default_ou()) service = Service.objects.create(name='Service2', slug='service2', ou=get_default_ou()) - login_stats['filters'][2]['options'].append({'id': 'service1 default', 'label': 'Service1'}) - login_stats['filters'][2]['options'].append({'id': 'service2 default', 'label': 'Service2'}) + login_stats['filters'][3]['options'].append({'id': 'service1 default', 'label': 'Service1'}) + login_stats['filters'][3]['options'].append({'id': 'service2 default', 'label': 'Service2'}) resp = app.get('/api/statistics/', headers=headers) assert login_stats in resp.json['data'] @@ -2093,13 +2098,15 @@ def test_api_statistics_list(app, admin): 'event_type_name,event_name', [('user.login', 'login'), ('user.registration', 'registration')] ) def test_api_statistics(app, admin, freezer, event_type_name, event_name): + OU = get_ou_model() headers = basic_authorization_header(admin) resp = app.get('/api/statistics/login/?time_interval=month', headers=headers) assert resp.json == {"data": {"series": [], "x_labels": []}, "err": 0} - user = User.objects.create(username='john.doe', email='john.doe@example.com') - portal = Service.objects.create(name='portal', slug='portal', ou=get_default_ou()) + user = User.objects.create(username='john.doe', email='john.doe@example.com', ou=get_default_ou()) + ou = OU.objects.create(name='Second OU', slug='second') + portal = Service.objects.create(name='portal', slug='portal', ou=ou) agendas = Service.objects.create(name='agendas', slug='agendas', ou=get_default_ou()) method = {'how': 'password-on-https'} @@ -2110,7 +2117,7 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name): freezer.move_to('2020-02-03 12:00') event = Event.objects.create(type=event_type, references=[portal], data=method) - event = Event.objects.create(type=event_type, references=[agendas], data=method) + event = Event.objects.create(type=event_type, references=[agendas, user], data=method) freezer.move_to('2020-03-04 13:00') event = Event.objects.create(type=event_type, references=[agendas], data=method) @@ -2131,14 +2138,36 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name): data['series'].sort(key=lambda x: x['label']) assert month_data == data - resp = app.get('/api/statistics/%s/?time_interval=month&ou=default' % event_name, headers=headers) + resp = app.get( + '/api/statistics/%s/?time_interval=month&services_ou=default' % event_name, headers=headers + ) data = resp.json['data'] data['series'].sort(key=lambda x: x['label']) assert data == { 'x_labels': ['2020-02', '2020-03'], - 'series': [{'label': 'FranceConnect', 'data': [None, 1]}, {'label': 'password', 'data': [2, 1]}], + 'series': [{'label': 'password', 'data': [1, 1]}], } + # legacy way to filter by service OU + services_ou_data = data + resp = app.get('/api/statistics/%s/?time_interval=month&ou=default' % event_name, headers=headers) + data = resp.json['data'] + data['series'].sort(key=lambda x: x['label']) + assert services_ou_data == data + + resp = app.get( + '/api/statistics/%s/?time_interval=month&users_ou=default&service=agendas default' % event_name, headers=headers + ) + data = resp.json['data'] + assert data == { + 'x_labels': ['2020-02'], + 'series': [{'label': 'password', 'data': [1]}], + } + + resp = app.get('/api/statistics/%s/?time_interval=month&users_ou=default' % event_name, headers=headers) + data = resp.json['data'] + assert data == {'x_labels': ['2020-02'], 'series': [{'label': 'password', 'data': [1]}]} + resp = app.get( '/api/statistics/%s/?time_interval=month&service=agendas default' % event_name, headers=headers ) @@ -2168,7 +2197,7 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name): assert data == {'x_labels': ['2020-02'], 'series': [{'label': 'password', 'data': [2]}]} resp = app.get( - '/api/statistics/%s/?time_interval=year&service=portal default' % event_name, headers=headers + '/api/statistics/%s/?time_interval=year&service=portal second' % event_name, headers=headers ) data = resp.json['data'] data['series'].sort(key=lambda x: x['label']) @@ -2187,9 +2216,13 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name): resp = app.get('/api/statistics/service_ou_%s/?time_interval=month' % event_name, headers=headers) data = resp.json['data'] + data['series'].sort(key=lambda x: x['label']) assert data == { 'x_labels': ['2020-02', '2020-03'], - 'series': [{'label': 'Default organizational unit', 'data': [2, 2]}], + 'series': [ + {'label': 'Default organizational unit', 'data': [1, 1]}, + {'label': 'Second OU', 'data': [1, 1]}, + ], } diff --git a/tests/test_journal.py b/tests/test_journal.py index 1153c302..908b299e 100644 --- a/tests/test_journal.py +++ b/tests/test_journal.py @@ -450,9 +450,9 @@ def test_message_in_context_exception_handling(db, some_event_types, caplog): @pytest.mark.parametrize('event_type_name', ['user.login', 'user.registration']) def test_statistics(db, event_type_name, freezer): - user = User.objects.create(username='john.doe', email='john.doe@example.com') - user2 = User.objects.create(username='jane.doe', email='jane.doe@example.com') + user = User.objects.create(username='john.doe', email='john.doe@example.com', ou=get_default_ou()) ou = OU.objects.create(name='Second OU') + user2 = User.objects.create(username='jane.doe', email='jane.doe@example.com', ou=ou) portal = Service.objects.create(name='portal', slug='portal', ou=ou) agendas = Service.objects.create(name='agendas', slug='agendas', ou=get_default_ou()) @@ -514,19 +514,26 @@ def test_statistics(db, event_type_name, freezer): ], } - stats = event_type_definition.get_method_statistics('month', ou=get_default_ou()) + stats = event_type_definition.get_method_statistics('month', services_ou=get_default_ou()) assert stats == { 'x_labels': ['2020-03'], 'series': [{'label': 'password', 'data': [2]},], } - stats = event_type_definition.get_method_statistics('month', ou=ou) + stats = event_type_definition.get_method_statistics('month', services_ou=ou) stats['series'].sort(key=lambda x: x['label']) assert stats == { 'x_labels': ['2020-02', '2020-03'], 'series': [{'label': 'FranceConnect', 'data': [2, None]}, {'label': 'password', 'data': [2, 1]}], } + stats = event_type_definition.get_method_statistics('month', users_ou=ou) + stats['series'].sort(key=lambda x: x['label']) + assert stats == { + 'x_labels': ['2020-02'], + 'series': [{'label': 'FranceConnect', 'data': [1]}, {'label': 'password', 'data': [1]}], + } + stats = event_type_definition.get_method_statistics('month', service=portal) stats['series'].sort(key=lambda x: x['label']) assert stats == { @@ -534,6 +541,13 @@ def test_statistics(db, event_type_name, freezer): 'series': [{'label': 'FranceConnect', 'data': [2, None]}, {'label': 'password', 'data': [2, 1]}], } + stats = event_type_definition.get_method_statistics('month', service=agendas, users_ou=get_default_ou()) + stats['series'].sort(key=lambda x: x['label']) + assert stats == { + 'x_labels': ['2020-03'], + 'series': [{'label': 'password', 'data': [1]}], + } + stats = event_type_definition.get_method_statistics('year') stats['series'].sort(key=lambda x: x['label']) assert stats == { -- 2.20.1