From 34fcf17556a5fc78e6de02921d5cf905f290286c Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Mar 2023 13:35:18 +0000 Subject: [PATCH 1/5] Various fixes for update self behaviour --- .../src/components/portal/onboarding/tours.js | 7 +++--- .../components/settings/ProfileModal.svelte | 1 + packages/builder/src/stores/portal/auth.js | 11 ++++++--- packages/types/src/api/web/auth.ts | 1 + packages/types/src/sdk/user.ts | 1 + .../worker/src/api/controllers/global/self.ts | 11 +++++---- packages/worker/src/api/routes/global/self.ts | 2 +- .../worker/src/api/routes/validation/users.ts | 23 +++++++++++++------ packages/worker/src/sdk/users/users.ts | 2 +- 9 files changed, 38 insertions(+), 21 deletions(-) diff --git a/packages/builder/src/components/portal/onboarding/tours.js b/packages/builder/src/components/portal/onboarding/tours.js index e28c638a4a..296c631bfd 100644 --- a/packages/builder/src/components/portal/onboarding/tours.js +++ b/packages/builder/src/components/portal/onboarding/tours.js @@ -3,6 +3,7 @@ import { store } from "builderStore" import { users, auth } from "stores/portal" import analytics from "analytics" import { OnboardingData, OnboardingDesign, OnboardingPublish } from "./steps" +import { API } from "api" const ONBOARDING_EVENT_PREFIX = "onboarding" export const TOUR_STEP_KEYS = { @@ -83,8 +84,7 @@ const getTours = () => { // Mark the users onboarding as complete // Clear all tour related state if (get(auth).user) { - await users.save({ - ...get(auth).user, + await API.updateSelf({ onboardedAt: new Date().toISOString(), }) @@ -114,8 +114,7 @@ const getTours = () => { onComplete: async () => { // Push the onboarding forward if (get(auth).user) { - await users.save({ - ...get(auth).user, + await API.updateSelf({ onboardedAt: new Date().toISOString(), }) diff --git a/packages/builder/src/components/settings/ProfileModal.svelte b/packages/builder/src/components/settings/ProfileModal.svelte index daeeb791bb..caaf5154b3 100644 --- a/packages/builder/src/components/settings/ProfileModal.svelte +++ b/packages/builder/src/components/settings/ProfileModal.svelte @@ -13,6 +13,7 @@ await auth.updateSelf($values) notifications.success("Information updated successfully") } catch (error) { + console.error(error) notifications.error("Failed to update information") } } diff --git a/packages/builder/src/stores/portal/auth.js b/packages/builder/src/stores/portal/auth.js index d11033c142..2ab68b11b4 100644 --- a/packages/builder/src/stores/portal/auth.js +++ b/packages/builder/src/stores/portal/auth.js @@ -154,9 +154,14 @@ export function createAuthStore() { await setInitInfo({}) }, updateSelf: async fields => { - const newUser = { ...get(auth).user, ...fields } - await API.updateSelf(newUser) - setUser(newUser) + await API.updateSelf({ ...fields }) + // Refetch to enrich after update. + try { + const user = await API.fetchBuilderSelf() + setUser(user) + } catch (error) { + setUser(null) + } }, forgotPassword: async email => { const tenantId = get(store).tenantId diff --git a/packages/types/src/api/web/auth.ts b/packages/types/src/api/web/auth.ts index e31a151c48..46b1e8cec9 100644 --- a/packages/types/src/api/web/auth.ts +++ b/packages/types/src/api/web/auth.ts @@ -17,6 +17,7 @@ export interface UpdateSelfRequest { lastName?: string password?: string forceResetPassword?: boolean + onboardedAt?: string } export interface UpdateSelfResponse { diff --git a/packages/types/src/sdk/user.ts b/packages/types/src/sdk/user.ts index 1602eeb6c8..a126d49e34 100644 --- a/packages/types/src/sdk/user.ts +++ b/packages/types/src/sdk/user.ts @@ -3,6 +3,7 @@ export interface UpdateSelf { lastName?: string password?: string forceResetPassword?: boolean + onboardedAt?: string } export interface SaveUserOpts { diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index 889a3e6a27..75a1b89a75 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -123,11 +123,12 @@ export async function updateSelf( ctx: UserCtx ) { const body = ctx.request.body - const update: UpdateSelf = { - firstName: body.firstName, - lastName: body.lastName, - password: body.password, - forceResetPassword: body.forceResetPassword, + + const update: UpdateSelf = {} + for (let [key, value] of Object.entries(body)) { + if (value) { + update[key as keyof UpdateSelf] = value + } } const user = await userSdk.updateSelf(ctx.user._id!, update) diff --git a/packages/worker/src/api/routes/global/self.ts b/packages/worker/src/api/routes/global/self.ts index 8784bb8b20..c1f1b80543 100644 --- a/packages/worker/src/api/routes/global/self.ts +++ b/packages/worker/src/api/routes/global/self.ts @@ -11,7 +11,7 @@ router .get("/api/global/self", controller.getSelf) .post( "/api/global/self", - users.buildUserSaveValidation(true), + users.buildSelfSaveValidation(), controller.updateSelf ) diff --git a/packages/worker/src/api/routes/validation/users.ts b/packages/worker/src/api/routes/validation/users.ts index 35f293ce87..bce1cf582b 100644 --- a/packages/worker/src/api/routes/validation/users.ts +++ b/packages/worker/src/api/routes/validation/users.ts @@ -17,13 +17,22 @@ let schema: any = { roles: Joi.object().pattern(/.*/, Joi.string()).required().unknown(true), } -export const buildUserSaveValidation = (isSelf = false) => { - if (!isSelf) { - schema = { - ...schema, - _id: Joi.string(), - _rev: Joi.string(), - } +export const buildSelfSaveValidation = () => { + schema = { + password: Joi.string().allow(null, ""), + forceResetPassword: Joi.boolean().optional(), + firstName: Joi.string().allow(null, ""), + lastName: Joi.string().allow(null, ""), + onboardedAt: Joi.string().allow(null, ""), + } + return auth.joiValidator.body(Joi.object(schema).required().unknown(false)) +} + +export const buildUserSaveValidation = () => { + schema = { + ...schema, + _id: Joi.string(), + _rev: Joi.string(), } return auth.joiValidator.body(Joi.object(schema).required().unknown(true)) } diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 126f91d3e1..c8295ff400 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -233,7 +233,7 @@ export async function updateSelf(id: string, data: UpdateSelf) { ...user, ...data, } - return save(user) + return save(user, { requirePassword: false }) } export const save = async ( From 8d5b215d62e9feba638b5697c9edcc4d86b131b3 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Mar 2023 13:41:57 +0000 Subject: [PATCH 2/5] Linting --- packages/builder/src/components/portal/onboarding/tours.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/portal/onboarding/tours.js b/packages/builder/src/components/portal/onboarding/tours.js index 296c631bfd..3ca25b7481 100644 --- a/packages/builder/src/components/portal/onboarding/tours.js +++ b/packages/builder/src/components/portal/onboarding/tours.js @@ -1,6 +1,6 @@ import { get } from "svelte/store" import { store } from "builderStore" -import { users, auth } from "stores/portal" +import { auth } from "stores/portal" import analytics from "analytics" import { OnboardingData, OnboardingDesign, OnboardingPublish } from "./steps" import { API } from "api" From 13cf1bf9d7737839918c9f6e8d9b217fb10a3d2b Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Mar 2023 14:21:55 +0000 Subject: [PATCH 3/5] Test updates --- .../src/api/routes/global/tests/self.spec.ts | 38 +++++++++++++------ .../worker/src/api/routes/global/users.ts | 2 +- packages/worker/src/tests/api/self.ts | 5 +-- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/self.spec.ts b/packages/worker/src/api/routes/global/tests/self.spec.ts index 74d24c7c31..f3959c7521 100644 --- a/packages/worker/src/api/routes/global/tests/self.spec.ts +++ b/packages/worker/src/api/routes/global/tests/self.spec.ts @@ -18,30 +18,26 @@ describe("/api/global/self", () => { }) describe("update", () => { - it("should update self", async () => { + it("should reject updates with forbidden keys", async () => { const user = await config.createUser() await config.createSession(user) - delete user.password - const res = await config.api.self.updateSelf(user) - const dbUser = await config.getUser(user.email) - user._rev = dbUser._rev - user.dayPassRecordedAt = mocks.date.MOCK_DATE.toISOString() - expect(res.body._id).toBe(user._id) - expect(events.user.updated).toBeCalledTimes(1) - expect(events.user.updated).toBeCalledWith(dbUser) - expect(events.user.passwordUpdated).not.toBeCalled() + await config.api.self.updateSelf(user, user).expect(400) }) it("should update password", async () => { const user = await config.createUser() await config.createSession(user) - user.password = "newPassword" - const res = await config.api.self.updateSelf(user) + const res = await config.api.self + .updateSelf(user, { + password: "newPassword", + }) + .expect(200) const dbUser = await config.getUser(user.email) + user._rev = dbUser._rev user.dayPassRecordedAt = mocks.date.MOCK_DATE.toISOString() expect(res.body._id).toBe(user._id) @@ -51,4 +47,22 @@ describe("/api/global/self", () => { expect(events.user.passwordUpdated).toBeCalledWith(dbUser) }) }) + + it("should update onboarding", async () => { + const user = await config.createUser() + await config.createSession(user) + + const res = await config.api.self + .updateSelf(user, { + onboardedAt: "2023-03-07T14:10:54.869Z", + }) + .expect(200) + + const dbUser = await config.getUser(user.email) + + user._rev = dbUser._rev + user.dayPassRecordedAt = mocks.date.MOCK_DATE.toISOString() + expect(dbUser.onboardedAt).toBe("2023-03-07T14:10:54.869Z") + expect(res.body._id).toBe(user._id) + }) }) diff --git a/packages/worker/src/api/routes/global/users.ts b/packages/worker/src/api/routes/global/users.ts index a6312679c3..47e76c17be 100644 --- a/packages/worker/src/api/routes/global/users.ts +++ b/packages/worker/src/api/routes/global/users.ts @@ -128,7 +128,7 @@ router .get("/api/global/users/self", selfController.getSelf) .post( "/api/global/users/self", - users.buildUserSaveValidation(true), + users.buildUserSaveValidation(), selfController.updateSelf ) diff --git a/packages/worker/src/tests/api/self.ts b/packages/worker/src/tests/api/self.ts index dcc6c1a98b..1c1492f37f 100644 --- a/packages/worker/src/tests/api/self.ts +++ b/packages/worker/src/tests/api/self.ts @@ -7,13 +7,12 @@ export class SelfAPI extends TestAPI { super(config) } - updateSelf = (user: User) => { + updateSelf = (user: User, update: any) => { return this.request .post(`/api/global/self`) - .send(user) + .send(update) .set(this.config.authHeaders(user)) .expect("Content-Type", /json/) - .expect(200) } getSelf = (user: User) => { From 2ec35c1739c0fb5ffa5016504911019cd6212624 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Mar 2023 16:17:29 +0000 Subject: [PATCH 4/5] Review updates --- packages/worker/src/api/controllers/global/self.ts | 7 ++----- packages/worker/src/api/routes/validation/users.ts | 8 ++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index 75a1b89a75..2e7044da37 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -124,11 +124,8 @@ export async function updateSelf( ) { const body = ctx.request.body - const update: UpdateSelf = {} - for (let [key, value] of Object.entries(body)) { - if (value) { - update[key as keyof UpdateSelf] = value - } + const update: UpdateSelf = { + ...body, } const user = await userSdk.updateSelf(ctx.user._id!, update) diff --git a/packages/worker/src/api/routes/validation/users.ts b/packages/worker/src/api/routes/validation/users.ts index bce1cf582b..30a3d67434 100644 --- a/packages/worker/src/api/routes/validation/users.ts +++ b/packages/worker/src/api/routes/validation/users.ts @@ -19,11 +19,11 @@ let schema: any = { export const buildSelfSaveValidation = () => { schema = { - password: Joi.string().allow(null, ""), + password: Joi.string().optional(), forceResetPassword: Joi.boolean().optional(), - firstName: Joi.string().allow(null, ""), - lastName: Joi.string().allow(null, ""), - onboardedAt: Joi.string().allow(null, ""), + firstName: Joi.string().allow("").optional(), + lastName: Joi.string().allow("").optional(), + onboardedAt: Joi.string().optional(), } return auth.joiValidator.body(Joi.object(schema).required().unknown(false)) } From fb7e81fbc907eb6856044ba9c9d1ae565c5d7874 Mon Sep 17 00:00:00 2001 From: Dean Date: Tue, 7 Mar 2023 16:39:26 +0000 Subject: [PATCH 5/5] Review feedback --- packages/types/src/sdk/user.ts | 8 -------- .../worker/src/api/controllers/global/self.ts | 18 +++++++----------- packages/worker/src/sdk/users/users.ts | 10 ---------- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/packages/types/src/sdk/user.ts b/packages/types/src/sdk/user.ts index a126d49e34..2b970da1a9 100644 --- a/packages/types/src/sdk/user.ts +++ b/packages/types/src/sdk/user.ts @@ -1,11 +1,3 @@ -export interface UpdateSelf { - firstName?: string - lastName?: string - password?: string - forceResetPassword?: boolean - onboardedAt?: string -} - export interface SaveUserOpts { hashPassword?: boolean requirePassword?: boolean diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index 2e7044da37..78e5bf7164 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -10,12 +10,7 @@ import { } from "@budibase/backend-core" import env from "../../../environment" import { groups } from "@budibase/pro" -import { - UpdateSelfRequest, - UpdateSelfResponse, - UpdateSelf, - UserCtx, -} from "@budibase/types" +import { UpdateSelfRequest, UpdateSelfResponse, UserCtx } from "@budibase/types" const { getCookie, clearCookie, newid } = utils function newTestApiKey() { @@ -122,13 +117,14 @@ export async function getSelf(ctx: any) { export async function updateSelf( ctx: UserCtx ) { - const body = ctx.request.body + const update = ctx.request.body - const update: UpdateSelf = { - ...body, + let user = await userSdk.getUser(ctx.user._id!) + user = { + ...user, + ...update, } - - const user = await userSdk.updateSelf(ctx.user._id!, update) + user = await userSdk.save(user, { requirePassword: false }) if (update.password) { // Log all other sessions out apart from the current one diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index c8295ff400..c686690367 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -29,7 +29,6 @@ import { PlatformUserByEmail, RowResponse, SearchUsersRequest, - UpdateSelf, User, SaveUserOpts, } from "@budibase/types" @@ -227,15 +226,6 @@ export async function isPreventPasswordActions(user: User) { return !!(account && isSSOAccount(account)) } -export async function updateSelf(id: string, data: UpdateSelf) { - let user = await getUser(id) - user = { - ...user, - ...data, - } - return save(user, { requirePassword: false }) -} - export const save = async ( user: User, opts: SaveUserOpts = {}