Merge lp:~roadmr/canonical-identity-provider/gdpr-report into lp:canonical-identity-provider/release

[Daniel Manrique]

    Add GDPR report admin action for accounts.

    The intent is to have a read-only, copy-pasteable view for GDPR requests.
    Currently the information to be reported is scattered between the Account
    change form and the auth logs changelist. The Account form contains most of the
    relevant data but since it's a form, it can't be cleanly copy-pasted.

    If more GDPR-relevant information is required, this view can easily be expanded
    to present that as well.

    Add GDPR report admin action for accounts.

    The intent is to have a read-only, copy-pasteable view for GDPR requests.
    Currently the information to be reported is scattered between the Account
    change form and the auth logs changelist. The Account form contains most of the
    relevant data but since it's a form, it can't be cleanly copy-pasted.

    If more GDPR-relevant information is required, this view can easily be expanded
    to present that as well.

[Maximiliano Bertacchini (maxiberta)]

LGTM, but check a couple of questions inline. Thanks!

[Daniel Manrique (roadmr)]

Updated per review comments, thanks for your attention to deTALALAL :)

[Maximiliano Bertacchini (maxiberta)]

+1

review: Approve

=== modified file 'src/identityprovider/admin.py'
--- src/identityprovider/admin.py	2018-05-29 19:27:59 +0000
+++ src/identityprovider/admin.py	2019-03-27 14:22:19 +0000
@@ -16,6 +16,7 @@
 from django.core import validators
 from django.db import connections
 from django.db.models.query import QuerySet
+from django.shortcuts import render
 from django.template.defaultfilters import force_escape
 from django.urls import reverse
 from django.utils.html import format_html
@@ -292,7 +293,7 @@
 
 
 class AccountAdmin(LargeTableMixin, admin.ModelAdmin):
-    actions = ['suspend']
+    actions = ['suspend', 'gdpr_report']
     inlines = [AccountPasswordInline, EmailAddressInline,
                AuthenticationDeviceInline]
     readonly_fields = ('date_created',)
@@ -370,6 +371,16 @@
 
     suspend.short_description = "Suspend accounts"
 
+    def gdpr_report(self, request, queryset):
+        """Print a GDPR report with data we hold for the selected accounts."""
+        template = 'admin/gdpr_report.html'
+        context = {
+            'accounts': queryset.all()
+        }
+        return render(request, template, context)
+
+    gdpr_report.short_description = "GDPR report for accounts"
+
 
 class APIUserAdminForm(forms.ModelForm):
 

=== added file 'src/identityprovider/templates/admin/gdpr_report.html'
--- src/identityprovider/templates/admin/gdpr_report.html	1970-01-01 00:00:00 +0000
+++ src/identityprovider/templates/admin/gdpr_report.html	2019-03-27 14:22:19 +0000
@@ -0,0 +1,47 @@
+{% extends "admin/index.html" %}
+{% load i18n staticfiles %}
+
+{% comment %}
+Copyright 2019 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file  LICENSE).
+{% endcomment %}
+
+{% block breadcrumbs %}
+<div class="breadcrumbs"><a href="/admin/">
+        {% trans "Home" %}</a> &rsaquo;
+    <a href="/admin/identityprovider/">Identityprovider</a> &rsaquo;
+    {% trans "GDPR report" %}
+</div>
+{% endblock %}
+
+{% block content %}
+{% for account in accounts %}
+<h2>Account: {{account}}</h2>
+<h3>Personal data</h3>
+<table>
+    <tr><td>Display name: </td> <td>{{account.displayname}}</td></tr>
+    <tr><td>OpenID identifier:</td> <td>{{account.openid_identifier}}</td></tr>
+    <tr><td>Creation date: </td> <td>{{account.date_created}}</td></tr>
+    <tr><td>Last login: </td> <td>{{account.last_login}}</td></tr>
+    <tr><td>Status: </td> <td>{{account.get_status_display}}</td></tr>
+    <tr><td>Last status change: </td> <td>{{account.date_status_set}}</td></tr>
+    <tr><td>Account active: </td> <td>{{account.is_active}}</td></tr>
+    <tr><td>Creation source: </td> <td>{{account.creation_source}}</td></tr>
+    <tr><td>Launchpad name:</td> <td>{{account.person_name}}</td></tr>
+</table>
+<h3>E-mail addresses</h3>
+<ol>
+    {% for email in account.emailaddress_set.all %}
+    <li>{{email}} {% if email.is_preferred %}(preferred){% endif %}</li>
+    {% endfor %}
+</ol>
+<h3>Access logs</h3>
+<ol>
+    {% for authlog in account.authlog_set.all %}
+    <li>{{authlog}}</li>
+    {% endfor %}
+</ol>
+{% endfor %}
+{% endblock %}
+
+{% block sidebar %}{% endblock %}

=== modified file 'src/identityprovider/tests/test_admin.py'
--- src/identityprovider/tests/test_admin.py	2018-05-29 19:27:59 +0000
+++ src/identityprovider/tests/test_admin.py	2019-03-27 14:22:19 +0000
@@ -54,6 +54,7 @@
     EmailStatus,
 )
 from identityprovider.tests.utils import SSOBaseTestCase
+from testing.helpers import assert_no_extra_queries_after
 
 
 class AdminTestCase(SSOBaseTestCase):
@@ -733,3 +734,62 @@
         for account in self.accounts:
             account.refresh_from_db()
             self.assertEqual(account.status, AccountStatus.SUSPENDED)
+
+    def test_gdpr_report_all_accounts(self):
+        action_data = {
+            'action': 'gdpr_report',
+            '_selected_action': [account.pk for account in self.accounts]
+        }
+        with self.assertTemplateUsed('admin/gdpr_report.html'):
+            response = self.client.post(
+                reverse('admin:identityprovider_account_changelist'),
+                action_data, follow=True)
+        for account in self.accounts:
+            self.assertContains(response, account.displayname)
+            self.assertContains(
+                response, "%s (preferred)" % account.preferredemail)
+            for email in account.emailaddress_set.all():
+                self.assertContains(response, email)
+
+    def test_gdpr_query_count(self):
+        # We only test addition of e-mails to existing accounts;
+        # query stability for multiple accounts is not possible without
+        # significant refactoring of Account.user() but is also not a big
+        # performance hit because it's only one extra query per account.
+        def add_emails():
+            for account in self.accounts:
+                self.factory.make_email_for_account(account)
+
+        def get_gdpr_report():
+            action_data = {
+                'action': 'gdpr_report',
+                '_selected_action': [account.pk for account in self.accounts]
+            }
+            self.client.post(
+                reverse('admin:identityprovider_account_changelist'),
+                action_data, follow=True)
+
+        assert_no_extra_queries_after(add_emails, get_gdpr_report)
+
+    def test_gdpr_query_count_authlogs(self):
+        def add_authlogs():
+            for account in self.accounts:
+                for _ in range(10):
+                    account.authlog_set.create(
+                        log_type=1, login_email=account.preferredemail.email,
+                        referer="foo", remote_ip='1.2.3.4',
+                        user_agent="firefox")
+                    account.save()
+
+        def get_gdpr_report():
+            action_data = {
+                'action': 'gdpr_report',
+                '_selected_action': [account.pk for account in self.accounts]
+            }
+            self.client.post(
+                reverse('admin:identityprovider_account_changelist'),
+                action_data, follow=True)
+        # Add some auth logs first
+        add_authlogs()
+        # Now ensure add_authlogs again doesn't result in extra queries
+        assert_no_extra_queries_after(add_authlogs, get_gdpr_report)