From 0ed8464aabe1712d8407335cdadfbbcea94c856f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 16 Jan 2024 11:07:03 +0100 Subject: [PATCH 1/4] Display error on wrong password --- packages/worker/src/api/controllers/global/users.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index b0e3219656..aa41b20c83 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -425,6 +425,6 @@ export const inviteAccept = async ( ctx.throw(400, err) } console.warn("Error inviting user", err) - ctx.throw(400, "Unable to create new user, invitation invalid.") + ctx.throw(400, err || "Unable to create new user, invitation invalid.") } } From 0cce1425715aa772953e2bccae17c3b5aeb46cfd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 16 Jan 2024 11:20:27 +0100 Subject: [PATCH 2/4] Use lock to prevent race conditions on invite --- packages/types/src/sdk/locks.ts | 1 + .../src/api/controllers/global/users.ts | 85 +++++++++++-------- 2 files changed, 50 insertions(+), 36 deletions(-) diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index 0e6053a4db..c7c028a135 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -22,6 +22,7 @@ export enum LockName { QUOTA_USAGE_EVENT = "quota_usage_event", APP_MIGRATION = "app_migrations", PROCESS_AUTO_COLUMNS = "process_auto_columns", + PROCESS_USER_INVITE = "process_user_invite", } export type LockOptions = { diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index aa41b20c83..35a2a4d1af 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -12,6 +12,8 @@ import { InviteUserRequest, InviteUsersRequest, InviteUsersResponse, + LockName, + LockType, MigrationType, SaveUserResponse, SearchUsersRequest, @@ -27,6 +29,7 @@ import { platform, tenancy, db, + locks, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" import { isEmailConfigured } from "../../../utilities/email" @@ -380,45 +383,55 @@ export const inviteAccept = async ( ) => { const { inviteCode, password, firstName, lastName } = ctx.request.body try { - // info is an extension of the user object that was stored by global - const { email, info }: any = await cache.invite.getCode(inviteCode) - await cache.invite.deleteCode(inviteCode) - const user = await tenancy.doInTenant(info.tenantId, async () => { - let request: any = { - firstName, - lastName, - password, - email, - admin: { global: info?.admin?.global || false }, - roles: info.apps, - tenantId: info.tenantId, - } - let builder: { global: boolean; apps?: string[] } = { - global: info?.builder?.global || false, - } + await locks.doWithLock( + { + type: LockType.AUTO_EXTEND, + name: LockName.PROCESS_USER_INVITE, + resource: inviteCode, + }, + async () => { + // info is an extension of the user object that was stored by global + const { email, info } = await cache.invite.getCode(inviteCode) + const user = await tenancy.doInTenant(info.tenantId, async () => { + let request: any = { + firstName, + lastName, + password, + email, + admin: { global: info?.admin?.global || false }, + roles: info.apps, + tenantId: info.tenantId, + } + const builder: { global: boolean; apps?: string[] } = { + global: info?.builder?.global || false, + } - if (info?.builder?.apps) { - builder.apps = info.builder.apps - request.builder = builder - } - delete info.apps - request = { - ...request, - ...info, - } + if (info?.builder?.apps) { + builder.apps = info.builder.apps + request.builder = builder + } + delete info.apps + request = { + ...request, + ...info, + } - const saved = await userSdk.db.save(request) - const db = tenancy.getGlobalDB() - const user = await db.get(saved._id) - await events.user.inviteAccepted(user) - return saved - }) + const saved = await userSdk.db.save(request) + const db = tenancy.getGlobalDB() + const user = await db.get(saved._id) + await events.user.inviteAccepted(user) + return saved + }) - ctx.body = { - _id: user._id!, - _rev: user._rev!, - email: user.email, - } + await cache.invite.deleteCode(inviteCode) + + ctx.body = { + _id: user._id!, + _rev: user._rev!, + email: user.email, + } + } + ) } catch (err: any) { if (err.code === ErrorCode.USAGE_LIMIT_EXCEEDED) { // explicitly re-throw limit exceeded errors From 7e50986a2e2bce31ca6794bbd9690e2f36ddad26 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 16 Jan 2024 11:28:35 +0100 Subject: [PATCH 3/4] Remove unnecessary get --- packages/worker/src/api/controllers/global/users.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 35a2a4d1af..8de9afd24b 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -417,9 +417,7 @@ export const inviteAccept = async ( } const saved = await userSdk.db.save(request) - const db = tenancy.getGlobalDB() - const user = await db.get(saved._id) - await events.user.inviteAccepted(user) + await events.user.inviteAccepted(saved) return saved }) From e42784b5e97b9e38ece3403bdc706983ad54d009 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 16 Jan 2024 11:49:34 +0100 Subject: [PATCH 4/4] Use system lock --- packages/worker/src/api/controllers/global/users.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 8de9afd24b..28ba97b4e2 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -388,6 +388,7 @@ export const inviteAccept = async ( type: LockType.AUTO_EXTEND, name: LockName.PROCESS_USER_INVITE, resource: inviteCode, + systemLock: true, }, async () => { // info is an extension of the user object that was stored by global