From 2538b9b26d5f1c6a2f4114affeace6196303475d Mon Sep 17 00:00:00 2001 From: Saar Shen Date: Fri, 15 May 2026 21:32:53 -0700 Subject: [PATCH 1/2] refactor: introduce AuthClient abstraction (Phase 1 of #64) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor MSAL auth behind an AuthClient interface so future providers (e.g. Google Identity Services) can plug in without touching consumers. Strict refactor — zero behavior change. Microsoft remains the only provider. Architecture: - lib/auth/types.ts: AuthClient interface (acquireToken with silentOnly option, login, logout, getUser, isAuthenticated, busy) - lib/auth/microsoft/MicrosoftAuthClient.ts: wraps MSAL (loginPopup, acquireTokenSilent+Popup, logoutPopup, accounts[0]) - lib/auth/useAuthClient.ts: hook returning the active client (today always Microsoft) Implementation notes: - acquireToken reads accounts fresh from instance.getAllAccounts() rather than from useMsal-captured state, eliminating the post-loginPopup race identified in PR #63. The one-shot store hack in app/page.tsx is no longer needed. - getUser sources displayName and email from MSAL account info instead of calling Graph /me. Removes a wasted round-trip on the books page. Removed: - lib/hooks/useGraphToken.ts (logic absorbed into MicrosoftAuthClient) - getUserInfo + UserInfo type from graphService.ts (no longer used) Other: - All pages and useBookId now use useAuthClient instead of useGraphToken. - HamburgerMenu signout uses auth.logout() instead of instance.logoutPopup directly. - app/page.tsx silent background check uses acquireToken({ silentOnly: true }) instead of constructing a one-shot MultiBackendStore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/arrange-v4/app/books/page.tsx | 37 ++++----- src/arrange-v4/app/cancelled/page.tsx | 17 ++-- src/arrange-v4/app/matrix/page.tsx | 17 ++-- src/arrange-v4/app/page.tsx | 49 +++++------- src/arrange-v4/app/scrum/page.tsx | 17 ++-- src/arrange-v4/components/HamburgerMenu.tsx | 15 ++-- .../lib/auth/microsoft/MicrosoftAuthClient.ts | 79 +++++++++++++++++++ src/arrange-v4/lib/auth/types.ts | 47 +++++++++++ src/arrange-v4/lib/auth/useAuthClient.ts | 15 ++++ src/arrange-v4/lib/graphService.ts | 15 +--- src/arrange-v4/lib/hooks/useBookId.ts | 8 +- src/arrange-v4/lib/hooks/useGraphToken.ts | 39 --------- src/arrange-v4/lib/store/useStore.ts | 4 +- 13 files changed, 225 insertions(+), 134 deletions(-) create mode 100644 src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts create mode 100644 src/arrange-v4/lib/auth/types.ts create mode 100644 src/arrange-v4/lib/auth/useAuthClient.ts delete mode 100644 src/arrange-v4/lib/hooks/useGraphToken.ts diff --git a/src/arrange-v4/app/books/page.tsx b/src/arrange-v4/app/books/page.tsx index ec2830f..9660a29 100644 --- a/src/arrange-v4/app/books/page.tsx +++ b/src/arrange-v4/app/books/page.tsx @@ -1,17 +1,17 @@ 'use client'; import { useState, useEffect } from 'react'; -import { getUserInfo } from '@/lib/graphService'; import { useStore } from '@/lib/store/useStore'; import type { Book } from '@/lib/store/types'; -import { useGraphToken } from '@/lib/hooks/useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { useSetTopBarActions } from '@/components/TopBarProvider'; import CalendarList from '@/components/CalendarList'; import CreateCalendar from '@/components/CreateCalendar'; import styles from './page.module.css'; export default function BooksPage() { - const { acquireToken, isAuthenticated, inProgress, handleLogin: graphLogin, instance } = useGraphToken(); + const auth = useAuthClient(); + const { isAuthenticated, busy } = auth; const store = useStore(); const [books, setBooks] = useState([]); const [loading, setLoading] = useState(false); @@ -20,17 +20,19 @@ export default function BooksPage() { const handleLogin = async () => { try { - await graphLogin(); + await auth.login(); } catch (error) { console.error('Login failed:', error); setError('Login failed. Please try again.'); } }; - const handleLogout = () => { - instance.logoutPopup({ - postLogoutRedirectUri: '/', - }); + const handleLogout = async () => { + try { + await auth.logout(); + } catch (error) { + console.error('Logout failed:', error); + } }; const fetchBooks = async () => { @@ -40,9 +42,8 @@ export default function BooksPage() { setError(null); try { - const accessToken = await acquireToken(); - const userInfo = await getUserInfo(accessToken); - setUserName(userInfo.displayName || userInfo.userPrincipalName || ''); + const user = auth.getUser(); + setUserName(user?.displayName || user?.email || ''); const allBooks = await store.listBooks(); setBooks(allBooks); @@ -78,20 +79,20 @@ export default function BooksPage() { }; useEffect(() => { - if (isAuthenticated && inProgress === 'none') { + if (isAuthenticated && !busy) { fetchBooks(); } - }, [isAuthenticated, inProgress]); + }, [isAuthenticated, busy]); useSetTopBarActions( null, !isAuthenticated ? ( ) : ( <> @@ -114,7 +115,7 @@ export default function BooksPage() { ), - [isAuthenticated, inProgress, loading], + [isAuthenticated, busy, loading], ); return ( @@ -147,10 +148,10 @@ export default function BooksPage() {

)} diff --git a/src/arrange-v4/app/cancelled/page.tsx b/src/arrange-v4/app/cancelled/page.tsx index ae0c602..16d2d24 100644 --- a/src/arrange-v4/app/cancelled/page.tsx +++ b/src/arrange-v4/app/cancelled/page.tsx @@ -4,7 +4,7 @@ import React, { useState, useEffect, useCallback, useRef, Suspense } from 'react import { useStore } from '@/lib/store/useStore'; import type { TodoItem, TodoItemWithId } from '@/lib/store/types'; import { formatRelativeDate } from '@/lib/dateUtils'; -import { useGraphToken } from '@/lib/hooks/useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { useBookId } from '@/lib/hooks/useBookId'; import { useSetTopBarActions } from '@/components/TopBarProvider'; import ViewTodoItem from '@/components/ViewTodoItem'; @@ -12,7 +12,8 @@ import Link from 'next/link'; import styles from './page.module.css'; function CancelledPageContent() { - const { isAuthenticated, inProgress, handleLogin: graphLogin } = useGraphToken(); + const auth = useAuthClient(); + const { isAuthenticated, busy } = auth; const store = useStore(); const { bookId, books, handleBookSwitch, error: bookError } = useBookId('/cancelled'); @@ -50,10 +51,10 @@ function CancelledPageContent() { }, [isAuthenticated, bookId, store]); useEffect(() => { - if (isAuthenticated && inProgress === 'none' && bookId) { + if (isAuthenticated && !busy && bookId) { fetchEvents(); } - }, [isAuthenticated, inProgress, bookId, fetchEvents]); + }, [isAuthenticated, busy, bookId, fetchEvents]); const handleDeleteSelected = () => { if (selectedIds.size === 0) return; @@ -62,7 +63,7 @@ function CancelledPageContent() { const handleLogin = async () => { try { - await graphLogin(); + await auth.login(); } catch (err) { console.error('Login failed:', err); setError('Login failed. Please try again.'); @@ -87,10 +88,10 @@ function CancelledPageContent() { !isAuthenticated ? ( ) : ( <> @@ -110,7 +111,7 @@ function CancelledPageContent() { ), - [isAuthenticated, inProgress, loading, deleting, bookId, books, selectedIds.size], + [isAuthenticated, busy, loading, deleting, bookId, books, selectedIds.size], ); const toggleSelect = (id: string) => { diff --git a/src/arrange-v4/app/matrix/page.tsx b/src/arrange-v4/app/matrix/page.tsx index 9860edd..6d58026 100644 --- a/src/arrange-v4/app/matrix/page.tsx +++ b/src/arrange-v4/app/matrix/page.tsx @@ -5,7 +5,7 @@ import { useStore } from '@/lib/store/useStore'; import { TodoItem, TodoItemWithId, TodoStatus, ALL_STATUSES, STATUS_LABELS } from '@/lib/store/types'; import { formatRelativeDate } from '@/lib/dateUtils'; import { hasSessionSweepRun, isSessionSweepInProgress, markSessionSweepInProgress, clearSessionSweepInProgress, markSessionSweepDone } from '@/lib/bookStorage'; -import { useGraphToken } from '@/lib/hooks/useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { useBookId } from '@/lib/hooks/useBookId'; import { useSetTopBarActions } from '@/components/TopBarProvider'; import AddTodoItem from '@/components/AddTodoItem'; @@ -187,7 +187,8 @@ export default function MatrixPage() { } function MatrixPageContent() { - const { isAuthenticated, inProgress, handleLogin: graphLogin } = useGraphToken(); + const auth = useAuthClient(); + const { isAuthenticated, busy } = auth; const store = useStore(); const { bookId, books, handleBookSwitch, error: bookError } = useBookId('/matrix'); const bookIdRef = useRef(bookId); @@ -527,7 +528,7 @@ function MatrixPageContent() { const handleLogin = async () => { try { - await graphLogin(); + await auth.login(); } catch (error) { console.error('Login failed:', error); setError('Login failed. Please try again.'); @@ -535,10 +536,10 @@ function MatrixPageContent() { }; useEffect(() => { - if (isAuthenticated && inProgress === 'none' && bookId) { + if (isAuthenticated && !busy && bookId) { fetchEvents(); } - }, [isAuthenticated, inProgress, bookId]); + }, [isAuthenticated, busy, bookId]); // Push page actions into the shared top bar useSetTopBarActions( @@ -558,10 +559,10 @@ function MatrixPageContent() { !isAuthenticated ? ( ) : ( <> @@ -575,7 +576,7 @@ function MatrixPageContent() { ), - [isAuthenticated, inProgress, loading, bookId, books, allCategories], + [isAuthenticated, busy, loading, bookId, books, allCategories], ); if (!bookId) { diff --git a/src/arrange-v4/app/page.tsx b/src/arrange-v4/app/page.tsx index c2317ab..4e82c91 100644 --- a/src/arrange-v4/app/page.tsx +++ b/src/arrange-v4/app/page.tsx @@ -1,20 +1,20 @@ 'use client'; -import { useMsal } from '@azure/msal-react'; -import { loginRequest } from '@/lib/msalConfig'; import { useRouter } from 'next/navigation'; -import { MultiBackendStore } from '@/lib/store/multiStore'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; +import { useStore } from '@/lib/store/useStore'; import { normalizeBookId } from '@/lib/store/types'; import { getLastBookId } from '@/lib/bookStorage'; import { useState, useEffect } from 'react'; import styles from './page.module.css'; export default function Home() { - const { instance, accounts, inProgress } = useMsal(); + const auth = useAuthClient(); + const store = useStore(); const router = useRouter(); const [matrixAvailable, setMatrixAvailable] = useState<{ show: boolean; bookId?: string }>({ show: false }); - const isAuthenticated = accounts.length > 0; + const { isAuthenticated, busy } = auth; // Background availability check: silent-only token acquisition so we never // open an unexpected popup from a useEffect. If the silent acquisition fails @@ -25,17 +25,12 @@ export default function Home() { useEffect(() => { let cancelled = false; const check = async () => { - if (!isAuthenticated || !accounts[0]) return; + if (!isAuthenticated) return; try { - const response = await instance.acquireTokenSilent({ - ...loginRequest, - account: accounts[0], - }); + // silentOnly: true — never open a popup from this background check. + await auth.acquireToken({ silentOnly: true }); if (cancelled) return; - const silentStore = new MultiBackendStore({ - acquireToken: async () => response.accessToken, - }); - const books = await silentStore.listBooks(); + const books = await store.listBooks(); if (cancelled) return; if (books.length === 1) { @@ -53,8 +48,8 @@ export default function Home() { } catch (error) { if (cancelled) return; // Silent failure is fine for this background check — don't open a popup. - // But reset matrixAvailable so stale data from a previous account or - // a previously-successful check doesn't linger. + // Reset matrixAvailable so stale data from a previous account or a + // previously-successful check doesn't linger. setMatrixAvailable({ show: false }); console.error('Error checking matrix availability:', error); } @@ -64,24 +59,20 @@ export default function Home() { return () => { cancelled = true; }; - }, [isAuthenticated, accounts, instance]); + }, [isAuthenticated, auth, store]); const handleLogin = async () => { try { - const result = await instance.loginPopup(loginRequest); - - // MSAL's React state hasn't re-rendered yet — useGraphToken would still - // return a stale closure that throws "no account". Build a one-shot store - // bound to the access token we just got back from loginPopup. - const postLoginStore = new MultiBackendStore({ - acquireToken: async () => result.accessToken, - }); + await auth.login(); // Wrap the post-login routing decision in its own try/catch. A transient - // Graph error must not leave the user stuck on the landing page after a + // backend error must not leave the user stuck on the landing page after a // successful login — fall back to /books in that case. + // Safe to call store methods here: AuthClient.acquireToken reads its + // underlying SDK state fresh, so the post-login token works even before + // React has re-rendered with the new auth state. try { - const books = await postLoginStore.listBooks(); + const books = await store.listBooks(); if (books.length === 1) { router.push(`/matrix?bookId=${encodeURIComponent(books[0].id)}`); @@ -130,10 +121,10 @@ export default function Home() { {!isAuthenticated ? ( ) : ( <> diff --git a/src/arrange-v4/app/scrum/page.tsx b/src/arrange-v4/app/scrum/page.tsx index 810d590..0d2f7d5 100644 --- a/src/arrange-v4/app/scrum/page.tsx +++ b/src/arrange-v4/app/scrum/page.tsx @@ -3,7 +3,7 @@ import React, { useState, useEffect, useCallback, useMemo, Suspense } from 'react'; import { useStore } from '@/lib/store/useStore'; import { TodoItem, TodoItemWithId, TodoStatus, ALL_STATUSES, STATUS_LABELS } from '@/lib/store/types'; -import { useGraphToken } from '@/lib/hooks/useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { useBookId } from '@/lib/hooks/useBookId'; import { useSetTopBarActions } from '@/components/TopBarProvider'; import AddTodoItem from '@/components/AddTodoItem'; @@ -70,7 +70,8 @@ function sortByPriority(items: TodoItemWithId[]) { } function ScrumPageContent() { - const { isAuthenticated, inProgress, handleLogin: graphLogin } = useGraphToken(); + const auth = useAuthClient(); + const { isAuthenticated, busy } = auth; const store = useStore(); const { bookId, books, handleBookSwitch, error: bookError } = useBookId('/scrum'); @@ -161,10 +162,10 @@ function ScrumPageContent() { }, [isAuthenticated, bookId, store]); useEffect(() => { - if (isAuthenticated && inProgress === 'none' && bookId) { + if (isAuthenticated && !busy && bookId) { fetchEvents(); } - }, [isAuthenticated, inProgress, bookId, fetchEvents]); + }, [isAuthenticated, busy, bookId, fetchEvents]); const handleAddTodo = async (todoItem: TodoItem) => { if (!bookId) throw new Error('No book selected'); @@ -180,7 +181,7 @@ function ScrumPageContent() { const handleLogin = async () => { try { - await graphLogin(); + await auth.login(); } catch (err) { console.error('Login failed:', err); setError('Login failed. Please try again.'); @@ -204,10 +205,10 @@ function ScrumPageContent() { !isAuthenticated ? ( ) : ( <> @@ -221,7 +222,7 @@ function ScrumPageContent() { ), - [isAuthenticated, inProgress, loading, bookId, books, allCategories], + [isAuthenticated, busy, loading, bookId, books, allCategories], ); const handleDragStart = (todo: TodoItemWithId) => { diff --git a/src/arrange-v4/components/HamburgerMenu.tsx b/src/arrange-v4/components/HamburgerMenu.tsx index 1e33833..f115362 100644 --- a/src/arrange-v4/components/HamburgerMenu.tsx +++ b/src/arrange-v4/components/HamburgerMenu.tsx @@ -3,7 +3,7 @@ import { useState, useEffect, useRef, Suspense } from 'react'; import Link from 'next/link'; import { usePathname, useSearchParams } from 'next/navigation'; -import { useMsal } from '@azure/msal-react'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { getLastBookId } from '@/lib/bookStorage'; import { useTopBarActions } from './TopBarProvider'; import styles from './HamburgerMenu.module.css'; @@ -56,12 +56,12 @@ export default function HamburgerMenu() { const [isOpen, setIsOpen] = useState(false); const [navItems, setNavItems] = useState(BASE_NAV_ITEMS); const pathname = usePathname(); - const { instance, accounts } = useMsal(); + const auth = useAuthClient(); const sidebarRef = useRef(null); const triggerRef = useRef(null); const { leftActions, rightActions } = useTopBarActions(); - const isAuthenticated = accounts.length > 0; + const isAuthenticated = auth.isAuthenticated; const isOnMatrix = pathname.startsWith('/matrix'); const isOnScrum = pathname.startsWith('/scrum'); @@ -78,10 +78,13 @@ export default function HamburgerMenu() { const currentPage = navItems.find(item => isActive(item)); const pageLabel = currentPage?.label || 'Arrange'; - const handleSignOut = () => { + const handleSignOut = async () => { setIsOpen(false); - const basePath = process.env.NEXT_PUBLIC_BASE_PATH || ''; - instance.logoutPopup({ postLogoutRedirectUri: `${window.location.origin}${basePath}/` }); + try { + await auth.logout(); + } catch (error) { + console.error('Logout failed:', error); + } }; useEffect(() => { diff --git a/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts b/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts new file mode 100644 index 0000000..e47ae66 --- /dev/null +++ b/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts @@ -0,0 +1,79 @@ +'use client'; + +import { useCallback, useMemo } from 'react'; +import { useMsal } from '@azure/msal-react'; +import { InteractionRequiredAuthError } from '@azure/msal-browser'; +import { loginRequest } from '@/lib/msalConfig'; +import type { AuthClient, AuthUser } from '../types'; + +/** + * React hook that adapts MSAL into the `AuthClient` shape. + * + * All MSAL-specific knowledge stays in this file. Consumers depend only on + * the `AuthClient` interface. + */ +export function useMicrosoftAuthClient(): AuthClient { + const { instance, accounts, inProgress } = useMsal(); + const isAuthenticated = accounts.length > 0; + const busy = inProgress !== 'none'; + + const acquireToken = useCallback(async (options?: { silentOnly?: boolean }): Promise => { + // Read accounts fresh from the MSAL instance, not from the captured React + // state. The captured state can lag by one render after loginPopup resolves, + // which would cause the post-login "no account" race. + const account = instance.getAllAccounts()[0]; + if (!account) { + throw new Error( + 'No signed-in Microsoft account is available. Sign in before requesting a Microsoft Graph access token.', + ); + } + try { + const response = await instance.acquireTokenSilent({ ...loginRequest, account }); + return response.accessToken; + } catch (silentError: unknown) { + if (options?.silentOnly) { + throw silentError; + } + if (silentError instanceof InteractionRequiredAuthError) { + const response = await instance.acquireTokenPopup(loginRequest); + return response.accessToken; + } + throw silentError; + } + }, [instance]); + + const login = useCallback(async (): Promise => { + await instance.loginPopup(loginRequest); + }, [instance]); + + const logout = useCallback(async (): Promise => { + const basePath = process.env.NEXT_PUBLIC_BASE_PATH || ''; + const postLogoutRedirectUri = + typeof window !== 'undefined' + ? `${window.location.origin}${basePath}/` + : '/'; + await instance.logoutPopup({ postLogoutRedirectUri }); + }, [instance]); + + const getUser = useCallback((): AuthUser | null => { + const account = accounts[0]; + if (!account) return null; + return { + displayName: account.name || account.username, + email: account.username, + }; + }, [accounts]); + + return useMemo( + () => ({ + provider: 'microsoft', + isAuthenticated, + busy, + acquireToken, + login, + logout, + getUser, + }), + [isAuthenticated, busy, acquireToken, login, logout, getUser], + ); +} diff --git a/src/arrange-v4/lib/auth/types.ts b/src/arrange-v4/lib/auth/types.ts new file mode 100644 index 0000000..913c79b --- /dev/null +++ b/src/arrange-v4/lib/auth/types.ts @@ -0,0 +1,47 @@ +/** + * Authentication abstraction. + * + * Today the only provider is Microsoft (MSAL). The interface is designed to + * accept additional providers (e.g. Google Identity Services) without + * touching consumers. + */ + +export type AuthProvider = 'microsoft'; + +export const ALL_AUTH_PROVIDERS: AuthProvider[] = ['microsoft']; + +export interface AuthUser { + /** Human-friendly name for display. Always present (falls back to email if no name is set). */ + displayName: string; + /** Email address / UPN if the provider exposes one. */ + email?: string; +} + +export interface AuthClient { + /** Which provider this client implements. */ + readonly provider: AuthProvider; + /** True when a user is signed in. */ + readonly isAuthenticated: boolean; + /** True when login, logout, or token acquisition is in progress. */ + readonly busy: boolean; + + /** + * Acquires an access token suitable for the corresponding backend's API. + * Implementations should attempt silent acquisition first and fall back to + * an interactive popup only when necessary. + * + * Pass `{ silentOnly: true }` to disable the popup fallback — useful for + * background checks (e.g. from a `useEffect`) where an unexpected popup + * would be a poor UX. + */ + acquireToken(options?: { silentOnly?: boolean }): Promise; + + /** Starts an interactive sign-in flow. */ + login(): Promise; + + /** Signs the current user out and clears any cached state. */ + logout(): Promise; + + /** Returns the signed-in user, or null when not signed in. */ + getUser(): AuthUser | null; +} diff --git a/src/arrange-v4/lib/auth/useAuthClient.ts b/src/arrange-v4/lib/auth/useAuthClient.ts new file mode 100644 index 0000000..7fc14ac --- /dev/null +++ b/src/arrange-v4/lib/auth/useAuthClient.ts @@ -0,0 +1,15 @@ +'use client'; + +import { useMicrosoftAuthClient } from './microsoft/MicrosoftAuthClient'; +import type { AuthClient } from './types'; + +/** + * Returns the active `AuthClient` for the current user. + * + * Today this always returns the Microsoft client. When additional providers + * are added, this hook will select the right implementation based on the + * user's stored provider preference. + */ +export function useAuthClient(): AuthClient { + return useMicrosoftAuthClient(); +} diff --git a/src/arrange-v4/lib/graphService.ts b/src/arrange-v4/lib/graphService.ts index bdc1a85..2a92015 100644 --- a/src/arrange-v4/lib/graphService.ts +++ b/src/arrange-v4/lib/graphService.ts @@ -4,8 +4,9 @@ import { Client } from '@microsoft/microsoft-graph-client'; * Low-level Microsoft Graph utilities. * * After the storage abstraction landed, this file is intentionally minimal: - * just an authenticated client factory plus the one user-info endpoint that - * the Books page calls directly. All calendar CRUD lives in `lib/store/calendar/`. + * just an authenticated client factory used by the calendar store. All + * calendar CRUD lives in `lib/store/calendar/`. User info is sourced from + * the active AuthClient (`auth.getUser()`) instead of `/me`. */ export function createGraphClient(accessToken: string): Client { @@ -15,13 +16,3 @@ export function createGraphClient(accessToken: string): Client { }, }); } - -export interface UserInfo { - displayName?: string; - userPrincipalName?: string; -} - -export async function getUserInfo(accessToken: string): Promise { - const client = createGraphClient(accessToken); - return client.api('/me').get(); -} diff --git a/src/arrange-v4/lib/hooks/useBookId.ts b/src/arrange-v4/lib/hooks/useBookId.ts index 296d75b..88d1cfc 100644 --- a/src/arrange-v4/lib/hooks/useBookId.ts +++ b/src/arrange-v4/lib/hooks/useBookId.ts @@ -6,7 +6,7 @@ import { useStore } from '@/lib/store/useStore'; import { normalizeBookId } from '@/lib/store/types'; import type { Book } from '@/lib/store/types'; import { getLastBookId, setLastBookId, clearLastBookId } from '@/lib/bookStorage'; -import { useGraphToken } from './useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; /** * Shared hook for resolving the selected book. @@ -26,7 +26,7 @@ export function useBookId(routePrefix: string) { const rawBookId = searchParams.get('bookId'); const bookId = normalizeBookId(rawBookId); - const { isAuthenticated, inProgress } = useGraphToken(); + const { isAuthenticated, busy } = useAuthClient(); const store = useStore(); const [books, setBooks] = useState([]); @@ -55,7 +55,7 @@ export function useBookId(routePrefix: string) { }, [rawBookId, bookId, router, routePrefix]); const fetchBooks = useCallback(async () => { - if (!isAuthenticated || inProgress !== 'none') return; + if (!isAuthenticated || busy) return; setError(null); try { const all = await store.listBooks(); @@ -72,7 +72,7 @@ export function useBookId(routePrefix: string) { console.error('Error fetching books:', err); setError(message); } - }, [isAuthenticated, inProgress, store, bookId, router]); + }, [isAuthenticated, busy, store, bookId, router]); useEffect(() => { fetchBooks(); // eslint-disable-line react-hooks/set-state-in-effect -- async data fetching sets state after await diff --git a/src/arrange-v4/lib/hooks/useGraphToken.ts b/src/arrange-v4/lib/hooks/useGraphToken.ts deleted file mode 100644 index b718416..0000000 --- a/src/arrange-v4/lib/hooks/useGraphToken.ts +++ /dev/null @@ -1,39 +0,0 @@ -'use client'; - -import { useCallback } from 'react'; -import { useMsal } from '@azure/msal-react'; -import { InteractionRequiredAuthError } from '@azure/msal-browser'; -import { loginRequest } from '@/lib/msalConfig'; - -/** - * Shared hook for acquiring a Microsoft Graph API access token. - * Wraps the acquireTokenSilent + acquireTokenPopup fallback pattern - * that is used across multiple pages. - */ -export function useGraphToken() { - const { instance, accounts, inProgress } = useMsal(); - const isAuthenticated = accounts.length > 0; - - const acquireToken = useCallback(async (): Promise => { - const account = accounts[0]; - if (!account) { - throw new Error('No signed-in Microsoft account is available. Sign in before requesting a Microsoft Graph access token.'); - } - try { - const response = await instance.acquireTokenSilent({ ...loginRequest, account }); - return response.accessToken; - } catch (silentError: unknown) { - if (silentError instanceof InteractionRequiredAuthError) { - const response = await instance.acquireTokenPopup(loginRequest); - return response.accessToken; - } - throw silentError; - } - }, [instance, accounts]); - - const handleLogin = useCallback(async () => { - await instance.loginPopup(loginRequest); - }, [instance]); - - return { acquireToken, isAuthenticated, inProgress, handleLogin, instance }; -} diff --git a/src/arrange-v4/lib/store/useStore.ts b/src/arrange-v4/lib/store/useStore.ts index 0f10540..522b1ac 100644 --- a/src/arrange-v4/lib/store/useStore.ts +++ b/src/arrange-v4/lib/store/useStore.ts @@ -1,7 +1,7 @@ 'use client'; import { useMemo } from 'react'; -import { useGraphToken } from '@/lib/hooks/useGraphToken'; +import { useAuthClient } from '@/lib/auth/useAuthClient'; import { MultiBackendStore } from './multiStore'; /** @@ -11,6 +11,6 @@ import { MultiBackendStore } from './multiStore'; * from re-firing on every parent render. */ export function useStore(): MultiBackendStore { - const { acquireToken } = useGraphToken(); + const { acquireToken } = useAuthClient(); return useMemo(() => new MultiBackendStore({ acquireToken }), [acquireToken]); } From de22003231d5673aef5967083eff8f10e8726c44 Mon Sep 17 00:00:00 2001 From: Saar Shen Date: Fri, 15 May 2026 21:41:08 -0700 Subject: [PATCH 2/2] fix: read accounts fresh in getUser() for consistency with acquireToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous getUser() captured useMsal's accounts at hook render time, so a caller doing await auth.login(); auth.getUser() would get stale pre-login state for one render cycle. Read from instance.getAllAccounts() fresh — same fix as acquireToken. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts b/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts index e47ae66..ba12189 100644 --- a/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts +++ b/src/arrange-v4/lib/auth/microsoft/MicrosoftAuthClient.ts @@ -56,13 +56,15 @@ export function useMicrosoftAuthClient(): AuthClient { }, [instance]); const getUser = useCallback((): AuthUser | null => { - const account = accounts[0]; + // Read fresh from the MSAL instance for the same reason acquireToken does: + // captured React state can lag immediately after loginPopup/logoutPopup. + const account = instance.getAllAccounts()[0]; if (!account) return null; return { displayName: account.name || account.username, email: account.username, }; - }, [accounts]); + }, [instance]); return useMemo( () => ({