Merge pull request #7114 from Budibase/fix/session-perf

Increasing time between updating session TTLs

packages/backend-core/src/environment.ts

@@ -56,6 +56,7 @@ const env = {
   SERVICE: process.env.SERVICE || "budibase",
   MEMORY_LEAK_CHECK: process.env.MEMORY_LEAK_CHECK || false,
   LOG_LEVEL: process.env.LOG_LEVEL,
+  SESSION_UPDATE_PERIOD: process.env.SESSION_UPDATE_PERIOD,
   DEPLOYMENT_ENVIRONMENT:
     process.env.DEPLOYMENT_ENVIRONMENT || "docker-compose",
   _set(key: any, value: any) {

packages/backend-core/src/index.ts

@@ -9,7 +9,7 @@ import * as installation from "./installation"
 import env from "./environment"
 import tenancy from "./tenancy"
 import featureFlags from "./featureFlags"
-import sessions from "./security/sessions"
+import * as sessions from "./security/sessions"
 import deprovisioning from "./context/deprovision"
 import auth from "./auth"
 import constants from "./constants"

packages/backend-core/src/middleware/authenticated.js → packages/backend-core/src/middleware/authenticated.ts

@@ -1,28 +1,39 @@
-const { Cookies, Headers } = require("../constants")
-const { getCookie, clearCookie, openJwt } = require("../utils")
-const { getUser } = require("../cache/user")
-const { getSession, updateSessionTTL } = require("../security/sessions")
-const { buildMatcherRegex, matches } = require("./matchers")
-const env = require("../environment")
-const { SEPARATOR } = require("../db/constants")
-const { ViewNames } = require("../db/utils")
-const { queryGlobalView } = require("../db/views")
-const { getGlobalDB, doInTenant } = require("../tenancy")
-const { decrypt } = require("../security/encryption")
+import { Cookies, Headers } from "../constants"
+import { getCookie, clearCookie, openJwt } from "../utils"
+import { getUser } from "../cache/user"
+import { getSession, updateSessionTTL } from "../security/sessions"
+import { buildMatcherRegex, matches } from "./matchers"
+import { SEPARATOR } from "../db/constants"
+import { ViewNames } from "../db/utils"
+import { queryGlobalView } from "../db/views"
+import { getGlobalDB, doInTenant } from "../tenancy"
+import { decrypt } from "../security/encryption"
 const identity = require("../context/identity")
+const env = require("../environment")
+
+const ONE_MINUTE = env.SESSION_UPDATE_PERIOD || 60 * 1000
+
+interface FinaliseOpts {
+  authenticated?: boolean
+  internal?: boolean
+  publicEndpoint?: boolean
+  version?: string
+  user?: any
+}
 
-function finalise(
-  ctx,
-  { authenticated, user, internal, version, publicEndpoint } = {}
-) {
-  ctx.publicEndpoint = publicEndpoint || false
-  ctx.isAuthenticated = authenticated || false
-  ctx.user = user
-  ctx.internal = internal || false
-  ctx.version = version
+function timeMinusOneMinute() {
+  return new Date(Date.now() - ONE_MINUTE).toISOString()
 }
 
-async function checkApiKey(apiKey, populateUser) {
+function finalise(ctx: any, opts: FinaliseOpts = {}) {
+  ctx.publicEndpoint = opts.publicEndpoint || false
+  ctx.isAuthenticated = opts.authenticated || false
+  ctx.user = opts.user
+  ctx.internal = opts.internal || false
+  ctx.version = opts.version
+}
+
+async function checkApiKey(apiKey: string, populateUser?: Function) {
   if (apiKey === env.INTERNAL_API_KEY) {
     return { valid: true }
   }
@@ -56,10 +67,12 @@ async function checkApiKey(apiKey, populateUser) {
  */
 module.exports = (
   noAuthPatterns = [],
-  opts = { publicAllowed: false, populateUser: null }
+  opts: { publicAllowed: boolean; populateUser?: Function } = {
+    publicAllowed: false,
+  }
 ) => {
   const noAuthOptions = noAuthPatterns ? buildMatcherRegex(noAuthPatterns) : []
-  return async (ctx, next) => {
+  return async (ctx: any, next: any) => {
     let publicEndpoint = false
     const version = ctx.request.headers[Headers.API_VER]
     // the path is not authenticated
@@ -73,9 +86,9 @@ module.exports = (
       const authCookie = getCookie(ctx, Cookies.Auth) || openJwt(headerToken)
       let authenticated = false,
         user = null,
-        internal = false
+        internal = false,
+        error = null
       if (authCookie) {
-        let error = null
         const sessionId = authCookie.sessionId
         const userId = authCookie.userId
 
@@ -103,7 +116,7 @@ module.exports = (
           console.error("Auth Error", error)
           // remove the cookie as the user does not exist anymore
           clearCookie(ctx, Cookies.Auth)
-        } else {
+        } else if (session?.lastAccessedAt < timeMinusOneMinute()) {
           // make sure we denote that the session is still in use
           await updateSessionTTL(session)
         }
@@ -131,7 +144,7 @@ module.exports = (
         delete user.password
       }
       // be explicit
-      if (authenticated !== true) {
+      if (error || authenticated !== true) {
         authenticated = false
       }
       // isAuthenticated is a function, so use a variable to be able to check authed state
@@ -142,7 +155,7 @@ module.exports = (
       } else {
         return next()
       }
-    } catch (err) {
+    } catch (err: any) {
       // invalid token, clear the cookie
       if (err && err.name === "JsonWebTokenError") {
         clearCookie(ctx, Cookies.Auth)

packages/backend-core/src/security/sessions.js → packages/backend-core/src/security/sessions.ts

@@ -3,34 +3,51 @@ const { v4: uuidv4 } = require("uuid")
 const { logWarn } = require("../logging")
 const env = require("../environment")
 
+interface Session {
+  key: string
+  userId: string
+  sessionId: string
+  lastAccessedAt: string
+  createdAt: string
+  csrfToken?: string
+  value: string
+}
+
+type SessionKey = { key: string }[]
+
 // a week in seconds
 const EXPIRY_SECONDS = 86400 * 7
 
-async function getSessionsForUser(userId) {
-  const client = await redis.getSessionClient()
-  const sessions = await client.scan(userId)
-  return sessions.map(session => session.value)
+function makeSessionID(userId: string, sessionId: string) {
+  return `${userId}/${sessionId}`
 }
 
-function makeSessionID(userId, sessionId) {
-  return `${userId}/${sessionId}`
+export async function getSessionsForUser(userId: string) {
+  const client = await redis.getSessionClient()
+  const sessions = await client.scan(userId)
+  return sessions.map((session: Session) => session.value)
 }
 
-async function invalidateSessions(userId, sessionIds = null) {
+export async function invalidateSessions(
+  userId: string,
+  opts: { sessionIds?: string[]; reason?: string } = {}
+) {
   try {
-    let sessions = []
+    const reason = opts?.reason || "unknown"
+    let sessionIds: string[] = opts.sessionIds || []
+    let sessions: SessionKey
 
     // If no sessionIds, get all the sessions for the user
     if (!sessionIds) {
       sessions = await getSessionsForUser(userId)
       sessions.forEach(
-        session =>
+        (session: any) =>
           (session.key = makeSessionID(session.userId, session.sessionId))
       )
     } else {
       // use the passed array of sessionIds
-      sessions = Array.isArray(sessionIds) ? sessionIds : [sessionIds]
-      sessions = sessions.map(sessionId => ({
+      sessionIds = Array.isArray(sessionIds) ? sessionIds : [sessionIds]
+      sessions = sessionIds.map((sessionId: string) => ({
         key: makeSessionID(userId, sessionId),
       }))
     }
@@ -43,7 +60,7 @@ async function invalidateSessions(userId, sessionIds = null) {
       }
       if (!env.isTest()) {
         logWarn(
-          `Invalidating sessions for ${userId} - ${sessions
+          `Invalidating sessions for ${userId} (reason: ${reason}) - ${sessions
             .map(session => session.key)
             .join(", ")}`
         )
@@ -55,9 +72,9 @@ async function invalidateSessions(userId, sessionIds = null) {
   }
 }
 
-exports.createASession = async (userId, session) => {
+export async function createASession(userId: string, session: Session) {
   // invalidate all other sessions
-  await invalidateSessions(userId)
+  await invalidateSessions(userId, { reason: "creation" })
 
   const client = await redis.getSessionClient()
   const sessionId = session.sessionId
@@ -65,27 +82,27 @@ exports.createASession = async (userId, session) => {
     session.csrfToken = uuidv4()
   }
   session = {
+    ...session,
     createdAt: new Date().toISOString(),
     lastAccessedAt: new Date().toISOString(),
-    ...session,
     userId,
   }
   await client.store(makeSessionID(userId, sessionId), session, EXPIRY_SECONDS)
 }
 
-exports.updateSessionTTL = async session => {
+export async function updateSessionTTL(session: Session) {
   const client = await redis.getSessionClient()
   const key = makeSessionID(session.userId, session.sessionId)
   session.lastAccessedAt = new Date().toISOString()
   await client.store(key, session, EXPIRY_SECONDS)
 }
 
-exports.endSession = async (userId, sessionId) => {
+export async function endSession(userId: string, sessionId: string) {
   const client = await redis.getSessionClient()
   await client.delete(makeSessionID(userId, sessionId))
 }
 
-exports.getSession = async (userId, sessionId) => {
+export async function getSession(userId: string, sessionId: string) {
   try {
     const client = await redis.getSessionClient()
     return client.get(makeSessionID(userId, sessionId))
@@ -96,11 +113,8 @@ exports.getSession = async (userId, sessionId) => {
   }
 }
 
-exports.getAllSessions = async () => {
+export async function getAllSessions() {
   const client = await redis.getSessionClient()
   const sessions = await client.scan()
-  return sessions.map(session => session.value)
+  return sessions.map((session: Session) => session.value)
 }
-
-exports.getUserSessions = getSessionsForUser
-exports.invalidateSessions = invalidateSessions

packages/backend-core/src/utils.js

@@ -10,7 +10,10 @@ const { queryGlobalView } = require("./db/views")
 const { Headers, Cookies, MAX_VALID_DATE } = require("./constants")
 const env = require("./environment")
 const userCache = require("./cache/user")
-const { getUserSessions, invalidateSessions } = require("./security/sessions")
+const {
+  getSessionsForUser,
+  invalidateSessions,
+} = require("./security/sessions")
 const events = require("./events")
 const tenancy = require("./tenancy")
 
@@ -178,7 +181,7 @@ exports.platformLogout = async ({ ctx, userId, keepActiveSession }) => {
   if (!ctx) throw new Error("Koa context must be supplied to logout.")
 
   const currentSession = exports.getCookie(ctx, Cookies.Auth)
-  let sessions = await getUserSessions(userId)
+  let sessions = await getSessionsForUser(userId)
 
   if (keepActiveSession) {
     sessions = sessions.filter(
@@ -190,10 +193,8 @@ exports.platformLogout = async ({ ctx, userId, keepActiveSession }) => {
     exports.clearCookie(ctx, Cookies.CurrentApp)
   }
 
-  await invalidateSessions(
-    userId,
-    sessions.map(({ sessionId }) => sessionId)
-  )
+  const sessionIds = sessions.map(({ sessionId }) => sessionId)
+  await invalidateSessions(userId, { sessionIds, reason: "logout" })
   await events.auth.logout()
   await userCache.invalidateUser(userId)
 }

packages/server/src/environment.js

@@ -63,6 +63,7 @@ module.exports = {
   DISABLE_ACCOUNT_PORTAL: process.env.DISABLE_ACCOUNT_PORTAL,
   TEMPLATE_REPOSITORY: process.env.TEMPLATE_REPOSITORY || "app",
   DISABLE_AUTO_PROD_APP_SYNC: process.env.DISABLE_AUTO_PROD_APP_SYNC,
+  SESSION_UPDATE_PERIOD: process.env.SESSION_UPDATE_PERIOD,
   // minor
   SALT_ROUNDS: process.env.SALT_ROUNDS,
   LOGGER: process.env.LOGGER,

packages/worker/src/environment.js

@@ -61,6 +61,7 @@ module.exports = {
   SMTP_FROM_ADDRESS: process.env.SMTP_FROM_ADDRESS,
   // other
   CHECKLIST_CACHE_TTL: parseIntSafe(process.env.CHECKLIST_CACHE_TTL) || 3600,
+  SESSION_UPDATE_PERIOD: process.env.SESSION_UPDATE_PERIOD,
   _set(key, value) {
     process.env[key] = value
     module.exports[key] = value

packages/worker/src/sdk/users/users.ts

@@ -370,6 +370,7 @@ export const bulkDelete = async (userIds: any) => {
 export const destroy = async (id: string, currentUser: any) => {
   const db = tenancy.getGlobalDB()
   const dbUser = await db.get(id)
+  const userId = dbUser._id as string
   let groups = dbUser.userGroups
 
   if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) {
@@ -387,7 +388,7 @@ export const destroy = async (id: string, currentUser: any) => {
 
   await deprovisioning.removeUserFromInfoDB(dbUser)
 
-  await db.remove(dbUser._id, dbUser._rev)
+  await db.remove(userId, dbUser._rev)
 
   if (groups) {
     await groupUtils.deleteGroupUsers(groups, dbUser)
@@ -395,17 +396,18 @@ export const destroy = async (id: string, currentUser: any) => {
 
   await eventHelpers.handleDeleteEvents(dbUser)
   await quotas.removeUser(dbUser)
-  await cache.user.invalidateUser(dbUser._id)
-  await sessions.invalidateSessions(dbUser._id)
+  await cache.user.invalidateUser(userId)
+  await sessions.invalidateSessions(userId, { reason: "deletion" })
   // let server know to sync user
-  await apps.syncUserInApps(dbUser._id)
+  await apps.syncUserInApps(userId)
 }
 
 const bulkDeleteProcessing = async (dbUser: User) => {
+  const userId = dbUser._id as string
   await deprovisioning.removeUserFromInfoDB(dbUser)
   await eventHelpers.handleDeleteEvents(dbUser)
-  await cache.user.invalidateUser(dbUser._id)
-  await sessions.invalidateSessions(dbUser._id)
+  await cache.user.invalidateUser(userId)
+  await sessions.invalidateSessions(userId, { reason: "bulk-deletion" })
   // let server know to sync user
-  await apps.syncUserInApps(dbUser._id)
+  await apps.syncUserInApps(userId)
 }