diff --git a/src/__mocks__/state-mocks.ts b/src/__mocks__/state-mocks.ts index 9622c32c9..90bf49f23 100644 --- a/src/__mocks__/state-mocks.ts +++ b/src/__mocks__/state-mocks.ts @@ -1,10 +1,10 @@ import { type AuthState, - type EnterpriseAccount, type GitifyUser, type SettingsState, Theme, } from '../types'; +import type { EnterpriseAccount } from '../utils/auth/types'; export const mockEnterpriseAccounts: EnterpriseAccount[] = [ { diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 3b6591c51..25a6201d8 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -35,8 +35,7 @@ describe('context/App.tsx', () => { jest.clearAllMocks(); }); - describe('api methods', () => { - const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); + describe('notification methods', () => { const getNotificationCountMock = jest.spyOn( notifications, 'getNotificationCount', @@ -269,18 +268,31 @@ describe('context/App.tsx', () => { 'github.com', ); }); + }); + describe('authentication methods', () => { + const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); + const fetchNotificationsMock = jest.fn(); - it('should call validateToken', async () => { + beforeEach(() => { + (useNotifications as jest.Mock).mockReturnValue({ + fetchNotifications: fetchNotificationsMock, + }); + }); + + it('should call loginWithPersonalAccessToken', async () => { apiRequestAuthMock.mockResolvedValueOnce(null); const TestComponent = () => { - const { validateToken } = useContext(AppContext); + const { loginWithPersonalAccessToken } = useContext(AppContext); return ( - ); - }; + return ( + + ); + }; - const { getByText } = customRender(); + const { getByText } = customRender(); - act(() => { - fireEvent.click(getByText('Test Case')); - }); + act(() => { + fireEvent.click(getByText('Test Case')); + }); - expect(saveStateMock).toHaveBeenCalledWith({ - auth: { enterpriseAccounts: [], token: null, user: null }, - settings: { - participating: true, - playSound: true, - showNotifications: true, - showBots: true, - showNotificationsCountInTray: false, - openAtStartup: false, - theme: 'SYSTEM', - detailedNotifications: true, - markAsDoneOnOpen: false, - showAccountHostname: false, - delayNotificationState: false, - }, + expect(saveStateMock).toHaveBeenCalledWith({ + auth: { enterpriseAccounts: [], token: null, user: null }, + settings: { + participating: true, + playSound: true, + showNotifications: true, + showBots: true, + showNotificationsCountInTray: false, + openAtStartup: false, + theme: 'SYSTEM', + detailedNotifications: true, + markAsDoneOnOpen: false, + showAccountHostname: false, + delayNotificationState: false, + }, + }); }); - }); - it('should call updateSetting and set auto launch(openAtStartup)', async () => { - const setAutoLaunchMock = jest.spyOn(comms, 'setAutoLaunch'); - const saveStateMock = jest - .spyOn(storage, 'saveState') - .mockImplementation(jest.fn()); + it('should call updateSetting and set auto launch(openAtStartup)', async () => { + const setAutoLaunchMock = jest.spyOn(comms, 'setAutoLaunch'); + const saveStateMock = jest + .spyOn(storage, 'saveState') + .mockImplementation(jest.fn()); - const TestComponent = () => { - const { updateSetting } = useContext(AppContext); + const TestComponent = () => { + const { updateSetting } = useContext(AppContext); - return ( - - ); - }; + return ( + + ); + }; - const { getByText } = customRender(); + const { getByText } = customRender(); - act(() => { - fireEvent.click(getByText('Test Case')); - }); + act(() => { + fireEvent.click(getByText('Test Case')); + }); - expect(setAutoLaunchMock).toHaveBeenCalledWith(true); - - expect(saveStateMock).toHaveBeenCalledWith({ - auth: { enterpriseAccounts: [], token: null, user: null }, - settings: { - participating: false, - playSound: true, - showNotifications: true, - showBots: true, - showNotificationsCountInTray: false, - openAtStartup: true, - theme: 'SYSTEM', - detailedNotifications: true, - markAsDoneOnOpen: false, - showAccountHostname: false, - delayNotificationState: false, - }, + expect(setAutoLaunchMock).toHaveBeenCalledWith(true); + + expect(saveStateMock).toHaveBeenCalledWith({ + auth: { enterpriseAccounts: [], token: null, user: null }, + settings: { + participating: false, + playSound: true, + showNotifications: true, + showBots: true, + showNotificationsCountInTray: false, + openAtStartup: true, + theme: 'SYSTEM', + detailedNotifications: true, + markAsDoneOnOpen: false, + showAccountHostname: false, + delayNotificationState: false, + }, + }); }); }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index 789188467..b29edebef 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -10,16 +10,23 @@ import { useInterval } from '../hooks/useInterval'; import { useNotifications } from '../hooks/useNotifications'; import { type AccountNotifications, - type AuthOptions, type AuthState, - type AuthTokenOptions, type GitifyError, type SettingsState, type Status, Theme, } from '../types'; import { headNotifications } from '../utils/api/client'; -import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth'; +import type { + LoginOAuthAppOptions, + LoginPersonalAccessTokenOptions, +} from '../utils/auth/types'; +import { + addAccount, + authGitHub, + getToken, + getUserData, +} from '../utils/auth/utils'; import { setAutoLaunch, updateTrayTitle } from '../utils/comms'; import Constants from '../utils/constants'; import { getNotificationCount } from '../utils/notifications'; @@ -49,9 +56,9 @@ export const defaultSettings: SettingsState = { interface AppContextState { auth: AuthState; isLoggedIn: boolean; - login: () => void; - loginEnterprise: (data: AuthOptions) => void; - validateToken: (data: AuthTokenOptions) => void; + loginWithGitHubApp: () => void; + loginWithOAuthApp: (data: LoginOAuthAppOptions) => void; + loginWithPersonalAccessToken: (data: LoginPersonalAccessTokenOptions) => void; logout: () => void; notifications: AccountNotifications[]; @@ -145,7 +152,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { return !!auth.token || auth.enterpriseAccounts.length > 0; }, [auth]); - const login = useCallback(async () => { + const loginWithGitHubApp = useCallback(async () => { const { authCode } = await authGitHub(); const { token } = await getToken(authCode); const hostname = Constants.DEFAULT_AUTH_OPTIONS.hostname; @@ -155,8 +162,8 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { saveState({ auth: updatedAuth, settings }); }, [auth, settings]); - const loginEnterprise = useCallback( - async (data: AuthOptions) => { + const loginWithOAuthApp = useCallback( + async (data: LoginOAuthAppOptions) => { const { authOptions, authCode } = await authGitHub(data); const { token, hostname } = await getToken(authCode, authOptions); const updatedAuth = addAccount(auth, token, hostname); @@ -166,14 +173,14 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { [auth, settings], ); - const validateToken = useCallback( - async ({ token, hostname }: AuthTokenOptions) => { + const loginWithPersonalAccessToken = useCallback( + async ({ token, hostname }: LoginPersonalAccessTokenOptions) => { await headNotifications(hostname, token); const user = await getUserData(token, hostname); - const updatedAccounts = addAccount(auth, token, hostname, user); - setAuth(updatedAccounts); - saveState({ auth: updatedAccounts, settings }); + const updatedAuth = addAccount(auth, token, hostname, user); + setAuth(updatedAuth); + saveState({ auth: updatedAuth, settings }); }, [auth, settings], ); @@ -236,9 +243,9 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { value={{ auth, isLoggedIn, - login, - loginEnterprise, - validateToken, + loginWithGitHubApp, + loginWithOAuthApp, + loginWithPersonalAccessToken, logout, notifications, diff --git a/src/routes/LoginWithOAuthApp.tsx b/src/routes/LoginWithOAuthApp.tsx index 4620d266c..f95ff345f 100644 --- a/src/routes/LoginWithOAuthApp.tsx +++ b/src/routes/LoginWithOAuthApp.tsx @@ -11,13 +11,13 @@ import { useNavigate } from 'react-router-dom'; import { Button } from '../components/fields/Button'; import { FieldInput } from '../components/fields/FieldInput'; import { AppContext } from '../context/App'; -import type { AuthOptions } from '../types'; +import type { LoginOAuthAppOptions } from '../utils/auth/types'; import { getNewOAuthAppURL, isValidClientId, isValidHostname, isValidToken, -} from '../utils/auth'; +} from '../utils/auth/utils'; import Constants from '../utils/constants'; interface IValues { @@ -59,7 +59,7 @@ export const validate = (values: IValues): IFormErrors => { export const LoginWithOAuthApp: FC = () => { const { auth: { enterpriseAccounts }, - loginEnterprise, + loginWithOAuthApp: loginEnterprise, } = useContext(AppContext); const navigate = useNavigate(); @@ -129,7 +129,7 @@ export const LoginWithOAuthApp: FC = () => { const login = useCallback(async (data: IValues) => { try { - await loginEnterprise(data as AuthOptions); + await loginEnterprise(data as LoginOAuthAppOptions); } catch (err) { // Skip } diff --git a/src/routes/LoginWithPersonalAccessToken.test.tsx b/src/routes/LoginWithPersonalAccessToken.test.tsx index 04fe33a30..a6ea100c6 100644 --- a/src/routes/LoginWithPersonalAccessToken.test.tsx +++ b/src/routes/LoginWithPersonalAccessToken.test.tsx @@ -75,7 +75,9 @@ describe('routes/LoginWithPersonalAccessToken.tsx', () => { describe("'Generate a PAT' button", () => { it('should be disabled if no hostname configured', async () => { render( - + @@ -93,7 +95,9 @@ describe('routes/LoginWithPersonalAccessToken.tsx', () => { it('should open in browser if hostname configured', async () => { render( - + @@ -110,7 +114,9 @@ describe('routes/LoginWithPersonalAccessToken.tsx', () => { mockValidateToken.mockResolvedValueOnce(null); render( - + @@ -136,7 +142,9 @@ describe('routes/LoginWithPersonalAccessToken.tsx', () => { mockValidateToken.mockRejectedValueOnce(null); render( - + @@ -181,7 +189,9 @@ describe('routes/LoginWithPersonalAccessToken.tsx', () => { it('should open help docs in the browser', async () => { render( - + diff --git a/src/routes/LoginWithPersonalAccessToken.tsx b/src/routes/LoginWithPersonalAccessToken.tsx index 1f53d09a5..95648278e 100644 --- a/src/routes/LoginWithPersonalAccessToken.tsx +++ b/src/routes/LoginWithPersonalAccessToken.tsx @@ -10,8 +10,12 @@ import { useNavigate } from 'react-router-dom'; import { Button } from '../components/fields/Button'; import { FieldInput } from '../components/fields/FieldInput'; import { AppContext } from '../context/App'; -import type { AuthTokenOptions } from '../types'; -import { getNewTokenURL, isValidHostname, isValidToken } from '../utils/auth'; +import type { LoginPersonalAccessTokenOptions } from '../utils/auth/types'; +import { + getNewTokenURL, + isValidHostname, + isValidToken, +} from '../utils/auth/utils'; import { Constants } from '../utils/constants'; interface IValues { @@ -43,7 +47,8 @@ export const validate = (values: IValues): IFormErrors => { }; export const LoginWithPersonalAccessToken: FC = () => { - const { validateToken } = useContext(AppContext); + const { loginWithPersonalAccessToken: validateToken } = + useContext(AppContext); const navigate = useNavigate(); const [isValidToken, setIsValidToken] = useState(true); @@ -119,7 +124,7 @@ export const LoginWithPersonalAccessToken: FC = () => { const login = useCallback(async (data: IValues) => { setIsValidToken(true); try { - await validateToken(data as AuthTokenOptions); + await validateToken(data as LoginPersonalAccessTokenOptions); navigate(-1); } catch (err) { setIsValidToken(false); diff --git a/src/types.ts b/src/types.ts index b504dd7d8..66d593e62 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,6 +1,7 @@ import type { OcticonProps } from '@primer/octicons-react'; import type { FC } from 'react'; import type { Notification } from './typesGitHub'; +import type { EnterpriseAccount } from './utils/auth/types'; export interface AuthState { token?: string; @@ -50,36 +51,11 @@ export type RadioGroupItem = { value: string; }; -export interface EnterpriseAccount { - hostname: string; - token: string; -} - export interface AccountNotifications { hostname: string; notifications: Notification[]; } -export interface AuthOptions { - hostname: string; - clientId: string; - clientSecret: string; -} - -export interface AuthTokenOptions { - hostname: string; - token: string; -} - -export interface AuthResponse { - authCode: string; - authOptions: AuthOptions; -} -export interface AuthTokenResponse { - hostname: string; - token: string; -} - export interface GitifyUser { login: string; name: string; diff --git a/src/utils/auth.test.ts b/src/utils/auth/auth.test.ts similarity index 97% rename from src/utils/auth.test.ts rename to src/utils/auth/auth.test.ts index ee517fc1b..2a5810ecc 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth/auth.test.ts @@ -1,9 +1,9 @@ import remote from '@electron/remote'; import type { AxiosPromise, AxiosResponse } from 'axios'; -import type { AuthState } from '../types'; -import * as apiRequests from './api/request'; -import * as auth from './auth'; -import { getNewOAuthAppURL, getNewTokenURL } from './auth'; +import type { AuthState } from '../../types'; +import * as apiRequests from '../api/request'; +import * as auth from './utils'; +import { getNewOAuthAppURL, getNewTokenURL } from './utils'; const browserWindow = new remote.BrowserWindow(); diff --git a/src/utils/auth/types.ts b/src/utils/auth/types.ts new file mode 100644 index 000000000..d2b72b7f2 --- /dev/null +++ b/src/utils/auth/types.ts @@ -0,0 +1,25 @@ +export interface LoginOAuthAppOptions { + hostname: string; + clientId: string; + clientSecret: string; +} + +export interface LoginPersonalAccessTokenOptions { + hostname: string; + token: string; +} + +export interface AuthResponse { + authCode: string; + authOptions: LoginOAuthAppOptions; +} + +export interface AuthTokenResponse { + hostname: string; + token: string; +} + +export interface EnterpriseAccount { + hostname: string; + token: string; +} diff --git a/src/utils/auth/utils.test.ts b/src/utils/auth/utils.test.ts new file mode 100644 index 000000000..46edb356f --- /dev/null +++ b/src/utils/auth/utils.test.ts @@ -0,0 +1,213 @@ +import remote from '@electron/remote'; +import type { AxiosPromise, AxiosResponse } from 'axios'; +import type { AuthState } from '../../types'; +import * as apiRequests from '../api/request'; +import * as auth from './utils'; +import { getNewOAuthAppURL, getNewTokenURL } from './utils'; + +const browserWindow = new remote.BrowserWindow(); + +describe('utils/auth/utils.ts', () => { + describe('authGitHub', () => { + const loadURLMock = jest.spyOn(browserWindow, 'loadURL'); + + beforeEach(() => { + loadURLMock.mockReset(); + }); + + it('should call authGitHub - success', async () => { + // Casting to jest.Mock avoids Typescript errors, where the spy is expected to match all the original + // function's typing. I might fix all that if the return type of this was actually used, or if I was + // writing this test for a new feature. Since I'm just upgrading Jest, jest.Mock is a nice escape hatch + ( + jest.spyOn(browserWindow.webContents, 'on') as jest.Mock + ).mockImplementation((event, callback): void => { + if (event === 'will-redirect') { + const event = new Event('will-redirect'); + callback(event, 'http://github.com/?code=123-456'); + } + }); + + const res = await auth.authGitHub(); + + expect(res.authCode).toBe('123-456'); + + expect( + browserWindow.webContents.session.clearStorageData, + ).toHaveBeenCalledTimes(1); + + expect(loadURLMock).toHaveBeenCalledTimes(1); + expect(loadURLMock).toHaveBeenCalledWith( + 'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read:user,notifications,repo', + ); + + expect(browserWindow.destroy).toHaveBeenCalledTimes(1); + }); + + it('should call authGitHub - failure', async () => { + ( + jest.spyOn(browserWindow.webContents, 'on') as jest.Mock + ).mockImplementation((event, callback): void => { + if (event === 'will-redirect') { + const event = new Event('will-redirect'); + callback(event, 'http://www.github.com/?error=Oops'); + } + }); + + await expect(async () => await auth.authGitHub()).rejects.toEqual( + "Oops! Something went wrong and we couldn't log you in using GitHub. Please try again.", + ); + expect(loadURLMock).toHaveBeenCalledTimes(1); + }); + }); + + describe('getToken', () => { + const authCode = '123-456'; + const apiRequestMock = jest.spyOn(apiRequests, 'apiRequest'); + + it('should get a token - success', async () => { + const requestPromise = new Promise((resolve) => + resolve({ data: { access_token: 'this-is-a-token' } } as AxiosResponse), + ) as AxiosPromise; + + apiRequestMock.mockResolvedValueOnce(requestPromise); + + const res = await auth.getToken(authCode); + + expect(apiRequests.apiRequest).toHaveBeenCalledWith( + 'https://github.com/login/oauth/access_token', + 'POST', + { + client_id: 'FAKE_CLIENT_ID_123', + client_secret: 'FAKE_CLIENT_SECRET_123', + code: '123-456', + }, + ); + expect(res.token).toBe('this-is-a-token'); + expect(res.hostname).toBe('github.com'); + }); + + it('should get a token - failure', async () => { + const message = 'Something went wrong.'; + + const requestPromise = new Promise((_, reject) => + reject({ data: { message } } as AxiosResponse), + ) as AxiosPromise; + + apiRequestMock.mockResolvedValueOnce(requestPromise); + + const call = async () => await auth.getToken(authCode); + + await expect(call).rejects.toEqual({ data: { message } }); + }); + }); + + describe('addAccount', () => { + const mockAuth: AuthState = { + token: null, + enterpriseAccounts: [], + user: null, + }; + + it('should add a github.com account', () => { + const result = auth.addAccount(mockAuth, '123-456', 'github.com'); + + expect(result).toEqual({ ...mockAuth, token: '123-456' }); + }); + + it('should add an enterprise account', () => { + const result = auth.addAccount(mockAuth, '123-456', 'github.gitify.io'); + + expect(result).toEqual({ + ...mockAuth, + enterpriseAccounts: [ + { hostname: 'github.gitify.io', token: '123-456' }, + ], + }); + }); + }); + + describe('getNewTokenURL', () => { + it('should generate new PAT url - github cloud', () => { + expect( + getNewTokenURL('github.com').startsWith( + 'https://github.com/settings/tokens/new', + ), + ).toBeTruthy(); + }); + + it('should generate new PAT url - github server', () => { + expect( + getNewTokenURL('github.gitify.io').startsWith( + 'https://github.gitify.io/settings/tokens/new', + ), + ).toBeTruthy(); + }); + }); + + describe('getNewOAuthAppURL', () => { + it('should generate new oauth app url - github cloud', () => { + expect( + getNewOAuthAppURL('github.com').startsWith( + 'https://github.com/settings/applications/new', + ), + ).toBeTruthy(); + }); + + it('should generate new oauth app url - github server', () => { + expect( + getNewOAuthAppURL('github.gitify.io').startsWith( + 'https://github.gitify.io/settings/applications/new', + ), + ).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(); + }); + }); +}); diff --git a/src/utils/auth.ts b/src/utils/auth/utils.ts similarity index 92% rename from src/utils/auth.ts rename to src/utils/auth/utils.ts index c1d7f5671..c09dab276 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth/utils.ts @@ -1,16 +1,12 @@ import { BrowserWindow } from '@electron/remote'; import { format } from 'date-fns'; -import type { - AuthResponse, - AuthState, - AuthTokenResponse, - GitifyUser, -} from '../types'; -import type { UserDetails } from '../typesGitHub'; -import { Constants } from '../utils/constants'; -import { getAuthenticatedUser } from './api/client'; -import { apiRequest } from './api/request'; -import { isEnterpriseHost } from './helpers'; +import type { AuthState, GitifyUser } from '../../types'; +import type { UserDetails } from '../../typesGitHub'; +import { getAuthenticatedUser } from '../api/client'; +import { apiRequest } from '../api/request'; +import { Constants } from '../constants'; +import { isEnterpriseHost } from '../helpers'; +import type { AuthResponse, AuthTokenResponse } from './types'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS,