Browse Source

feat(hybridcloud) Update unsubscribe links to use react views (#59350)

Implement new signed link generation that generates links that contain
either a customer domain path, or a slug based path. The signature data
is for the API endpoint used by the react views.

I've used an option so that I can test this in the test-silos before
enabling it in saas.
Mark Story 1 year ago
parent
commit
db60e099da

+ 18 - 8
src/sentry/mail/notifications.py

@@ -17,7 +17,7 @@ from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
 from sentry.utils.email import MessageBuilder, group_id_to_email
-from sentry.utils.linksign import generate_signed_link
+from sentry.utils.linksign import generate_signed_link, generate_signed_unsubscribe_link
 
 logger = logging.getLogger(__name__)
 
@@ -62,13 +62,23 @@ def get_subject_with_prefix(
 
 
 def get_unsubscribe_link(user_id: int, data: UnsubscribeContext) -> str:
-    signed_link: str = generate_signed_link(
-        user_id,
-        f"sentry-account-email-unsubscribe-{data.key}",
-        data.referrer,
-        kwargs={f"{data.key}_id": data.resource_id},
-    )
-    return signed_link
+    use_react = options.get("unsubscribe_link.use_react_views")
+    if use_react:
+        return generate_signed_unsubscribe_link(
+            organization=data.organization,
+            user_id=user_id,
+            resource=data.key,
+            referrer=data.referrer,
+            resource_id=data.resource_id,
+        )
+
+    else:
+        return generate_signed_link(
+            user_id,
+            f"sentry-account-email-unsubscribe-{data.key}",
+            data.referrer,
+            kwargs={f"{data.key}_id": data.resource_id},
+        )
 
 
 def _log_message(notification: BaseNotification, recipient: RpcActor) -> None:

+ 1 - 0
src/sentry/notifications/notifications/activity/base.py

@@ -108,6 +108,7 @@ class GroupActivityNotification(ActivityNotification, abc.ABC):
 
     def get_unsubscribe_key(self) -> UnsubscribeContext | None:
         return UnsubscribeContext(
+            organization=self.group.organization,
             key="issue",
             resource_id=self.group.id,
         )

+ 4 - 1
src/sentry/notifications/notifications/digest.py

@@ -63,7 +63,10 @@ class DigestNotification(ProjectNotification):
 
     def get_unsubscribe_key(self) -> UnsubscribeContext | None:
         return UnsubscribeContext(
-            key="project", resource_id=self.project.id, referrer="alert_digest"
+            organization=self.project.organization,
+            key="project",
+            resource_id=self.project.id,
+            referrer="alert_digest",
         )
 
     def get_subject(self, context: Mapping[str, Any] | None = None) -> str:

+ 5 - 1
src/sentry/notifications/types.py

@@ -2,10 +2,13 @@ from __future__ import annotations
 
 from dataclasses import dataclass
 from enum import Enum
-from typing import Optional
+from typing import TYPE_CHECKING, Optional
 
 from sentry.services.hybrid_cloud import ValueEqualityEnum
 
+if TYPE_CHECKING:
+    from sentry.models.organization import Organization
+
 """
 TODO(postgres): We've encoded these enums as integers to facilitate
 communication with the DB. We'd prefer to encode them as strings to facilitate
@@ -361,6 +364,7 @@ class GroupSubscriptionStatus:
 
 @dataclass
 class UnsubscribeContext:
+    organization: Organization
     resource_id: int
     key: str
     referrer: str | None = None

+ 2 - 0
src/sentry/options/defaults.py

@@ -1698,3 +1698,5 @@ register(
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )
 # END: SDK Crash Detection
+
+register("unsubscribe_link.use_react_views", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE)

+ 38 - 1
src/sentry/utils/linksign.py

@@ -7,7 +7,8 @@ from django.core import signing
 from django.urls import reverse
 from sentry_sdk.api import capture_exception
 
-from sentry import options
+from sentry import features, options
+from sentry.models.organization import Organization
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.types.region import get_local_region
 from sentry.utils.numbers import base36_decode, base36_encode
@@ -47,6 +48,42 @@ def generate_signed_link(
     return signed_link
 
 
+def generate_signed_unsubscribe_link(
+    organization: Organization,
+    user_id: int,
+    resource: str,
+    resource_id: str | int,
+    referrer: str | None = None,
+):
+    """
+    Generate an absolute URL to the react rendered unsubscribe views
+
+    The URL will include a signature for the API endpoint that does read/writes.
+    The signature encodes the specific API path and userid that the action
+    is valid for.
+
+    The generated link will honour the customer-domain option for
+    the organization.
+    """
+    html_viewname = f"sentry-organization-unsubscribe-{resource}"
+    api_endpointname = f"sentry-api-0-organization-unsubscribe-{resource}"
+    url_args = [organization.slug, resource_id]
+    if features.has("organizations:customer-domains", organization):
+        url_args = [resource_id]
+        html_viewname = f"sentry-customer-domain-unsubscribe-{resource}"
+
+    htmlpath = reverse(html_viewname, args=url_args)
+    apipath = reverse(api_endpointname, args=[organization.slug, resource_id])
+
+    item = "{}|{}|{}".format(options.get("system.url-prefix"), apipath, base36_encode(user_id))
+    signature = ":".join(get_signer().sign(item).rsplit(":", 2)[1:])
+
+    query = f"_={base36_encode(user_id)}:{signature}"
+    if referrer:
+        query = query + "&" + urlencode({"referrer": referrer})
+    return organization.absolute_url(path=htmlpath, query=query)
+
+
 def find_signature(request) -> str | None:
     return request.GET.get("_")
 

+ 20 - 20
src/sentry/web/urls.py

@@ -644,26 +644,6 @@ urlpatterns += [
                     react_page_view,
                     name="sentry-customer-domain-legal-settings",
                 ),
-                re_path(
-                    r"^unsubscribe/(?P<organization_slug>\w+)/project/(?P<project_id>\d+)/$",
-                    react_page_view,
-                    name="sentry-organization-unsubscribe-project",
-                ),
-                re_path(
-                    r"^unsubscribe/project/(?P<project_id>\d+)/$",
-                    react_page_view,
-                    name="sentry-customer-domain-unsubscribe-project",
-                ),
-                re_path(
-                    r"^unsubscribe/(?P<organization_slug>\w+)/issue/(?P<issue_id>\d+)/$",
-                    react_page_view,
-                    name="sentry-organization-unsubscribe-issue",
-                ),
-                re_path(
-                    r"^unsubscribe/issue/(?P<issue_id>\d+)/$",
-                    react_page_view,
-                    name="sentry-customer-domain-unsubscribe-issue",
-                ),
                 re_path(
                     r"^(?P<organization_slug>[\w_-]+)/$",
                     react_page_view,
@@ -706,6 +686,26 @@ urlpatterns += [
         react_page_view,
         name="integration-installation",
     ),
+    re_path(
+        r"^unsubscribe/(?P<organization_slug>\w+)/project/(?P<project_id>\d+)/$",
+        GenericReactPageView.as_view(auth_required=False),
+        name="sentry-organization-unsubscribe-project",
+    ),
+    re_path(
+        r"^unsubscribe/project/(?P<project_id>\d+)/$",
+        GenericReactPageView.as_view(auth_required=False),
+        name="sentry-customer-domain-unsubscribe-project",
+    ),
+    re_path(
+        r"^unsubscribe/(?P<organization_slug>\w+)/issue/(?P<issue_id>\d+)/$",
+        GenericReactPageView.as_view(auth_required=False),
+        name="sentry-organization-unsubscribe-issue",
+    ),
+    re_path(
+        r"^unsubscribe/issue/(?P<issue_id>\d+)/$",
+        GenericReactPageView.as_view(auth_required=False),
+        name="sentry-customer-domain-unsubscribe-issue",
+    ),
     # Issues
     re_path(
         r"^issues/(?P<project_slug>[\w_-]+)/(?P<group_id>\d+)/tags/(?P<key>[^\/]+)/export/$",

+ 1 - 0
static/app/api.tsx

@@ -119,6 +119,7 @@ const ALLOWED_ANON_PAGES = [
   /^\/share\//,
   /^\/auth\/login\//,
   /^\/join-request\//,
+  /^\/unsubscribe\//,
 ];
 
 /**

+ 4 - 1
static/app/views/unsubscribe/issue.spec.tsx

@@ -58,6 +58,9 @@ describe('UnsubscribeIssue', function () {
     const button = screen.getByRole('button', {name: 'Unsubscribe'});
     await userEvent.click(button);
 
-    expect(mockUpdate).toHaveBeenCalled();
+    expect(mockUpdate).toHaveBeenCalledWith(
+      '/organizations/acme/unsubscribe/issue/9876/?_=signature-value',
+      expect.objectContaining({data: {cancel: 1}})
+    );
   });
 });

+ 7 - 4
static/app/views/unsubscribe/issue.tsx

@@ -1,5 +1,5 @@
 import {Fragment} from 'react';
-import {browserHistory, RouteComponentProps} from 'react-router';
+import {RouteComponentProps} from 'react-router';
 
 import Alert from 'sentry/components/alert';
 import ApiForm from 'sentry/components/forms/apiForm';
@@ -89,13 +89,16 @@ function UnsubscribeBody({orgSlug, issueId, signature}: BodyProps) {
         submitLabel={t('Unsubscribe')}
         cancelLabel={t('Cancel')}
         onCancel={() => {
-          browserHistory.push('/auth/login/');
+          // Use window.location as we're going to an HTML view
+          window.location.assign('/auth/login/');
         }}
         onSubmitSuccess={() => {
-          browserHistory.push('/auth/login/');
+          // Use window.location as we're going to an HTML view
+          window.location.assign('/auth/login/');
         }}
+        initialData={{cancel: 1}}
       >
-        <HiddenField name="cancel" value="1" />
+        <HiddenField name="cancel" />
       </ApiForm>
     </Fragment>
   );

Some files were not shown because too many files changed in this diff