fix(client-admin): bound and validate sign-in proof and show the reason once

Proof that cannot be sent in a header, or that does not arrive within ten seconds, is treated as unavailable. The reason for a failed sign-in is shown once. Page reloads go through one module so that tests can observe them.

Author: rudyfraser

client-admin/src/app-login.test.js

@@ -6,6 +6,10 @@ import { ThemeUIProvider } from 'theme-ui'
 import App from './app'
 import rootReducer from './reducers'
 import theme from './theme'
+import { reloadPage, replacePage } from './util/page-navigation'
+
+jest.setTimeout(30000)
+const PATIENT = { timeout: 15000 }
 
 const mockInit = jest.fn()
 const mockRestore = jest.fn()
@@ -33,6 +37,11 @@ jest.mock('@atproto/api', () => ({
   }
 }))
 
+jest.mock('./util/page-navigation', () => ({
+  reloadPage: jest.fn(),
+  replacePage: jest.fn()
+}))
+
 jest.mock('./components/conversations-and-account/conversations', () => {
   return function MockConversations() {
     return <div>All Conversations</div>
@@ -50,7 +59,7 @@ const PROOF_REQUEST = {
 const PROOF_MESSAGE =
   'We could not confirm this account with its host. Sign in again. If this keeps happening, your host may not support this sign-in yet.'
 const UNSUPPORTED_DID_MESSAGE =
-  'Accounts with a did:web identifier cannot sign in to the admin console yet.'
+  'This kind of account identifier cannot sign in to the admin console yet.'
 const LOOKUP_MESSAGE = 'We could not look up your account just now. Try again in a minute.'
 
 const did = 'did:plc:abcdefghijklmnopqrstuvwx'
@@ -126,40 +135,48 @@ describe('admin console sign-in', () => {
 
     it('restores the OAuth session of that identity and signs in with proof', async () => {
       mockRestore.mockResolvedValue(session)
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(serverAnswers(200, { token: 'admin-1', uid: 7 }))
 
       openConsole('/')
 
-      await waitFor(() => expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBe('admin-1'))
+      await waitFor(() => expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBe('admin-1'), PATIENT)
       expect(mockRestore.mock.calls).toEqual([[did]])
       expect(mockAgent.mock.calls).toEqual([[session]])
       expect(mockGetServiceAuth.mock.calls).toEqual([[PROOF_REQUEST]])
       expect(loginRequests()).toEqual([
         {
-          headers: { 'Content-Type': 'application/json', Authorization: 'Bearer proof-1' },
+          headers: {
+            'Content-Type': 'application/json',
+            Authorization: 'Bearer proof.number1.signature'
+          },
           body: storedIdentity
         }
       ])
       expect(JSON.parse(localStorage.getItem(IDENTITY_KEY))).toEqual(storedIdentity)
+      await waitFor(() => expect(reloadPage).toHaveBeenCalledTimes(1), PATIENT)
+      expect(replacePage).toHaveBeenCalledTimes(0)
     })
 
     it('ends signed out on the sign-in page with the reason when the server refuses the proof', async () => {
       mockRestore.mockResolvedValue(session)
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(
         serverAnswers(401, { error: 'polis_err_atproto_auth_invalid', status: 401 })
       )
 
       openConsole('/')
 
-      expect(await screen.findAllByText(PROOF_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(PROOF_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(window.location.pathname).toBe('/signin')
       expect(screen.getByRole('button', { name: 'Sign In' })).toBeInTheDocument()
       expect(screen.queryByText('All Conversations')).toBeNull()
       expect(loginRequests()).toEqual([
         {
-          headers: { 'Content-Type': 'application/json', Authorization: 'Bearer proof-1' },
+          headers: {
+            'Content-Type': 'application/json',
+            Authorization: 'Bearer proof.number1.signature'
+          },
           body: storedIdentity
         }
       ])
@@ -175,7 +192,7 @@ describe('admin console sign-in', () => {
 
       openConsole('/')
 
-      expect(await screen.findAllByText(PROOF_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(PROOF_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(window.location.pathname).toBe('/signin')
       expect(mockRestore.mock.calls).toEqual([[did]])
       expect(mockGetServiceAuth).toHaveBeenCalledTimes(0)
@@ -197,7 +214,7 @@ describe('admin console sign-in', () => {
 
       openConsole('/')
 
-      expect(await screen.findAllByText(PROOF_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(PROOF_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(mockGetServiceAuth.mock.calls).toEqual([[PROOF_REQUEST]])
       expect(loginRequests()).toEqual([
         { headers: { 'Content-Type': 'application/json' }, body: storedIdentity }
@@ -208,14 +225,14 @@ describe('admin console sign-in', () => {
 
     it('shows the lookup message when the server cannot look the account up', async () => {
       mockRestore.mockResolvedValue(session)
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(
         serverAnswers(503, { error: 'polis_err_atproto_did_resolution_failed', status: 503 })
       )
 
       openConsole('/')
 
-      expect(await screen.findAllByText(LOOKUP_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(LOOKUP_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(window.location.pathname).toBe('/signin')
       expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBeNull()
       expect(JSON.parse(localStorage.getItem(IDENTITY_KEY))).toEqual(storedIdentity)
@@ -225,39 +242,62 @@ describe('admin console sign-in', () => {
   describe('returning from the authorization server', () => {
     it('signs in with proof from the new session', async () => {
       mockInit.mockResolvedValue({ session })
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(serverAnswers(200, { token: 'admin-1', uid: 7 }))
 
       openConsole('/?state=s1&code=c1')
 
-      await waitFor(() => expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBe('admin-1'))
+      await waitFor(() => expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBe('admin-1'), PATIENT)
       expect(mockInit).toHaveBeenCalledTimes(1)
       expect(mockRestore).toHaveBeenCalledTimes(0)
       expect(mockAgent.mock.calls).toEqual([[session]])
       expect(mockGetServiceAuth.mock.calls).toEqual([[PROOF_REQUEST]])
       expect(loginRequests()).toEqual([
         {
-          headers: { 'Content-Type': 'application/json', Authorization: 'Bearer proof-1' },
+          headers: {
+            'Content-Type': 'application/json',
+            Authorization: 'Bearer proof.number1.signature'
+          },
           body: { ...freshIdentity, email }
         }
       ])
       expect(JSON.parse(localStorage.getItem(IDENTITY_KEY))).toEqual(freshIdentity)
+      await waitFor(() => expect(replacePage.mock.calls).toEqual([['/']]), PATIENT)
+      expect(reloadPage).toHaveBeenCalledTimes(0)
+    })
+
+    it('shows the reason on the sign-in page when the return address is the callback route', async () => {
+      mockInit.mockResolvedValueOnce({ session }).mockResolvedValue(null)
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
+      window.fetch.mockResolvedValue(
+        serverAnswers(401, { error: 'polis_err_atproto_auth_invalid', status: 401 })
+      )
+
+      openConsole('/auth/callback?state=s1&code=c1')
+
+      expect(await screen.findAllByText(PROOF_MESSAGE, {}, PATIENT)).toHaveLength(1)
+      expect(window.location.pathname).toBe('/signin')
+      expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBeNull()
+      expect(replacePage).toHaveBeenCalledTimes(0)
     })
 
     it('ends on the sign-in page with the reason and stores nothing when the server refuses', async () => {
       mockInit.mockResolvedValue({ session })
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(
         serverAnswers(400, { error: 'polis_err_atproto_unsupported_did', status: 400 })
       )
 
       openConsole('/?state=s1&code=c1')
 
-      expect(await screen.findAllByText(UNSUPPORTED_DID_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(UNSUPPORTED_DID_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(window.location.pathname).toBe('/signin')
       expect(loginRequests()).toEqual([
         {
-          headers: { 'Content-Type': 'application/json', Authorization: 'Bearer proof-1' },
+          headers: {
+            'Content-Type': 'application/json',
+            Authorization: 'Bearer proof.number1.signature'
+          },
           body: { ...freshIdentity, email }
         }
       ])
@@ -267,14 +307,14 @@ describe('admin console sign-in', () => {
     it('does not remove an identity stored earlier when the server refuses', async () => {
       localStorage.setItem(IDENTITY_KEY, JSON.stringify(storedIdentity))
       mockInit.mockResolvedValue({ session })
-      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+      mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
       window.fetch.mockResolvedValue(
         serverAnswers(401, { error: 'polis_err_atproto_auth_replayed', status: 401 })
       )
 
       openConsole('/?state=s1&code=c1')
 
-      expect(await screen.findAllByText(PROOF_MESSAGE)).toHaveLength(1)
+      expect(await screen.findAllByText(PROOF_MESSAGE, {}, PATIENT)).toHaveLength(1)
       expect(localStorage.getItem(ADMIN_TOKEN_KEY)).toBeNull()
       expect(JSON.parse(localStorage.getItem(IDENTITY_KEY))).toEqual(storedIdentity)
     })

client-admin/src/app.js

@@ -35,6 +35,7 @@ import {
   isAdminTokenExpired
 } from './util/atproto-oauth'
 import { ADMIN_TOKEN_KEY, loginWithProof, signInErrorMessage } from './util/atproto-login'
+import { reloadPage, replacePage } from './util/page-navigation'
 import { Agent } from '@atproto/api'
 
 const AUTH_LOADING_TIMEOUT = 3000
@@ -142,7 +143,7 @@ const App = () => {
         .then((success) => {
           if (success) {
             // Reload to cleanly initialize with the new identity and admin JWT
-            window.location.replace('/')
+            replacePage('/')
           } else {
             setIsLoading(false)
           }
@@ -163,7 +164,7 @@ const App = () => {
       } else if (identity && !adminToken) {
         loginWithStoredIdentity(identity)
           .then(() => {
-            window.location.reload()
+            reloadPage()
           })
           .catch((err) => {
             console.error('Sign-in with the stored identity failed:', err)
@@ -229,6 +230,10 @@ const App = () => {
     }
   }, [loadUserDataIfNeeded])
 
+  const clearSignInError = useCallback(() => {
+    setSignInError('')
+  }, [])
+
   // Re-check auth when returning from OAuth callback
   const handleAuthComplete = useCallback(() => {
     const identity = getAtprotoIdentity()
@@ -241,7 +246,16 @@ const App = () => {
       <Routes>
         {/* Public routes */}
         <Route path="/home" element={<Home />} />
-        <Route path="/signin" element={<SignIn authed={isAuthed()} signInError={signInError} />} />
+        <Route
+          path="/signin"
+          element={
+            <SignIn
+              authed={isAuthed()}
+              signInError={signInError}
+              onSignInErrorShown={clearSignInError}
+            />
+          }
+        />
         <Route path="/signout" element={<SignOut />} />
         <Route path="/tos" element={<TOS />} />
         <Route path="/privacy" element={<Privacy />} />

client-admin/src/components/auth-callback.test.js

@@ -4,6 +4,9 @@ import { ThemeUIProvider } from 'theme-ui'
 import theme from '../theme'
 import AuthCallback from './auth-callback'
 
+jest.setTimeout(30000)
+const PATIENT = { timeout: 15000 }
+
 const mockInit = jest.fn()
 const mockGetProfile = jest.fn()
 const mockGetSession = jest.fn()
@@ -84,7 +87,7 @@ describe('AuthCallback', () => {
       }
     })
     mockGetSession.mockResolvedValue({ data: { email } })
-    mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof-1' } })
+    mockGetServiceAuth.mockResolvedValue({ data: { token: 'proof.number1.signature' } })
   })
 
   afterEach(() => {
@@ -111,7 +114,10 @@ describe('AuthCallback', () => {
     expect(loginRequests()).toEqual([
       {
         url: 'http://localhost/api/v3/auth/atproto-login',
-        headers: { 'Content-Type': 'application/json', Authorization: 'Bearer proof-1' },
+        headers: {
+          'Content-Type': 'application/json',
+          Authorization: 'Bearer proof.number1.signature'
+        },
         body: { ...identity, email }
       }
     ])
@@ -136,4 +142,15 @@ describe('AuthCallback', () => {
     expect(window.fetch).toHaveBeenCalledTimes(1)
     expect(localStorage.length).toBe(0)
   })
+
+  it('goes to the sign-in page when the server refuses and nobody listens for the reason', async () => {
+    window.fetch.mockResolvedValue(
+      serverAnswers(401, { error: 'polis_err_atproto_auth_invalid', status: 401 })
+    )
+
+    openCallback({})
+
+    expect(await screen.findAllByText('sign-in page', {}, PATIENT)).toHaveLength(1)
+    expect(localStorage.length).toBe(0)
+  })
 })

client-admin/src/components/landers/signin-error.test.js

@@ -59,4 +59,22 @@ describe('SignIn reason for a failed sign-in', () => {
     expect(mockSignIn).toHaveBeenCalledWith('organizer.example.com')
     expect(screen.queryByText(PROOF_MESSAGE)).toBeNull()
   })
+
+  it('reports once that the message was shown, so that it is not shown again later', () => {
+    const onSignInErrorShown = jest.fn()
+    const { rerender } = render(page({ signInError: PROOF_MESSAGE, onSignInErrorShown }))
+
+    rerender(page({ signInError: '', onSignInErrorShown }))
+
+    expect(onSignInErrorShown).toHaveBeenCalledTimes(1)
+    expect(screen.getAllByText(PROOF_MESSAGE)).toHaveLength(1)
+  })
+
+  it('reports nothing when there is no message', () => {
+    const onSignInErrorShown = jest.fn()
+
+    render(page({ signInError: '', onSignInErrorShown }))
+
+    expect(onSignInErrorShown).toHaveBeenCalledTimes(0)
+  })
 })

client-admin/src/components/landers/signin.js

@@ -7,14 +7,16 @@ import { Heading, Box, Button, Input, Text } from 'theme-ui'
 import StaticLayout from './lander-layout'
 import { getOAuthClient } from '../../util/atproto-oauth'
 
-const SignIn = ({ authed, signInError }) => {
+const SignIn = ({ authed, signInError, onSignInErrorShown }) => {
   const [handle, setHandle] = useState('')
   const [isProcessing, setIsProcessing] = useState(false)
   const [error, setError] = useState('')
 
   useEffect(() => {
-    if (signInError) setError(signInError)
-  }, [signInError])
+    if (!signInError) return
+    setError(signInError)
+    if (onSignInErrorShown) onSignInErrorShown()
+  }, [signInError, onSignInErrorShown])
 
   const handleSignIn = async () => {
     const trimmed = handle.trim()
@@ -73,7 +75,8 @@ const SignIn = ({ authed, signInError }) => {
 
 SignIn.propTypes = {
   authed: PropTypes.bool,
-  signInError: PropTypes.string
+  signInError: PropTypes.string,
+  onSignInErrorShown: PropTypes.func
 }
 
 export default SignIn