Browse Source

feat(integrations): Add confirmation modal to token revocation and clientSecret rotation (#69494)

These actions could be disruptive, so let's add one more confirmation
check.

* Token revocation on the Auth User Tokens page
* Token revocation and Client Secret rotation on the Edit
Internal/Public Integration page
* Client Secret rotation on the API Application Details page



https://github.com/getsentry/sentry/assets/1127549/6ed3d0ad-8760-4d87-8837-d262e13dcf42


https://github.com/getsentry/sentry/assets/1127549/62f6bb93-abec-4aab-bec3-b693267ec644


https://github.com/getsentry/sentry/assets/1127549/cac4c0e7-194b-4168-ac0c-0edb67195763

Also adding the last characters to the confirmation modal on the
Organization Auth Tokens page to make sure we revoke the correct one.


![image](https://github.com/getsentry/sentry/assets/1127549/f66771ec-f3e1-442e-b10e-99b15eb25ee6)
Alexander Tarasov 10 months ago
parent
commit
7730e56c5d

+ 2 - 0
static/app/views/settings/account/apiApplications/details.spec.tsx

@@ -103,6 +103,8 @@ describe('ApiApplications', function () {
       screen.getByRole('button', {name: 'Rotate client secret'})
     ).toBeInTheDocument();
     await userEvent.click(screen.getByRole('button', {name: 'Rotate client secret'}));
+    // Confirm modal
+    await userEvent.click(screen.getByRole('button', {name: 'Confirm'}));
 
     expect(
       screen.getByText('This will be the only time your client secret is visible!')

+ 9 - 3
static/app/views/settings/account/apiApplications/details.tsx

@@ -6,6 +6,7 @@ import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {openModal} from 'sentry/actionCreators/modal';
 import Button from 'sentry/components/actions/button';
 import {Alert} from 'sentry/components/alert';
+import Confirm from 'sentry/components/confirm';
 import Form from 'sentry/components/forms/form';
 import FormField from 'sentry/components/forms/formField';
 import JsonForm from 'sentry/components/forms/jsonForm';
@@ -107,9 +108,14 @@ class ApiApplicationsDetails extends DeprecatedAsyncView<Props, State> {
                   ) : (
                     <ClientSecret>
                       <HiddenSecret>{t('hidden')}</HiddenSecret>
-                      <Button onClick={this.rotateClientSecret} priority="danger">
-                        Rotate client secret
-                      </Button>
+                      <Confirm
+                        onConfirm={this.rotateClientSecret}
+                        message={t(
+                          'Are you sure you want to rotate the client secret? The current one will not be usable anymore, and this cannot be undone.'
+                        )}
+                      >
+                        <Button priority="danger">Rotate client secret</Button>
+                      </Confirm>
                     </ClientSecret>
                   )
                 }

+ 8 - 1
static/app/views/settings/account/apiTokenRow.spec.tsx

@@ -1,6 +1,11 @@
 import {ApiTokenFixture} from 'sentry-fixture/apiToken';
 
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {
+  render,
+  renderGlobalModal,
+  screen,
+  userEvent,
+} from 'sentry-test/reactTestingLibrary';
 
 import ApiTokenRow from 'sentry/views/settings/account/apiTokenRow';
 
@@ -12,8 +17,10 @@ describe('ApiTokenRow', () => {
   it('calls onRemove callback when trash can is clicked', async () => {
     const cb = jest.fn();
     render(<ApiTokenRow onRemove={cb} token={ApiTokenFixture()} />);
+    renderGlobalModal();
 
     await userEvent.click(screen.getByLabelText('Remove'));
+    await userEvent.click(screen.getByLabelText('Confirm'));
     expect(cb).toHaveBeenCalled();
   });
 

+ 14 - 6
static/app/views/settings/account/apiTokenRow.tsx

@@ -1,6 +1,7 @@
 import styled from '@emotion/styled';
 
 import {Button} from 'sentry/components/button';
+import Confirm from 'sentry/components/confirm';
 import {DateTime} from 'sentry/components/dateTime';
 import PanelItem from 'sentry/components/panels/panelItem';
 import {IconSubtract} from 'sentry/icons';
@@ -22,13 +23,20 @@ function ApiTokenRow({token, onRemove, tokenPrefix = ''}: Props) {
       <Controls>
         {token.name ? token.name : ''}
         <ButtonWrapper>
-          <Button
-            data-test-id="token-delete"
-            onClick={() => onRemove(token)}
-            icon={<IconSubtract isCircled size="xs" />}
+          <Confirm
+            onConfirm={() => onRemove(token)}
+            message={t(
+              'Are you sure you want to revoke %s token? It will not be usable anymore, and this cannot be undone.',
+              tokenPreview(token.tokenLastCharacters, tokenPrefix)
+            )}
           >
-            {t('Remove')}
-          </Button>
+            <Button
+              data-test-id="token-delete"
+              icon={<IconSubtract isCircled size="xs" />}
+            >
+              {t('Remove')}
+            </Button>
+          </Confirm>
         </ButtonWrapper>
       </Controls>
 

+ 11 - 3
static/app/views/settings/account/apiTokens.spec.tsx

@@ -1,7 +1,12 @@
 import {ApiTokenFixture} from 'sentry-fixture/apiToken';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 
-import {fireEvent, render, screen} from 'sentry-test/reactTestingLibrary';
+import {
+  render,
+  renderGlobalModal,
+  screen,
+  userEvent,
+} from 'sentry-test/reactTestingLibrary';
 
 import {ApiTokens} from 'sentry/views/settings/account/apiTokens';
 
@@ -30,7 +35,7 @@ describe('ApiTokens', function () {
     render(<ApiTokens organization={organization} />);
   });
 
-  it('can delete token', function () {
+  it('can delete token', async function () {
     MockApiClient.addMockResponse({
       url: '/api-tokens/',
       body: [ApiTokenFixture()],
@@ -43,8 +48,11 @@ describe('ApiTokens', function () {
     expect(mock).not.toHaveBeenCalled();
 
     render(<ApiTokens organization={organization} />);
+    renderGlobalModal();
 
-    fireEvent.click(screen.getByLabelText('Remove'));
+    await userEvent.click(screen.getByRole('button', {name: 'Remove'}));
+    // Confirm modal
+    await userEvent.click(screen.getByRole('button', {name: 'Confirm'}));
 
     // Should be loading
     expect(mock).toHaveBeenCalledTimes(1);

+ 2 - 1
static/app/views/settings/organizationAuthTokens/authTokenRow.tsx

@@ -151,7 +151,8 @@ export function OrganizationAuthTokensAuthTokenRow({
             disabled={!revokeToken || isRevoking}
             onConfirm={revokeToken ? () => revokeToken(token) : undefined}
             message={t(
-              'Are you sure you want to revoke this token? The token will not be usable anymore, and this cannot be undone.'
+              'Are you sure you want to revoke %s token? It will not be usable anymore, and this cannot be undone.',
+              tokenPreview(token.tokenLastCharacters || '', 'sntrys_')
             )}
           >
             <Button

+ 6 - 0
static/app/views/settings/organizationDeveloperSettings/sentryApplicationDetails.spec.tsx

@@ -431,7 +431,11 @@ describe('Sentry Application Details', function () {
       });
 
       renderComponent();
+      renderGlobalModal();
+
       await userEvent.click(screen.getByRole('button', {name: 'Remove'}));
+      // Confirm modal
+      await userEvent.click(screen.getByRole('button', {name: 'Confirm'}));
       expect(await screen.findByText('No tokens created yet.')).toBeInTheDocument();
     });
 
@@ -626,6 +630,8 @@ describe('Sentry Application Details', function () {
         screen.getByRole('button', {name: 'Rotate client secret'})
       ).toBeInTheDocument();
       await userEvent.click(screen.getByRole('button', {name: 'Rotate client secret'}));
+      // Confirm modal
+      await userEvent.click(screen.getByRole('button', {name: 'Confirm'}));
 
       expect(
         screen.getByText('This will be the only time your client secret is visible!')

+ 9 - 3
static/app/views/settings/organizationDeveloperSettings/sentryApplicationDetails.tsx

@@ -17,6 +17,7 @@ import Avatar from 'sentry/components/avatar';
 import type {Model} from 'sentry/components/avatarChooser';
 import AvatarChooser from 'sentry/components/avatarChooser';
 import {Button} from 'sentry/components/button';
+import Confirm from 'sentry/components/confirm';
 import EmptyMessage from 'sentry/components/emptyMessage';
 import Form from 'sentry/components/forms/form';
 import FormField from 'sentry/components/forms/formField';
@@ -541,9 +542,14 @@ class SentryApplicationDetails extends DeprecatedAsyncView<Props, State> {
                       <ClientSecret>
                         <HiddenSecret>{t('hidden')}</HiddenSecret>
                         {this.hasTokenAccess ? (
-                          <Button onClick={this.rotateClientSecret} priority="danger">
-                            Rotate client secret
-                          </Button>
+                          <Confirm
+                            onConfirm={this.rotateClientSecret}
+                            message={t(
+                              'Are you sure you want to rotate the client secret? The current one will not be usable anymore, and this cannot be undone.'
+                            )}
+                          >
+                            <Button priority="danger">Rotate client secret</Button>
+                          </Confirm>
                         ) : undefined}
                       </ClientSecret>
                     )

+ 1 - 0
tests/acceptance/test_organization_developer_settings.py

@@ -108,6 +108,7 @@ class OrganizationDeveloperSettingsEditAcceptanceTest(AcceptanceTestCase):
         self.load_page(url)
 
         self.browser.click('[data-test-id="token-delete"]')
+        self.browser.click('[data-test-id="confirm-button"]')
         self.browser.wait_until(".ref-success")
 
         assert self.browser.find_element(