From 2dea5c56149ad8fbc3edf451902e53d35a62f097 Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Fri, 5 Aug 2022 21:35:26 +0100
Subject: [PATCH] Some various session fixes based on current data.

---
 .../src/middleware/authenticated.ts           | 71 +++++++++----------
 .../backend-core/src/security/sessions.ts     | 21 +++---
 .../src/utilities/rowProcessor/index.js       |  2 +-
 3 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/packages/backend-core/src/middleware/authenticated.ts b/packages/backend-core/src/middleware/authenticated.ts
index 7280eba294..3406b00812 100644
--- a/packages/backend-core/src/middleware/authenticated.ts
+++ b/packages/backend-core/src/middleware/authenticated.ts
@@ -84,45 +84,40 @@ module.exports = (
       // check the actual user is authenticated first, try header or cookie
       const headerToken = ctx.request.headers[Headers.TOKEN]
       const authCookie = getCookie(ctx, Cookies.Auth) || openJwt(headerToken)
-      let authenticated = false,
-        user = null,
-        internal = false,
-        error = null
-      if (authCookie) {
-        const sessionId = authCookie.sessionId
-        const userId = authCookie.userId
-
-        const session = await getSession(userId, sessionId)
-        if (!session) {
-          error = `Session not found - ${userId} - ${sessionId}`
-        } else {
-          try {
-            if (opts && opts.populateUser) {
-              user = await getUser(
-                userId,
-                session.tenantId,
-                opts.populateUser(ctx)
-              )
-            } else {
-              user = await getUser(userId, session.tenantId)
-            }
-            user.csrfToken = session.csrfToken
-            authenticated = true
-          } catch (err) {
-            error = err
-          }
-        }
-        if (error) {
-          console.error("Auth Error", error)
-          // remove the cookie as the user does not exist anymore
-          clearCookie(ctx, Cookies.Auth)
-        } else if (session?.lastAccessedAt < timeMinusOneMinute()) {
-          // make sure we denote that the session is still in use
-          await updateSessionTTL(session)
-        }
-      }
       const apiKey = ctx.request.headers[Headers.API_KEY]
       const tenantId = ctx.request.headers[Headers.TENANT_ID]
+      let authenticated = false,
+        user = null,
+        internal = false
+      if (authCookie && !apiKey) {
+        const sessionId = authCookie.sessionId
+        const userId = authCookie.userId
+        let session
+        try {
+          // getting session handles error checking (if session exists etc)
+          session = await getSession(userId, sessionId)
+          if (opts && opts.populateUser) {
+            user = await getUser(
+              userId,
+              session.tenantId,
+              opts.populateUser(ctx)
+            )
+          } else {
+            user = await getUser(userId, session.tenantId)
+          }
+          user.csrfToken = session.csrfToken
+          if (session?.lastAccessedAt < timeMinusOneMinute()) {
+            // make sure we denote that the session is still in use
+            await updateSessionTTL(session)
+          }
+          authenticated = true
+        } catch (err: any) {
+          authenticated = false
+          console.error("Auth Error", err?.message || err)
+          // remove the cookie as the user does not exist anymore
+          clearCookie(ctx, Cookies.Auth)
+        }
+      }
       // this is an internal request, no user made it
       if (!authenticated && apiKey) {
         const populateUser = opts.populateUser ? opts.populateUser(ctx) : null
@@ -144,7 +139,7 @@ module.exports = (
         delete user.password
       }
       // be explicit
-      if (error || authenticated !== true) {
+      if (authenticated !== true) {
         authenticated = false
       }
       // isAuthenticated is a function, so use a variable to be able to check authed state
diff --git a/packages/backend-core/src/security/sessions.ts b/packages/backend-core/src/security/sessions.ts
index f8375f510b..39d24ee16a 100644
--- a/packages/backend-core/src/security/sessions.ts
+++ b/packages/backend-core/src/security/sessions.ts
@@ -38,7 +38,7 @@ export async function invalidateSessions(
     let sessions: SessionKey
 
     // If no sessionIds, get all the sessions for the user
-    if (!sessionIds) {
+    if (sessionIds.length === 0) {
       sessions = await getSessionsForUser(userId)
       sessions.forEach(
         (session: any) =>
@@ -103,18 +103,13 @@ export async function endSession(userId: string, sessionId: string) {
 }
 
 export async function getSession(userId: string, sessionId: string) {
-  try {
-    const client = await redis.getSessionClient()
-    return client.get(makeSessionID(userId, sessionId))
-  } catch (err) {
-    // if can't get session don't error, just don't return anything
-    console.error(err)
-    return null
+  if (!userId || !sessionId) {
+    throw new Error(`Invalid session details - ${userId} - ${sessionId}`)
   }
-}
-
-export async function getAllSessions() {
   const client = await redis.getSessionClient()
-  const sessions = await client.scan()
-  return sessions.map((session: Session) => session.value)
+  const session = await client.get(makeSessionID(userId, sessionId))
+  if (!session) {
+    throw new Error(`Session not found - ${userId} - ${sessionId}`)
+  }
+  return session
 }
diff --git a/packages/server/src/utilities/rowProcessor/index.js b/packages/server/src/utilities/rowProcessor/index.js
index 36a02eb9b1..e4c364eaf3 100644
--- a/packages/server/src/utilities/rowProcessor/index.js
+++ b/packages/server/src/utilities/rowProcessor/index.js
@@ -278,7 +278,7 @@ exports.outputProcessing = async (table, rows, opts = { squash: true }) => {
   for (let [property, column] of Object.entries(table.schema)) {
     if (column.type === FieldTypes.ATTACHMENT) {
       for (let row of enriched) {
-        if (row[property] == null || row[property].length === 0) {
+        if (row[property] == null || !Array.isArray(row[property])) {
           continue
         }
         row[property].forEach(attachment => {