fix(nhis): skip the last-logged update on read-only storages

api/src/auth/router.ts

@@ -345,7 +345,13 @@ router.post('/nhi-token', async (req, res) => {
   // setSessionCookies only stamps boundIp for adminMode, which an NHI never has, so no conflict
   if (user.nhi.ipBinding) payload.boundIp = clientIp
   const token = await setSessionCookies(req, res, reqSitePath(req), payload, 'nhi-session', userOrg, { skipExchangeToken: true, exp })
-  storages.globalStorage.updateLogged(user.id).catch((err: any) => internalError('nhi-update-logged', 'error while updating logged date', err))
+  // Guarded on `readonly`, like confirmLog on the password path: a read-only storage cannot record a
+  // last-logged date, and FileStorage's updateLogged throws SYNCHRONOUSLY — it is not async — so the
+  // .catch() below never attaches and the rejection became a 500 that failed the whole exchange.
+  // Without the guard an NHI could not obtain a session at all under file storage.
+  if (!storages.globalStorage.readonly) {
+    storages.globalStorage.updateLogged(user.id).catch((err: any) => internalError('nhi-update-logged', 'error while updating logged date', err))
+  }
   eventsLog.info('sd.auth.nhi.ok', `an NHI session was created for ${user.id}`, logContext)
   res.set('Cache-Control', 'no-store')
   res.send({ access_token: token, token_type: 'Bearer', expires_in: exp - nowSec })

tests/features/file-storage.unit.spec.ts

@@ -53,4 +53,18 @@ test.describe('file storage interface', () => {
     assert.equal(ntag.department, 'dep1')
     assert.equal(ntag.departmentName, 'Dep 1')
   })
+
+  test('declares itself readonly, which is what callers must branch on', () => {
+    // The NHI token exchange and the password login both record a last-logged date, which a
+    // read-only storage cannot do. Both must therefore branch on this flag rather than rely on
+    // catching a failure — see the next test for why catching does not work.
+    assert.equal(storage.readonly, true)
+  })
+
+  test('updateLogged throws synchronously, so a .catch() cannot absorb it', () => {
+    // This is the reason the NHI exchange needs an explicit `readonly` guard rather than a .catch():
+    // the method is not async, so the throw happens at the call site before any handler attaches and
+    // propagates as a 500. If it ever becomes an async rejection this test fails, and the guard can go.
+    assert.throws(() => storage.updateLogged('anyone'))
+  })
 })