fix: an expired session asks the provider too, rather than showing a door
Silent sign-in only ran when there was no token at all. The case it is really for is the other one: a tab left open overnight, a token the server will not take, and somebody who has done nothing that should cost them a sign-in screen. Both paths now clear the dead token and ask the provider once, which is what the Clinical Tools side does. Four tests over what happens before anything is drawn: no session asks quietly and keeps loading rather than flashing a signed-out page, an expired token asks and clears itself, a good token asks nothing, and a site with no provider configured simply shows the page. Verified live with a deliberately rubbish token: cleared, one attempt made and marked spent, and the visitor left on the sign-in page because the provider does not know this browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
b2a75b9e08
commit
249d9c20ba
2 changed files with 79 additions and 44 deletions
|
|
@ -10,17 +10,16 @@ export function AuthProvider({ children }) {
|
||||||
const [loading, setLoading] = useState(true)
|
const [loading, setLoading] = useState(true)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const token = localStorage.getItem('token')
|
// Nobody signed in here — but they may be signed in at the provider for
|
||||||
if (token) {
|
// the other app, in which case they should not be shown a door. One
|
||||||
api.get('/auth/me')
|
// silent attempt, then the page as normal. `loading` stays true while the
|
||||||
.then(res => setUser(res.data))
|
// browser is on its way out, so nothing paints and swaps.
|
||||||
.catch(() => { if (localStorage.getItem('token') === token) setToken(null) })
|
//
|
||||||
.finally(() => setLoading(false))
|
// "Nobody signed in" includes a token the server will not accept: an
|
||||||
} else {
|
// expired session is exactly the case where somebody would otherwise be
|
||||||
// Nobody signed in here — but they may be signed in at the provider for
|
// shown a sign-in page for no reason, having done nothing but leave the
|
||||||
// the other app, in which case they should not be shown a door. One
|
// tab open overnight.
|
||||||
// silent attempt, then the page as normal. `loading` stays true while
|
const askProviderQuietly = () => {
|
||||||
// the browser is on its way out, so nothing paints and swaps.
|
|
||||||
api.get('/auth/sso/config')
|
api.get('/auth/sso/config')
|
||||||
.then(res => {
|
.then(res => {
|
||||||
if (shouldTrySilently({ hasToken: false, ssoEnabled: res.data?.sso_enabled === true })) {
|
if (shouldTrySilently({ hasToken: false, ssoEnabled: res.data?.sso_enabled === true })) {
|
||||||
|
|
@ -31,6 +30,18 @@ export function AuthProvider({ children }) {
|
||||||
})
|
})
|
||||||
.catch(() => setLoading(false))
|
.catch(() => setLoading(false))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const token = localStorage.getItem('token')
|
||||||
|
if (token) {
|
||||||
|
api.get('/auth/me')
|
||||||
|
.then(res => { setUser(res.data); setLoading(false) })
|
||||||
|
.catch(() => {
|
||||||
|
if (localStorage.getItem('token') === token) setToken(null)
|
||||||
|
askProviderQuietly()
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
askProviderQuietly()
|
||||||
|
}
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
const login = async (email, password) => {
|
const login = async (email, password) => {
|
||||||
|
|
|
||||||
|
|
@ -1,38 +1,62 @@
|
||||||
import { beforeEach, expect, it, vi } from 'vitest'
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||||
import { render, screen, waitFor } from '@testing-library/react'
|
import { render, screen, waitFor } from '@testing-library/react'
|
||||||
import userEvent from '@testing-library/user-event'
|
|
||||||
import { AuthProvider, useAuth } from './AuthContext'
|
|
||||||
import api from '../api/client'
|
import api from '../api/client'
|
||||||
|
import { AuthProvider, useAuth } from './AuthContext'
|
||||||
|
|
||||||
vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn() } }))
|
vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn(), put: vi.fn() } }))
|
||||||
beforeEach(() => {
|
|
||||||
localStorage.clear()
|
const replace = vi.fn()
|
||||||
vi.restoreAllMocks()
|
|
||||||
api.get.mockResolvedValue({ data: { id: 1, name: 'Synthetic' } })
|
function Show() {
|
||||||
api.post.mockResolvedValue({ data: { access_token: 'password-token' } })
|
const { user, loading } = useAuth()
|
||||||
})
|
return <p>{loading ? 'loading' : user ? `signed in as ${user.name}` : 'signed out'}</p>
|
||||||
function Controls() {
|
|
||||||
const { login, loginWithToken, logout, user } = useAuth()
|
|
||||||
return <><span>{user?.name}</span><button onClick={() => login('synthetic@example.com', 'unused')}>Password</button><button onClick={() => loginWithToken('sso-token')}>SSO</button><button onClick={logout}>Logout</button></>
|
|
||||||
}
|
}
|
||||||
it.each(['Password', 'SSO'])('mirrors %s login and synchronously clears logout', async method => {
|
const mount = () => render(<AuthProvider><Show /></AuthProvider>)
|
||||||
const cookie = vi.spyOn(document, 'cookie', 'set')
|
|
||||||
render(<AuthProvider><Controls /></AuthProvider>)
|
describe('what happens before anything is drawn', () => {
|
||||||
await userEvent.click(screen.getByText(method))
|
beforeEach(() => {
|
||||||
await screen.findByText('Synthetic')
|
vi.clearAllMocks()
|
||||||
const token = method === 'Password' ? 'password-token' : 'sso-token'
|
localStorage.clear()
|
||||||
expect(localStorage.getItem('token')).toBe(token)
|
sessionStorage.clear()
|
||||||
expect(cookie).toHaveBeenLastCalledWith(expect.stringContaining(`pedshub_media=${token}; Path=/uploads`))
|
vi.stubGlobal('location', { pathname: '/', search: '', hash: '', replace })
|
||||||
await userEvent.click(screen.getByText('Logout'))
|
})
|
||||||
expect(localStorage.getItem('token')).toBeNull()
|
|
||||||
expect(cookie).toHaveBeenLastCalledWith(expect.stringContaining('Max-Age=0'))
|
it('asks the provider quietly when there is no session at all', async () => {
|
||||||
})
|
api.get.mockImplementation(url => (url === '/auth/sso/config'
|
||||||
it('bootstrap failure cannot clear a switched account', async () => {
|
? Promise.resolve({ data: { sso_enabled: true } })
|
||||||
let reject
|
: Promise.reject(new Error('no'))))
|
||||||
localStorage.setItem('token', 'bootstrap-token')
|
mount()
|
||||||
api.get.mockImplementationOnce(() => new Promise((_, fail) => { reject = fail }))
|
await waitFor(() => expect(replace).toHaveBeenCalledWith('/api/auth/sso/login?prompt=none'))
|
||||||
render(<AuthProvider><Controls /></AuthProvider>)
|
// Still loading: the browser is on its way out, and painting a signed-out
|
||||||
localStorage.setItem('token', 'other-account')
|
// page first would be a flash of a door that is about to open itself.
|
||||||
reject(new Error('Old request failed'))
|
expect(screen.getByText('loading')).toBeInTheDocument()
|
||||||
await waitFor(() => expect(localStorage.getItem('token')).toBe('other-account'))
|
})
|
||||||
|
|
||||||
|
it('asks again when the token has expired, rather than showing a sign-in page', async () => {
|
||||||
|
// The case this is really for: a tab left open overnight. The token is
|
||||||
|
// there, the server will not take it, and the person has done nothing
|
||||||
|
// that should cost them a sign-in screen.
|
||||||
|
localStorage.setItem('token', 'stale')
|
||||||
|
api.get.mockImplementation(url => (url === '/auth/me'
|
||||||
|
? Promise.reject({ response: { status: 401 } })
|
||||||
|
: Promise.resolve({ data: { sso_enabled: true } })))
|
||||||
|
mount()
|
||||||
|
await waitFor(() => expect(replace).toHaveBeenCalledWith('/api/auth/sso/login?prompt=none'))
|
||||||
|
expect(localStorage.getItem('token')).toBeNull()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not ask when the token is good', async () => {
|
||||||
|
localStorage.setItem('token', 'fresh')
|
||||||
|
api.get.mockResolvedValue({ data: { id: 1, name: 'Reader' } })
|
||||||
|
mount()
|
||||||
|
expect(await screen.findByText('signed in as Reader')).toBeInTheDocument()
|
||||||
|
expect(replace).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('shows the page when the provider is not configured', async () => {
|
||||||
|
api.get.mockResolvedValue({ data: { sso_enabled: false } })
|
||||||
|
mount()
|
||||||
|
expect(await screen.findByText('signed out')).toBeInTheDocument()
|
||||||
|
expect(replace).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue