Skip to content

Commit 4c7f672

Browse files
authored
User: fake user deletion, prevent pat access with disabled/deleted user (mozilla#4161)
Also: refactor circular imports into services.py
1 parent a03d4e1 commit 4c7f672

14 files changed

Lines changed: 168 additions & 144 deletions

File tree

pontoon/api/authentication.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ def authenticate(self, request):
4242
if pat.expires_at and pat.expires_at.astimezone() < timezone.now():
4343
raise AuthenticationFailed({"detail": "Token has expired."})
4444

45+
if not pat.user.is_active:
46+
raise AuthenticationFailed({"detail": "User is disabled."})
47+
4548
pat.last_used = timezone.now()
4649
pat.save(update_fields=["last_used"])
4750

pontoon/base/admin.py

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import uuid
2-
31
from django.contrib import admin
42
from django.contrib.auth import get_user_model
53
from django.contrib.auth.admin import UserAdmin as AuthUserAdmin
@@ -9,10 +7,10 @@
97
from django.urls import reverse
108
from django.utils.html import format_html
119

12-
from pontoon.actionlog.models import ActionLog
13-
from pontoon.base import models, utils
10+
from pontoon.base import models
11+
from pontoon.base.services import anonymize_user
12+
from pontoon.base.utils import get_m2m_changes
1413
from pontoon.teams.utils import log_user_groups
15-
from pontoon.terminology.models import Term
1614

1715

1816
AGGREGATED_STATS_FIELDS = (
@@ -57,7 +55,7 @@ def save_model(self, request, obj, form, change):
5755

5856
# Users can only be moved between groups upon editing, not creation
5957
if "groups" in form.cleaned_data:
60-
add_groups, remove_groups = utils.get_m2m_changes(
58+
add_groups, remove_groups = get_m2m_changes(
6159
obj.groups, form.cleaned_data["groups"]
6260
)
6361

@@ -68,38 +66,7 @@ def save_model(self, request, obj, form, change):
6866
# move user data we care about to it and then delete the user.
6967
# See bug 1561663 for details.
7068
def delete_model(self, request, obj):
71-
random_hash = uuid.uuid4().hex
72-
new_user = User.objects.create_user(
73-
username="deleted-user-" + random_hash,
74-
email="deleted-user-" + random_hash + "@example.com",
75-
first_name="Deleted User",
76-
is_active=False,
77-
)
78-
79-
ActionLog.objects.filter(performed_by=obj).update(performed_by=new_user)
80-
models.PermissionChangelog.objects.filter(performed_by=obj).update(
81-
performed_by=new_user
82-
)
83-
models.PermissionChangelog.objects.filter(performed_on=obj).update(
84-
performed_on=new_user
85-
)
86-
models.Project.objects.filter(contact=obj).update(contact=new_user)
87-
models.Translation.objects.filter(user=obj).update(user=new_user)
88-
models.Translation.objects.filter(approved_user=obj).update(
89-
approved_user=new_user
90-
)
91-
models.Translation.objects.filter(unapproved_user=obj).update(
92-
unapproved_user=new_user
93-
)
94-
models.Translation.objects.filter(rejected_user=obj).update(
95-
rejected_user=new_user
96-
)
97-
models.Translation.objects.filter(unrejected_user=obj).update(
98-
unrejected_user=new_user
99-
)
100-
Term.objects.filter(created_by=obj).update(created_by=new_user)
101-
models.Comment.objects.filter(author=obj).update(author=new_user)
102-
69+
anonymize_user(obj)
10370
super().delete_model(request, obj)
10471

10572
# This method is to override bulk delete method from the user list page

pontoon/base/services.py

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import uuid
2+
3+
from django.contrib.auth.models import User
4+
from django.db.models.query import QuerySet
5+
from django.http import Http404
6+
from django.shortcuts import redirect
7+
from django.urls import reverse
8+
9+
from pontoon.actionlog.models import ActionLog
10+
from pontoon.api.models import PersonalAccessToken
11+
from pontoon.base.models.comment import Comment
12+
from pontoon.base.models.locale import Locale, LocaleCodeHistory
13+
from pontoon.base.models.permission_changelog import PermissionChangelog
14+
from pontoon.base.models.project import Project, ProjectSlugHistory
15+
from pontoon.base.models.project_locale import ProjectLocale
16+
from pontoon.base.models.translation import Translation
17+
from pontoon.terminology.models import Term
18+
19+
20+
def readonly_exists(projects, locale):
21+
if not isinstance(projects, (QuerySet, tuple, list)):
22+
projects = [projects]
23+
24+
return ProjectLocale.objects.filter(
25+
project__in=projects,
26+
locale=locale,
27+
readonly=True,
28+
).exists()
29+
30+
31+
def get_project_or_redirect(
32+
slug, redirect_view_name, slug_arg_name, request_user, **kwargs
33+
):
34+
"""
35+
Attempts to get a project with the given slug. If the project doesn't exist, it checks if the slug is in the
36+
ProjectSlugHistory and if so, it redirects to the current project slug URL. If the old slug is not found in the
37+
history, it raises an Http404 error.
38+
"""
39+
40+
try:
41+
project = Project.objects.visible_for(request_user).available().get(slug=slug)
42+
return project
43+
except Project.DoesNotExist:
44+
slug_history = (
45+
ProjectSlugHistory.objects.filter(old_slug=slug)
46+
.order_by("-created_at")
47+
.first()
48+
)
49+
if slug_history is not None:
50+
redirect_kwargs = {slug_arg_name: slug_history.project.slug}
51+
redirect_kwargs.update(kwargs)
52+
redirect_url = reverse(redirect_view_name, kwargs=redirect_kwargs)
53+
return redirect(redirect_url)
54+
else:
55+
raise Http404
56+
57+
58+
def get_locale_or_redirect(code, redirect_view_name=None, url_arg_name=None, **kwargs):
59+
"""
60+
Attempts to retrieve a locale using the given code. If the locale does not exist, it checks the LocaleCodeHistory
61+
for a record of the old code. If an entry is found, it either redirects to the view specified by redirect_view_name
62+
using the new locale code or returns the Locale object if no redirect_view_name is provided.
63+
The url_arg_name parameter specifies the argument name for the locale code used in the URL pattern of the redirect view.
64+
If the old code is not found in the history, it raises an Http404 error.
65+
"""
66+
67+
try:
68+
return Locale.objects.get(code=code)
69+
except Locale.DoesNotExist:
70+
code_history = (
71+
LocaleCodeHistory.objects.filter(old_code=code)
72+
.order_by("-created_at")
73+
.first()
74+
)
75+
if code_history:
76+
if not redirect_view_name or not url_arg_name:
77+
return code_history.locale
78+
79+
redirect_kwargs = {url_arg_name: code_history.locale.code}
80+
redirect_kwargs.update(kwargs)
81+
redirect_url = reverse(redirect_view_name, kwargs=redirect_kwargs)
82+
return redirect(redirect_url)
83+
84+
raise Http404
85+
86+
87+
def anonymize_user(user):
88+
random_hash = uuid.uuid4().hex
89+
new_user = User.objects.create_user(
90+
username="deleted-user-" + random_hash,
91+
email="deleted-user-" + random_hash + "@example.com",
92+
first_name="Deleted User",
93+
is_active=False,
94+
)
95+
96+
ActionLog.objects.filter(performed_by=user).update(performed_by=new_user)
97+
PermissionChangelog.objects.filter(performed_by=user).update(performed_by=new_user)
98+
PermissionChangelog.objects.filter(performed_on=user).update(performed_on=new_user)
99+
Project.objects.filter(contact=user).update(contact=new_user)
100+
Translation.objects.filter(user=user).update(user=new_user)
101+
Translation.objects.filter(approved_user=user).update(approved_user=new_user)
102+
Translation.objects.filter(unapproved_user=user).update(unapproved_user=new_user)
103+
Translation.objects.filter(rejected_user=user).update(rejected_user=new_user)
104+
Translation.objects.filter(unrejected_user=user).update(unrejected_user=new_user)
105+
Term.objects.filter(created_by=user).update(created_by=new_user)
106+
Comment.objects.filter(author=user).update(author=new_user)
107+
PersonalAccessToken.objects.filter(user=user).update(revoked=True)

pontoon/base/utils.py

Lines changed: 1 addition & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@
99

1010
from django.core.exceptions import ValidationError
1111
from django.core.validators import validate_email
12-
from django.db.models.query import QuerySet
13-
from django.http import Http404, HttpResponseBadRequest
14-
from django.shortcuts import redirect
15-
from django.urls import reverse
12+
from django.http import HttpResponseBadRequest
1613
from django.utils.text import slugify
1714
from django.utils.timezone import make_aware
1815
from django.utils.translation import trans_real
@@ -221,26 +218,6 @@ def get_m2m_changes(current_qs, new_qs):
221218
return list(add_items), list(remove_items)
222219

223220

224-
def readonly_exists(projects, locale):
225-
"""
226-
:arg list projects: a list of Project instances.
227-
:arg Locale locale: Locale instance.
228-
:returns: True if a read-only ProjectLocale instance for given Projects and
229-
Locale exists.
230-
"""
231-
# Avoid circular import; someday we should refactor to avoid.
232-
from pontoon.base.models import ProjectLocale
233-
234-
if not isinstance(projects, (QuerySet, tuple, list)):
235-
projects = [projects]
236-
237-
return ProjectLocale.objects.filter(
238-
project__in=projects,
239-
locale=locale,
240-
readonly=True,
241-
).exists()
242-
243-
244221
def get_search_phrases(search):
245222
"""
246223
Split the search phrase into separate search queries.
@@ -283,65 +260,5 @@ def is_email(email):
283260
return False
284261

285262

286-
def get_project_or_redirect(
287-
slug, redirect_view_name, slug_arg_name, request_user, **kwargs
288-
):
289-
"""
290-
Attempts to get a project with the given slug. If the project doesn't exist, it checks if the slug is in the
291-
ProjectSlugHistory and if so, it redirects to the current project slug URL. If the old slug is not found in the
292-
history, it raises an Http404 error.
293-
"""
294-
# Avoid circular import; someday we should refactor to avoid.
295-
from pontoon.base.models import Project, ProjectSlugHistory
296-
297-
try:
298-
project = Project.objects.visible_for(request_user).available().get(slug=slug)
299-
return project
300-
except Project.DoesNotExist:
301-
slug_history = (
302-
ProjectSlugHistory.objects.filter(old_slug=slug)
303-
.order_by("-created_at")
304-
.first()
305-
)
306-
if slug_history is not None:
307-
redirect_kwargs = {slug_arg_name: slug_history.project.slug}
308-
redirect_kwargs.update(kwargs)
309-
redirect_url = reverse(redirect_view_name, kwargs=redirect_kwargs)
310-
return redirect(redirect_url)
311-
else:
312-
raise Http404
313-
314-
315-
def get_locale_or_redirect(code, redirect_view_name=None, url_arg_name=None, **kwargs):
316-
"""
317-
Attempts to retrieve a locale using the given code. If the locale does not exist, it checks the LocaleCodeHistory
318-
for a record of the old code. If an entry is found, it either redirects to the view specified by redirect_view_name
319-
using the new locale code or returns the Locale object if no redirect_view_name is provided.
320-
The url_arg_name parameter specifies the argument name for the locale code used in the URL pattern of the redirect view.
321-
If the old code is not found in the history, it raises an Http404 error.
322-
"""
323-
# Avoid circular import; someday we should refactor to avoid.
324-
from pontoon.base.models import Locale, LocaleCodeHistory
325-
326-
try:
327-
return Locale.objects.get(code=code)
328-
except Locale.DoesNotExist:
329-
code_history = (
330-
LocaleCodeHistory.objects.filter(old_code=code)
331-
.order_by("-created_at")
332-
.first()
333-
)
334-
if code_history:
335-
if not redirect_view_name or not url_arg_name:
336-
return code_history.locale
337-
338-
redirect_kwargs = {url_arg_name: code_history.locale.code}
339-
redirect_kwargs.update(kwargs)
340-
redirect_url = reverse(redirect_view_name, kwargs=redirect_kwargs)
341-
return redirect(redirect_url)
342-
343-
raise Http404
344-
345-
346263
def parse_bool(value) -> bool:
347264
return str(value).lower() in ("1", "true", "yes", "on")

pontoon/base/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
TranslationMemoryEntry,
4747
UserProfile,
4848
)
49+
from pontoon.base.services import readonly_exists
4950
from pontoon.base.templatetags.helpers import provider_login_url
5051
from pontoon.checks.libraries import run_checks
5152
from pontoon.checks.utils import are_blocking_checks
@@ -920,7 +921,7 @@ def upload(request):
920921
project = get_object_or_404(Project.objects.visible_for(request.user), slug=slug)
921922
if not request.user.can_translate(
922923
project=project, locale=locale
923-
) or utils.readonly_exists(project, locale):
924+
) or readonly_exists(project, locale):
924925
return HttpResponseForbidden("You don't have permission to upload files.")
925926
get_object_or_404(Resource, project=project, path=res_path)
926927

pontoon/batch/views.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@
1414
Translation,
1515
TranslationMemoryEntry,
1616
)
17-
from pontoon.base.utils import (
18-
readonly_exists,
19-
require_AJAX,
20-
)
17+
from pontoon.base.services import readonly_exists
18+
from pontoon.base.utils import require_AJAX
2119
from pontoon.batch import forms
2220
from pontoon.batch.actions import ACTIONS_FN_MAP
2321

pontoon/contributors/tests/test_views.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010

1111
from pontoon.api.models import PersonalAccessToken
1212
from pontoon.base.models import User, UserBanLog
13+
from pontoon.base.models.translation import Translation
1314
from pontoon.base.tests import (
1415
LocaleFactory,
1516
TranslationFactory,
1617
)
1718
from pontoon.base.utils import aware_datetime
1819
from pontoon.contributors import views
20+
from pontoon.test.factories import EntityFactory, ProjectFactory, ResourceFactory
1921

2022

2123
def commajoin(*items):
@@ -415,7 +417,32 @@ def test_personal_access_token_deletion_correct_permissions(member, user_b):
415417

416418
@pytest.mark.django_db
417419
def test_delete_user(member):
418-
"""Test if user can delete their own account."""
420+
"""
421+
Test if user can delete their own account. Check if user relationships are assigned
422+
to a new placeholder user.
423+
"""
424+
locale_a = LocaleFactory(
425+
code="kg",
426+
name="Klingon",
427+
)
428+
project_a = ProjectFactory(
429+
slug="project_a",
430+
name="Project A",
431+
)
432+
resource_a = ResourceFactory.create(
433+
path="resource_a",
434+
project=project_a,
435+
)
436+
entity_a = EntityFactory.create(string="Test String", resource=resource_a)
437+
438+
TranslationFactory.create(
439+
string="String A",
440+
locale=locale_a,
441+
user=member.user,
442+
entity=entity_a,
443+
)
444+
assert Translation.objects.filter(user=member.user).exists()
445+
419446
url = reverse("pontoon.contributors.delete_user")
420447
user_pk = member.user.pk
421448

@@ -425,3 +452,4 @@ def test_delete_user(member):
425452
assert response.json()["status"] == "success"
426453
assert response.json()["message"] == "User deleted successfully."
427454
assert not User.objects.filter(pk=user_pk).exists()
455+
assert not Translation.objects.filter(user=member.user).exists()

pontoon/contributors/views.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
from pontoon.api.models import PersonalAccessToken
3434
from pontoon.base import forms
3535
from pontoon.base.models import Locale, Project, UserBanLog, UserProfile
36-
from pontoon.base.utils import get_locale_or_redirect, require_AJAX
36+
from pontoon.base.services import anonymize_user, get_locale_or_redirect
37+
from pontoon.base.utils import require_AJAX
3738
from pontoon.contributors import utils
3839
from pontoon.messaging.emails import send_verification_email
3940
from pontoon.settings import (
@@ -605,6 +606,7 @@ def delete_token(request, token_id):
605606
@transaction.atomic
606607
def delete_user(request):
607608
try:
609+
anonymize_user(request.user)
608610
request.user.delete()
609611
logout(request)
610612
messages.success(request, "Your account has been deleted.")

0 commit comments

Comments
 (0)