From 393b0788746a73d59ed986b4d4fd9384e94be68b Mon Sep 17 00:00:00 2001 From: relaxolotl <5597345+relaxolotl@users.noreply.github.com> Date: Tue, 9 Nov 2021 12:37:16 -0500 Subject: [PATCH] ref(appstore-connect): Make iTunes credentials optional and stop (re)writing them to DB (#29863) --- .../project_app_store_connect_credentials.py | 19 ----- src/sentry/lang/native/appconnect.py | 33 +++++---- src/sentry/lang/native/symbolicator.py | 7 -- tests/sentry/lang/native/test_appconnect.py | 71 +++++-------------- tests/sentry/tasks/test_app_store_connect.py | 6 -- 5 files changed, 35 insertions(+), 101 deletions(-) diff --git a/src/sentry/api/endpoints/project_app_store_connect_credentials.py b/src/sentry/api/endpoints/project_app_store_connect_credentials.py index 7908c1270fdb68..bcdb418bcafd9d 100644 --- a/src/sentry/api/endpoints/project_app_store_connect_credentials.py +++ b/src/sentry/api/endpoints/project_app_store_connect_credentials.py @@ -27,7 +27,6 @@ validates and includes the status of API credentials associated with this app. See :class:`AppStoreConnectCredentialsValidateEndpoint`. """ -import datetime import logging from typing import Dict, Optional, Union from uuid import uuid4 @@ -210,15 +209,6 @@ def post(self, request: Request, project: Project) -> Response: config["id"] = uuid4().hex config["name"] = config["appName"] - # TODO(itunes): Deprecated fields. Needs to be removed alongside a migration, so this uses - # placeholders as a temporary workaround until that migration happens. - config["itunesCreated"] = datetime.datetime.now() - config["itunesSession"] = "deprecated-field-do-not-use" - config["orgPublicId"] = "deprecated-field-please-do-not-use--" - config["orgName"] = "deprecated-field-do-not-use" - config["itunesUser"] = "deprecated-field-do-not-use" - config["itunesPassword"] = "deprecated-field-do-not-use" - try: validated_config = appconnect.AppStoreConnectConfig.from_json(config) except ValueError: @@ -295,15 +285,6 @@ def post(self, request: Request, project: Project, credentials_id: str) -> Respo except KeyError: return Response(status=404) - # Deprecated fields. Needs to be removed alongside a migration, so this uses - # placeholders as a temporary workaround until that migration happens. - data["itunesCreated"] = datetime.datetime.now() - data["itunesSession"] = "deprecated-field-do-not-use" - data["orgPublicId"] = "deprecated-field-please-do-not-use--" - data["orgName"] = "deprecated-field-do-not-use" - data["itunesUser"] = "deprecated-field-do-not-use" - data["itunesPassword"] = "deprecated-field-do-not-use" - # Any secrets set to None during validation are meant to be no-ops, so remove them to avoid # erasing the existing values for secret in secret_fields(symbol_source_config.type): diff --git a/src/sentry/lang/native/appconnect.py b/src/sentry/lang/native/appconnect.py index a5e9609d32b729..08ea6defe85efb 100644 --- a/src/sentry/lang/native/appconnect.py +++ b/src/sentry/lang/native/appconnect.py @@ -7,10 +7,8 @@ import dataclasses import logging import pathlib -from datetime import datetime from typing import Any, Dict, List -import dateutil import jsonschema import requests import sentry_sdk @@ -53,6 +51,17 @@ class NoDsymsError(Exception): pass +# TODO(itunes): Remove when the fields are removed from DB +DEPRECATED_FIELDS = [ + "itunesUser", + "itunesCreated", + "itunesPassword", + "itunesSession", + "orgPublicId", + "orgName", +] + + @dataclasses.dataclass(frozen=True) class AppStoreConnectConfig: """The symbol source configuration for an App Store Connect source. @@ -93,14 +102,6 @@ class AppStoreConnectConfig: # This is guaranteed to be unique and should map 1:1 to ``appId``. bundleId: str - # TODO(itunes): Deprecated fields. These must be removed alongside a migration. - itunesUser: str - itunesPassword: str - itunesSession: str - itunesCreated: datetime - orgPublicId: str - orgName: str - def __post_init__(self) -> None: # All fields are required. for field in dataclasses.fields(self): @@ -120,13 +121,13 @@ def from_json(cls, data: Dict[str, Any]) -> "AppStoreConnectConfig": symbol source configuration. """ # TODO(itunes): Remove logic related to iTunes fields when the fields are removed - if isinstance(data["itunesCreated"], datetime): - data["itunesCreated"] = data["itunesCreated"].isoformat() + for field in DEPRECATED_FIELDS: + if field in data: + del data[field] try: jsonschema.validate(data, APP_STORE_CONNECT_SCHEMA) except jsonschema.exceptions.ValidationError as e: raise InvalidConfigError from e - data["itunesCreated"] = dateutil.parser.isoparse(data["itunesCreated"]) return cls(**data) @classmethod @@ -177,9 +178,6 @@ def to_json(self) -> Dict[str, Any]: data = dict() for field in dataclasses.fields(self): value = getattr(self, field.name) - # TODO(itunes): Remove logic related to iTunes fields when the fields are removed - if field.name == "itunesCreated": - value = value.isoformat() data[field.name] = value try: jsonschema.validate(data, APP_STORE_CONNECT_SCHEMA) @@ -197,7 +195,8 @@ def to_redacted_json(self) -> Dict[str, Any]: """ data = self.to_json() for to_redact in secret_fields("appStoreConnect"): - data[to_redact] = {"hidden-secret": True} + if to_redact in data: + data[to_redact] = {"hidden-secret": True} return data def update_project_symbol_source(self, project: Project, allow_multiple: bool) -> json.JSONData: diff --git a/src/sentry/lang/native/symbolicator.py b/src/sentry/lang/native/symbolicator.py index faca3953cf7a10..f8e60e03029f9b 100644 --- a/src/sentry/lang/native/symbolicator.py +++ b/src/sentry/lang/native/symbolicator.py @@ -80,13 +80,6 @@ "appName", "appId", "bundleId", - # TODO(itunes): All of the below fields are deprecated. Remove together with migration. - "itunesUser", - "itunesCreated", - "itunesPassword", - "itunesSession", - "orgPublicId", - "orgName", ], "additionalProperties": False, } diff --git a/tests/sentry/lang/native/test_appconnect.py b/tests/sentry/lang/native/test_appconnect.py index 4c6e84bd20987c..82aa6ebbbb23a8 100644 --- a/tests/sentry/lang/native/test_appconnect.py +++ b/tests/sentry/lang/native/test_appconnect.py @@ -31,15 +31,9 @@ def data(self, now: datetime) -> json.JSONData: "appconnectIssuer": "abc123" * 6, "appconnectKey": "abc123", "appconnectPrivateKey": "---- BEGIN PRIVATE KEY ---- ABC123...", - "itunesUser": "someone@example.com", - "itunesPassword": "a secret", - "itunesSession": "ABC123", - "itunesCreated": now.isoformat(), "appName": "Sample Application", "appId": "1234", "bundleId": "com.example.app", - "orgPublicId": "71105f98-7743-4844-ab70-2c901e2ea13d", - "orgName": "Example Organisation", } def test_from_json_basic(self, data: json.JSONData, now: datetime) -> None: @@ -49,45 +43,38 @@ def test_from_json_basic(self, data: json.JSONData, now: datetime) -> None: assert config.name == data["name"] assert config.appconnectIssuer == data["appconnectIssuer"] assert config.appconnectPrivateKey == data["appconnectPrivateKey"] - assert config.itunesUser == data["itunesUser"] - assert config.itunesPassword == data["itunesPassword"] - assert config.itunesSession == data["itunesSession"] - assert config.itunesCreated == now assert config.appName == data["appName"] assert config.bundleId == data["bundleId"] - assert config.orgPublicId == data["orgPublicId"] - assert config.orgName == data["orgName"] - - def test_from_json_isoformat(self, data: json.JSONData, now: datetime) -> None: - data["itunesCreated"] = now.isoformat() - config = appconnect.AppStoreConnectConfig.from_json(data) - assert config.itunesCreated == now - - def test_from_json_datetime(self, data: json.JSONData, now: datetime) -> None: - data["itunesCreated"] = now - config = appconnect.AppStoreConnectConfig.from_json(data) - assert config.itunesCreated == now def test_to_json(self, data: json.JSONData, now: datetime) -> None: config = appconnect.AppStoreConnectConfig.from_json(data) new_data = config.to_json() - # Fixup our input to expected JSON format - data["itunesCreated"] = now.isoformat() - assert new_data == data def test_to_redacted_json(self, data: json.JSONData, now: datetime) -> None: config = appconnect.AppStoreConnectConfig.from_json(data) new_data = config.to_redacted_json() - # Fixup our input to expected JSON format - data["itunesCreated"] = now.isoformat() + # Redacted secrets + data["appconnectPrivateKey"] = {"hidden-secret": True} + assert "itunesPassword" not in data + assert "itunesSession" not in data + + assert new_data == data + + def test_to_redacted_json_with_deprecated(self, data: json.JSONData, now: datetime) -> None: + data_with_deprecated = data + data_with_deprecated["itunesPassword"] = "honk" + data_with_deprecated["itunesSession"] = "beep" + + config = appconnect.AppStoreConnectConfig.from_json(data) + new_data = config.to_redacted_json() # Redacted secrets data["appconnectPrivateKey"] = {"hidden-secret": True} - data["itunesPassword"] = {"hidden-secret": True} - data["itunesSession"] = {"hidden-secret": True} + assert "itunesPassword" not in data + assert "itunesSession" not in data assert new_data == data @@ -110,15 +97,9 @@ def config(self) -> appconnect.AppStoreConnectConfig: appconnectIssuer="abc123" * 6, appconnectKey="abc123key", appconnectPrivateKey="----BEGIN PRIVATE KEY---- blabla", - itunesUser="me@example.com", - itunesPassword="secret", - itunesSession="THE-COOKIE", - itunesCreated=datetime.utcnow(), appName="My App", appId="123", bundleId="com.example.app", - orgPublicId="71105f98-7743-4844-ab70-2c901e2ea13d", - orgName="Example Com", ) @pytest.mark.django_db # type: ignore @@ -156,7 +137,6 @@ def test_new_sources_with_existing( new_sources.append(cfg.to_json()) assert stored_sources == new_sources - # TODO(itunes): Update when iTunes fields are removed @pytest.mark.django_db # type: ignore def test_update( self, default_project: "Project", config: appconnect.AppStoreConnectConfig @@ -169,24 +149,17 @@ def test_update( name=config.name, appconnectIssuer=config.appconnectIssuer, appconnectKey=config.appconnectKey, - appconnectPrivateKey=config.appconnectPrivateKey, - itunesUser=config.itunesUser, - itunesPassword=config.itunesPassword, - itunesSession="A NEW COOKIE", - itunesCreated=datetime.utcnow(), + appconnectPrivateKey="A NEW KEY", appName=config.appName, appId=config.appId, bundleId=config.bundleId, - orgPublicId=config.orgPublicId, - orgName=config.orgName, ) updated.update_project_symbol_source(default_project, allow_multiple=False) current = appconnect.AppStoreConnectConfig.from_project_config(default_project, config.id) - assert current.itunesSession == "A NEW COOKIE" + assert current.appconnectPrivateKey == "A NEW KEY" - # TODO(itunes): Update when iTunes fields are removed @pytest.mark.django_db # type: ignore def test_update_no_matching_id( self, default_project: "Project", config: appconnect.AppStoreConnectConfig @@ -199,16 +172,10 @@ def test_update_no_matching_id( name=config.name, appconnectIssuer=config.appconnectIssuer, appconnectKey=config.appconnectKey, - appconnectPrivateKey=config.appconnectPrivateKey, - itunesUser=config.itunesUser, - itunesPassword=config.itunesPassword, - itunesSession="A NEW COOKIE", - itunesCreated=datetime.utcnow(), + appconnectPrivateKey="A NEW KEY", appName=config.appName, appId=config.appId, bundleId=config.bundleId, - orgPublicId=config.orgPublicId, - orgName=config.orgName, ) with pytest.raises(ValueError): diff --git a/tests/sentry/tasks/test_app_store_connect.py b/tests/sentry/tasks/test_app_store_connect.py index c19f6ba701a499..388b96f20f5e6b 100644 --- a/tests/sentry/tasks/test_app_store_connect.py +++ b/tests/sentry/tasks/test_app_store_connect.py @@ -20,15 +20,9 @@ def config(self): appconnectIssuer="abc123" * 6, appconnectKey="abc123key", appconnectPrivateKey="----BEGIN PRIVATE KEY---- blabla", - itunesUser="me@example.com", - itunesPassword="secret", - itunesSession="THE-COOKIE", - itunesCreated=datetime.utcnow(), appName="My App", appId="123", bundleId="com.example.app", - orgPublicId="71105f98-7743-4844-ab70-2c901e2ea13d", - orgName="Example Com", ) @pytest.fixture