Skip to content

Commit f7521fd

Browse files
authored
Enforce one acceptance legal response per current docs (#777)
* Potentially duplicated legal docs responses should not block the app * Prevent duplicated legal docs responses * Still allow for declined legal docs to be recorded as many times as the user declined
1 parent a2676ae commit f7521fd

4 files changed

Lines changed: 114 additions & 2 deletions

File tree

src/thunderbird_accounts/core/tests/test_views.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,36 @@ def test_needs_tos_acceptance_true_when_only_declined(self):
610610
self.assertEqual(response.status_code, 200)
611611
self.assertTrue(response.context['needs_tos_acceptance'])
612612

613+
def test_needs_tos_acceptance_false_with_duplicate_acceptances(self):
614+
"""Duplicate acceptance responses should not cause the check to fail."""
615+
tos = LegalDocument.objects.create(
616+
document_type=LegalDocument.DocumentType.TOS,
617+
version='2.0',
618+
is_current=True,
619+
content_path='tos/v2.0',
620+
)
621+
privacy = LegalDocument.objects.create(
622+
document_type=LegalDocument.DocumentType.PRIVACY,
623+
version='2.0',
624+
is_current=True,
625+
content_path='privacy/v2.0',
626+
)
627+
628+
# Force duplicate responses
629+
LegalDocumentResponse.objects.create(
630+
user=self.user, document=tos, action=LegalDocumentResponse.Action.ACCEPTED,
631+
)
632+
LegalDocumentResponse.objects.create(
633+
user=self.user, document=tos, action=LegalDocumentResponse.Action.ACCEPTED,
634+
)
635+
LegalDocumentResponse.objects.create(
636+
user=self.user, document=privacy, action=LegalDocumentResponse.Action.ACCEPTED,
637+
)
638+
639+
response = self._login_and_get_home()
640+
self.assertEqual(response.status_code, 200)
641+
self.assertFalse(response.context['needs_tos_acceptance'])
642+
613643
def test_needs_tos_acceptance_ignores_non_current_docs(self):
614644
LegalDocument.objects.create(
615645
document_type=LegalDocument.DocumentType.TOS,

src/thunderbird_accounts/core/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def home(request: HttpRequest):
121121
is_current=True,
122122
responses__user=request.user,
123123
responses__action=LegalDocumentResponse.Action.ACCEPTED,
124-
).count()
124+
).distinct().count()
125125
needs_tos_acceptance = legal_doc_count != accepted_current_doc_count
126126

127127
form_data = request.session.get('form_data')

src/thunderbird_accounts/legal/tests/test_views.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,71 @@ def test_source_context_defaults_to_empty(self):
254254
resp = LegalDocumentResponse.objects.get(user=self.user)
255255
self.assertEqual(resp.source_context, '')
256256

257+
def test_duplicate_accept_does_not_create_new_responses(self):
258+
oidc_force_login(self.client, self.user)
259+
260+
LegalDocument.objects.create(
261+
document_type=LegalDocument.DocumentType.TOS,
262+
version='2.0',
263+
is_current=True,
264+
content_path='tos/v2.0',
265+
)
266+
LegalDocument.objects.create(
267+
document_type=LegalDocument.DocumentType.PRIVACY,
268+
version='2.0',
269+
is_current=True,
270+
content_path='privacy/v2.0',
271+
)
272+
273+
payload = json.dumps({'source_context': 'sign-up'})
274+
275+
response1 = self.client.post(self.url, data=payload, content_type='application/json')
276+
self.assertEqual(response1.status_code, 200)
277+
data1 = json.loads(response1.content)
278+
self.assertEqual(len(data1['responses']), 2)
279+
280+
response2 = self.client.post(self.url, data=payload, content_type='application/json')
281+
self.assertEqual(response2.status_code, 200)
282+
data2 = json.loads(response2.content)
283+
self.assertEqual(len(data2['responses']), 0)
284+
285+
self.assertEqual(
286+
LegalDocumentResponse.objects.filter(
287+
user=self.user, action=LegalDocumentResponse.Action.ACCEPTED
288+
).count(),
289+
2,
290+
)
291+
292+
def test_decline_still_creates_duplicate_responses(self):
293+
"""Decline responses are always recorded for audit purposes."""
294+
oidc_force_login(self.client, self.user)
295+
296+
# Give user an active subscription so decline doesn't delete user
297+
Subscription.objects.create(user=self.user, status=Subscription.StatusValues.ACTIVE)
298+
299+
LegalDocument.objects.create(
300+
document_type=LegalDocument.DocumentType.TOS,
301+
version='2.0',
302+
is_current=True,
303+
content_path='tos/v2.0',
304+
)
305+
306+
payload = json.dumps({'source_context': 'dashboard'})
307+
decline_url = self.url.replace('accept', 'decline')
308+
309+
response1 = self.client.post(decline_url, data=payload, content_type='application/json')
310+
self.assertEqual(response1.status_code, 200)
311+
312+
response2 = self.client.post(decline_url, data=payload, content_type='application/json')
313+
self.assertEqual(response2.status_code, 200)
314+
315+
self.assertEqual(
316+
LegalDocumentResponse.objects.filter(
317+
user=self.user, action=LegalDocumentResponse.Action.DECLINED
318+
).count(),
319+
2,
320+
)
321+
257322
def test_rejects_non_post_methods(self):
258323
oidc_force_login(self.client, self.user)
259324
response = self.client.get(self.url)

src/thunderbird_accounts/legal/views.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,31 @@ def _read_legal_content(content_path: str, locale: str) -> str:
4949

5050

5151
def _record_response(request, action: str) -> JsonResponse:
52-
"""Record an accept or decline response for all current legal documents."""
52+
"""Record an accept or decline response for all current legal documents.
53+
54+
For acceptance, skips documents that the user has already accepted
55+
but we still want to record the decline action as many times as they declined.
56+
"""
5357
data = json.loads(request.body)
5458
source_context = data.get('source_context', '')
5559

5660
current_docs = LegalDocument.objects.filter(is_current=True)
5761

62+
if action == LegalDocumentResponse.Action.ACCEPTED:
63+
already_accepted_ids = set(
64+
LegalDocumentResponse.objects.filter(
65+
user=request.user,
66+
document__in=current_docs,
67+
action=LegalDocumentResponse.Action.ACCEPTED,
68+
).values_list('document_id', flat=True)
69+
)
70+
else:
71+
already_accepted_ids = set()
72+
5873
created = []
5974
for doc in current_docs:
75+
if doc.pk in already_accepted_ids:
76+
continue
6077
response = LegalDocumentResponse.objects.create(
6178
user=request.user,
6279
document=doc,

0 commit comments

Comments
 (0)