Repository: gitify-app/gitify
Pull request #1129 (2024-05-21 19:43:29)

refactor: auth validation logic
🧼 


Patch:
diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts
index 6594be1d..4a930d0e 100644
--- a/src/__mocks__/mock-state.ts
+++ b/src/__mocks__/mock-state.ts
@@ -15,7 +15,7 @@ export const mockSettings: SettingsState = {
   showNotificationsCountInTray: false,
   openAtStartup: false,
   theme: Theme.SYSTEM,
-  detailedNotifications: false,
+  detailedNotifications: true,
   markAsDoneOnOpen: false,
   showAccountHostname: false,
   delayNotificationState: false,
diff --git a/src/context/App.tsx b/src/context/App.tsx
index 45eacda1..77c32fe2 100644
--- a/src/context/App.tsx
+++ b/src/context/App.tsx
@@ -41,7 +41,7 @@ export const defaultSettings: SettingsState = {
   showNotificationsCountInTray: false,
   openAtStartup: false,
   theme: Theme.SYSTEM,
-  detailedNotifications: false,
+  detailedNotifications: true,
   markAsDoneOnOpen: false,
   showAccountHostname: false,
   delayNotificationState: false,
diff --git a/src/routes/LoginWithOAuthApp.tsx b/src/routes/LoginWithOAuthApp.tsx
index a51057da..79d40e00 100644
--- a/src/routes/LoginWithOAuthApp.tsx
+++ b/src/routes/LoginWithOAuthApp.tsx
@@ -15,7 +15,12 @@ import { Button } from '../components/fields/Button';
 import { FieldInput } from '../components/fields/FieldInput';
 import { AppContext } from '../context/App';
 import type { AuthOptions } from '../types';
-import { getNewOAuthAppURL } from '../utils/auth';
+import {
+  getNewOAuthAppURL,
+  isValidClientId,
+  isValidHostname,
+  isValidToken,
+} from '../utils/auth';
 import Constants from '../utils/constants';
 
 interface IValues {
@@ -35,25 +40,19 @@ export const validate = (values: IValues): IFormErrors => {
 
   if (!values.hostname) {
     errors.hostname = 'Required';
-  } else if (
-    !/^([a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,6}$/i.test(
-      values.hostname,
-    )
-  ) {
+  } else if (!isValidHostname(values.hostname)) {
     errors.hostname = 'Invalid hostname.';
   }
 
   if (!values.clientId) {
-    // 20
     errors.clientId = 'Required';
-  } else if (!/^[A-Z0-9]{20}$/i.test(values.clientId)) {
+  } else if (!isValidClientId(values.clientId)) {
     errors.clientId = 'Invalid client id.';
   }
 
   if (!values.clientSecret) {
-    // 40
     errors.clientSecret = 'Required';
-  } else if (!/^[A-Z0-9]{40}$/i.test(values.clientSecret)) {
+  } else if (!isValidToken(values.clientSecret)) {
     errors.clientSecret = 'Invalid client secret.';
   }
 
diff --git a/src/routes/LoginWithPersonalAccessToken.tsx b/src/routes/LoginWithPersonalAccessToken.tsx
index 5fc1b6ac..55726ed6 100644
--- a/src/routes/LoginWithPersonalAccessToken.tsx
+++ b/src/routes/LoginWithPersonalAccessToken.tsx
@@ -15,7 +15,7 @@ import type { AuthTokenOptions } from '../types';
 import { Constants } from '../utils/constants';
 
 import { Button } from '../components/fields/Button';
-import { getNewTokenURL } from '../utils/auth';
+import { getNewTokenURL, isValidHostname, isValidToken } from '../utils/auth';
 
 interface IValues {
   token?: string;
@@ -32,17 +32,13 @@ export const validate = (values: IValues): IFormErrors => {
 
   if (!values.hostname) {
     errors.hostname = 'Required';
-  } else if (
-    !/^([a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,6}$/i.test(
-      values.hostname,
-    )
-  ) {
+  } else if (!isValidHostname(values.hostname)) {
     errors.hostname = 'Invalid hostname.';
   }
 
   if (!values.token) {
     errors.token = 'Required';
-  } else if (!/^[A-Z0-9_]{40}$/i.test(values.token)) {
+  } else if (!isValidToken(values.token)) {
     errors.token = 'Invalid token.';
   }
 
diff --git a/src/utils/auth.ts b/src/utils/auth.ts
index fba2e2c0..62a052ba 100644
--- a/src/utils/auth.ts
+++ b/src/utils/auth.ts
@@ -167,3 +167,17 @@ export function getNewOAuthAppURL(hostname: string): string {
 
   return newOAuthAppURL.toString();
 }
+
+export function isValidHostname(hostname: string) {
+  return /^([a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,6}$/i.test(
+    hostname,
+  );
+}
+
+export function isValidClientId(clientId: string) {
+  return /^[A-Z0-9_]{20}$/i.test(clientId);
+}
+
+export function isValidToken(token: string) {
+  return /^[A-Z0-9_]{40}$/i.test(token);
+}


Test patch:
diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx
index f687aeed..bf8c297f 100644
--- a/src/context/App.test.tsx
+++ b/src/context/App.test.tsx
@@ -330,7 +330,7 @@ describe('context/App.tsx', () => {
         showNotificationsCountInTray: false,
         openAtStartup: false,
         theme: 'SYSTEM',
-        detailedNotifications: false,
+        detailedNotifications: true,
         markAsDoneOnOpen: false,
         showAccountHostname: false,
         delayNotificationState: false,
@@ -375,7 +375,7 @@ describe('context/App.tsx', () => {
         showNotificationsCountInTray: false,
         openAtStartup: true,
         theme: 'SYSTEM',
-        detailedNotifications: false,
+        detailedNotifications: true,
         markAsDoneOnOpen: false,
         showAccountHostname: false,
         delayNotificationState: false,
diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts
index 11b96b29..29e4be3b 100644
--- a/src/hooks/useNotifications.test.ts
+++ b/src/hooks/useNotifications.test.ts
@@ -34,7 +34,10 @@ describe('hooks/useNotifications.ts', () => {
         const { result } = renderHook(() => useNotifications());
 
         act(() => {
-          result.current.fetchNotifications(mockAccounts, mockSettings);
+          result.current.fetchNotifications(mockAccounts, {
+            ...mockSettings,
+            detailedNotifications: false,
+          });
         });
 
         expect(result.current.status).toBe('loading');
@@ -116,7 +119,10 @@ describe('hooks/useNotifications.ts', () => {
         const { result } = renderHook(() => useNotifications());
 
         act(() => {
-          result.current.fetchNotifications(accounts, mockSettings);
+          result.current.fetchNotifications(accounts, {
+            ...mockSettings,
+            detailedNotifications: false,
+          });
         });
 
         await waitFor(() => {
@@ -179,7 +185,10 @@ describe('hooks/useNotifications.ts', () => {
         const { result } = renderHook(() => useNotifications());
 
         act(() => {
-          result.current.fetchNotifications(accounts, mockSettings);
+          result.current.fetchNotifications(accounts, {
+            ...mockSettings,
+            detailedNotifications: false,
+          });
         });
 
         await waitFor(() => {
@@ -417,7 +426,10 @@ describe('hooks/useNotifications.ts', () => {
       const { result } = renderHook(() => useNotifications());
 
       act(() => {
-        result.current.fetchNotifications(mockAccounts, mockSettings);
+        result.current.fetchNotifications(mockAccounts, {
+          ...mockSettings,
+          detailedNotifications: false,
+        });
       });
 
       await waitFor(() => {
diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx
index 9f256605..3484ef88 100644
--- a/src/routes/Settings.test.tsx
+++ b/src/routes/Settings.test.tsx
@@ -113,7 +113,10 @@ describe('routes/Settings.tsx', () => {
       fireEvent.click(screen.getByLabelText('Detailed notifications'));
 
       expect(updateSetting).toHaveBeenCalledTimes(1);
-      expect(updateSetting).toHaveBeenCalledWith('detailedNotifications', true);
+      expect(updateSetting).toHaveBeenCalledWith(
+        'detailedNotifications',
+        false,
+      );
     });
   });
 
diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap
index ad020946..212b0003 100644
--- a/src/routes/__snapshots__/Settings.test.tsx.snap
+++ b/src/routes/__snapshots__/Settings.test.tsx.snap
@@ -133,6 +133,7 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
             class="flex items-center h-5"
           >
             <input
+              checked=""
               class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
               id="detailedNotifications"
               type="checkbox"
@@ -266,7 +267,6 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
             <input
               checked=""
               class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
-              disabled=""
               id="showBots"
               type="checkbox"
             />
@@ -277,7 +277,6 @@ exports[`routes/Settings.tsx General should render itself & its children 1`] = `
             <label
               class="font-medium text-gray-700 dark:text-gray-200"
               for="showBots"
-              style="text-decoration: line-through;"
             >
               Show notifications from Bot accounts
               <span
diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts
index e92ac835..d2e904a7 100644
--- a/src/utils/auth.test.ts
+++ b/src/utils/auth.test.ts
@@ -167,4 +167,52 @@ describe('utils/auth.tsx', () => {
       ).toBeTruthy();
     });
   });
+
+  describe('isValidHostname', () => {
+    it('should validate hostname - github cloud', () => {
+      expect(auth.isValidHostname('github.com')).toBeTruthy();
+    });
+
+    it('should validate hostname - github enterprise server', () => {
+      expect(auth.isValidHostname('github.gitify.io')).toBeTruthy();
+    });
+
+    it('should invalidate hostname - empty', () => {
+      expect(auth.isValidHostname('')).toBeFalsy();
+    });
+
+    it('should invalidate hostname - invalid', () => {
+      expect(auth.isValidHostname('github')).toBeFalsy();
+    });
+  });
+
+  describe('isValidClientId', () => {
+    it('should validate client id - valid', () => {
+      expect(auth.isValidClientId('1234567890_ASDFGHJKL')).toBeTruthy();
+    });
+
+    it('should validate client id - empty', () => {
+      expect(auth.isValidClientId('')).toBeFalsy();
+    });
+
+    it('should validate client id - invalid', () => {
+      expect(auth.isValidClientId('1234567890asdfg')).toBeFalsy();
+    });
+  });
+
+  describe('isValidToken', () => {
+    it('should validate token - valid', () => {
+      expect(
+        auth.isValidToken('1234567890_asdfghjklPOIUYTREWQ0987654321'),
+      ).toBeTruthy();
+    });
+
+    it('should validate token - empty', () => {
+      expect(auth.isValidToken('')).toBeFalsy();
+    });
+
+    it('should validate token - invalid', () => {
+      expect(auth.isValidToken('1234567890asdfg')).toBeFalsy();
+    });
+  });
 });