diff --git a/src/sentry/analytics/events/sentry_app_installation_updated.py b/src/sentry/analytics/events/sentry_app_installation_updated.py new file mode 100644 index 00000000000000..19c6fb8345d187 --- /dev/null +++ b/src/sentry/analytics/events/sentry_app_installation_updated.py @@ -0,0 +1,16 @@ +from __future__ import absolute_import + +from sentry import analytics + + +class SentryAppInstallationUpdatedEvent(analytics.Event): + type = 'sentry_app_installation.updated' + + attributes = ( + analytics.Attribute('sentry_app_installation_id'), + analytics.Attribute('sentry_app_id'), + analytics.Attribute('organization_id'), + ) + + +analytics.register(SentryAppInstallationUpdatedEvent) diff --git a/src/sentry/api/bases/sentryapps.py b/src/sentry/api/bases/sentryapps.py index 23aa22bd887e31..ef443b4f5465c7 100644 --- a/src/sentry/api/bases/sentryapps.py +++ b/src/sentry/api/bases/sentryapps.py @@ -227,6 +227,12 @@ class SentryAppInstallationPermission(SentryPermission): 'POST': ('org:integrations', 'event:write', 'event:admin'), } + def has_permission(self, request, *args, **kwargs): + # To let the app mark the installation as installed, we don't care about permissions + if request.user.is_sentry_app and request.method == 'PUT': + return True + return super(SentryAppInstallationPermission, self).has_permission(request, *args, **kwargs) + def has_object_permission(self, request, view, installation): if not hasattr(request, 'user') or not request.user: return False @@ -236,6 +242,10 @@ def has_object_permission(self, request, view, installation): if is_active_superuser(request): return True + # if user is an app, make sure it's for that same app + if request.user.is_sentry_app: + return request.user == installation.sentry_app.proxy_user + if installation.organization not in request.user.get_orgs(): raise Http404 diff --git a/src/sentry/api/endpoints/sentry_app_installation_details.py b/src/sentry/api/endpoints/sentry_app_installation_details.py index f580e71baa0e95..699d6de38fd225 100644 --- a/src/sentry/api/endpoints/sentry_app_installation_details.py +++ b/src/sentry/api/endpoints/sentry_app_installation_details.py @@ -4,7 +4,8 @@ from sentry.api.bases import SentryAppInstallationBaseEndpoint from sentry.api.serializers import serialize -from sentry.mediators.sentry_app_installations import Destroyer +from sentry.mediators.sentry_app_installations import Destroyer, Updater +from sentry.api.serializers.rest_framework import SentryAppInstallationSerializer class SentryAppInstallationDetailsEndpoint(SentryAppInstallationBaseEndpoint): @@ -19,3 +20,22 @@ def delete(self, request, installation): request=request, ) return Response(status=204) + + def put(self, request, installation): + serializer = SentryAppInstallationSerializer( + installation, + data=request.data, + partial=True, + ) + + if serializer.is_valid(): + result = serializer.validated_data + + updated_installation = Updater.run( + user=request.user, + sentry_app_installation=installation, + status=result.get('status'), + ) + + return Response(serialize(updated_installation, request.user)) + return Response(serializer.errors, status=400) diff --git a/src/sentry/api/serializers/models/sentry_app_installation.py b/src/sentry/api/serializers/models/sentry_app_installation.py index b26c911698837a..dda961969c0e96 100644 --- a/src/sentry/api/serializers/models/sentry_app_installation.py +++ b/src/sentry/api/serializers/models/sentry_app_installation.py @@ -3,6 +3,7 @@ from sentry.api.serializers import Serializer, register from sentry.models import SentryAppInstallation +from sentry.constants import SentryAppInstallationStatus @register(SentryAppInstallation) @@ -17,6 +18,7 @@ def serialize(self, install, attrs, user): 'slug': install.organization.slug, }, 'uuid': install.uuid, + 'status': SentryAppInstallationStatus.as_str(install.status), } if install.api_grant: diff --git a/src/sentry/api/serializers/rest_framework/sentry_app_installation.py b/src/sentry/api/serializers/rest_framework/sentry_app_installation.py new file mode 100644 index 00000000000000..97c798ac126112 --- /dev/null +++ b/src/sentry/api/serializers/rest_framework/sentry_app_installation.py @@ -0,0 +1,19 @@ +from __future__ import absolute_import + + +from rest_framework import serializers +from rest_framework.serializers import Serializer, ValidationError +from sentry.constants import SentryAppInstallationStatus + + +class SentryAppInstallationSerializer(Serializer): + status = serializers.CharField() + + def validate_status(self, new_status): + # can only set status to installed + if new_status != SentryAppInstallationStatus.INSTALLED_STR: + raise ValidationError( + u"Invalid value '{}' for status. Valid values: '{}'".format( + new_status, SentryAppInstallationStatus.INSTALLED_STR)) + + return new_status diff --git a/src/sentry/constants.py b/src/sentry/constants.py index afd7a91f701ffa..dcddf7da54892c 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -402,20 +402,22 @@ def as_str(cls, status): class SentryAppInstallationStatus(object): PENDING = 0 INSTALLED = 1 + PENDING_STR = 'pending' + INSTALLED_STR = 'installed' @classmethod def as_choices(cls): return ( - (cls.PENDING, 'pending'), - (cls.INSTALLED, 'installed'), + (cls.PENDING, cls.PENDING_STR), + (cls.INSTALLED, cls.INSTALLED_STR), ) @classmethod def as_str(cls, status): if status == cls.PENDING: - return 'pending' + return cls.PENDING_STR elif status == cls.INSTALLED: - return 'installed' + return cls.INSTALLED_STR StatsPeriod = namedtuple('StatsPeriod', ('segments', 'interval')) diff --git a/src/sentry/mediators/sentry_app_installations/__init__.py b/src/sentry/mediators/sentry_app_installations/__init__.py index cd76833604be95..d9c9ab597fc3a1 100644 --- a/src/sentry/mediators/sentry_app_installations/__init__.py +++ b/src/sentry/mediators/sentry_app_installations/__init__.py @@ -2,4 +2,5 @@ from .creator import Creator # NOQA from .destroyer import Destroyer # NOQA +from .updater import Updater # NOQA from .installation_notifier import InstallationNotifier # NOQA diff --git a/src/sentry/mediators/sentry_app_installations/updater.py b/src/sentry/mediators/sentry_app_installations/updater.py new file mode 100644 index 00000000000000..8895ef7a9aeada --- /dev/null +++ b/src/sentry/mediators/sentry_app_installations/updater.py @@ -0,0 +1,34 @@ +from __future__ import absolute_import + +import six + + +from sentry import analytics +from sentry.constants import SentryAppInstallationStatus +from sentry.mediators import Mediator, Param +from sentry.mediators.param import if_param + + +class Updater(Mediator): + sentry_app_installation = Param('sentry.models.SentryAppInstallation') + status = Param(six.string_types, required=False) + + def call(self): + self._update_status() + self.sentry_app_installation.save() + return self.sentry_app_installation + + @if_param('status') + def _update_status(self): + # convert from string to integer + if self.status == SentryAppInstallationStatus.INSTALLED_STR: + self.sentry_app_installation.status = SentryAppInstallationStatus.INSTALLED + + def record_analytics(self): + # TODO: Add analytics + analytics.record( + 'sentry_app_installation.updated', + sentry_app_installation_id=self.sentry_app_installation.id, + sentry_app_id=self.sentry_app_installation.sentry_app.id, + organization_id=self.sentry_app_installation.organization.id + ) diff --git a/tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py b/tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py index 8ddbb7ae44c61c..4a61d1e5e97103 100644 --- a/tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py +++ b/tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py @@ -1,9 +1,11 @@ from __future__ import absolute_import from django.core.urlresolvers import reverse +from mock import patch from sentry.testutils import APITestCase import responses +from sentry.mediators.token_exchange import GrantExchanger class SentryAppInstallationDetailsTest(APITestCase): @@ -17,6 +19,7 @@ def setUp(self): name='Test', organization=self.super_org, published=True, + scopes=('org:write', 'team:admin'), ) self.installation = self.create_sentry_app_installation( @@ -58,6 +61,7 @@ def test_access_within_installs_organization(self): }, 'uuid': self.installation2.uuid, 'code': self.installation2.api_grant.code, + 'status': 'pending' } def test_no_access_outside_install_organization(self): @@ -95,3 +99,69 @@ def test_member_cannot_delete_install(self): response = self.client.delete(self.url, format='json') assert response.status_code == 403 + + +class MarkInstalledSentryAppInstallationsTest(SentryAppInstallationDetailsTest): + def setUp(self): + super(MarkInstalledSentryAppInstallationsTest, self).setUp() + self.token = GrantExchanger.run( + install=self.installation, + code=self.installation.api_grant.code, + client_id=self.published_app.application.client_id, + user=self.published_app.proxy_user, + ) + + @patch('sentry.analytics.record') + def test_sentry_app_installation_mark_installed(self, record): + self.url = reverse( + 'sentry-api-0-sentry-app-installation-details', + args=[self.installation.uuid], + ) + response = self.client.put( + self.url, + data={'status': 'installed'}, + HTTP_AUTHORIZATION=u'Bearer {}'.format(self.token.token), + format='json', + ) + assert response.status_code == 200 + assert response.data['status'] == 'installed' + + record.assert_called_with( + 'sentry_app_installation.updated', + sentry_app_installation_id=self.installation.id, + sentry_app_id=self.installation.sentry_app.id, + organization_id=self.installation.organization.id + ) + + def test_sentry_app_installation_mark_pending_status(self): + self.url = reverse( + 'sentry-api-0-sentry-app-installation-details', + args=[self.installation.uuid], + ) + response = self.client.put( + self.url, + data={'status': 'pending'}, + HTTP_AUTHORIZATION=u'Bearer {}'.format(self.token.token), + format='json', + ) + assert response.status_code == 400 + assert response.data['status'][0] == u"Invalid value 'pending' for status. Valid values: 'installed'" + + def test_sentry_app_installation_mark_installed_wrong_app(self): + self.token = GrantExchanger.run( + install=self.installation2, + code=self.installation2.api_grant.code, + client_id=self.unpublished_app.application.client_id, + user=self.unpublished_app.proxy_user, + ) + self.url = reverse( + 'sentry-api-0-sentry-app-installation-details', + args=[self.installation.uuid], + ) + response = self.client.put( + self.url, + data={'status': 'installed'}, + HTTP_AUTHORIZATION=u'Bearer {}'.format(self.token.token), + format='json', + ) + assert response.status_code == 403 diff --git a/tests/sentry/api/endpoints/test_organization_sentry_app_installations.py b/tests/sentry/api/endpoints/test_organization_sentry_app_installations.py index 7f5be4fbdbb491..05af48e586756e 100644 --- a/tests/sentry/api/endpoints/test_organization_sentry_app_installations.py +++ b/tests/sentry/api/endpoints/test_organization_sentry_app_installations.py @@ -58,6 +58,7 @@ def test_superuser_sees_all_installs(self): }, 'uuid': self.installation2.uuid, 'code': self.installation2.api_grant.code, + 'status': 'pending', }] url = reverse( @@ -78,6 +79,7 @@ def test_superuser_sees_all_installs(self): }, 'uuid': self.installation.uuid, 'code': self.installation.api_grant.code, + 'status': 'pending' }] def test_users_only_sees_installs_on_their_org(self): @@ -95,6 +97,7 @@ def test_users_only_sees_installs_on_their_org(self): }, 'uuid': self.installation2.uuid, 'code': self.installation2.api_grant.code, + 'status': 'pending', }] # Org the User is not a part of diff --git a/tests/sentry/mediators/sentry_app_installations/test_installation_notifier.py b/tests/sentry/mediators/sentry_app_installations/test_installation_notifier.py index 27be975ce224e8..a1aed14014e00d 100644 --- a/tests/sentry/mediators/sentry_app_installations/test_installation_notifier.py +++ b/tests/sentry/mediators/sentry_app_installations/test_installation_notifier.py @@ -63,6 +63,7 @@ def test_task_enqueued(self, safe_urlopen): }, 'uuid': self.install.uuid, 'code': self.install.api_grant.code, + 'status': 'pending' }, }, 'actor': { @@ -106,6 +107,7 @@ def test_uninstallation_enqueued(self, safe_urlopen): }, 'uuid': self.install.uuid, 'code': self.install.api_grant.code, + 'status': 'pending' }, }, 'actor': {