From 00a5dda89c8320cccb8bc25b486c17999a899fea Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 10 Apr 2024 14:18:36 -0600 Subject: [PATCH] backend: Add ability to delete a user Signed-off-by: Taylor Smock --- backend/api/users/resources.py | 44 +++++++++++++ backend/models/postgis/application.py | 6 ++ backend/models/postgis/message.py | 4 +- backend/services/users/osm_service.py | 21 ++++++ backend/services/users/user_service.py | 56 +++++++++++++++- .../services/users/test_user_service.py | 64 +++++++++++++++++++ 6 files changed, 192 insertions(+), 3 deletions(-) diff --git a/backend/api/users/resources.py b/backend/api/users/resources.py index ab08d78fd9..df27cdc2c3 100644 --- a/backend/api/users/resources.py +++ b/backend/api/users/resources.py @@ -1,11 +1,16 @@ from distutils.util import strtobool +from typing import Optional + +from flask import stream_with_context, Response from flask_restful import Resource, current_app, request from schematics.exceptions import DataError from backend.models.dtos.user_dto import UserSearchQuery +from backend.models.postgis.user import User from backend.services.users.authentication_service import token_auth from backend.services.users.user_service import UserService from backend.services.project_service import ProjectService +from backend.services.users.osm_service import OSMService class UsersRestAPI(Resource): @@ -44,6 +49,27 @@ def get(self, user_id): user_dto = UserService.get_user_dto_by_id(user_id, token_auth.current_user()) return user_dto.to_primitive(), 200 + @token_auth.login_required + def delete(self, user_id: Optional[int] = None): + """ + Delete user information by id. + :param user_id: The user to delete + :return: RFC7464 compliant sequence of user objects deleted + 200: User deleted + 401: Unauthorized - Invalid credentials + 404: User not found + 500: Internal Server Error + """ + if user_id == token_auth.current_user() or UserService.is_user_an_admin( + token_auth.current_user() + ): + return ( + UserService.delete_user_by_id( + user_id, token_auth.current_user() + ).to_primitive(), + 200, + ) + class UsersAllAPI(Resource): @token_auth.login_required @@ -115,6 +141,24 @@ def get(self): users_dto = UserService.get_all_users(query) return users_dto.to_primitive(), 200 + @token_auth.login_required + def delete(self): + if UserService.is_user_an_admin(token_auth.current_user()): + + def delete_users(): + for user in User.get_all_users_not_paginated(): + # We specifically want to remove users that have deleted their OSM accounts. + if OSMService.is_osm_user_gone(user.id): + data = UserService.delete_user_by_id( + user.id, token_auth.current_user() + ).to_primitive() + yield f"\u001e{data}\n" + + return Response( + stream_with_context(delete_users()), + headers={"Content-Type": "application/json-seq"}, + ) + class UsersQueriesUsernameAPI(Resource): @token_auth.login_required diff --git a/backend/models/postgis/application.py b/backend/models/postgis/application.py index 75fb8c913d..b0d7cf9712 100644 --- a/backend/models/postgis/application.py +++ b/backend/models/postgis/application.py @@ -60,6 +60,12 @@ def get_all_for_user(user: int): applications_dto.applications.append(application_dto) return applications_dto + @staticmethod + def delete_all_for_user(user: int): + for r in db.session.query(Application).filter(Application.user == user): + db.session.delete(r) + db.session.commit() + def as_dto(self): app_dto = ApplicationDTO() app_dto.user = self.user diff --git a/backend/models/postgis/message.py b/backend/models/postgis/message.py index 32516d5a57..5bc4ad88b9 100644 --- a/backend/models/postgis/message.py +++ b/backend/models/postgis/message.py @@ -1,3 +1,5 @@ +from typing import List + from sqlalchemy.sql.expression import false from backend import db @@ -178,7 +180,7 @@ def delete_multiple_messages(message_ids: list, user_id: int): db.session.commit() @staticmethod - def delete_all_messages(user_id: int, message_type_filters: list = None): + def delete_all_messages(user_id: int, message_type_filters: List[int] = None): """Deletes all messages to the user ----------------------------------- :param user_id: user id of the user whose messages are to be deleted diff --git a/backend/services/users/osm_service.py b/backend/services/users/osm_service.py index 33feb7ae17..87d57ff8cf 100644 --- a/backend/services/users/osm_service.py +++ b/backend/services/users/osm_service.py @@ -13,6 +13,25 @@ def __init__(self, message): class OSMService: + @staticmethod + def is_osm_user_gone(user_id: int) -> bool: + """ + Check if OSM details for the user from OSM API are available + :param user_id: user_id in scope + :raises OSMServiceError + """ + osm_user_details_url = ( + f"{current_app.config['OSM_SERVER_URL']}/api/0.6/user/{user_id}.json" + ) + response = requests.head(osm_user_details_url) + + if response.status_code == 410: + return True + if response.status_code != 200: + raise OSMServiceError("Bad response from OSM") + + return False + @staticmethod def get_osm_details_for_user(user_id: int) -> UserOSMDTO: """ @@ -25,6 +44,8 @@ def get_osm_details_for_user(user_id: int) -> UserOSMDTO: ) response = requests.get(osm_user_details_url) + if response.status_code == 410: + raise OSMServiceError("User no longer exists on OSM") if response.status_code != 200: raise OSMServiceError("Bad response from OSM") diff --git a/backend/services/users/user_service.py b/backend/services/users/user_service.py index 8324cd1477..48c23990f0 100644 --- a/backend/services/users/user_service.py +++ b/backend/services/users/user_service.py @@ -1,3 +1,5 @@ +from typing import Optional + from cachetools import TTLCache, cached from flask import current_app import datetime @@ -27,14 +29,14 @@ from backend.models.postgis.task import TaskHistory, TaskAction, Task from backend.models.dtos.user_dto import UserTaskDTOs from backend.models.dtos.stats_dto import Pagination +from backend.models.postgis.project_chat import ProjectChat from backend.models.postgis.statuses import TaskStatus, ProjectStatus -from backend.services.users.osm_service import OSMService, OSMServiceError from backend.services.messaging.smtp_service import SMTPService from backend.services.messaging.template_service import ( get_txt_template, template_var_replacing, ) - +from backend.services.users.osm_service import OSMService, OSMServiceError user_filter_cache = TTLCache(maxsize=1024, ttl=600) @@ -190,6 +192,56 @@ def get_user_dto_by_id(user: int, request_user: int) -> UserDTO: return user.as_dto(request_username) return user.as_dto() + @staticmethod + def delete_user_by_id(user_id: int, request_user_id: int) -> Optional[UserDTO]: + if user_id == request_user_id or UserService.is_user_an_admin(request_user_id): + user = User.get_by_id(user_id) + original_dto = UserService.get_user_dto_by_id(user_id, request_user_id) + user.accepted_licenses = [] + user.city = None + user.country = None + user.email_address = None + user.facebook_id = None + user.gender = None + user.interests = [] + user.irc_id = None + user.is_email_verified = False + user.is_expert = False + user.linkedin_id = None + user.name = None + user.picture_url = None + user.self_description_gender = None + user.skype_id = None + user.slack_id = None + user.twitter_id = None + # FIXME: Should we keep user_id since that will make conversations easier to follow? + # Keep in mind that OSM uses user_ on deleted accounts. + user.username = f"user_{user_id}" + + # Remove permissions from admin users, keep role for blocked users. + if UserService.is_user_an_admin(user_id): + user.set_user_role(UserRole.MAPPER) + user.save() + + # Remove messages that might contain user identifying information. + for message in ProjectChat.query.filter_by(user_id=user_id): + # TODO detect image links and try to delete them + message.message = f"[Deleted user_{user_id} message]" + db.session.commit() + + # Drop application keys + from backend.models.postgis.application import Application + + Application.delete_all_for_user(user_id) + + # Delete all messages (AKA notifications) for the user + Message.delete_all_messages( + user_id, [message_type.value for message_type in MessageType] + ) + # Leave interests, licenses, organizations, and tasks alone for now. + return original_dto + return None + @staticmethod def get_interests_stats(user_id): # Get all projects that the user has contributed. diff --git a/tests/backend/integration/services/users/test_user_service.py b/tests/backend/integration/services/users/test_user_service.py index 3857625a84..45980032e1 100644 --- a/tests/backend/integration/services/users/test_user_service.py +++ b/tests/backend/integration/services/users/test_user_service.py @@ -2,6 +2,7 @@ from tests.backend.base import BaseTestCase from backend.models.postgis.message import Message +from backend.models.postgis.statuses import UserRole, UserGender from backend.services.users.user_service import ( UserService, MappingLevel, @@ -97,3 +98,66 @@ def test_register_user_creates_new_user(self): # Assert self.assertEqual(expected_user.username, test_user.username) self.assertEqual(expected_user.mapping_level, MappingLevel.INTERMEDIATE.value) + + def add_user_identifying_information(self, user: User) -> User: + user.username = "Test User" + user.email_address = "test@example.com" + user.twitter_id = "@Test" + user.facebook_id = "@FBTest" + user.linkedin_id = "@LinkedIn" + user.slack_id = "@Slack" + user.skype_id = "@Skype" + user.irc_id = "IRC" + user.name = "Some name here" + user.city = "Some city" + user.country = "Some country" + user.picture_url = "https://test.com/path/to/picture.png" + user.gender = UserGender.MALE.value + user.self_description_gender = "I am male" + user.default_editor = "ID" + user.save() + return user + + def check_user_details_deleted(self, user: User, deleted: bool): + if deleted: + check = self.assertIsNone + self.assertNotEquals(UserRole.ADMIN.value, user.role) + self.assertEqual(f"user_{user.id}", user.username) + else: + self.assertNotEquals(f"user_{user.id}", user.username) + check = self.assertIsNotNone + check(user.email_address) + check(user.twitter_id) + check(user.facebook_id) + check(user.linkedin_id) + check(user.slack_id) + check(user.skype_id) + check(user.irc_id) + check(user.name) + check(user.city) + check(user.country) + check(user.picture_url) + check(user.gender) + check(user.self_description_gender) + self.assertEqual([], user.accepted_licenses) + self.assertEqual([], user.interests) + + def test_delete_user_same_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + UserService.delete_user_by_id(test_user.id, test_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=True) + + def test_delete_user_different_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + other_user = return_canned_user("someone", test_user.id + 1) + other_user.create() + UserService.delete_user_by_id(test_user.id, other_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=False) + + def test_delete_user_different_admin_user(self): + test_user = self.add_user_identifying_information(create_canned_user()) + other_user = return_canned_user("someone", test_user.id + 1) + other_user.set_user_role(UserRole.ADMIN) + other_user.create() + UserService.delete_user_by_id(test_user.id, other_user.id) + self.check_user_details_deleted(User().get_by_id(test_user.id), deleted=True)