From 52d16d1099fd2346bf8e7f485fc37ebffbedb243 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 5 Aug 2022 17:13:03 +0100 Subject: [PATCH] Updating sessions to TS, adding env var to set the session update length, adding reasons for invalidation, making sure errors are never considered authenticated. --- packages/backend-core/src/environment.ts | 1 + packages/backend-core/src/index.ts | 2 +- .../src/middleware/authenticated.ts | 8 +-- .../src/security/{sessions.js => sessions.ts} | 62 ++++++++++++------- packages/backend-core/src/utils.js | 13 ++-- packages/server/src/environment.js | 1 + packages/worker/src/environment.js | 1 + packages/worker/src/sdk/users/users.ts | 16 ++--- 8 files changed, 62 insertions(+), 42 deletions(-) rename packages/backend-core/src/security/{sessions.js => sessions.ts} (63%) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 51cc721ded..0348d921ab 100644 --- a/packages/backend-core/src/environment.ts +++ b/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) { diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index ced4630fb7..e585d4b6c3 100644 --- a/packages/backend-core/src/index.ts +++ b/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" diff --git a/packages/backend-core/src/middleware/authenticated.ts b/packages/backend-core/src/middleware/authenticated.ts index e5b2232a42..7280eba294 100644 --- a/packages/backend-core/src/middleware/authenticated.ts +++ b/packages/backend-core/src/middleware/authenticated.ts @@ -11,7 +11,7 @@ import { decrypt } from "../security/encryption" const identity = require("../context/identity") const env = require("../environment") -const ONE_MINUTE = 60 * 1000 +const ONE_MINUTE = env.SESSION_UPDATE_PERIOD || 60 * 1000 interface FinaliseOpts { authenticated?: boolean @@ -86,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 @@ -144,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 diff --git a/packages/backend-core/src/security/sessions.js b/packages/backend-core/src/security/sessions.ts similarity index 63% rename from packages/backend-core/src/security/sessions.js rename to packages/backend-core/src/security/sessions.ts index a3be0a1a58..f8375f510b 100644 --- a/packages/backend-core/src/security/sessions.js +++ b/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, sessionId) { +function makeSessionID(userId: string, sessionId: string) { return `${userId}/${sessionId}` } -async function invalidateSessions(userId, sessionIds = null) { +export async function getSessionsForUser(userId: string) { + const client = await redis.getSessionClient() + const sessions = await client.scan(userId) + return sessions.map((session: Session) => session.value) +} + +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 diff --git a/packages/backend-core/src/utils.js b/packages/backend-core/src/utils.js index cf32539c58..1e143968d9 100644 --- a/packages/backend-core/src/utils.js +++ b/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) } diff --git a/packages/server/src/environment.js b/packages/server/src/environment.js index 99d099f8d5..c2e2815e00 100644 --- a/packages/server/src/environment.js +++ b/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, diff --git a/packages/worker/src/environment.js b/packages/worker/src/environment.js index 4c3cab1cab..bb45c1dd78 100644 --- a/packages/worker/src/environment.js +++ b/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 diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index e6b3f0a21d..58c2decabf 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/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) }