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

Proposed by Daniel Manrique on 2019-03-26
Status: Merged
Approved by: Daniel Manrique on 2019-03-27
Approved revision: 1683
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~roadmr/canonical-identity-provider/gdpr-report
Merge into: lp:canonical-identity-provider/release
Diff against target: 164 lines (+119/-1)
3 files modified
src/identityprovider/admin.py (+12/-1)
src/identityprovider/templates/admin/gdpr_report.html (+47/-0)
src/identityprovider/tests/test_admin.py (+60/-0)
To merge this branch: bzr merge lp:~roadmr/canonical-identity-provider/gdpr-report
Reviewer Review Type Date Requested Status
Maximiliano Bertacchini 2019-03-26 Approve on 2019-03-27
Review via email: mp+365134@code.launchpad.net

Commit message

    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.

Description of the change

    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.

To post a comment you must log in.
Maximiliano Bertacchini (maxiberta) wrote :

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

1683. By Daniel Manrique on 2019-03-27

Fix review comments

Daniel Manrique (roadmr) wrote :

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

Maximiliano Bertacchini (maxiberta) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/identityprovider/admin.py'
2--- src/identityprovider/admin.py 2018-05-29 19:27:59 +0000
3+++ src/identityprovider/admin.py 2019-03-27 14:22:19 +0000
4@@ -16,6 +16,7 @@
5 from django.core import validators
6 from django.db import connections
7 from django.db.models.query import QuerySet
8+from django.shortcuts import render
9 from django.template.defaultfilters import force_escape
10 from django.urls import reverse
11 from django.utils.html import format_html
12@@ -292,7 +293,7 @@
13
14
15 class AccountAdmin(LargeTableMixin, admin.ModelAdmin):
16- actions = ['suspend']
17+ actions = ['suspend', 'gdpr_report']
18 inlines = [AccountPasswordInline, EmailAddressInline,
19 AuthenticationDeviceInline]
20 readonly_fields = ('date_created',)
21@@ -370,6 +371,16 @@
22
23 suspend.short_description = "Suspend accounts"
24
25+ def gdpr_report(self, request, queryset):
26+ """Print a GDPR report with data we hold for the selected accounts."""
27+ template = 'admin/gdpr_report.html'
28+ context = {
29+ 'accounts': queryset.all()
30+ }
31+ return render(request, template, context)
32+
33+ gdpr_report.short_description = "GDPR report for accounts"
34+
35
36 class APIUserAdminForm(forms.ModelForm):
37
38
39=== added file 'src/identityprovider/templates/admin/gdpr_report.html'
40--- src/identityprovider/templates/admin/gdpr_report.html 1970-01-01 00:00:00 +0000
41+++ src/identityprovider/templates/admin/gdpr_report.html 2019-03-27 14:22:19 +0000
42@@ -0,0 +1,47 @@
43+{% extends "admin/index.html" %}
44+{% load i18n staticfiles %}
45+
46+{% comment %}
47+Copyright 2019 Canonical Ltd. This software is licensed under the
48+GNU Affero General Public License version 3 (see the file LICENSE).
49+{% endcomment %}
50+
51+{% block breadcrumbs %}
52+<div class="breadcrumbs"><a href="/admin/">
53+ {% trans "Home" %}</a> &rsaquo;
54+ <a href="/admin/identityprovider/">Identityprovider</a> &rsaquo;
55+ {% trans "GDPR report" %}
56+</div>
57+{% endblock %}
58+
59+{% block content %}
60+{% for account in accounts %}
61+<h2>Account: {{account}}</h2>
62+<h3>Personal data</h3>
63+<table>
64+ <tr><td>Display name: </td> <td>{{account.displayname}}</td></tr>
65+ <tr><td>OpenID identifier:</td> <td>{{account.openid_identifier}}</td></tr>
66+ <tr><td>Creation date: </td> <td>{{account.date_created}}</td></tr>
67+ <tr><td>Last login: </td> <td>{{account.last_login}}</td></tr>
68+ <tr><td>Status: </td> <td>{{account.get_status_display}}</td></tr>
69+ <tr><td>Last status change: </td> <td>{{account.date_status_set}}</td></tr>
70+ <tr><td>Account active: </td> <td>{{account.is_active}}</td></tr>
71+ <tr><td>Creation source: </td> <td>{{account.creation_source}}</td></tr>
72+ <tr><td>Launchpad name:</td> <td>{{account.person_name}}</td></tr>
73+</table>
74+<h3>E-mail addresses</h3>
75+<ol>
76+ {% for email in account.emailaddress_set.all %}
77+ <li>{{email}} {% if email.is_preferred %}(preferred){% endif %}</li>
78+ {% endfor %}
79+</ol>
80+<h3>Access logs</h3>
81+<ol>
82+ {% for authlog in account.authlog_set.all %}
83+ <li>{{authlog}}</li>
84+ {% endfor %}
85+</ol>
86+{% endfor %}
87+{% endblock %}
88+
89+{% block sidebar %}{% endblock %}
90
91=== modified file 'src/identityprovider/tests/test_admin.py'
92--- src/identityprovider/tests/test_admin.py 2018-05-29 19:27:59 +0000
93+++ src/identityprovider/tests/test_admin.py 2019-03-27 14:22:19 +0000
94@@ -54,6 +54,7 @@
95 EmailStatus,
96 )
97 from identityprovider.tests.utils import SSOBaseTestCase
98+from testing.helpers import assert_no_extra_queries_after
99
100
101 class AdminTestCase(SSOBaseTestCase):
102@@ -733,3 +734,62 @@
103 for account in self.accounts:
104 account.refresh_from_db()
105 self.assertEqual(account.status, AccountStatus.SUSPENDED)
106+
107+ def test_gdpr_report_all_accounts(self):
108+ action_data = {
109+ 'action': 'gdpr_report',
110+ '_selected_action': [account.pk for account in self.accounts]
111+ }
112+ with self.assertTemplateUsed('admin/gdpr_report.html'):
113+ response = self.client.post(
114+ reverse('admin:identityprovider_account_changelist'),
115+ action_data, follow=True)
116+ for account in self.accounts:
117+ self.assertContains(response, account.displayname)
118+ self.assertContains(
119+ response, "%s (preferred)" % account.preferredemail)
120+ for email in account.emailaddress_set.all():
121+ self.assertContains(response, email)
122+
123+ def test_gdpr_query_count(self):
124+ # We only test addition of e-mails to existing accounts;
125+ # query stability for multiple accounts is not possible without
126+ # significant refactoring of Account.user() but is also not a big
127+ # performance hit because it's only one extra query per account.
128+ def add_emails():
129+ for account in self.accounts:
130+ self.factory.make_email_for_account(account)
131+
132+ def get_gdpr_report():
133+ action_data = {
134+ 'action': 'gdpr_report',
135+ '_selected_action': [account.pk for account in self.accounts]
136+ }
137+ self.client.post(
138+ reverse('admin:identityprovider_account_changelist'),
139+ action_data, follow=True)
140+
141+ assert_no_extra_queries_after(add_emails, get_gdpr_report)
142+
143+ def test_gdpr_query_count_authlogs(self):
144+ def add_authlogs():
145+ for account in self.accounts:
146+ for _ in range(10):
147+ account.authlog_set.create(
148+ log_type=1, login_email=account.preferredemail.email,
149+ referer="foo", remote_ip='1.2.3.4',
150+ user_agent="firefox")
151+ account.save()
152+
153+ def get_gdpr_report():
154+ action_data = {
155+ 'action': 'gdpr_report',
156+ '_selected_action': [account.pk for account in self.accounts]
157+ }
158+ self.client.post(
159+ reverse('admin:identityprovider_account_changelist'),
160+ action_data, follow=True)
161+ # Add some auth logs first
162+ add_authlogs()
163+ # Now ensure add_authlogs again doesn't result in extra queries
164+ assert_no_extra_queries_after(add_authlogs, get_gdpr_report)