From 264b9f82009398674b1af4affb4ee7fe847ad0ca Mon Sep 17 00:00:00 2001 From: Robert Luby Date: Mon, 15 Jun 2026 11:21:22 +0200 Subject: [PATCH 1/6] SRVOCF-862: added disabled github auth button and divider --- src/common/components/UserAvatar.tsx | 42 ++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/common/components/UserAvatar.tsx b/src/common/components/UserAvatar.tsx index c21c55d..b641a0d 100644 --- a/src/common/components/UserAvatar.tsx +++ b/src/common/components/UserAvatar.tsx @@ -1,15 +1,22 @@ import { Alert, Button, + Divider, + Flex, + FlexItem, Form, FormGroup, + FormHelperText, + HelperText, + HelperTextItem, Modal, ModalBody, ModalFooter, ModalHeader, TextInput, + Tooltip, } from '@patternfly/react-core'; -import { KeyIcon, UserIcon } from '@patternfly/react-icons'; +import { GithubIcon, KeyIcon, UserIcon } from '@patternfly/react-icons'; import { useTranslation } from 'react-i18next'; import { ForgeUser, PAT_KEY, USER_KEY } from '../services/types'; import { useContext, useState } from 'react'; @@ -102,6 +109,31 @@ function PatModal({ isOpen, onClose, onConnect }: PatModalProps) { {error && ( )} + + + + + + + + {t('or')} + + + +
setPat(value)} /> + + + + {t('Enter your GitHub Personal Access Token to connect your repositories.')} + + +
- {t('Enter your GitHub Personal Access Token to connect your repositories.')} + + + + ); +} diff --git a/src/common/components/PatModal.test.tsx b/src/common/components/PatModal.test.tsx new file mode 100644 index 0000000..94afb4c --- /dev/null +++ b/src/common/components/PatModal.test.tsx @@ -0,0 +1,224 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { PatModal } from './PatModal'; +import { OAuthConfig } from '../services/oauth/OAuthService'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +describe('PatModal', () => { + const defaultProps = { + isOpen: true, + oauthConfig: null as OAuthConfig | null, + onClose: vi.fn(), + onConnect: vi.fn().mockResolvedValue(undefined), + onOAuth: vi.fn().mockResolvedValue(undefined), + }; + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('rendering', () => { + it('renders modal title and form elements', () => { + render(); + + expect(screen.getByText('Connect to GitHub')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Sign in with GitHub/ })).toBeInTheDocument(); + expect(screen.getByText('or')).toBeInTheDocument(); + expect(screen.getByLabelText('Personal Access Token')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + }); + + it('does not render content when closed', () => { + render(); + + expect(screen.queryByText('Connect to GitHub')).not.toBeInTheDocument(); + }); + }); + + describe('OAuth button', () => { + it('is aria-disabled when oauthConfig is null', () => { + render(); + + expect(screen.getByRole('button', { name: /Sign in with GitHub/ })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + }); + + it('is aria-disabled when oauth is not enabled', () => { + render(); + + expect(screen.getByRole('button', { name: /Sign in with GitHub/ })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + }); + + it('is active when oauth is configured', () => { + render( + , + ); + + expect(screen.getByRole('button', { name: /Sign in with GitHub/ })).not.toHaveAttribute( + 'aria-disabled', + ); + }); + + it('calls onOAuth when clicked and enabled', async () => { + const onOAuth = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: /Sign in with GitHub/ })); + + expect(onOAuth).toHaveBeenCalledOnce(); + }); + + it('shows error when OAuth flow fails', async () => { + const onOAuth = vi.fn().mockRejectedValue(new Error('Popup blocked')); + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: /Sign in with GitHub/ })); + + expect(await screen.findByText('Popup blocked')).toBeInTheDocument(); + }); + }); + + describe('PAT input', () => { + it('Connect button is disabled when input is empty', () => { + render(); + + expect(screen.getByRole('button', { name: 'Connect' })).toBeDisabled(); + }); + + it('Connect button is enabled when input has value', async () => { + const user = userEvent.setup(); + + render(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_test'); + + expect(screen.getByRole('button', { name: 'Connect' })).toBeEnabled(); + }); + + it('calls onConnect with PAT value', async () => { + const onConnect = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup(); + + render(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + await waitFor(() => { + expect(onConnect).toHaveBeenCalledWith('ghp_valid'); + }); + }); + + it('clears PAT input after successful connect', async () => { + const onConnect = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup(); + + render(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + await waitFor(() => { + expect(screen.getByLabelText('Personal Access Token')).toHaveValue(''); + }); + }); + + it('shows error when connect fails', async () => { + const onConnect = vi.fn().mockRejectedValue(new Error('Bad credentials')); + const user = userEvent.setup(); + + render(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_bad'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + expect(await screen.findByText('Bad credentials')).toBeInTheDocument(); + }); + + it('disables Cancel while validating', async () => { + const user = userEvent.setup(); + let resolveConnect: () => void; + const onConnect = vi.fn().mockReturnValue( + new Promise((resolve) => { + resolveConnect = resolve; + }), + ); + + render(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_slow'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled(); + + resolveConnect!(); + }); + }); + + describe('close', () => { + it('calls onClose when Cancel is clicked', async () => { + const onClose = vi.fn(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(onClose).toHaveBeenCalledOnce(); + }); + + it('clears PAT and error on cancel', async () => { + const onConnect = vi.fn().mockRejectedValue(new Error('Bad credentials')); + const onClose = vi.fn(); + const user = userEvent.setup(); + + const { rerender } = render( + , + ); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_bad'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + expect(await screen.findByText('Bad credentials')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + rerender( + , + ); + + expect(screen.getByLabelText('Personal Access Token')).toHaveValue(''); + expect(screen.queryByText('Bad credentials')).not.toBeInTheDocument(); + }); + }); +}); diff --git a/src/common/components/PatModal.tsx b/src/common/components/PatModal.tsx new file mode 100644 index 0000000..4462c9e --- /dev/null +++ b/src/common/components/PatModal.tsx @@ -0,0 +1,172 @@ +import { + Alert, + Button, + Divider, + Flex, + FlexItem, + Form, + FormGroup, + FormHelperText, + HelperText, + HelperTextItem, + Modal, + ModalBody, + ModalFooter, + ModalHeader, + TextInput, + Tooltip, +} from '@patternfly/react-core'; +import { GithubIcon } from '@patternfly/react-icons'; +import { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { OAuthConfig } from '../services/oauth/OAuthService'; +import { errorMessage } from '../utils/utils'; + +interface PatModalProps { + isOpen: boolean; + oauthConfig: OAuthConfig | null; + onClose: () => void; + onConnect: (pat: string) => Promise; + onOAuth: () => Promise; +} + +export function PatModal({ isOpen, oauthConfig, onClose, onConnect, onOAuth }: PatModalProps) { + const { t } = useTranslation('plugin__console-functions-plugin'); + const { + pat, + isValidating, + isOAuthInProgress, + error, + setPat, + handleConnect, + handleOAuth, + handleClose, + } = usePatModal(onClose, onConnect, onOAuth); + + const oauthEnabled = oauthConfig?.enabled ?? false; + const isBusy = isValidating || isOAuthInProgress; + + const oauthButton = ( + + ); + + return ( + + + + {error && ( + + )} + {oauthEnabled ? oauthButton : {oauthButton}} + + + + + {t('or')} + + + + +
+ + setPat(value)} + isDisabled={isBusy} + /> + + + + {t('Enter your GitHub Personal Access Token to connect your repositories.')} + + + + +
+ + + + + + + ); +} + +function usePatModal( + onClose: () => void, + onConnect: (pat: string) => Promise, + onOAuth: () => Promise, +) { + const [pat, setPat] = useState(''); + const [isValidating, setIsValidating] = useState(false); + const [isOAuthInProgress, setIsOAuthInProgress] = useState(false); + const [error, setError] = useState(null); + + const handleConnect = async () => { + setIsValidating(true); + setError(null); + try { + await onConnect(pat); + setPat(''); + } catch (err) { + setError(errorMessage(err)); + } finally { + setIsValidating(false); + } + }; + + const handleOAuth = async () => { + setIsOAuthInProgress(true); + setError(null); + try { + await onOAuth(); + } catch (err) { + setError(errorMessage(err)); + } finally { + setIsOAuthInProgress(false); + } + }; + + const handleClose = () => { + setPat(''); + setError(null); + onClose(); + }; + + return { + pat, + isValidating, + isOAuthInProgress, + error, + setPat, + handleConnect, + handleOAuth, + handleClose, + }; +} diff --git a/src/common/components/UserAvatar.test.tsx b/src/common/components/UserAvatar.test.tsx index 49ff48f..ec71495 100644 --- a/src/common/components/UserAvatar.test.tsx +++ b/src/common/components/UserAvatar.test.tsx @@ -1,7 +1,7 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { UserAvatar } from './UserAvatar'; -import { PAT_KEY, USER_KEY } from '../services/types'; +import { TOKEN_KEY, USER_KEY } from '../services/types'; import { ForgeConnectionContext } from '../context/ForgeConnectionProvider'; import { ReactNode } from 'react'; @@ -16,6 +16,15 @@ vi.mock('../services/source-control/useSourceControlService', () => ({ }), })); +const mockFetchConfig = vi.fn().mockResolvedValue({ enabled: false, client_id: '' }); +const mockStartFlow = vi.fn(); +vi.mock('../services/oauth/useOAuthService', () => ({ + useOAuthService: () => ({ + fetchConfig: mockFetchConfig, + startFlow: mockStartFlow, + }), +})); + const testUser = { name: 'twoGiants' }; function renderWithContext( @@ -48,7 +57,7 @@ describe('UserAvatar', () => { }); it('renders username when user is stored in sessionStorage', () => { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); renderWithContext(); @@ -56,17 +65,16 @@ describe('UserAvatar', () => { expect(screen.getByText('twoGiants')).toBeInTheDocument(); }); - it('button is clickable when enableReconnect is true', async () => { + it('shows dropdown when logged-in user is clicked', async () => { const user = userEvent.setup(); - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); renderWithContext(); - const button = screen.getByRole('button', { name: 'twoGiants' }); - await user.click(button); + await user.click(screen.getByRole('button', { name: 'twoGiants' })); - expect(screen.getByText('Personal Access Token')).toBeInTheDocument(); + expect(screen.getByText('Disconnect')).toBeInTheDocument(); }); it('button is disabled when enableReconnect is false', async () => { @@ -90,7 +98,7 @@ describe('UserAvatar', () => { }); it('does not auto-open modal when PAT is already stored', () => { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); renderWithContext(); @@ -132,7 +140,7 @@ describe('UserAvatar', () => { }); expect(screen.getByText('twoGiants')).toBeInTheDocument(); - expect(sessionStorage.getItem(PAT_KEY)).toBe('ghp_valid'); + expect(sessionStorage.getItem(TOKEN_KEY)).toBe('ghp_valid'); expect(JSON.parse(sessionStorage.getItem(USER_KEY)!)).toEqual(testUser); expect(connectToForge).toHaveBeenCalled(); }); @@ -161,7 +169,7 @@ describe('UserAvatar', () => { expect(screen.queryByText('Personal Access Token')).not.toBeInTheDocument(); }); - it('clears PAT input after successful connect', async () => { + it('shows dropdown after successful connect', async () => { const user = userEvent.setup(); const connectToForge = vi.fn(); mockFetchUserInfo.mockResolvedValue(testUser); @@ -182,7 +190,7 @@ describe('UserAvatar', () => { await user.click(screen.getByRole('button', { name: 'twoGiants' })); - expect(screen.getByLabelText('Personal Access Token')).toHaveValue(''); + expect(screen.getByText('Disconnect')).toBeInTheDocument(); }); it('clears PAT input and error on cancel', async () => { @@ -222,4 +230,25 @@ describe('UserAvatar', () => { resolveConnect!(); }); }); + + describe('disconnect', () => { + it('shows confirmation modal and disconnects on confirm', async () => { + const user = userEvent.setup(); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); + sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); + + renderWithContext(); + + await user.click(screen.getByRole('button', { name: 'twoGiants' })); + await user.click(screen.getByRole('menuitem', { name: 'Disconnect' })); + + expect(screen.getByText('Disconnect from GitHub')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Disconnect' })); + + expect(sessionStorage.getItem(TOKEN_KEY)).toBeNull(); + expect(sessionStorage.getItem(USER_KEY)).toBeNull(); + expect(screen.getByText('Connect to GitHub')).toBeInTheDocument(); + }); + }); }); diff --git a/src/common/components/UserAvatar.tsx b/src/common/components/UserAvatar.tsx index b641a0d..2c8b27d 100644 --- a/src/common/components/UserAvatar.tsx +++ b/src/common/components/UserAvatar.tsx @@ -1,28 +1,14 @@ -import { - Alert, - Button, - Divider, - Flex, - FlexItem, - Form, - FormGroup, - FormHelperText, - HelperText, - HelperTextItem, - Modal, - ModalBody, - ModalFooter, - ModalHeader, - TextInput, - Tooltip, -} from '@patternfly/react-core'; -import { GithubIcon, KeyIcon, UserIcon } from '@patternfly/react-icons'; +import { Button, Dropdown, DropdownItem, DropdownList, MenuToggle } from '@patternfly/react-core'; +import { KeyIcon, UserIcon } from '@patternfly/react-icons'; +import { useContext, useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { ForgeUser, PAT_KEY, USER_KEY } from '../services/types'; -import { useContext, useState } from 'react'; import { ForgeConnectionContext } from '../context/ForgeConnectionProvider'; +import { OAuthConfig } from '../services/oauth/OAuthService'; +import { useOAuthService } from '../services/oauth/useOAuthService'; import { useSourceControlService } from '../services/source-control/useSourceControlService'; -import { errorMessage } from '../utils/utils'; +import { AUTH_METHOD_KEY, ForgeUser, TOKEN_KEY, USER_KEY } from '../services/types'; +import { DisconnectConfirmModal } from './DisconnectConfirmModal'; +import { PatModal } from './PatModal'; interface UserAvatarProps { enableReconnect: boolean; @@ -30,55 +16,155 @@ interface UserAvatarProps { export function UserAvatar({ enableReconnect }: UserAvatarProps) { const { t } = useTranslation('plugin__console-functions-plugin'); - const { user, isModalOpen, openModal, closeModal, login } = useUserAvatar(enableReconnect); - - const icon = user ? : ; - const label = user ? user.name : t('Connect to GitHub'); + const { + user, + isModalOpen, + isDropdownOpen, + isConfirmOpen, + oauthConfig, + openModal, + closeModal, + loginWithPat, + loginWithOAuth, + toggleDropdown, + closeDropdown, + openConfirm, + closeConfirm, + disconnect, + } = useUserAvatar(enableReconnect); + + if (user) { + return ( + <> + !setOpen && closeDropdown()} + toggle={(toggleRef) => ( + } + > + {user.name} + + )} + popperProps={{ position: 'end' }} + > + + + {t('Disconnect')} + + + + + + ); + } return ( <> - + ); } function useUserAvatar(enableReconnect: boolean) { const sourceControlService = useSourceControlService(); + const oauthService = useOAuthService(); const connectToForge = useContext(ForgeConnectionContext).connectToForge; const [user, setUser] = useState(() => readStoredUser()); const [isModalOpen, setIsModalOpen] = useState( - () => enableReconnect && !sessionStorage.getItem(PAT_KEY), + () => enableReconnect && !sessionStorage.getItem(TOKEN_KEY), ); - - const login = async (pat: string) => { - const forgeUser = await sourceControlService.fetchUserInfo(pat); - sessionStorage.setItem(PAT_KEY, pat); + const [oauthConfig, setOAuthConfig] = useState(null); + const [isDropdownOpen, setIsDropdownOpen] = useState(false); + const [isConfirmOpen, setIsConfirmOpen] = useState(false); + + useEffect(() => { + oauthService + .fetchConfig() + .then(setOAuthConfig) + .catch(() => {}); + }, [oauthService]); + + const finishLogin = (token: string, forgeUser: ForgeUser, method: 'pat' | 'oauth') => { + sessionStorage.setItem(TOKEN_KEY, token); sessionStorage.setItem(USER_KEY, JSON.stringify(forgeUser)); + sessionStorage.setItem(AUTH_METHOD_KEY, method); setUser(forgeUser); setIsModalOpen(false); connectToForge(forgeUser); }; + const loginWithPat = async (pat: string) => { + const forgeUser = await sourceControlService.fetchUserInfo(pat); + finishLogin(pat, forgeUser, 'pat'); + }; + + const loginWithOAuth = async () => { + const token = await oauthService.startFlow(); + const forgeUser = await sourceControlService.fetchUserInfo(token); + finishLogin(token, forgeUser, 'oauth'); + }; + + const disconnect = () => { + sessionStorage.removeItem(TOKEN_KEY); + sessionStorage.removeItem(USER_KEY); + sessionStorage.removeItem(AUTH_METHOD_KEY); + setUser(null); + setIsConfirmOpen(false); + }; + const openModal = () => setIsModalOpen(true); const closeModal = () => setIsModalOpen(false); - - return { user, isModalOpen, openModal, closeModal, login }; + const toggleDropdown = () => setIsDropdownOpen((prev) => !prev); + const closeDropdown = () => setIsDropdownOpen(false); + const openConfirm = () => setIsConfirmOpen(true); + const closeConfirm = () => setIsConfirmOpen(false); + + return { + user, + isModalOpen, + isDropdownOpen, + isConfirmOpen, + oauthConfig, + openModal, + closeModal, + loginWithPat, + loginWithOAuth, + toggleDropdown, + closeDropdown, + openConfirm, + closeConfirm, + disconnect, + }; } function readStoredUser(): ForgeUser | null { - const pat = sessionStorage.getItem(PAT_KEY); + const token = sessionStorage.getItem(TOKEN_KEY); const userJson = sessionStorage.getItem(USER_KEY); - if (!pat || !userJson) { + if (!token || !userJson) { return null; } @@ -88,110 +174,3 @@ function readStoredUser(): ForgeUser | null { return null; } } - -interface PatModalProps { - isOpen: boolean; - onClose: () => void; - onConnect: (pat: string) => Promise; -} - -function PatModal({ isOpen, onClose, onConnect }: PatModalProps) { - const { t } = useTranslation('plugin__console-functions-plugin'); - const { pat, isValidating, error, setPat, handleConnect, handleClose } = usePatModal( - onClose, - onConnect, - ); - - return ( - - - - {error && ( - - )} - - - - - - - - {t('or')} - - - - -
- - setPat(value)} - /> - - - - {t('Enter your GitHub Personal Access Token to connect your repositories.')} - - - - -
- - - - - - - ); -} - -function usePatModal(onClose: () => void, onConnect: (pat: string) => Promise) { - const [pat, setPat] = useState(''); - const [isValidating, setIsValidating] = useState(false); - const [error, setError] = useState(null); - - const handleConnect = async () => { - setIsValidating(true); - setError(null); - try { - await onConnect(pat); - setPat(''); - } catch (err) { - setError(errorMessage(err)); - } finally { - setIsValidating(false); - } - }; - - const handleClose = () => { - setPat(''); - setError(null); - onClose(); - }; - - return { pat, isValidating, error, setPat, handleConnect, handleClose }; -} diff --git a/src/common/context/ForgeConnectionProvider.tsx b/src/common/context/ForgeConnectionProvider.tsx index 6dd87ce..822d374 100644 --- a/src/common/context/ForgeConnectionProvider.tsx +++ b/src/common/context/ForgeConnectionProvider.tsx @@ -1,5 +1,5 @@ import { createContext, ReactNode, useState } from 'react'; -import { ForgeUser, PAT_KEY, USER_KEY } from '../services/types'; +import { ForgeUser, TOKEN_KEY, USER_KEY } from '../services/types'; interface ForgeConnection { isActive: boolean; @@ -16,7 +16,7 @@ export const ForgeConnectionContext = createContext({ }); export function ForgeConnectionProvider({ children }: Readonly<{ children: ReactNode }>) { - const [isActive, setIsActive] = useState(() => !!sessionStorage.getItem(PAT_KEY)); + const [isActive, setIsActive] = useState(() => !!sessionStorage.getItem(TOKEN_KEY)); const [user, setUser] = useState(readStoredUser); const [connectionId, setConnectionId] = useState(0); diff --git a/src/common/services/oauth/OAuthService.ts b/src/common/services/oauth/OAuthService.ts new file mode 100644 index 0000000..e1a7ae9 --- /dev/null +++ b/src/common/services/oauth/OAuthService.ts @@ -0,0 +1,129 @@ +import { consoleFetchJSON } from '@openshift-console/dynamic-plugin-sdk'; + +const PROXY_BASE = '/api/proxy/plugin/console-functions-plugin/backend'; +const STATE_KEY = 'func-console-oauth-state'; +const VERIFIER_KEY = 'func-console-oauth-verifier'; + +export interface OAuthConfig { + client_id: string; + enabled: boolean; +} + +interface OAuthMessage { + type: 'oauth-callback' | 'oauth-error'; + code?: string; + state?: string; + error?: string; +} + +export class OAuthService { + #configCache: OAuthConfig | null = null; + + async fetchConfig(): Promise { + if (this.#configCache) return this.#configCache; + this.#configCache = await consoleFetchJSON(`${PROXY_BASE}/api/oauth/config`); + return this.#configCache!; + } + + async startFlow(): Promise { + const config = await this.fetchConfig(); + if (!config.enabled) throw new Error('OAuth is not configured on this cluster'); + + const state = generateRandomString(32); + const codeVerifier = generateRandomString(64); + const codeChallenge = await generateCodeChallenge(codeVerifier); + + sessionStorage.setItem(STATE_KEY, state); + sessionStorage.setItem(VERIFIER_KEY, codeVerifier); + + const params = new URLSearchParams({ + client_id: config.client_id, + redirect_uri: `${window.location.origin}/faas/oauth/callback`, + scope: 'repo', + state, + code_challenge: codeChallenge, + code_challenge_method: 'S256', + }); + + const popup = window.open( + `https://github.com/login/oauth/authorize?${params}`, + 'github-oauth', + 'width=600,height=700', + ); + if (!popup) { + cleanup(); + throw new Error('Popup was blocked by the browser. Please allow popups for this site.'); + } + + return new Promise((resolve, reject) => { + const closedCheck = setInterval(() => { + if (popup.closed) { + clearInterval(closedCheck); + window.removeEventListener('message', onMessage); + cleanup(); + reject(new Error('Authorization window was closed')); + } + }, 500); + + const onMessage = async (event: MessageEvent) => { + if (event.origin !== window.location.origin) return; + if (!event.data?.type?.startsWith('oauth-')) return; + + clearInterval(closedCheck); + window.removeEventListener('message', onMessage); + popup.close(); + + if (event.data.type === 'oauth-error') { + cleanup(); + reject(new Error(event.data.error ?? 'Authorization denied')); + return; + } + + const storedState = sessionStorage.getItem(STATE_KEY); + if (event.data.state !== storedState) { + cleanup(); + reject(new Error('State mismatch, possible CSRF attack')); + return; + } + + try { + const storedVerifier = sessionStorage.getItem(VERIFIER_KEY)!; + const tokenResponse = await consoleFetchJSON.post(`${PROXY_BASE}/api/oauth/callback`, { + code: event.data.code, + code_verifier: storedVerifier, + }); + cleanup(); + resolve(tokenResponse.access_token); + } catch (err) { + cleanup(); + reject(err); + } + }; + + window.addEventListener('message', onMessage); + }); + } +} + +function cleanup() { + sessionStorage.removeItem(STATE_KEY); + sessionStorage.removeItem(VERIFIER_KEY); +} + +function generateRandomString(length: number): string { + const bytes = crypto.getRandomValues(new Uint8Array(length)); + return base64UrlEncode(bytes.buffer); +} + +async function generateCodeChallenge(verifier: string): Promise { + const encoded = new TextEncoder().encode(verifier); + const digest = await crypto.subtle.digest('SHA-256', encoded); + return base64UrlEncode(digest); +} + +function base64UrlEncode(buffer: ArrayBuffer): string { + const bytes = new Uint8Array(buffer); + let binary = ''; + for (const b of bytes) binary += String.fromCharCode(b); + return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} diff --git a/src/common/services/oauth/useOAuthService.ts b/src/common/services/oauth/useOAuthService.ts new file mode 100644 index 0000000..20a8d62 --- /dev/null +++ b/src/common/services/oauth/useOAuthService.ts @@ -0,0 +1,7 @@ +import { OAuthService } from './OAuthService'; + +const instance = new OAuthService(); + +export function useOAuthService(): OAuthService { + return instance; +} diff --git a/src/common/services/source-control/useSourceControlService.ts b/src/common/services/source-control/useSourceControlService.ts index 4f484f2..8ff69fa 100644 --- a/src/common/services/source-control/useSourceControlService.ts +++ b/src/common/services/source-control/useSourceControlService.ts @@ -1,7 +1,8 @@ +import { TOKEN_KEY } from '../types'; import { GithubService } from './GithubService'; import { SourceControlService } from './SourceControlService'; -const instance = new GithubService(() => sessionStorage.getItem('func-console-pat') || ''); +const instance = new GithubService(() => sessionStorage.getItem(TOKEN_KEY) || ''); export function useSourceControlService(): SourceControlService { return instance; diff --git a/src/common/services/types.ts b/src/common/services/types.ts index e4e783f..1f937af 100644 --- a/src/common/services/types.ts +++ b/src/common/services/types.ts @@ -1,5 +1,8 @@ -export const PAT_KEY = 'func-console-pat'; +export const TOKEN_KEY = 'func-console-token'; +export const PAT_KEY = TOKEN_KEY; export const USER_KEY = 'func-console-user'; +export const AUTH_METHOD_KEY = 'func-console-auth-method'; +export type AuthMethod = 'pat' | 'oauth'; export interface FileEntry { path: string; diff --git a/src/pages/function-edit/FunctionEditPage.test.tsx b/src/pages/function-edit/FunctionEditPage.test.tsx index 7a65709..f07de8e 100644 --- a/src/pages/function-edit/FunctionEditPage.test.tsx +++ b/src/pages/function-edit/FunctionEditPage.test.tsx @@ -4,6 +4,7 @@ import { http, HttpResponse, delay } from 'msw'; import { server } from '../../../testing/msw/server'; import { MemoryRouter, Route, Routes } from 'react-router-dom-v5-compat'; import FunctionEditPage from './FunctionEditPage'; +import { TOKEN_KEY } from '../../common/services/types'; const GITHUB_API = 'https://api.github.com'; @@ -79,7 +80,7 @@ function setupFetchHandlers() { describe('FunctionEditPage', () => { beforeAll(() => { - sessionStorage.setItem('func-console-pat', 'test-pat'); + sessionStorage.setItem(TOKEN_KEY, 'test-pat'); }); afterAll(() => { diff --git a/src/pages/oauth-callback/OAuthCallbackPage.tsx b/src/pages/oauth-callback/OAuthCallbackPage.tsx new file mode 100644 index 0000000..5e7a9e0 --- /dev/null +++ b/src/pages/oauth-callback/OAuthCallbackPage.tsx @@ -0,0 +1,56 @@ +import { EmptyState, EmptyStateBody, Spinner } from '@patternfly/react-core'; +import { useEffect, useRef } from 'react'; +import { useTranslation } from 'react-i18next'; + +export default function OAuthCallbackPage() { + const { t } = useTranslation('plugin__console-functions-plugin'); + const sent = useRef(false); + + const params = new URLSearchParams(window.location.search); + const code = params.get('code'); + const state = params.get('state'); + const error = params.get('error'); + const errorDescription = params.get('error_description'); + + useEffect(() => { + if (sent.current) return; + sent.current = true; + + if (!window.opener) return; + + if (error) { + window.opener.postMessage( + { type: 'oauth-error', error: errorDescription || error }, + window.location.origin, + ); + } else if (code && state) { + window.opener.postMessage({ type: 'oauth-callback', code, state }, window.location.origin); + } + + setTimeout(() => window.close(), 300); + }, [code, state, error, errorDescription]); + + if (!window.opener) { + return ( + + + {t('This page should be opened via the GitHub sign-in flow.')} + + + ); + } + + if (error) { + return ( + + {errorDescription || error} + + ); + } + + return ( + + {t('This window will close automatically.')} + + ); +} From fefbadf1fdc52c9d547acae4057237d105e8814f Mon Sep 17 00:00:00 2001 From: Robert Luby Date: Mon, 15 Jun 2026 13:36:42 +0200 Subject: [PATCH 3/6] SRVOCF-856: fix lint styling --- src/common/components/DisconnectConfirmModal.test.tsx | 4 +++- src/common/components/PatModal.test.tsx | 11 ++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/common/components/DisconnectConfirmModal.test.tsx b/src/common/components/DisconnectConfirmModal.test.tsx index 3797317..d4de9cf 100644 --- a/src/common/components/DisconnectConfirmModal.test.tsx +++ b/src/common/components/DisconnectConfirmModal.test.tsx @@ -21,7 +21,9 @@ describe('DisconnectConfirmModal', () => { render(); expect(screen.getByText('Disconnect from GitHub')).toBeInTheDocument(); - expect(screen.getByText('Are you sure you want to disconnect from GitHub?')).toBeInTheDocument(); + expect( + screen.getByText('Are you sure you want to disconnect from GitHub?'), + ).toBeInTheDocument(); }); it('renders Disconnect and Cancel buttons', () => { diff --git a/src/common/components/PatModal.test.tsx b/src/common/components/PatModal.test.tsx index 94afb4c..61d4397 100644 --- a/src/common/components/PatModal.test.tsx +++ b/src/common/components/PatModal.test.tsx @@ -59,9 +59,7 @@ describe('PatModal', () => { }); it('is active when oauth is configured', () => { - render( - , - ); + render(); expect(screen.getByRole('button', { name: /Sign in with GitHub/ })).not.toHaveAttribute( 'aria-disabled', @@ -209,12 +207,7 @@ describe('PatModal', () => { await user.click(screen.getByRole('button', { name: 'Cancel' })); rerender( - , + , ); expect(screen.getByLabelText('Personal Access Token')).toHaveValue(''); From 0054d12e59fb4e478d445da7d576f901f0ee62e7 Mon Sep 17 00:00:00 2001 From: Robert Luby Date: Mon, 15 Jun 2026 14:02:25 +0200 Subject: [PATCH 4/6] SRVOCF-856: update tooltip content on disabled auth button --- src/common/components/PatModal.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/common/components/PatModal.tsx b/src/common/components/PatModal.tsx index 4462c9e..d135318 100644 --- a/src/common/components/PatModal.tsx +++ b/src/common/components/PatModal.tsx @@ -68,7 +68,13 @@ export function PatModal({ isOpen, oauthConfig, onClose, onConnect, onOAuth }: P {error && ( )} - {oauthEnabled ? oauthButton : {oauthButton}} + {oauthEnabled ? ( + oauthButton + ) : ( + + {oauthButton} + + )} Date: Mon, 15 Jun 2026 16:53:22 +0200 Subject: [PATCH 5/6] SRVOCF-856: fix review comments --- backend/main.go | 19 +++++++++++++++++-- src/common/components/PatModal.tsx | 4 +--- src/common/components/UserAvatar.test.tsx | 10 +++++++++- src/common/components/UserAvatar.tsx | 5 +++-- .../context/ForgeConnectionProvider.tsx | 11 ++++++++++- src/common/services/types.ts | 2 -- .../FunctionCreatePage.test.tsx | 8 ++++---- .../components/CreateFunctionForm.test.tsx | 1 + .../function-list/FunctionsListPage.test.tsx | 4 ++-- 9 files changed, 47 insertions(+), 17 deletions(-) diff --git a/backend/main.go b/backend/main.go index caaae96..a80ba92 100644 --- a/backend/main.go +++ b/backend/main.go @@ -228,9 +228,13 @@ var ( func handleOAuthConfig(w http.ResponseWriter, _ *http.Request) { enabled := githubClientID != "" && githubClientSecret != "" + cid := "" + if enabled { + cid = githubClientID + } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{ - "client_id": githubClientID, + "client_id": cid, "enabled": enabled, }) } @@ -284,6 +288,11 @@ func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + jsonError(w, fmt.Sprintf("GitHub returned HTTP %d", resp.StatusCode), http.StatusBadGateway) + return + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) if err != nil { jsonError(w, "failed to read GitHub response", http.StatusBadGateway) @@ -302,8 +311,14 @@ func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { return } + token, _ := ghResp["access_token"].(string) + if token == "" { + jsonError(w, "no access_token in GitHub response", http.StatusBadGateway) + return + } + w.Header().Set("Content-Type", "application/json") - w.Write(body) + json.NewEncoder(w).Encode(map[string]string{"access_token": token}) } const ocpInternalRegistry = "image-registry.openshift-image-registry.svc:5000/" diff --git a/src/common/components/PatModal.tsx b/src/common/components/PatModal.tsx index d135318..00d882f 100644 --- a/src/common/components/PatModal.tsx +++ b/src/common/components/PatModal.tsx @@ -65,9 +65,7 @@ export function PatModal({ isOpen, oauthConfig, onClose, onConnect, onOAuth }: P - {error && ( - - )} + {error && } {oauthEnabled ? ( oauthButton ) : ( diff --git a/src/common/components/UserAvatar.test.tsx b/src/common/components/UserAvatar.test.tsx index ec71495..f8b570c 100644 --- a/src/common/components/UserAvatar.test.tsx +++ b/src/common/components/UserAvatar.test.tsx @@ -29,7 +29,13 @@ const testUser = { name: 'twoGiants' }; function renderWithContext( ui: ReactNode, - contextValue = { isActive: false, user: testUser, connectionId: 0, connectToForge: vi.fn() }, + contextValue = { + isActive: false, + user: testUser, + connectionId: 0, + connectToForge: vi.fn(), + disconnectFromForge: vi.fn(), + }, ) { return render( {ui}, @@ -130,6 +136,7 @@ describe('UserAvatar', () => { user: testUser, connectionId: 0, connectToForge, + disconnectFromForge: vi.fn(), }); await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid'); @@ -179,6 +186,7 @@ describe('UserAvatar', () => { user: testUser, connectionId: 0, connectToForge, + disconnectFromForge: vi.fn(), }); await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid'); diff --git a/src/common/components/UserAvatar.tsx b/src/common/components/UserAvatar.tsx index 2c8b27d..a62594b 100644 --- a/src/common/components/UserAvatar.tsx +++ b/src/common/components/UserAvatar.tsx @@ -39,7 +39,7 @@ export function UserAvatar({ enableReconnect }: UserAvatarProps) { !setOpen && closeDropdown()} + onOpenChange={(open) => !open && closeDropdown()} toggle={(toggleRef) => ( (() => readStoredUser()); const [isModalOpen, setIsModalOpen] = useState( () => enableReconnect && !sessionStorage.getItem(TOKEN_KEY), @@ -133,6 +133,7 @@ function useUserAvatar(enableReconnect: boolean) { sessionStorage.removeItem(AUTH_METHOD_KEY); setUser(null); setIsConfirmOpen(false); + disconnectFromForge(); }; const openModal = () => setIsModalOpen(true); diff --git a/src/common/context/ForgeConnectionProvider.tsx b/src/common/context/ForgeConnectionProvider.tsx index 822d374..99951b8 100644 --- a/src/common/context/ForgeConnectionProvider.tsx +++ b/src/common/context/ForgeConnectionProvider.tsx @@ -6,6 +6,7 @@ interface ForgeConnection { user: ForgeUser; connectionId: number; connectToForge: (user: ForgeUser) => void; + disconnectFromForge: () => void; } export const ForgeConnectionContext = createContext({ @@ -13,6 +14,7 @@ export const ForgeConnectionContext = createContext({ user: { name: '' }, connectionId: 0, connectToForge: () => {}, + disconnectFromForge: () => {}, }); export function ForgeConnectionProvider({ children }: Readonly<{ children: ReactNode }>) { @@ -26,8 +28,15 @@ export function ForgeConnectionProvider({ children }: Readonly<{ children: React setConnectionId((id) => id + 1); }; + const disconnectFromForge = () => { + setUser({ name: '' }); + setIsActive(false); + }; + return ( - + {children} ); diff --git a/src/common/services/types.ts b/src/common/services/types.ts index 1f937af..3c35a5a 100644 --- a/src/common/services/types.ts +++ b/src/common/services/types.ts @@ -1,8 +1,6 @@ export const TOKEN_KEY = 'func-console-token'; -export const PAT_KEY = TOKEN_KEY; export const USER_KEY = 'func-console-user'; export const AUTH_METHOD_KEY = 'func-console-auth-method'; -export type AuthMethod = 'pat' | 'oauth'; export interface FileEntry { path: string; diff --git a/src/pages/function-create/FunctionCreatePage.test.tsx b/src/pages/function-create/FunctionCreatePage.test.tsx index 227da35..f2f8a1c 100644 --- a/src/pages/function-create/FunctionCreatePage.test.tsx +++ b/src/pages/function-create/FunctionCreatePage.test.tsx @@ -2,7 +2,7 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MemoryRouter } from 'react-router-dom'; import FunctionCreatePage from './FunctionCreatePage'; -import { PAT_KEY, USER_KEY } from '../../common/services/types'; +import { TOKEN_KEY, USER_KEY } from '../../common/services/types'; const mockGenerateFunction = vi.fn(); const mockCreateRepoWithSecret = vi.fn(); @@ -83,7 +83,7 @@ describe('FunctionCreatePage', () => { }); it('renders CreateFunctionForm', () => { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); renderPage(); @@ -92,7 +92,7 @@ describe('FunctionCreatePage', () => { }); it('calls generateFunction, creates repo with secrets, then navigates on submit', async () => { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); sessionStorage.setItem(USER_KEY, JSON.stringify({ name: 'testuser' })); const user = userEvent.setup(); const files = [{ path: 'func.yaml', mode: '100644', content: 'name: f', type: 'blob' }]; @@ -134,7 +134,7 @@ describe('FunctionCreatePage', () => { }); it('shows an alert on error', async () => { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); sessionStorage.setItem(USER_KEY, JSON.stringify({ name: 'testuser' })); const user = userEvent.setup(); mockGenerateFunction.mockRejectedValue(new Error('Backend error')); diff --git a/src/pages/function-create/components/CreateFunctionForm.test.tsx b/src/pages/function-create/components/CreateFunctionForm.test.tsx index 228491c..07ef18c 100644 --- a/src/pages/function-create/components/CreateFunctionForm.test.tsx +++ b/src/pages/function-create/components/CreateFunctionForm.test.tsx @@ -10,6 +10,7 @@ const forgeContext = { user: testUser, connectionId: 0, connectToForge: vi.fn(), + disconnectFromForge: vi.fn(), }; function renderWithContext(ui: React.ReactElement) { diff --git a/src/pages/function-list/FunctionsListPage.test.tsx b/src/pages/function-list/FunctionsListPage.test.tsx index c6a1f83..b3b713f 100644 --- a/src/pages/function-list/FunctionsListPage.test.tsx +++ b/src/pages/function-list/FunctionsListPage.test.tsx @@ -2,7 +2,7 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MemoryRouter } from 'react-router-dom-v5-compat'; import FunctionsListPage from './FunctionsListPage'; -import { PAT_KEY } from '../../common/services/types'; +import { TOKEN_KEY } from '../../common/services/types'; vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }), @@ -68,7 +68,7 @@ function clusterData( } function renderAuthenticated() { - sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(TOKEN_KEY, 'ghp_test'); } function repoFixture(name: string) { From c1a8cdd34621cfb6c41e921db77e63625188003e Mon Sep 17 00:00:00 2001 From: Robert Luby Date: Tue, 16 Jun 2026 10:02:47 +0200 Subject: [PATCH 6/6] SRVOCF-856: extract ouath_handler.go and add test for OAuthService --- backend/main.go | 101 ------ backend/oauth_handler.go | 112 +++++++ .../services/oauth/OAuthService.test.ts | 288 ++++++++++++++++++ 3 files changed, 400 insertions(+), 101 deletions(-) create mode 100644 backend/oauth_handler.go create mode 100644 src/common/services/oauth/OAuthService.test.ts diff --git a/backend/main.go b/backend/main.go index a80ba92..287afcf 100644 --- a/backend/main.go +++ b/backend/main.go @@ -12,7 +12,6 @@ import ( "log" "net" "net/http" - "net/url" "os" "path/filepath" "regexp" @@ -221,106 +220,6 @@ func handleFuncCreate(w http.ResponseWriter, r *http.Request) { } } -var ( - githubClientID = os.Getenv("GITHUB_CLIENT_ID") - githubClientSecret = os.Getenv("GITHUB_CLIENT_SECRET") -) - -func handleOAuthConfig(w http.ResponseWriter, _ *http.Request) { - enabled := githubClientID != "" && githubClientSecret != "" - cid := "" - if enabled { - cid = githubClientID - } - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "client_id": cid, - "enabled": enabled, - }) -} - -type oauthCallbackRequest struct { - Code string `json:"code"` - CodeVerifier string `json:"code_verifier"` -} - -var validOAuthParam = regexp.MustCompile(`^[a-zA-Z0-9_\-]+$`) - -func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { - if githubClientID == "" || githubClientSecret == "" { - jsonError(w, "OAuth is not configured on this server", http.StatusServiceUnavailable) - return - } - - var req oauthCallbackRequest - r.Body = http.MaxBytesReader(w, r.Body, 4096) - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - jsonError(w, "invalid request body: "+err.Error(), http.StatusBadRequest) - return - } - if !validOAuthParam.MatchString(req.Code) { - jsonError(w, "invalid authorization code", http.StatusBadRequest) - return - } - if !validOAuthParam.MatchString(req.CodeVerifier) { - jsonError(w, "invalid code verifier", http.StatusBadRequest) - return - } - - form := url.Values{ - "client_id": {githubClientID}, - "client_secret": {githubClientSecret}, - "code": {req.Code}, - "code_verifier": {req.CodeVerifier}, - } - ghReq, err := http.NewRequestWithContext(r.Context(), "POST", "https://github.com/login/oauth/access_token", strings.NewReader(form.Encode())) - if err != nil { - jsonError(w, "failed to build token request", http.StatusInternalServerError) - return - } - ghReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") - ghReq.Header.Set("Accept", "application/json") - - resp, err := http.DefaultClient.Do(ghReq) - if err != nil { - jsonError(w, "failed to exchange token with GitHub: "+err.Error(), http.StatusBadGateway) - return - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - jsonError(w, fmt.Sprintf("GitHub returned HTTP %d", resp.StatusCode), http.StatusBadGateway) - return - } - - body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) - if err != nil { - jsonError(w, "failed to read GitHub response", http.StatusBadGateway) - return - } - - var ghResp map[string]any - if err := json.Unmarshal(body, &ghResp); err != nil { - jsonError(w, "invalid response from GitHub", http.StatusBadGateway) - return - } - - if errMsg, ok := ghResp["error"]; ok { - desc, _ := ghResp["error_description"].(string) - jsonError(w, fmt.Sprintf("%v: %s", errMsg, desc), http.StatusUnauthorized) - return - } - - token, _ := ghResp["access_token"].(string) - if token == "" { - jsonError(w, "no access_token in GitHub response", http.StatusBadGateway) - return - } - - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]string{"access_token": token}) -} - const ocpInternalRegistry = "image-registry.openshift-image-registry.svc:5000/" func generateCIWorkflow(root, runtime, branch, registry string) error { diff --git a/backend/oauth_handler.go b/backend/oauth_handler.go new file mode 100644 index 0000000..203e0ee --- /dev/null +++ b/backend/oauth_handler.go @@ -0,0 +1,112 @@ +package main + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os" + "regexp" + "strings" +) + +var ( + githubClientID = os.Getenv("GITHUB_CLIENT_ID") + githubClientSecret = os.Getenv("GITHUB_CLIENT_SECRET") +) + +func handleOAuthConfig(w http.ResponseWriter, _ *http.Request) { + enabled := githubClientID != "" && githubClientSecret != "" + cid := "" + if enabled { + cid = githubClientID + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "client_id": cid, + "enabled": enabled, + }) +} + +type oauthCallbackRequest struct { + Code string `json:"code"` + CodeVerifier string `json:"code_verifier"` +} + +var validOAuthParam = regexp.MustCompile(`^[a-zA-Z0-9_\-]+$`) + +func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { + if githubClientID == "" || githubClientSecret == "" { + jsonError(w, "OAuth is not configured on this server", http.StatusServiceUnavailable) + return + } + + var req oauthCallbackRequest + r.Body = http.MaxBytesReader(w, r.Body, 4096) + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + jsonError(w, "invalid request body: "+err.Error(), http.StatusBadRequest) + return + } + if !validOAuthParam.MatchString(req.Code) { + jsonError(w, "invalid authorization code", http.StatusBadRequest) + return + } + if !validOAuthParam.MatchString(req.CodeVerifier) { + jsonError(w, "invalid code verifier", http.StatusBadRequest) + return + } + + form := url.Values{ + "client_id": {githubClientID}, + "client_secret": {githubClientSecret}, + "code": {req.Code}, + "code_verifier": {req.CodeVerifier}, + } + ghReq, err := http.NewRequestWithContext(r.Context(), "POST", "https://github.com/login/oauth/access_token", strings.NewReader(form.Encode())) + if err != nil { + jsonError(w, "failed to build token request", http.StatusInternalServerError) + return + } + ghReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") + ghReq.Header.Set("Accept", "application/json") + + resp, err := http.DefaultClient.Do(ghReq) + if err != nil { + jsonError(w, "failed to exchange token with GitHub: "+err.Error(), http.StatusBadGateway) + return + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + jsonError(w, fmt.Sprintf("GitHub returned HTTP %d", resp.StatusCode), http.StatusBadGateway) + return + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) + if err != nil { + jsonError(w, "failed to read GitHub response", http.StatusBadGateway) + return + } + + var ghResp map[string]any + if err := json.Unmarshal(body, &ghResp); err != nil { + jsonError(w, "invalid response from GitHub", http.StatusBadGateway) + return + } + + if errMsg, ok := ghResp["error"]; ok { + desc, _ := ghResp["error_description"].(string) + jsonError(w, fmt.Sprintf("%v: %s", errMsg, desc), http.StatusUnauthorized) + return + } + + token, _ := ghResp["access_token"].(string) + if token == "" { + jsonError(w, "no access_token in GitHub response", http.StatusBadGateway) + return + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{"access_token": token}) +} diff --git a/src/common/services/oauth/OAuthService.test.ts b/src/common/services/oauth/OAuthService.test.ts new file mode 100644 index 0000000..fbb8a65 --- /dev/null +++ b/src/common/services/oauth/OAuthService.test.ts @@ -0,0 +1,288 @@ +import { consoleFetchJSON } from '@openshift-console/dynamic-plugin-sdk'; +import { OAuthService, OAuthConfig } from './OAuthService'; + +vi.mock('@openshift-console/dynamic-plugin-sdk', () => ({ + consoleFetchJSON: Object.assign(vi.fn(), { + post: vi.fn(), + }), +})); + +const enabledConfig: OAuthConfig = { client_id: 'test-client-id', enabled: true }; +const disabledConfig: OAuthConfig = { client_id: '', enabled: false }; + +const STATE_KEY = 'func-console-oauth-state'; +const VERIFIER_KEY = 'func-console-oauth-verifier'; + +function flushMicrotasks(): Promise { + return new Promise((resolve) => queueMicrotask(resolve)); +} + +async function waitForSetup(): Promise { + for (let i = 0; i < 10; i++) await flushMicrotasks(); +} + +describe('OAuthService', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.clearAllTimers(); + vi.restoreAllMocks(); + vi.useRealTimers(); + sessionStorage.clear(); + }); + + describe('fetchConfig', () => { + it('fetches and returns OAuth config from the backend', async () => { + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + + const service = new OAuthService(); + const result = await service.fetchConfig(); + + expect(consoleFetchJSON).toHaveBeenCalledWith( + '/api/proxy/plugin/console-functions-plugin/backend/api/oauth/config', + ); + expect(result).toEqual(enabledConfig); + }); + + it('caches the config after the first fetch', async () => { + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + + const service = new OAuthService(); + await service.fetchConfig(); + await service.fetchConfig(); + + expect(consoleFetchJSON).toHaveBeenCalledTimes(1); + }); + }); + + describe('startFlow', () => { + beforeEach(() => { + vi.spyOn(crypto.subtle, 'digest').mockResolvedValue(new ArrayBuffer(32)); + }); + + it('throws when OAuth is not enabled', async () => { + vi.mocked(consoleFetchJSON).mockResolvedValue(disabledConfig); + + const service = new OAuthService(); + + await expect(service.startFlow()).rejects.toThrow('OAuth is not configured on this cluster'); + }); + + it('throws when popup is blocked', async () => { + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + vi.spyOn(window, 'open').mockReturnValue(null); + + const service = new OAuthService(); + + await expect(service.startFlow()).rejects.toThrow('Popup was blocked by the browser'); + expect(sessionStorage.getItem(STATE_KEY)).toBeNull(); + expect(sessionStorage.getItem(VERIFIER_KEY)).toBeNull(); + }); + + it('opens popup with correct OAuth params', async () => { + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: true, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + try { + await service.startFlow(); + } catch { + // expected - popup is immediately "closed" + } + + const url = vi.mocked(window.open).mock.calls[0][0] as string; + expect(url).toContain('client_id=test-client-id'); + expect(url).toContain('scope=repo'); + expect(url).toContain('code_challenge_method=S256'); + expect(url).toContain('redirect_uri='); + expect(url).toContain('state='); + expect(url).toContain('code_challenge='); + }); + + it('rejects when popup is closed without completing auth', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + popup.closed = true; + await vi.advanceTimersByTimeAsync(500); + + await expect(flowPromise).rejects.toThrow('Authorization window was closed'); + expect(sessionStorage.getItem(STATE_KEY)).toBeNull(); + }); + + it('rejects on oauth-error message', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + window.dispatchEvent( + new MessageEvent('message', { + origin: window.location.origin, + data: { type: 'oauth-error', error: 'access_denied' }, + }), + ); + + await expect(flowPromise).rejects.toThrow('access_denied'); + expect(popup.close).toHaveBeenCalled(); + expect(sessionStorage.getItem(STATE_KEY)).toBeNull(); + }); + + it('rejects on state mismatch', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + window.dispatchEvent( + new MessageEvent('message', { + origin: window.location.origin, + data: { type: 'oauth-callback', code: 'test-code', state: 'wrong-state' }, + }), + ); + + await expect(flowPromise).rejects.toThrow('State mismatch'); + }); + + it('exchanges code for token and resolves with access_token', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + vi.mocked(consoleFetchJSON.post).mockResolvedValue({ access_token: 'gho_abc123' }); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + + await waitForSetup(); + + const storedState = sessionStorage.getItem(STATE_KEY)!; + const storedVerifier = sessionStorage.getItem(VERIFIER_KEY)!; + expect(storedState).toBeTruthy(); + expect(storedVerifier).toBeTruthy(); + + window.dispatchEvent( + new MessageEvent('message', { + origin: window.location.origin, + data: { type: 'oauth-callback', code: 'auth-code', state: storedState }, + }), + ); + + await vi.advanceTimersByTimeAsync(0); + + const token = await flowPromise; + + expect(token).toBe('gho_abc123'); + expect(consoleFetchJSON.post).toHaveBeenCalledWith( + '/api/proxy/plugin/console-functions-plugin/backend/api/oauth/callback', + { code: 'auth-code', code_verifier: storedVerifier }, + ); + expect(popup.close).toHaveBeenCalled(); + expect(sessionStorage.getItem(STATE_KEY)).toBeNull(); + expect(sessionStorage.getItem(VERIFIER_KEY)).toBeNull(); + }); + + it('rejects when token exchange fails', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + vi.mocked(consoleFetchJSON.post).mockRejectedValue(new Error('exchange failed')); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + const storedState = sessionStorage.getItem(STATE_KEY)!; + + window.dispatchEvent( + new MessageEvent('message', { + origin: window.location.origin, + data: { type: 'oauth-callback', code: 'auth-code', state: storedState }, + }), + ); + + await vi.advanceTimersByTimeAsync(0); + + await expect(flowPromise).rejects.toThrow('exchange failed'); + expect(sessionStorage.getItem(STATE_KEY)).toBeNull(); + }); + + it('ignores messages from different origins', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + window.dispatchEvent( + new MessageEvent('message', { + origin: 'https://evil.com', + data: { type: 'oauth-callback', code: 'stolen-code', state: 'x' }, + }), + ); + + popup.closed = true; + await vi.advanceTimersByTimeAsync(500); + + await expect(flowPromise).rejects.toThrow('Authorization window was closed'); + expect(consoleFetchJSON.post).not.toHaveBeenCalled(); + }); + + it('ignores messages with non-oauth type', async () => { + vi.useFakeTimers(); + vi.mocked(consoleFetchJSON).mockResolvedValue(enabledConfig); + const popup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window); + + const service = new OAuthService(); + const flowPromise = service.startFlow(); + flowPromise.catch(() => {}); + + await waitForSetup(); + + window.dispatchEvent( + new MessageEvent('message', { + origin: window.location.origin, + data: { type: 'unrelated-message' }, + }), + ); + + popup.closed = true; + await vi.advanceTimersByTimeAsync(500); + + await expect(flowPromise).rejects.toThrow('Authorization window was closed'); + expect(consoleFetchJSON.post).not.toHaveBeenCalled(); + }); + }); +});