From 7e05c8da1052f419602ca0702b79122388b6eb5c Mon Sep 17 00:00:00 2001 From: Alex Zaslavsky Date: Fri, 8 Sep 2023 16:42:34 -0700 Subject: [PATCH] ref(backup): Remove SanitizeUserImportsMixin (#55875) This was redundant code that basically duplicated the functionality of the `__relocation_scope__` we set on each model. What the models that used it really wanted was to be in the global relocation scope, which is now the case. Issue: getsentry/team-ospo#181 --- .../backup/model_dependencies/detailed.json | 8 +++--- src/sentry/backup/mixins.py | 26 ------------------- src/sentry/models/authenticator.py | 7 ++--- src/sentry/models/userpermission.py | 7 ++--- src/sentry/models/userrole.py | 13 ++++++---- 5 files changed, 20 insertions(+), 41 deletions(-) delete mode 100644 src/sentry/backup/mixins.py diff --git a/fixtures/backup/model_dependencies/detailed.json b/fixtures/backup/model_dependencies/detailed.json index a77c028c38ed0a..c58579384a0c80 100644 --- a/fixtures/backup/model_dependencies/detailed.json +++ b/fixtures/backup/model_dependencies/detailed.json @@ -435,7 +435,7 @@ } }, "model": "sentry.Authenticator", - "relocation_scope": "User", + "relocation_scope": "Global", "silos": [ "Control" ] @@ -3305,7 +3305,7 @@ } }, "model": "sentry.UserPermission", - "relocation_scope": "User", + "relocation_scope": "Global", "silos": [ "Control" ] @@ -3338,7 +3338,7 @@ "sentry.UserRole": { "foreign_keys": {}, "model": "sentry.UserRole", - "relocation_scope": "User", + "relocation_scope": "Global", "silos": [ "Control" ] @@ -3355,7 +3355,7 @@ } }, "model": "sentry.UserRoleUser", - "relocation_scope": "User", + "relocation_scope": "Global", "silos": [ "Control" ] diff --git a/src/sentry/backup/mixins.py b/src/sentry/backup/mixins.py deleted file mode 100644 index ac603a670802bd..00000000000000 --- a/src/sentry/backup/mixins.py +++ /dev/null @@ -1,26 +0,0 @@ -from __future__ import annotations - -from typing import Optional, Tuple - -from sentry.backup.dependencies import ImportKind, PrimaryKeyMap -from sentry.backup.helpers import ImportFlags -from sentry.backup.scopes import ImportScope - - -class SanitizeUserImportsMixin: - """ - The only realistic reason to do a `Global`ly-scoped import is when restoring some full-instance - backup to a clean install. In this case, one may want to import so-called "superusers": users - with powerful various instance-wide permissions generally reserved for admins and instance - maintainers. Thus, for security reasons, running this import in any `ImportScope` other than - `Global` will sanitize user imports by ignoring imports of the `Authenticator`, - `UserPermission`, `UserRole`, and `UserRoleUser` models. - """ - - def write_relocation_import( - self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags - ) -> Optional[Tuple[int, int, ImportKind]]: - if scope != ImportScope.Global: - return None - - return super().write_relocation_import(pk_map, scope, flags) # type: ignore[misc] diff --git a/src/sentry/models/authenticator.py b/src/sentry/models/authenticator.py index 64733b2b79298e..22b713ec6ffc3c 100644 --- a/src/sentry/models/authenticator.py +++ b/src/sentry/models/authenticator.py @@ -17,7 +17,6 @@ available_authenticators, ) from sentry.auth.authenticators.base import EnrollmentStatus -from sentry.backup.mixins import SanitizeUserImportsMixin from sentry.backup.scopes import RelocationScope from sentry.db.models import ( BaseManager, @@ -140,8 +139,10 @@ def to_python(self, value): @control_silo_only_model -class Authenticator(SanitizeUserImportsMixin, BaseModel): - __relocation_scope__ = RelocationScope.User +class Authenticator(BaseModel): + # It only makes sense to import/export this data when doing a full global backup/restore, so it + # lives in the `Global` scope, even though it only depends on the `User` model. + __relocation_scope__ = RelocationScope.Global id = BoundedAutoField(primary_key=True) user = FlexibleForeignKey("sentry.User", db_index=True) diff --git a/src/sentry/models/userpermission.py b/src/sentry/models/userpermission.py index 9c83e384300160..f7139c5a270df8 100644 --- a/src/sentry/models/userpermission.py +++ b/src/sentry/models/userpermission.py @@ -2,20 +2,21 @@ from django.db import models -from sentry.backup.mixins import SanitizeUserImportsMixin from sentry.backup.scopes import RelocationScope from sentry.db.models import FlexibleForeignKey, Model, control_silo_only_model, sane_repr @control_silo_only_model -class UserPermission(SanitizeUserImportsMixin, Model): +class UserPermission(Model): """ Permissions are applied to administrative users and control explicit scope-like permissions within the API. Generally speaking, they should only apply to active superuser sessions. """ - __relocation_scope__ = RelocationScope.User + # It only makes sense to import/export this data when doing a full global backup/restore, so it + # lives in the `Global` scope, even though it only depends on the `User` model. + __relocation_scope__ = RelocationScope.Global user = FlexibleForeignKey("sentry.User") # permissions should be in the form of 'service-name.permission-name' diff --git a/src/sentry/models/userrole.py b/src/sentry/models/userrole.py index cdb8f0f6589807..0e17aaede067e5 100644 --- a/src/sentry/models/userrole.py +++ b/src/sentry/models/userrole.py @@ -3,7 +3,6 @@ from django.conf import settings from django.db import models -from sentry.backup.mixins import SanitizeUserImportsMixin from sentry.backup.scopes import RelocationScope from sentry.db.models import ArrayField, DefaultFieldsModel, control_silo_only_model, sane_repr from sentry.db.models.fields.foreignkey import FlexibleForeignKey @@ -12,12 +11,14 @@ @control_silo_only_model -class UserRole(SanitizeUserImportsMixin, DefaultFieldsModel): +class UserRole(DefaultFieldsModel): """ Roles are applied to administrative users and apply a set of `UserPermission`. """ - __relocation_scope__ = RelocationScope.User + # It only makes sense to import/export this data when doing a full global backup/restore, so it + # lives in the `Global` scope, even though it only depends on the `User` model. + __relocation_scope__ = RelocationScope.Global name = models.CharField(max_length=32, unique=True) permissions = ArrayField() @@ -42,8 +43,10 @@ def permissions_for_user(cls, user_id: int) -> FrozenSet[str]: @control_silo_only_model -class UserRoleUser(SanitizeUserImportsMixin, DefaultFieldsModel): - __relocation_scope__ = RelocationScope.User +class UserRoleUser(DefaultFieldsModel): + # It only makes sense to import/export this data when doing a full global backup/restore, so it + # lives in the `Global` scope, even though it only depends on the `User` model. + __relocation_scope__ = RelocationScope.Global user = FlexibleForeignKey("sentry.User") role = FlexibleForeignKey("sentry.UserRole")