Skip to content

Commit f46366b

Browse files
committed
backend: Add ability to delete a user
Signed-off-by: Taylor Smock <tsmock@meta.com>
1 parent 7d26e0c commit f46366b

File tree

8 files changed

+343
-10
lines changed

8 files changed

+343
-10
lines changed

backend/api/users/resources.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
from distutils.util import strtobool
2+
from typing import Optional
3+
4+
from flask import stream_with_context, Response
25
from flask_restful import Resource, current_app, request
36
from schematics.exceptions import DataError
47

58
from backend.models.dtos.user_dto import UserSearchQuery
9+
from backend.models.postgis.user import User
610
from backend.services.users.authentication_service import token_auth
711
from backend.services.users.user_service import UserService
812
from backend.services.project_service import ProjectService
13+
from backend.services.users.osm_service import OSMService
14+
from backend.exceptions import Unauthorized
915

1016

1117
class UsersRestAPI(Resource):
@@ -44,6 +50,28 @@ def get(self, user_id):
4450
user_dto = UserService.get_user_dto_by_id(user_id, token_auth.current_user())
4551
return user_dto.to_primitive(), 200
4652

53+
@token_auth.login_required
54+
def delete(self, user_id: Optional[int] = None):
55+
"""
56+
Delete user information by id.
57+
:param user_id: The user to delete
58+
:return: RFC7464 compliant sequence of user objects deleted
59+
200: User deleted
60+
401: Unauthorized - Invalid credentials
61+
404: User not found
62+
500: Internal Server Error
63+
"""
64+
if user_id == token_auth.current_user() or UserService.is_user_an_admin(
65+
token_auth.current_user()
66+
):
67+
return (
68+
UserService.delete_user_by_id(
69+
user_id, token_auth.current_user()
70+
).to_primitive(),
71+
200,
72+
)
73+
raise Unauthorized()
74+
4775

4876
class UsersAllAPI(Resource):
4977
@token_auth.login_required
@@ -115,6 +143,25 @@ def get(self):
115143
users_dto = UserService.get_all_users(query)
116144
return users_dto.to_primitive(), 200
117145

146+
@token_auth.login_required
147+
def delete(self):
148+
if UserService.is_user_an_admin(token_auth.current_user()):
149+
150+
def delete_users():
151+
for user in User.get_all_users_not_paginated():
152+
# We specifically want to remove users that have deleted their OSM accounts.
153+
if OSMService.is_osm_user_gone(user.id):
154+
data = UserService.delete_user_by_id(
155+
user.id, token_auth.current_user()
156+
).to_primitive()
157+
yield f"\u001e{data}\n"
158+
159+
return Response(
160+
stream_with_context(delete_users()),
161+
headers={"Content-Type": "application/json-seq"},
162+
)
163+
raise Unauthorized()
164+
118165

119166
class UsersQueriesUsernameAPI(Resource):
120167
@token_auth.login_required

backend/models/postgis/application.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ def get_all_for_user(user: int):
6060
applications_dto.applications.append(application_dto)
6161
return applications_dto
6262

63+
@staticmethod
64+
def delete_all_for_user(user: int):
65+
for r in db.session.query(Application).filter(Application.user == user):
66+
db.session.delete(r)
67+
db.session.commit()
68+
6369
def as_dto(self):
6470
app_dto = ApplicationDTO()
6571
app_dto.user = self.user

backend/models/postgis/message.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import List
2+
13
from sqlalchemy.sql.expression import false
24

35
from backend import db
@@ -178,7 +180,7 @@ def delete_multiple_messages(message_ids: list, user_id: int):
178180
db.session.commit()
179181

180182
@staticmethod
181-
def delete_all_messages(user_id: int, message_type_filters: list = None):
183+
def delete_all_messages(user_id: int, message_type_filters: List[int] = None):
182184
"""Deletes all messages to the user
183185
-----------------------------------
184186
:param user_id: user id of the user whose messages are to be deleted

backend/services/users/osm_service.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@ def __init__(self, message):
1313

1414

1515
class OSMService:
16+
@staticmethod
17+
def is_osm_user_gone(user_id: int) -> bool:
18+
"""
19+
Check if OSM details for the user from OSM API are available
20+
:param user_id: user_id in scope
21+
:raises OSMServiceError
22+
"""
23+
osm_user_details_url = (
24+
f"{current_app.config['OSM_SERVER_URL']}/api/0.6/user/{user_id}.json"
25+
)
26+
response = requests.head(osm_user_details_url)
27+
28+
if response.status_code == 410:
29+
return True
30+
if response.status_code != 200:
31+
raise OSMServiceError("Bad response from OSM")
32+
33+
return False
34+
1635
@staticmethod
1736
def get_osm_details_for_user(user_id: int) -> UserOSMDTO:
1837
"""
@@ -25,6 +44,8 @@ def get_osm_details_for_user(user_id: int) -> UserOSMDTO:
2544
)
2645
response = requests.get(osm_user_details_url)
2746

47+
if response.status_code == 410:
48+
raise OSMServiceError("User no longer exists on OSM")
2849
if response.status_code != 200:
2950
raise OSMServiceError("Bad response from OSM")
3051

backend/services/users/user_service.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Optional
2+
13
from cachetools import TTLCache, cached
24
from flask import current_app
35
import datetime
@@ -27,14 +29,14 @@
2729
from backend.models.postgis.task import TaskHistory, TaskAction, Task
2830
from backend.models.dtos.user_dto import UserTaskDTOs
2931
from backend.models.dtos.stats_dto import Pagination
32+
from backend.models.postgis.project_chat import ProjectChat
3033
from backend.models.postgis.statuses import TaskStatus, ProjectStatus
31-
from backend.services.users.osm_service import OSMService, OSMServiceError
3234
from backend.services.messaging.smtp_service import SMTPService
3335
from backend.services.messaging.template_service import (
3436
get_txt_template,
3537
template_var_replacing,
3638
)
37-
39+
from backend.services.users.osm_service import OSMService, OSMServiceError
3840

3941
user_filter_cache = TTLCache(maxsize=1024, ttl=600)
4042

@@ -190,6 +192,56 @@ def get_user_dto_by_id(user: int, request_user: int) -> UserDTO:
190192
return user.as_dto(request_username)
191193
return user.as_dto()
192194

195+
@staticmethod
196+
def delete_user_by_id(user_id: int, request_user_id: int) -> Optional[UserDTO]:
197+
if user_id == request_user_id or UserService.is_user_an_admin(request_user_id):
198+
user = User.get_by_id(user_id)
199+
original_dto = UserService.get_user_dto_by_id(user_id, request_user_id)
200+
user.accepted_licenses = []
201+
user.city = None
202+
user.country = None
203+
user.email_address = None
204+
user.facebook_id = None
205+
user.gender = None
206+
user.interests = []
207+
user.irc_id = None
208+
user.is_email_verified = False
209+
user.is_expert = False
210+
user.linkedin_id = None
211+
user.name = None
212+
user.picture_url = None
213+
user.self_description_gender = None
214+
user.skype_id = None
215+
user.slack_id = None
216+
user.twitter_id = None
217+
# FIXME: Should we keep user_id since that will make conversations easier to follow?
218+
# Keep in mind that OSM uses user_<int:user_id> on deleted accounts.
219+
user.username = f"user_{user_id}"
220+
221+
# Remove permissions from admin users, keep role for blocked users.
222+
if UserService.is_user_an_admin(user_id):
223+
user.set_user_role(UserRole.MAPPER)
224+
user.save()
225+
226+
# Remove messages that might contain user identifying information.
227+
for message in ProjectChat.query.filter_by(user_id=user_id):
228+
# TODO detect image links and try to delete them
229+
message.message = f"[Deleted user_{user_id} message]"
230+
db.session.commit()
231+
232+
# Drop application keys
233+
from backend.models.postgis.application import Application
234+
235+
Application.delete_all_for_user(user_id)
236+
237+
# Delete all messages (AKA notifications) for the user
238+
Message.delete_all_messages(
239+
user_id, [message_type.value for message_type in MessageType]
240+
)
241+
# Leave interests, licenses, organizations, and tasks alone for now.
242+
return original_dto
243+
return None
244+
193245
@staticmethod
194246
def get_interests_stats(user_id):
195247
# Get all projects that the user has contributed.

tests/backend/integration/api/users/test_resources.py

Lines changed: 143 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
from typing import Optional
2+
13
from backend.models.postgis.task import Task, TaskStatus
24
from backend.models.postgis.statuses import UserGender, UserRole, MappingLevel
35
from backend.exceptions import get_message_from_sub_code
46

5-
67
from tests.backend.base import BaseTestCase
78
from tests.backend.helpers.test_helpers import (
89
return_canned_user,
@@ -11,7 +12,6 @@
1112
create_canned_interest,
1213
)
1314

14-
1515
TEST_USERNAME = "test_user"
1616
TEST_USER_ID = 1111111
1717
TEST_EMAIL = "test@hotmail.com"
@@ -93,11 +93,11 @@ def test_returns_404_if_user_not_found(self):
9393
@staticmethod
9494
def assert_user_detail_response(
9595
response,
96-
user_id=TEST_USER_ID,
97-
username=TEST_USERNAME,
98-
email=TEST_EMAIL,
99-
gender=None,
100-
own_info=True,
96+
user_id: Optional[int] = TEST_USER_ID,
97+
username: Optional[str] = TEST_USERNAME,
98+
email: Optional[str] = TEST_EMAIL,
99+
gender: Optional[str] = None,
100+
own_info: bool = True,
101101
):
102102
assert response.status_code == 200
103103
assert response.json["id"] == user_id
@@ -556,3 +556,139 @@ def test_email_and_gender_not_returned_if_requested_by_other(self):
556556
TestUsersQueriesUsernameAPI.assert_user_detail_response(
557557
response, TEST_USER_ID, TEST_USERNAME, None, None, False
558558
)
559+
560+
def test_user_can_delete_self(self):
561+
"""Check that a user can delete (redact personal information) themselves"""
562+
# Arrange
563+
self.user.email_address = TEST_EMAIL
564+
self.user.gender = UserGender.FEMALE.value
565+
self.user.save()
566+
# Act
567+
response = self.client.delete(
568+
self.url, headers={"Authorization": self.user_session_token}
569+
)
570+
next_response = self.client.get(
571+
f"/api/v2/users/{TEST_USER_ID}/",
572+
headers={"Authorization": self.user_session_token},
573+
)
574+
# Assert
575+
# Note that we return the deleted user information at this time
576+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
577+
response,
578+
TEST_USER_ID,
579+
TEST_USERNAME,
580+
TEST_EMAIL,
581+
UserGender.FEMALE.name,
582+
True,
583+
)
584+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
585+
next_response, TEST_USER_ID, f"user_{TEST_USER_ID}", None, None, True
586+
)
587+
588+
def test_other_user_cannot_delete_self(self):
589+
"""Check that another user cannot delete (redact personal information) about a different user"""
590+
# Arrange
591+
self.user.email_address = TEST_EMAIL
592+
self.user.gender = UserGender.FEMALE.value
593+
self.user.save()
594+
user_2 = return_canned_user("user_2", 2222222)
595+
user_2.create()
596+
user_2_session_token = generate_encoded_token(user_2.id)
597+
# Act
598+
response = self.client.delete(
599+
self.url, headers={"Authorization": user_2_session_token}
600+
)
601+
# Assert
602+
self.assertEqual(401, response.status_code)
603+
rjson = response.json
604+
self.assertDictEqual(
605+
rjson,
606+
{
607+
"error": {
608+
"code": 401,
609+
"details": {},
610+
"message": "Authentication credentials were missing or incorrect.",
611+
"sub_code": "UNAUTHORIZED",
612+
}
613+
},
614+
)
615+
616+
def test_other_admin_user_can_delete_self(self):
617+
"""Check that another user cannot delete (redact personal information) about a different user"""
618+
# Arrange
619+
self.user.email_address = TEST_EMAIL
620+
self.user.gender = UserGender.FEMALE.value
621+
self.user.save()
622+
user_2 = return_canned_user("user_2", 2222222)
623+
user_2.set_user_role(UserRole.ADMIN)
624+
user_2.create()
625+
user_2_session_token = generate_encoded_token(user_2.id)
626+
# Act
627+
response = self.client.delete(
628+
self.url, headers={"Authorization": user_2_session_token}
629+
)
630+
next_response = self.client.get(
631+
f"/api/v2/users/{TEST_USER_ID}/",
632+
headers={"Authorization": user_2_session_token},
633+
)
634+
# Assert
635+
# Note that we return the deleted user information at this time
636+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
637+
response,
638+
TEST_USER_ID,
639+
TEST_USERNAME,
640+
TEST_EMAIL,
641+
UserGender.FEMALE.name,
642+
False,
643+
)
644+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
645+
next_response, TEST_USER_ID, f"user_{TEST_USER_ID}", None, None, False
646+
)
647+
648+
def test_admin_user_can_remove_redacted_osm_accounts(self):
649+
"""Check that an admin can redact redacted OSM accounts"""
650+
# Arrange
651+
self.user.email_address = TEST_EMAIL
652+
self.user.gender = UserGender.FEMALE.value
653+
self.user.id = 4
654+
self.user.save()
655+
user_2 = return_canned_user("user_2", 2222222)
656+
user_2.set_user_role(UserRole.ADMIN)
657+
user_2.create()
658+
user_2_session_token = generate_encoded_token(user_2.id)
659+
# Act
660+
response = self.client.delete(
661+
"/api/v2/users/", headers={"Authorization": user_2_session_token}
662+
)
663+
next_response = self.client.get(
664+
"/api/v2/users/4/", headers={"Authorization": user_2_session_token}
665+
)
666+
# Assert
667+
self.assertEqual(200, response.status_code)
668+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
669+
next_response, 4, "user_4", None, None, False
670+
)
671+
672+
def test_user_cannot_remove_redacted_osm_accounts(self):
673+
"""Check that a user cannot redact redacted OSM accounts"""
674+
# Arrange
675+
self.user.email_address = TEST_EMAIL
676+
self.user.gender = UserGender.FEMALE.value
677+
self.user.id = 4
678+
self.user.save()
679+
user_2 = return_canned_user("user_2", 2222222)
680+
user_2.set_user_role(UserRole.MAPPER)
681+
user_2.create()
682+
user_2_session_token = generate_encoded_token(user_2.id)
683+
# Act
684+
response = self.client.delete(
685+
"/api/v2/users/", headers={"Authorization": user_2_session_token}
686+
)
687+
next_response = self.client.get(
688+
"/api/v2/users/4/", headers={"Authorization": user_2_session_token}
689+
)
690+
# Assert
691+
self.assertEqual(401, response.status_code)
692+
TestUsersQueriesUsernameAPI.assert_user_detail_response(
693+
next_response, 4, TEST_USERNAME, None, None, False
694+
)

tests/backend/integration/services/users/test_osm_service.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,7 @@ def test_get_osm_details_for_user_returns_user_details_if_valid_user_id(self):
1313
dto = OSMService.get_osm_details_for_user(13526430)
1414
# Assert
1515
self.assertEqual(dto.account_created, "2021-06-10T01:27:18Z")
16+
17+
def test_is_user_deleted(self):
18+
self.assertTrue(OSMService.is_osm_user_gone(535043))
19+
self.assertFalse(OSMService.is_osm_user_gone(2078753))

0 commit comments

Comments
 (0)