OpenConceptLab/ocl_online#535 | An expired session signs the user back in instead of ending on Keycloak's "Invalid redirect uri"

src/common/__tests__/ssoSession.test.js

@@ -15,7 +15,7 @@ globalThis.window = {
   location: { origin: 'http://localhost:4004' },
   OIDC_SERVER_URL: 'https://sso.example.org/auth/',
   OIDC_REALM: 'ocl',
-  OIDC_RP_CLIENT_ID: 'oclmap',
+  OIDC_RP_CLIENT_ID: 'mapper',
   addEventListener: (type, fn) => { listeners[type] = fn },
   removeEventListener: type => { delete listeners[type] },
 }
@@ -24,7 +24,11 @@ globalThis.document = {
   body: { appendChild: el => appended.push(el) },
 }
 
-const { isSSOSessionEnabled, buildSilentAuthURL, silentSignIn, ensureIdToken } = await import('../ssoSession.js')
+const {
+  isSSOSessionEnabled, buildSilentAuthURL, silentSignIn, ensureIdToken,
+  buildSSOLogoutURL, prepareSignOut, takeSigninToResume, settleForcedSignOut, renewSessionSilently,
+  FORCED_SIGN_OUT_WINDOW_MS,
+} = await import('../ssoSession.js')
 
 const reset = () => { appended.length = 0 }
 const answer = (data, overrides = {}) => listeners.message({
@@ -46,7 +50,7 @@ test('is enabled only when server, realm and client id are set', () => {
 
 test('builds a prompt=none PKCE auth URL against Keycloak', () => {
   const url = new URL(buildSilentAuthURL({
-    serverURL: 'https://sso.example.org/auth', realm: 'ocl', clientId: 'oclmap',
+    serverURL: 'https://sso.example.org/auth', realm: 'ocl', clientId: 'mapper',
     redirectURI: 'http://localhost:4004/silent-sso.html', state: 's', nonce: 'n', codeChallenge: 'c',
   }))
   assert.equal(url.pathname, '/auth/realms/ocl/protocol/openid-connect/auth')
@@ -60,12 +64,12 @@ test('exchanges the code and removes the iframe when Keycloak has a session', as
   let sent
   const promise = silentSignIn(async body => { sent = body; return { access_token: 't', id_token: 'i' } })
   await tick()
-  answer({ code: 'abc', state: stateOf() })
+  answer({ code: 'code-1', state: stateOf() })
   const tokens = await promise
   assert.equal(tokens.access_token, 't')
-  assert.equal(sent.code, 'abc')
+  assert.equal(sent.code, 'code-1')
   assert.equal(sent.redirect_uri, 'http://localhost:4004/silent-sso.html')
-  assert.equal(sent.client_id, 'oclmap')
+  assert.equal(sent.client_id, 'mapper')
   assert.ok(sent.code_verifier.length >= 43)
   assert.equal(appended[0].removed, true)
 })
@@ -100,7 +104,7 @@ test('resolves null when the exchange fails', async () => {
   reset()
   const promise = silentSignIn(async () => { throw new Error('boom') })
   await tick()
-  answer({ code: 'abc', state: stateOf() })
+  answer({ code: 'code-1', state: stateOf() })
   assert.equal(await promise, null)
 })
 
@@ -138,7 +142,7 @@ test('ensureIdToken fetches a fresh id token only when one is missing', async ()
   delete store.id_token
   const promise = ensureIdToken(exchange)
   await tick()
-  answer({ code: 'abc', state: stateOf() })
+  answer({ code: 'code-1', state: stateOf() })
   await promise
   assert.equal(store.id_token, 'fresh')
 })
@@ -158,3 +162,105 @@ test('ensureIdToken leaves storage alone when there is no Keycloak session or SS
   await ensureIdToken(async () => ({}))
   assert.equal(appended.length, 0)
 })
+
+test('buildSSOLogoutURL sends the whole post-logout address, encoded', () => {
+  const url = new URL(buildSSOLogoutURL('https://api.example.org', 'id-token', 'http://localhost:4004/#/signin?returnTo=x'))
+  assert.equal(url.pathname, '/users/logout/')
+  assert.equal(url.hash, '')
+  assert.equal(url.searchParams.get('id_token_hint'), 'id-token')
+  assert.equal(url.searchParams.get('post_logout_redirect_uri'), 'http://localhost:4004/#/signin?returnTo=x')
+  assert.equal(buildSSOLogoutURL('https://api.example.org', '', 'http://localhost:4004'), undefined)
+  assert.equal(buildSSOLogoutURL('https://api.example.org', 'id-token', ''), undefined)
+})
+
+const useSessionStorage = () => {
+  const store = {}
+  globalThis.sessionStorage = {
+    getItem: key => store[key] ?? null, setItem: (key, value) => { store[key] = value }, removeItem: key => { delete store[key] },
+  }
+  return store
+}
+const signOut = overrides => prepareSignOut({
+  forced: true, apiURL: 'https://api.example.org', idToken: 'id-token', loginRedirectURL: 'http://localhost:4004',
+  returnTo: 'http://localhost:4004/map-projects/1/?tab=2', now: 1000, ...overrides,
+})
+const home = { pathname: '/', search: '' }
+
+test('a forced sign-out returns to LOGIN_REDIRECT_URL and signs back in to the page once', () => {
+  useSessionStorage()
+  const { logoutURL, signInAgain } = signOut()
+  const url = new URL(logoutURL)
+  assert.equal(url.searchParams.get('post_logout_redirect_uri'), 'http://localhost:4004')
+  assert.equal(url.hash, '')
+  assert.equal(signInAgain, true)
+
+  assert.equal(takeSigninToResume(home, 2000), 'http://localhost:4004/map-projects/1/?tab=2')
+  assert.equal(takeSigninToResume(home, 2000), null)
+})
+
+test('a normal sign-out returns to LOGIN_REDIRECT_URL and drops any forced sign-out', () => {
+  useSessionStorage()
+  signOut()
+  const { logoutURL, signInAgain } = signOut({ forced: false })
+  assert.equal(new URL(logoutURL).searchParams.get('post_logout_redirect_uri'), 'http://localhost:4004')
+  assert.equal(signInAgain, false)
+  assert.equal(takeSigninToResume(home, 2000), null)
+})
+
+test('without an id token there is no logout URL and nothing to resume on load', () => {
+  useSessionStorage()
+  const { logoutURL, signInAgain } = signOut({ idToken: undefined })
+  assert.equal(logoutURL, undefined)
+  assert.equal(signInAgain, true)
+  assert.equal(takeSigninToResume(home, 2000), null)
+})
+
+test('a second forced sign-out soon after the first does not sign in again', () => {
+  useSessionStorage()
+  signOut({ now: 1000 })
+  assert.ok(takeSigninToResume(home, 2000))
+  const { signInAgain } = signOut({ now: 1000 + FORCED_SIGN_OUT_WINDOW_MS - 1 })
+  assert.equal(signInAgain, false)
+  assert.equal(takeSigninToResume(home, 1000 + FORCED_SIGN_OUT_WINDOW_MS), null)
+
+  assert.equal(signOut({ now: 1000 + 3 * FORCED_SIGN_OUT_WINDOW_MS }).signInAgain, true)
+})
+
+test('the page to resume expires and is dropped', () => {
+  useSessionStorage()
+  signOut({ now: 1000 })
+  assert.equal(takeSigninToResume(home, 1000 + FORCED_SIGN_OUT_WINDOW_MS), null)
+  assert.equal(takeSigninToResume(home, 2000), null)
+})
+
+test('the sign-in callback is left alone and settles the forced sign-out', () => {
+  useSessionStorage()
+  signOut()
+  assert.equal(takeSigninToResume({ pathname: '/oidc/login/', search: '?code=code-1&state=s' }, 2000), null)
+  assert.equal(takeSigninToResume({ pathname: '/', search: '?code=code-1&state=s' }, 2000), null)
+  settleForcedSignOut()
+  assert.equal(takeSigninToResume(home, 2000), null)
+})
+
+test('unavailable storage never throws', () => {
+  globalThis.sessionStorage = { getItem() { throw new Error('denied') }, setItem() { throw new Error('denied') }, removeItem() { throw new Error('denied') } }
+  assert.ok(signOut().logoutURL)
+  assert.equal(takeSigninToResume(home, 2000), null)
+  settleForcedSignOut()
+})
+
+test('renewSessionSilently tries at most once a window', async () => {
+  reset()
+  useSessionStorage()
+  window.OIDC_SERVER_URL = 'https://sso.example.org/auth'
+  const exchange = async () => ({ access_token: 't', id_token: 'i' })
+
+  const promise = renewSessionSilently(exchange, 1000)
+  await tick()
+  answer({ code: 'code-1', state: stateOf() })
+  assert.equal((await promise).access_token, 't')
+
+  reset()
+  assert.equal(await renewSessionSilently(exchange, 1000 + FORCED_SIGN_OUT_WINDOW_MS - 1), null)
+  assert.equal(appended.length, 0)
+})

src/common/ssoSession.js

@@ -139,3 +139,72 @@ export const ensureIdToken = async exchangeCode => {
   if(tokens?.id_token)
     localStorage.setItem('id_token', tokens.id_token)
 }
+
+export const buildSSOLogoutURL = (apiURL, idToken, redirectURL) => {
+  if(idToken && redirectURL)
+    return `${apiURL}/users/logout/?id_token_hint=${encodeURIComponent(idToken)}&post_logout_redirect_uri=${encodeURIComponent(redirectURL)}`
+}
+
+// Keycloak's logout only returns to an address registered for the client (LOGIN_REDIRECT_URL), so after a
+// forced sign-out the page to sign back in to waits here, in this tab, until the app loads again. The record
+// outlives the sign-in for a while, so a token the API keeps turning down stops after one retry, not a loop.
+const FORCED_SIGN_OUT_KEY = 'forced_sign_out'
+const SILENT_RENEWAL_KEY = 'silent_renewal_at'
+export const FORCED_SIGN_OUT_WINDOW_MS = 10 * 60 * 1000
+
+const readSession = key => {
+  try {
+    return JSON.parse(sessionStorage.getItem(key))
+  } catch {
+    return null
+  }
+}
+
+const writeSession = (key, value) => {
+  try {
+    if(value === null)
+      sessionStorage.removeItem(key)
+    else
+      sessionStorage.setItem(key, JSON.stringify(value))
+    return true
+  } catch {
+    return false
+  }
+}
+
+const isRecent = (at, now) => Number.isFinite(at) && now - at < FORCED_SIGN_OUT_WINDOW_MS
+
+export const prepareSignOut = ({ forced, apiURL, idToken, loginRedirectURL, returnTo, now = Date.now() }) => {
+  const logoutURL = buildSSOLogoutURL(apiURL, idToken, loginRedirectURL)
+  if(!forced) {
+    writeSession(FORCED_SIGN_OUT_KEY, null)
+    return { logoutURL, signInAgain: false }
+  }
+  const signInAgain = !isRecent(readSession(FORCED_SIGN_OUT_KEY)?.at, now)
+  writeSession(FORCED_SIGN_OUT_KEY, { returnTo, at: now, resume: Boolean(logoutURL) && signInAgain })
+  return { logoutURL, signInAgain }
+}
+
+const isSignInCallback = ({ pathname, search }) => /^\/oidc\/login(\/|$)/i.test(pathname) || /[?&]code=/.test(search)
+
+export const takeSigninToResume = (location, now = Date.now()) => {
+  const record = readSession(FORCED_SIGN_OUT_KEY)
+  if(!record?.resume || isSignInCallback(location))
+    return null
+  writeSession(FORCED_SIGN_OUT_KEY, { ...record, resume: false })
+  return isRecent(record.at, now) && record.returnTo ? record.returnTo : null
+}
+
+export const settleForcedSignOut = () => {
+  const record = readSession(FORCED_SIGN_OUT_KEY)
+  if(record?.resume)
+    writeSession(FORCED_SIGN_OUT_KEY, { ...record, resume: false })
+}
+
+// A token the API turns down may only have expired while the Keycloak session lives on: renew it without a
+// prompt, at most once a window, so a token the API keeps rejecting can't reload the page forever.
+export const renewSessionSilently = async (exchangeCode, now = Date.now()) => {
+  if(isRecent(readSession(SILENT_RENEWAL_KEY), now) || !writeSession(SILENT_RENEWAL_KEY, now))
+    return null
+  return silentSignIn(exchangeCode)
+}

src/common/utils.js

@@ -14,7 +14,7 @@ import {
 } from './constants';
 import APIService from '../services/APIService';
 import GAService from '../services/GAService';
-import { ensureIdToken } from './ssoSession';
+import { ensureIdToken, prepareSignOut, settleForcedSignOut, renewSessionSilently } from './ssoSession';
 import { SERVER_CONFIGS } from './serverConfigs';
 import { legacyHashRoute, isSameSitePath } from './history';
 
@@ -243,12 +243,13 @@ export const arrayToCSV = objArray => {
   }, str);
 }
 
-export const refreshCurrentUserCache = callback => {
+export const refreshCurrentUserCache = (callback, onFailure) => {
   APIService.user().get(null, null, {includeAuthGroups: true, includeSubscribedOrgs: true, includeCapabilities: true}).then(response => {
-    if(response.status === 200) {
+    if(response?.status === 200) {
       localStorage.setItem('user', JSON.stringify(response.data));
       if(callback) callback(response);
-    }
+    } else if(onFailure)
+      onFailure(response);
   });
 }
 
@@ -631,7 +632,32 @@ export const getRandomColor = () => `#${Math.floor(Math.random()*16777215).toStr
 
 const exchangeSilentCode = body => APIService.users().appendToUrl('oidc/code-exchange/').post(body, false).then(res => res.data);
 
-export const logoutUser = (...args) => ensureIdToken(exchangeSilentCode).then(() => signOutUser(...args));
+// Several requests can be turned down at once; the first one decides, so later ones can't undo its sign-out.
+let signingOut = null
+export const logoutUser = (...args) => {
+  signingOut = signingOut || ensureIdToken(exchangeSilentCode).then(() => signOutUser(...args)).catch(error => {
+    signingOut = null
+    throw error
+  })
+  return signingOut
+}
+
+// The API turned the token down: renew it silently while the Keycloak session lives on, and sign out only if
+// that fails. The reload sends every request again with the new token.
+let renewing = null
+export const renewOrSignOut = () => {
+  renewing = renewing || renewSessionSilently(exchangeSilentCode).then(tokens => {
+    if(!tokens?.id_token)
+      return logoutUser(true, true)
+    localStorage.removeItem('server_configs')
+    localStorage.setItem('token', tokens.access_token)
+    localStorage.setItem('id_token', tokens.id_token)
+    settleForcedSignOut()
+    const reload = () => window.location.reload()
+    refreshCurrentUserCache(reload, reload)
+  })
+  return renewing
+}
 
 const signOutUser = (redirectToLogin, forced) => {
   const clearTokens = () => {
@@ -642,17 +668,19 @@ const signOutUser = (redirectToLogin, forced) => {
   }
 
   const returnTo = window.location.origin + window.location.pathname + window.location.search
-  if(forced)
-    sessionStorage.setItem('session_expired', 'true')
-
-  const redirectURL = forced ?
-    window.location.origin + '/#/signin?returnTo=' + encodeURIComponent(returnTo) :
-    undefined
-  const logoutURL = getSSOLogoutURL(redirectURL)
+  const { logoutURL, signInAgain } = prepareSignOut({
+    forced,
+    returnTo,
+    apiURL: getAPIURL(),
+    idToken: localStorage.id_token,
+    loginRedirectURL: window.LOGIN_REDIRECT_URL || process.env.LOGIN_REDIRECT_URL,
+  })
 
   clearTokens()
   if(logoutURL)
     window.location = logoutURL
+  else if(forced && !signInAgain)
+    return // turned down again right after signing back in: stay signed out here rather than reload in a loop
   else if(redirectToLogin)
     getLoginURL(forced ? returnTo : undefined).then(url => { window.location.href = url })
   else
@@ -931,14 +959,6 @@ export const getRegisterURL = async returnTo => {
 }
 
 
-export const getSSOLogoutURL = returnTo => {
-  const redirectURL = returnTo || window.LOGIN_REDIRECT_URL || process.env.LOGIN_REDIRECT_URL
-  const idToken = localStorage.id_token
-  if(redirectURL && idToken)
-    return `${getAPIURL()}/users/logout/?id_token_hint=${idToken}&post_logout_redirect_uri=${redirectURL}`
-}
-
-
 export const urlSearchParamsToObject = urlSearchParams => {
   const result = {}
   for(const [key, value] of urlSearchParams.entries()) { // each 'entry' is a [key, value] tuple

src/components/app/App.jsx

@@ -6,7 +6,7 @@ import {
   getEnv
 } from '../../common/utils';
 import GAService from '../../services/GAService';
-import { silentSignIn, isSSOSessionEnabled } from '../../common/ssoSession';
+import { silentSignIn, isSSOSessionEnabled, settleForcedSignOut } from '../../common/ssoSession';
 import Error404 from '../errors/Error404';
 import Error403 from '../errors/Error403';
 import Error401 from '../errors/Error401';
@@ -113,7 +113,7 @@ const App = props => {
       localStorage.removeItem('server_configs')
       localStorage.setItem('token', tokens.access_token)
       localStorage.setItem('id_token', tokens.id_token)
-      sessionStorage.removeItem('session_expired')
+      settleForcedSignOut()
       refreshCurrentUserCache(() => setAuthVersion(version => version + 1))
     }).finally(() => { silentCheckRunning.current = false })
   }

src/components/users/OIDLoginCallback.jsx

@@ -7,6 +7,7 @@ import {
   isSignupOAuthState, isLoggedIn, getLoginURL
 } from '../../common/utils';
 import { navigate } from '../../common/history';
+import { settleForcedSignOut } from '../../common/ssoSession';
 import APIService from '../../services/APIService'
 import GAService from '../../services/GAService'
 import { OperationsContext } from '../app/LayoutContext';
@@ -51,7 +52,7 @@ class OIDLoginCallback extends React.Component {
             localStorage.removeItem('server_configs')
             localStorage.setItem('token', res.data.access_token)
             localStorage.setItem('id_token', res.data.id_token)
-            sessionStorage.removeItem('session_expired')
+            settleForcedSignOut()
             setAlert({
               duration: 2000,
               severity: 'success',