Browse Source

fix(api): Stop relying and sending the token value (#63485)

Now that we always have access to the last token characters, we don't
need the conditional logic for showing one vs the other, so cleaning up
this code. After that, our delete endpoint now also allows us to delete
by using the ID instead of relying on the full token value, so changing
the API call to use the new parameter instead, and help reduce exposing
the token.

[This BE PR](https://github.com/getsentry/sentry/pull/63483) needs to
get merged in first before this can be merged and deployed.



https://github.com/getsentry/sentry/assets/5581484/905aa803-e181-4e03-9cf2-0fdd943bdedb


Resolves:
https://github.com/getsentry/team-enterprise/issues/21#issue-2047202292
Yash Kamothi 1 year ago
parent
commit
0211233efe

+ 1 - 0
fixtures/js-stubs/apiToken.tsx

@@ -12,6 +12,7 @@ export function ApiTokenFixture(
     refreshToken: 'refresh_token',
     expiresAt: new Date('Thu Jan 11 2018 18:01:41 GMT-0800 (PST)').toISOString(),
     state: 'active',
+    tokenLastCharacters: 'n123',
     ...params,
   };
 }

+ 1 - 0
fixtures/js-stubs/sentryAppToken.tsx

@@ -12,6 +12,7 @@ export function SentryAppTokenFixture(
     application: null,
     id: '1',
     state: 'active',
+    tokenLastCharacters: 'oken',
     ...params,
   };
 }

+ 1 - 1
static/app/types/user.tsx

@@ -81,7 +81,7 @@ export interface InternalAppApiToken extends BaseApiToken {
   application: null;
   refreshToken: string;
   token: string;
-  tokenLastCharacters?: string;
+  tokenLastCharacters: string;
 }
 
 export type ApiApplication = {

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

@@ -25,12 +25,4 @@ describe('ApiTokenRow', () => {
     render(<ApiTokenRow onRemove={cb} token={token} />);
     expect(screen.queryByLabelText('Token preview')).toBeInTheDocument();
   });
-
-  it('uses old logic when lastTokenCharacters field is not found', () => {
-    const token = ApiTokenFixture();
-
-    const cb = jest.fn();
-    render(<ApiTokenRow onRemove={cb} token={token} />);
-    expect(screen.queryByLabelText('Token preview')).not.toBeInTheDocument();
-  });
 });

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

@@ -3,7 +3,6 @@ import styled from '@emotion/styled';
 import {Button} from 'sentry/components/button';
 import DateTime from 'sentry/components/dateTime';
 import PanelItem from 'sentry/components/panels/panelItem';
-import TextCopyInput from 'sentry/components/textCopyInput';
 import {IconSubtract} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
@@ -22,22 +21,14 @@ function ApiTokenRow({token, onRemove}: Props) {
   return (
     <StyledPanelItem>
       <Controls>
-        {token.tokenLastCharacters ? (
-          <TokenPreview aria-label={t('Token preview')}>
-            {tokenPreview(
-              getDynamicText({
-                value: token.tokenLastCharacters,
-                fixed: 'ABCD',
-              })
-            )}
-          </TokenPreview>
-        ) : (
-          <InputWrapper>
-            <TextCopyInput>
-              {getDynamicText({value: token.token, fixed: 'CI_AUTH_TOKEN'})}
-            </TextCopyInput>
-          </InputWrapper>
-        )}
+        <TokenPreview aria-label={t('Token preview')}>
+          {tokenPreview(
+            getDynamicText({
+              value: token.tokenLastCharacters,
+              fixed: 'ABCD',
+            })
+          )}
+        </TokenPreview>
         <ButtonWrapper>
           <Button
             onClick={() => onRemove(token)}
@@ -80,12 +71,6 @@ const Controls = styled('div')`
   margin-bottom: ${space(1)};
 `;
 
-const InputWrapper = styled('div')`
-  font-size: ${p => p.theme.fontSizeRelativeSmall};
-  flex: 1;
-  margin-right: ${space(1)};
-`;
-
 const Details = styled('div')`
   display: flex;
   margin-top: ${space(1)};

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

@@ -48,13 +48,13 @@ export class ApiTokens extends DeprecatedAsyncView<Props, State> {
 
     this.setState(
       state => ({
-        tokenList: state.tokenList?.filter(tk => tk.token !== token.token) ?? [],
+        tokenList: state.tokenList?.filter(tk => tk.id !== token.id) ?? [],
       }),
       async () => {
         try {
           await this.api.requestPromise('/api-tokens/', {
             method: 'DELETE',
-            data: {token: token.token},
+            data: {tokenId: token.id},
           });
 
           addSuccessMessage(t('Removed token'));
@@ -118,7 +118,7 @@ export class ApiTokens extends DeprecatedAsyncView<Props, State> {
 
             {tokenList?.map(token => (
               <ApiTokenRow
-                key={token.token}
+                key={token.id}
                 token={token}
                 onRemove={this.handleRemoveToken}
               />