fix(api): Delete API tokens by ID instead of using value (#63647)

We were previously exposing the API tokens in our application, and using
inconsitent behavior with our API auth token handling. We should be
leveraging existing components to create a standard, safe, experience
for users on the application. This works[ with the partnering BE PR that
allows these internal app api tokens to be deleted by
id](https://github.com/getsentry/sentry/pull/63624). Now everywhere on
the application, wherever we are showing a new api auth token, it will
have the same flow, and only be accessible at that time. Afterwards, we
have no need to ever grab the token.


https://github.com/getsentry/sentry/assets/5581484/d0b3e081-8d99-4c94-bcc4-a0f2ad4bd681

Resolves:
https://github.com/getsentry/team-enterprise/issues/21#issue-2047202292

fixtures/js-stubs/apiToken.tsx

@@ -1,8 +1,8 @@
-import type {InternalAppApiToken} from 'sentry/types';
+import type {NewInternalAppApiToken} from 'sentry/types';
 
 export function ApiTokenFixture(
-  params: Partial<InternalAppApiToken> = {}
-): InternalAppApiToken {
+  params: Partial<NewInternalAppApiToken> = {}
+): NewInternalAppApiToken {
   return {
     id: '1',
     token: 'apitoken123',

fixtures/js-stubs/sentryAppToken.tsx

@@ -1,8 +1,8 @@
-import {InternalAppApiToken} from 'sentry/types';
+import {NewInternalAppApiToken} from 'sentry/types';
 
 export function SentryAppTokenFixture(
-  params: Partial<InternalAppApiToken> = {}
-): InternalAppApiToken {
+  params: Partial<NewInternalAppApiToken> = {}
+): NewInternalAppApiToken {
   return {
     token: '123456123456123456123456-token',
     dateCreated: '2019-03-02T18:30:26Z',

static/app/actionCreators/sentryAppTokens.tsx

@@ -5,7 +5,7 @@ import {
 } from 'sentry/actionCreators/indicator';
 import {Client} from 'sentry/api';
 import {t} from 'sentry/locale';
-import {InternalAppApiToken, SentryApp} from 'sentry/types';
+import {NewInternalAppApiToken, SentryApp} from 'sentry/types';
 
 /**
  * Install a sentry application
@@ -16,7 +16,7 @@ import {InternalAppApiToken, SentryApp} from 'sentry/types';
 export async function addSentryAppToken(
   client: Client,
   app: SentryApp
-): Promise<InternalAppApiToken> {
+): Promise<NewInternalAppApiToken> {
   addLoadingMessage();
   try {
     const token = await client.requestPromise(`/sentry-apps/${app.slug}/api-tokens/`, {
@@ -35,16 +35,16 @@ export async function addSentryAppToken(
  *
  * @param {Object} client ApiClient
  * @param {Object} app SentryApp
- * @param {String} token Token string
+ * @param {String} tokenId Id of the token
  */
 export async function removeSentryAppToken(
   client: Client,
   app: SentryApp,
-  token: string
+  tokenId: string
 ): Promise<void> {
   addLoadingMessage();
   try {
-    await client.requestPromise(`/sentry-apps/${app.slug}/api-tokens/${token}/`, {
+    await client.requestPromise(`/sentry-apps/${app.slug}/api-tokens/${tokenId}/`, {
       method: 'DELETE',
     });
     addSuccessMessage(t('Token successfully deleted.'));

static/app/types/user.tsx

@@ -76,14 +76,18 @@ interface BaseApiToken {
   state: string;
 }
 
-// We include the token for API tokens used for internal apps
+// API Tokens should not be using and storing the token values in the application, as the tokens are secrets.
 export interface InternalAppApiToken extends BaseApiToken {
   application: null;
   refreshToken: string;
-  token: string;
   tokenLastCharacters: string;
 }
 
+// We include the token for new API tokens
+export interface NewInternalAppApiToken extends InternalAppApiToken {
+  token: string;
+}
+
 export type ApiApplication = {
   allowedOrigins: string[];
   clientID: string;

static/app/views/settings/account/apiNewToken.tsx

@@ -8,7 +8,7 @@ import PanelBody from 'sentry/components/panels/panelBody';
 import PanelHeader from 'sentry/components/panels/panelHeader';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t, tct} from 'sentry/locale';
-import {InternalAppApiToken, Permissions} from 'sentry/types';
+import {NewInternalAppApiToken, Permissions} from 'sentry/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import NewTokenHandler from 'sentry/views/settings/components/newTokenHandler';
@@ -18,7 +18,7 @@ import PermissionSelection from 'sentry/views/settings/organizationDeveloperSett
 
 const API_INDEX_ROUTE = '/settings/account/api/auth-tokens/';
 type State = {
-  newToken: InternalAppApiToken | null;
+  newToken: NewInternalAppApiToken | null;
   permissions: Permissions;
 };
 

static/app/views/settings/account/apiTokenRow.tsx

@@ -16,8 +16,6 @@ type Props = {
   tokenPrefix?: string;
 };
 
-// TODO: After the BE portion of code changes have been released, remove the conditional rendering of the token.
-// We are currently doing the conditional logic to do safe blue/green deploys and handle contract changes.
 function ApiTokenRow({token, onRemove, tokenPrefix = ''}: Props) {
   return (
     <StyledPanelItem>
@@ -33,6 +31,7 @@ function ApiTokenRow({token, onRemove, tokenPrefix = ''}: Props) {
         </TokenPreview>
         <ButtonWrapper>
           <Button
+            data-test-id="token-delete"
             onClick={() => onRemove(token)}
             icon={<IconSubtract isCircled size="xs" />}
           >

static/app/views/settings/organizationDeveloperSettings/sentryApplicationDetails.spec.tsx

@@ -16,7 +16,7 @@ describe('Sentry Application Details', function () {
   let createAppRequest;
   let editAppRequest;
 
-  const maskedValue = '*'.repeat(64);
+  const maskedValue = '************oken';
 
   const router = RouterFixture();
 
@@ -290,13 +290,11 @@ describe('Sentry Application Details', function () {
       expect(screen.getByText('Small Icon')).toBeInTheDocument();
     });
 
-    it('shows tokens', function () {
+    it('has tokens', function () {
       renderComponent();
 
       expect(screen.getByText('Tokens')).toBeInTheDocument();
-      expect(screen.getByRole('textbox', {name: 'Token value'})).toHaveValue(
-        '123456123456123456123456-token'
-      );
+      expect(screen.getByLabelText('Token preview')).toHaveTextContent('oken');
     });
 
     it('shows just clientSecret', function () {
@@ -345,7 +343,7 @@ describe('Sentry Application Details', function () {
 
     it('shows masked tokens', function () {
       renderComponent();
-      expect(screen.getByRole('textbox', {name: 'Token value'})).toHaveValue(maskedValue);
+      expect(screen.getByLabelText('Token preview')).toHaveTextContent(maskedValue);
     });
 
     it('shows masked clientSecret', function () {
@@ -400,27 +398,34 @@ describe('Sentry Application Details', function () {
           SentryAppTokenFixture({
             token: '392847329',
             dateCreated: '2018-03-02T18:30:26Z',
+            id: '234',
           }),
         ],
       });
 
       renderComponent();
+      expect(screen.queryByLabelText('Generated token')).not.toBeInTheDocument();
+      expect(screen.getAllByLabelText('Token preview')).toHaveLength(1);
+
       await userEvent.click(screen.getByRole('button', {name: 'New Token'}));
 
       await waitFor(() => {
-        expect(screen.getAllByRole('textbox', {name: 'Token value'})).toHaveLength(2);
+        expect(screen.getAllByLabelText('Token preview')).toHaveLength(1);
+      });
+      await waitFor(() => {
+        expect(screen.getAllByLabelText('Generated token')).toHaveLength(1);
       });
     });
 
     it('removing token from list', async function () {
       MockApiClient.addMockResponse({
-        url: `/sentry-apps/${sentryApp.slug}/api-tokens/${token.token}/`,
+        url: `/sentry-apps/${sentryApp.slug}/api-tokens/${token.id}/`,
         method: 'DELETE',
         body: {},
       });
 
       renderComponent();
-      await userEvent.click(screen.getByRole('button', {name: 'Revoke'}));
+      await userEvent.click(screen.getByRole('button', {name: 'Remove'}));
       expect(await screen.findByText('No tokens created yet.')).toBeInTheDocument();
     });
 

static/app/views/settings/organizationDeveloperSettings/sentryApplicationDetails.tsx

@@ -13,7 +13,6 @@ import {
 import Avatar from 'sentry/components/avatar';
 import AvatarChooser, {Model} from 'sentry/components/avatarChooser';
 import {Button} from 'sentry/components/button';
-import DateTime from 'sentry/components/dateTime';
 import EmptyMessage from 'sentry/components/emptyMessage';
 import Form from 'sentry/components/forms/form';
 import FormField from 'sentry/components/forms/formField';
@@ -23,7 +22,6 @@ import ExternalLink from 'sentry/components/links/externalLink';
 import Panel from 'sentry/components/panels/panel';
 import PanelBody from 'sentry/components/panels/panelBody';
 import PanelHeader from 'sentry/components/panels/panelHeader';
-import PanelItem from 'sentry/components/panels/panelItem';
 import TextCopyInput from 'sentry/components/textCopyInput';
 import {Tooltip} from 'sentry/components/tooltip';
 import {SENTRY_APP_PERMISSIONS} from 'sentry/constants';
@@ -31,14 +29,22 @@ import {
   internalIntegrationForms,
   publicIntegrationForms,
 } from 'sentry/data/forms/sentryApplication';
-import {IconAdd, IconDelete} from 'sentry/icons';
+import {IconAdd} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {InternalAppApiToken, Organization, Scope, SentryApp} from 'sentry/types';
+import {
+  InternalAppApiToken,
+  NewInternalAppApiToken,
+  Organization,
+  Scope,
+  SentryApp,
+} from 'sentry/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import withOrganization from 'sentry/utils/withOrganization';
 import DeprecatedAsyncView from 'sentry/views/deprecatedAsyncView';
+import ApiTokenRow from 'sentry/views/settings/account/apiTokenRow';
+import NewTokenHandler from 'sentry/views/settings/components/newTokenHandler';
 import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';
 import PermissionsObserver from 'sentry/views/settings/organizationDeveloperSettings/permissionsObserver';
 
@@ -144,6 +150,7 @@ type Props = RouteComponentProps<{appSlug?: string}, {}> & {
 
 type State = DeprecatedAsyncView['state'] & {
   app: SentryApp | null;
+  newTokens: NewInternalAppApiToken[];
   tokens: InternalAppApiToken[];
 };
 
@@ -155,6 +162,7 @@ class SentryApplicationDetails extends DeprecatedAsyncView<Props, State> {
       ...super.getDefaultState(),
       app: null,
       tokens: [],
+      newTokens: [],
     };
   }
 
@@ -235,7 +243,7 @@ class SentryApplicationDetails extends DeprecatedAsyncView<Props, State> {
 
   onAddToken = async (evt: React.MouseEvent): Promise<void> => {
     evt.preventDefault();
-    const {app, tokens} = this.state;
+    const {app, newTokens} = this.state;
     if (!app) {
       return;
     }
@@ -243,64 +251,55 @@ class SentryApplicationDetails extends DeprecatedAsyncView<Props, State> {
     const api = this.api;
 
     const token = await addSentryAppToken(api, app);
-    const newTokens = tokens.concat(token);
-    this.setState({tokens: newTokens});
+    const updatedNewTokens = newTokens.concat(token);
+    this.setState({newTokens: updatedNewTokens});
   };
 
-  onRemoveToken = async (token: InternalAppApiToken, evt: React.MouseEvent) => {
-    evt.preventDefault();
+  onRemoveToken = async (token: InternalAppApiToken) => {
     const {app, tokens} = this.state;
     if (!app) {
       return;
     }
 
     const api = this.api;
-    const newTokens = tokens.filter(tok => tok.token !== token.token);
+    const newTokens = tokens.filter(tok => tok.id !== token.id);
 
-    await removeSentryAppToken(api, app, token.token);
+    await removeSentryAppToken(api, app, token.id);
     this.setState({tokens: newTokens});
   };
 
+  handleFinishNewToken = (newToken: NewInternalAppApiToken) => {
+    const {tokens, newTokens} = this.state;
+    const updatedNewTokens = newTokens.filter(token => token.id !== newToken.id);
+    const updatedTokens = tokens.concat(newToken as InternalAppApiToken);
+    this.setState({tokens: updatedTokens, newTokens: updatedNewTokens});
+  };
+
   renderTokens = () => {
-    const {tokens} = this.state;
-    if (tokens.length > 0) {
-      return tokens.map(token => (
-        <StyledPanelItem key={token.token}>
-          <TokenItem>
-            <Tooltip
-              disabled={this.showAuthInfo}
-              position="right"
-              containerDisplayMode="inline"
-              title={t(
-                'You do not have access to view these credentials because the permissions for this integration exceed those of your role.'
-              )}
-            >
-              <TextCopyInput aria-label={t('Token value')}>
-                {getDynamicText({value: token.token, fixed: 'xxxxxx'})}
-              </TextCopyInput>
-            </Tooltip>
-          </TokenItem>
-          <CreatedDate>
-            <CreatedTitle>Created:</CreatedTitle>
-            <DateTime
-              date={getDynamicText({
-                value: token.dateCreated,
-                fixed: new Date(1508208080000),
-              })}
-            />
-          </CreatedDate>
-          <Button
-            onClick={this.onRemoveToken.bind(this, token)}
-            size="sm"
-            icon={<IconDelete />}
-            data-test-id="token-delete"
-          >
-            {t('Revoke')}
-          </Button>
-        </StyledPanelItem>
-      ));
+    const {tokens, newTokens} = this.state;
+    if (tokens.length < 1 && newTokens.length < 1) {
+      return <EmptyMessage description={t('No tokens created yet.')} />;
     }
-    return <EmptyMessage description={t('No tokens created yet.')} />;
+    const tokensToDisplay = tokens.map(token => (
+      <ApiTokenRow
+        data-test-id="api-token"
+        key={token.id}
+        token={token}
+        onRemove={this.onRemoveToken}
+      />
+    ));
+    tokensToDisplay.push(
+      ...newTokens.map(newToken => (
+        <NewTokenHandler
+          data-test-id="new-api-token"
+          key={newToken.id}
+          token={getDynamicText({value: newToken.token, fixed: 'ORG_AUTH_TOKEN'})}
+          handleGoBack={() => this.handleFinishNewToken(newToken)}
+        />
+      ))
+    );
+
+    return tokensToDisplay;
   };
 
   onFieldChange = (name: string, value: FieldValue): void => {
@@ -503,28 +502,6 @@ class SentryApplicationDetails extends DeprecatedAsyncView<Props, State> {
 
 export default withOrganization(SentryApplicationDetails);
 
-const StyledPanelItem = styled(PanelItem)`
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
-`;
-
-const TokenItem = styled('div')`
-  width: 70%;
-`;
-
-const CreatedTitle = styled('span')`
-  color: ${p => p.theme.gray300};
-  margin-bottom: 2px;
-`;
-
-const CreatedDate = styled('div')`
-  display: flex;
-  flex-direction: column;
-  font-size: 14px;
-  margin: 0 10px;
-`;
-
 const AvatarPreview = styled('div')`
   flex: 1;
   display: grid;

tests/acceptance/test_organization_developer_settings.py

@@ -119,7 +119,9 @@ class OrganizationDeveloperSettingsEditAcceptanceTest(AcceptanceTestCase):
 
         self.load_page(url)
 
+        assert self.browser.element_exists('[aria-label="Generated token"]') is False
+
         self.browser.click('[data-test-id="token-add"]')
         self.browser.wait_until(".ref-success")
 
-        assert len(self.browser.elements('[data-test-id="token-delete"]')) == 2
+        assert len(self.browser.elements('[aria-label="Generated token"]')) == 1