From 978d93fb07d7bca8d2c50c1f43fd9d5b5fa89adb Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 30 Sep 2026 11:45:17 -0600 Subject: [PATCH] fix(auth): handle revoked GitHub access with reconnect flow - Return 401 GITHUB_AUTH_REVOKED from /storages, /storage/discover and /storage/create when the GitHub client is missing or GitHub rejects the token - Keep the 401 status when discovery fails to look up the GitHub user - ConnectView offers Reconnect GitHub instead of creating storage on revoke - Cancelling on a revoked connection now logs out before redirecting to sign-in --- backend/routes/auth.js | 40 ++++++--- backend/services/storageService.js | 4 +- backend/tests/auth.test.js | 92 +++++++++++++++++++++ frontend/src/App.jsx | 43 +++++++++- frontend/src/__tests__/connectView.test.jsx | 56 +++++++++++++ frontend/src/views/ConnectView.jsx | 56 +++++++++++-- 6 files changed, 267 insertions(+), 24 deletions(-) diff --git a/backend/routes/auth.js b/backend/routes/auth.js index 44b774f..5ac68c2 100644 --- a/backend/routes/auth.js +++ b/backend/routes/auth.js @@ -64,6 +64,16 @@ function makeAuthRouter({ authService, storageService }) { }); }); + /** Returns the usable client, or sends a 401 GITHUB_AUTH_REVOKED and returns null. */ + function getOctokit(clients, res) { + const octokit = clients.githubUserClient || clients.githubClient; + if (!octokit) { + githubAccessRevoked(res); + return null; + } + return octokit; + } + router.get('/storages', requireAuth, async (req, res) => { try { const provider = req.query.provider; @@ -74,11 +84,11 @@ function makeAuthRouter({ authService, storageService }) { } const clients = await authService.clientsFor(req.user); - if (!clients.githubClient) { - return res.status(400).json({ error: 'GitHub client is not available' }); - } - const repos = await storageService.listGitHubRepos(clients.githubClient); + const octokit = getOctokit(clients, res); + if (!octokit) return; + + const repos = await storageService.listGitHubRepos(octokit); return res.json({ provider: 'github', storages: repos.map((repo) => ({ @@ -90,6 +100,9 @@ function makeAuthRouter({ authService, storageService }) { })), }); } catch (err) { + if (err.status === 401) { + return githubAccessRevoked(res); + } console.error(err); return res.status(500).json({ error: 'Failed to list storages' }); } @@ -105,15 +118,17 @@ function makeAuthRouter({ authService, storageService }) { } const clients = await authService.clientsFor(req.user); - if (!clients.githubUserClient && !clients.githubClient) { - return res.status(400).json({ error: 'GitHub client is not available' }); - } + + if (!getOctokit(clients, res)) return; const result = await storageService.discoverAccountStores(provider, clients, { sessionStorageRef: req.user?.storage || null, }); return res.json(result); } catch (err) { + if (err.status === 401) { + return githubAccessRevoked(res); + } console.error(err); return res.status(500).json({ error: err.message || 'Failed to discover storage', @@ -131,9 +146,7 @@ function makeAuthRouter({ authService, storageService }) { } const clients = await authService.clientsFor(req.user); - if (!clients.githubUserClient && !clients.githubClient) { - return res.status(400).json({ error: 'GitHub client is not available' }); - } + if (!getOctokit(clients, res)) return; const installUrl = await authService.getInstallationSetupUrl(); const result = @@ -260,6 +273,13 @@ function makeAuthRouter({ authService, storageService }) { return router; } +function githubAccessRevoked(res) { + return res.status(401).json({ + error: 'GitHub access was revoked. Sign in with GitHub again.', + code: 'GITHUB_AUTH_REVOKED', + }); +} + /** @param {import('express').Request} req */ function requireAuth(req, res, next) { if (req.isAuthenticated()) { diff --git a/backend/services/storageService.js b/backend/services/storageService.js index f890104..6249f94 100644 --- a/backend/services/storageService.js +++ b/backend/services/storageService.js @@ -314,11 +314,13 @@ class StorageService { const { data: user } = await octokit.rest.users.getAuthenticated(); owner = user.login; } catch (err) { - throw new Error( + const wrapped = new Error( err?.status === 401 ? 'GitHub authentication failed. Sign out and sign in again.' : 'Could not look up your GitHub username.', ); + wrapped.status = err?.status; + throw wrapped; } try { diff --git a/backend/tests/auth.test.js b/backend/tests/auth.test.js index 7955832..0408c9d 100644 --- a/backend/tests/auth.test.js +++ b/backend/tests/auth.test.js @@ -318,6 +318,98 @@ test('GET /api/auth/storages returns mapped GitHub repos', async () => { assert.equal(res.body.storages[0].id, 'R_kg'); }); +test('GET /api/auth/storages returns GITHUB_AUTH_REVOKED when no client is available', async () => { + const app = createAuthedApp({ + user: AUTHED_USER, + authService: { + clientsFor: async () => ({}), + }, + storageService: { + listGitHubRepos: async () => { + throw new Error('should not be called without a client'); + }, + }, + }); + const res = await request(app).get('/api/auth/storages?provider=github'); + assert.equal(res.status, 401); + assert.equal(res.body.code, 'GITHUB_AUTH_REVOKED'); +}); + +test('GET /api/auth/storages returns GITHUB_AUTH_REVOKED when GitHub rejects the token', async () => { + const app = createAuthedApp({ + user: AUTHED_USER, + authService: { + clientsFor: async () => ({ githubClient: { mock: true } }), + }, + storageService: { + listGitHubRepos: async () => { + const err = new Error('Bad credentials'); + err.status = 401; + throw err; + }, + }, + }); + const res = await request(app).get('/api/auth/storages?provider=github'); + assert.equal(res.status, 401); + assert.equal(res.body.code, 'GITHUB_AUTH_REVOKED'); +}); + +test('GET /api/auth/storage/discover returns GITHUB_AUTH_REVOKED when no client is available', async () => { + const app = createAuthedApp({ + user: AUTHED_USER, + authService: { + clientsFor: async () => ({}), + }, + storageService: { + discoverAccountStores: async () => { + throw new Error('should not be called without a client'); + }, + }, + }); + const res = await request(app).get('/api/auth/storage/discover?provider=github'); + assert.equal(res.status, 401); + assert.equal(res.body.code, 'GITHUB_AUTH_REVOKED'); +}); + +test('GET /api/auth/storage/discover returns GITHUB_AUTH_REVOKED when GitHub rejects the token', async () => { + const app = createAuthedApp({ + user: AUTHED_USER, + authService: { + clientsFor: async () => ({ githubUserClient: { mock: true } }), + }, + storageService: { + discoverAccountStores: async () => { + const err = new Error('GitHub authentication failed. Sign out and sign in again.'); + err.status = 401; + throw err; + }, + }, + }); + const res = await request(app).get('/api/auth/storage/discover?provider=github'); + assert.equal(res.status, 401); + assert.equal(res.body.code, 'GITHUB_AUTH_REVOKED'); +}); + +test('POST /api/auth/storage/create returns GITHUB_AUTH_REVOKED when no client is available', async () => { + const app = createAuthedApp({ + user: AUTHED_USER, + authService: { + clientsFor: async () => ({}), + getInstallationSetupUrl: async () => 'https://github.com/settings/installations', + }, + storageService: { + createGitHubRepository: async () => { + throw new Error('should not be called without a client'); + }, + }, + }); + const res = await request(app) + .post('/api/auth/storage/create') + .send({ name: 'vizably-new' }); + assert.equal(res.status, 401); + assert.equal(res.body.code, 'GITHUB_AUTH_REVOKED'); +}); + test('POST /api/auth/storage/create returns storageRef and needsInstall', async () => { const app = createAuthedApp({ user: AUTHED_USER, diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index b8e9a1a..c8ccb7e 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -332,6 +332,16 @@ function AppRoutes() { navigate(PATHS.dashboard) } + const reconnectGitHub = async () => { + try { + await apiClient.logout() + } catch { + // Clear local state even if the network call fails. + } + setUser(null) + apiClient.githubLogin() + } + const signOut = async () => { try { await apiClient.logout() @@ -342,6 +352,32 @@ function AppRoutes() { navigate(PATHS.landing) } + const redirectToSignIn = async () => { + try { + await apiClient.logout() + } catch { + // Clear local state even if the network call fails. + } + navigate(PATHS.signin) + setUser(null) + setAuthLoading(false) + setScan(null) + setProblem(null) + } + + /** + * Cancel out of storage setup. Only forces a fresh sign-in when GitHub + * access was actually revoked — someone who just clicked Connect by + * mistake keeps their session and can pick up onboarding again later. + */ + const cancelConnect = (isRevoked) => { + if (isRevoked) { + redirectToSignIn() + return + } + navigate(PATHS.landing) + } + const route = routeKeyFor(location.pathname) useEffect(() => { @@ -363,7 +399,8 @@ function AppRoutes() { navigate(PATHS.signin)} + onCancel={cancelConnect} + onReconnect={reconnectGitHub} storageError={storageError} /> ) @@ -397,9 +434,7 @@ function AppRoutes() { element={ authed && storageReady ? - : authed - ? - : + : } /> } /> diff --git a/frontend/src/__tests__/connectView.test.jsx b/frontend/src/__tests__/connectView.test.jsx index 5ebf852..04ddea4 100644 --- a/frontend/src/__tests__/connectView.test.jsx +++ b/frontend/src/__tests__/connectView.test.jsx @@ -195,6 +195,62 @@ describe('ConnectView', () => { await waitFor(() => expect(client.discoverStorages).toHaveBeenCalled()) }) + it('Back does not signal a revoke when access is fine', async () => { + const onCancel = vi.fn() + const client = mockClient() + render( + , + ) + + expect(await screen.findByText('Vizably account found')).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /^back$/i })) + expect(onCancel).toHaveBeenCalledWith(false) + }) + + it('offers reconnect instead of create when GitHub access was revoked', async () => { + const onReconnect = vi.fn() + const err = new Error('GitHub access was revoked. Sign in with GitHub again.') + err.status = 401 + err.code = 'GITHUB_AUTH_REVOKED' + const client = mockClient({ + discoverStorages: vi.fn().mockRejectedValue(err), + }) + + render( + , + ) + + expect(await screen.findByText(/Reconnect to authorize/i)).toBeInTheDocument() + expect(screen.getByText(/GitHub access revoked/i)).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /reconnect github/i })) + await waitFor(() => expect(onReconnect).toHaveBeenCalled()) + expect(client.createStorage).not.toHaveBeenCalled() + }) + + it('Back signals a revoke so the caller can force a fresh sign-in', async () => { + const onCancel = vi.fn() + const err = new Error('GitHub access was revoked. Sign in with GitHub again.') + err.status = 401 + err.code = 'GITHUB_AUTH_REVOKED' + const client = mockClient({ + discoverStorages: vi.fn().mockRejectedValue(err), + }) + + render( + , + ) + + expect(await screen.findByText(/Reconnect to authorize/i)).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /^back$/i })) + expect(onCancel).toHaveBeenCalledWith(true) + }) + it('shows install hop when create returns needsInstall', async () => { const client = mockClient({ discoverStorages: vi.fn().mockResolvedValue({ diff --git a/frontend/src/views/ConnectView.jsx b/frontend/src/views/ConnectView.jsx index 63a5e81..8cf0c10 100644 --- a/frontend/src/views/ConnectView.jsx +++ b/frontend/src/views/ConnectView.jsx @@ -43,6 +43,14 @@ const STATUS_UI = { }, } +function isGitHubAccessLost(err) { + return ( + err?.code === 'GITHUB_AUTH_REVOKED' || + err?.status === 401 || + /GitHub client is not available/i.test(err?.message || '') + ) +} + function reasonMessage(reason) { switch (reason) { case 'malformed_manifest': @@ -78,7 +86,8 @@ function storageRefFromHit(hit) { * @param {object} props * @param {'github' | 'google'} props.provider * @param {() => void} props.onDone - * @param {() => void} props.onCancel + * @param {(isRevoked?: boolean) => void} props.onCancel + * @param {() => void | Promise} [props.onReconnect] * @param {string} [props.storageError] * @param {import('../lib/apiClient').ApiClient} [props.client] */ @@ -86,6 +95,7 @@ export default function ConnectView({ provider, onDone, onCancel, + onReconnect, storageError = null, client = apiClient, }) { @@ -103,6 +113,7 @@ export default function ConnectView({ const [validating, setValidating] = useState(false) const [confirming, setConfirming] = useState(false) const [error, setError] = useState(storageError) + const [needsReconnect, setNeedsReconnect] = useState(false) const selectedHit = useMemo( () => stores.find((hit) => hit.storageRef.id === selectedId) ?? null, @@ -128,10 +139,11 @@ export default function ConnectView({ !activeStorageRef)) const confirmLabel = useMemo(() => { + if (needsReconnect) return 'Reconnect GitHub' if (emptyAccount) return 'Set up Vizably storage' if (statusUi?.button) return statusUi.button return 'Continue' - }, [emptyAccount, statusUi]) + }, [needsReconnect, emptyAccount, statusUi]) const runValidation = useCallback( async (storageRef) => { @@ -161,6 +173,7 @@ export default function ConnectView({ if (!isGitHub) return [] setDiscovering(true) setError(null) + setNeedsReconnect(false) try { const result = await client.discoverStorages('github') const list = result.stores ?? [] @@ -171,7 +184,13 @@ export default function ConnectView({ }) return list } catch (err) { - setError(err.message || 'Failed to look up Vizably storage') + const lost = isGitHubAccessLost(err) + setNeedsReconnect(lost) + setError( + lost + ? 'GitHub access was revoked. Reconnect to authorize Vizably again.' + : err.message || 'Failed to look up Vizably storage', + ) setStores([]) setSelectedId('') return [] @@ -263,7 +282,24 @@ export default function ConnectView({ await runValidation(ref) } + const handleReconnect = async () => { + if (onReconnect) { + await onReconnect() + return + } + try { + await client.logout() + } catch { + // Continue to GitHub even if logout fails (stale cookie). + } + client.githubLogin() + } + const handleConfirm = async () => { + if (needsReconnect) { + await handleReconnect() + return + } if (emptyAccount) { await handleCreateDefault() return @@ -302,7 +338,7 @@ export default function ConnectView({

Google Drive storage uses the Google Picker and is not wired yet. Use GitHub for now.

- @@ -336,9 +372,11 @@ export default function ConnectView({ > {providerIcon} - {pv.name} connected + {needsReconnect ? `${pv.name} access revoked` : `${pv.name} connected`} + + + {Ico(needsReconnect ? 'TriangleAlert' : 'Check', 15, 'currentColor')} - {Ico('Check', 15, 'currentColor')}

Connect your Vizably storage @@ -561,7 +599,7 @@ export default function ConnectView({
-