fix: PATs should have an unique description (per user) (#2191)

* fix: PATs should have an unique description * add pat validation on the back-end service * Update src/lib/services/pat-service.ts Co-authored-by: Simon Hornby <liquidwicked64@gmail.com> * fix: only consider current user's PATs * fix tests * cleanup * Update frontend/src/component/user/Profile/PersonalAPITokensTab/CreatePersonalAPIToken/CreatePersonalAPIToken.tsx Co-authored-by: Thomas Heartman <thomas@getunleash.ai> * Update src/test/e2e/api/admin/user/pat.e2e.test.ts Co-authored-by: Thomas Heartman <thomas@getunleash.ai> Co-authored-by: Simon Hornby <liquidwicked64@gmail.com> Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
Unleash · Oct 14, 2022 · 076a007 · 076a007
1 parent 06ebe4f
commit 076a007
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 6 deletions.
diff --git a/...onent/user/Profile/PersonalAPITokensTab/CreatePersonalAPIToken/CreatePersonalAPIToken.tsx b/...onent/user/Profile/PersonalAPITokensTab/CreatePersonalAPIToken/CreatePersonalAPIToken.tsx
@@ -93,7 +93,7 @@ export const CreatePersonalAPIToken: FC<ICreatePersonalAPITokenProps> = ({
     setOpen,
     newToken,
 }) => {
-    const { refetchTokens } = usePersonalAPITokens();
+    const { tokens, refetchTokens } = usePersonalAPITokens();
     const { createPersonalAPIToken, loading } = usePersonalAPITokensApi();
     const { setToastApiError } = useToast();
     const { uiConfig } = useUiConfig();
@@ -103,6 +103,11 @@ export const CreatePersonalAPIToken: FC<ICreatePersonalAPITokenProps> = ({
     const [expiration, setExpiration] = useState<ExpirationOption>(
         ExpirationOption['30DAYS']
     );
+    const [errors, setErrors] = useState<{ [key: string]: string }>({});
+
+    const clearErrors = () => {
+        setErrors({});
+    };
 
     const calculateDate = () => {
         const expiresAt = new Date();
@@ -157,6 +162,23 @@ export const CreatePersonalAPIToken: FC<ICreatePersonalAPITokenProps> = ({
     --data-raw '${JSON.stringify(getPersonalAPITokenPayload(), undefined, 2)}'`;
     };
 
+    const isDescriptionEmpty = (description: string) => description.length;
+    const isDescriptionUnique = (description: string) =>
+        !tokens?.some(token => token.description === description);
+    const isValid =
+        isDescriptionEmpty(description) && isDescriptionUnique(description);
+
+    const onSetDescription = (description: string) => {
+        clearErrors();
+        if (!isDescriptionUnique(description)) {
+            setErrors({
+                description:
+                    'A personal API token with that description already exists.',
+            });
+        }
+        setDescription(description);
+    };
+
     return (
         <SidebarModal
             open={open}
@@ -184,8 +206,10 @@ export const CreatePersonalAPIToken: FC<ICreatePersonalAPITokenProps> = ({
                         <StyledInput
                             autoFocus
                             label="Description"
+                            error={Boolean(errors.description)}
+                            errorText={errors.description}
                             value={description}
-                            onChange={e => setDescription(e.target.value)}
+                            onChange={e => onSetDescription(e.target.value)}
                             required
                         />
                         <StyledInputDescription>
@@ -226,6 +250,7 @@ export const CreatePersonalAPIToken: FC<ICreatePersonalAPITokenProps> = ({
                             type="submit"
                             variant="contained"
                             color="primary"
+                            disabled={!isValid}
                         >
                             Create token
                         </Button>

diff --git a/src/lib/db/pat-store.ts b/src/lib/db/pat-store.ts
@@ -75,6 +75,18 @@ export default class PatStore implements IPatStore {
         return present;
     }
 
+    async existsWithDescriptionByUser(
+        description: string,
+        userId: number,
+    ): Promise<boolean> {
+        const result = await this.db.raw(
+            `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE description = ? AND user_id = ?) AS present`,
+            [description, userId],
+        );
+        const { present } = result.rows[0];
+        return present;
+    }
+
     async get(id: number): Promise<Pat> {
         const row = await this.db(TABLE).where({ id }).first();
         return fromRow(row);

diff --git a/src/lib/services/pat-service.ts b/src/lib/services/pat-service.ts
@@ -6,6 +6,8 @@ import { PAT_CREATED } from '../types/events';
 import { IPat } from '../types/models/pat';
 import crypto from 'crypto';
 import User from '../types/user';
+import BadDataError from '../error/bad-data-error';
+import NameExistsError from '../error/name-exists-error';
 
 export default class PatService {
     private config: IUnleashConfig;
@@ -30,9 +32,7 @@ export default class PatService {
     }
 
     async createPat(pat: IPat, user: User): Promise<IPat> {
-        if (new Date(pat.expiresAt) < new Date()) {
-            throw new Error('The expiry date should be in future.');
-        }
+        await this.validatePat(pat, user.id);
         pat.secret = this.generateSecretKey();
         pat.userId = user.id;
         const newPat = await this.patStore.create(pat);
@@ -55,6 +55,25 @@ export default class PatService {
         return this.patStore.deleteForUser(id, userId);
     }
 
+    async validatePat(
+        { description, expiresAt }: IPat,
+        userId: number,
+    ): Promise<void> {
+        if (!description) {
+            throw new BadDataError('PAT description cannot be empty');
+        }
+
+        if (new Date(expiresAt) < new Date()) {
+            throw new BadDataError('The expiry date should be in future.');
+        }
+
+        if (
+            await this.patStore.existsWithDescriptionByUser(description, userId)
+        ) {
+            throw new NameExistsError('PAT description already exists');
+        }
+    }
+
     private generateSecretKey() {
         const randomStr = crypto.randomBytes(28).toString('hex');
         return `user:${randomStr}`;

diff --git a/src/lib/types/stores/pat-store.ts b/src/lib/types/stores/pat-store.ts
@@ -5,4 +5,8 @@ export interface IPatStore extends Store<IPat, number> {
     create(group: IPat): Promise<IPat>;
     getAllByUser(userId: number): Promise<IPat[]>;
     deleteForUser(id: number, userId: number): Promise<void>;
+    existsWithDescriptionByUser(
+        description: string,
+        userId: number,
+    ): Promise<boolean>;
 }
diff --git a/src/test/e2e/api/admin/user/pat.e2e.test.ts b/src/test/e2e/api/admin/user/pat.e2e.test.ts
@@ -59,11 +59,13 @@ test('should create a PAT', async () => {
 });
 
 test('should delete the PAT', async () => {
+    const description = 'pat to be deleted';
     const { request } = app;
 
     const { body } = await request
         .post('/api/admin/user/tokens')
         .send({
+            description,
             expiresAt: tomorrow,
         } as IPat)
         .set('Content-Type', 'application/json')
@@ -128,6 +130,7 @@ test('should get only current user PATs', async () => {
     await request
         .post('/api/admin/user/tokens')
         .send({
+            description: 'my pat',
             expiresAt: tomorrow,
         } as IPat)
         .set('Content-Type', 'application/json')
@@ -149,10 +152,72 @@ test('should fail creation of PAT with passed expiry', async () => {
     await request
         .post('/api/admin/user/tokens')
         .send({
+            description: 'my expired pat',
             expiresAt: yesterday,
         } as IPat)
         .set('Content-Type', 'application/json')
-        .expect(500);
+        .expect(400);
+});
+
+test('should fail creation of PAT without a description', async () => {
+    await app.request
+        .post('/api/admin/user/tokens')
+        .send({
+            expiresAt: tomorrow,
+        } as IPat)
+        .set('Content-Type', 'application/json')
+        .expect(400);
+});
+
+test('should fail creation of PAT with a description that already exists for the current user', async () => {
+    const description = 'duplicate description';
+
+    await app.request
+        .post('/api/admin/user/tokens')
+        .send({
+            description,
+            expiresAt: tomorrow,
+        } as IPat)
+        .set('Content-Type', 'application/json')
+        .expect(201);
+
+    await app.request
+        .post('/api/admin/user/tokens')
+        .send({
+            description,
+            expiresAt: tomorrow,
+        } as IPat)
+        .set('Content-Type', 'application/json')
+        .expect(409);
+});
+
+test('should not fail creation of PAT when a description already exists for another user PAT', async () => {
+    const description = 'another duplicate description';
+
+    await app.request
+        .post('/api/admin/user/tokens')
+        .send({
+            description,
+            expiresAt: tomorrow,
+        } as IPat)
+        .set('Content-Type', 'application/json')
+        .expect(201);
+
+    await app.request
+        .post(`/auth/demo/login`)
+        .send({
+            email: 'user-other@getunleash.io',
+        })
+        .expect(200);
+
+    await app.request
+        .post('/api/admin/user/tokens')
+        .send({
+            description,
+            expiresAt: tomorrow,
+        } as IPat)
+        .set('Content-Type', 'application/json')
+        .expect(201);
 });
 
 test('should get user id 1', async () => {

diff --git a/src/test/fixtures/fake-pat-store.ts b/src/test/fixtures/fake-pat-store.ts
@@ -20,6 +20,13 @@ export default class FakePatStore implements IPatStore {
         throw new Error('Method not implemented.');
     }
 
+    existsWithDescriptionByUser(
+        description: string,
+        userId: number,
+    ): Promise<boolean> {
+        throw new Error('Method not implemented.');
+    }
+
     get(key: number): Promise<IPat> {
         throw new Error('Method not implemented.');
     }