diff --git a/packages/backend-core/src/middleware/passport/google.ts b/packages/backend-core/src/middleware/passport/google.ts index deba849233..dd3dc8b86d 100644 --- a/packages/backend-core/src/middleware/passport/google.ts +++ b/packages/backend-core/src/middleware/passport/google.ts @@ -1,9 +1,9 @@ import { ssoCallbackUrl } from "./utils" -import { authenticateThirdParty } from "./third-party-common" +import { authenticateThirdParty, SaveUserFunction } from "./third-party-common" import { ConfigType, GoogleConfig, Database, SSOProfile } from "@budibase/types" const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy -export function buildVerifyFn(saveUserFn?: Function) { +export function buildVerifyFn(saveUserFn?: SaveUserFunction) { return ( accessToken: string, refreshToken: string, @@ -39,7 +39,7 @@ export function buildVerifyFn(saveUserFn?: Function) { export async function strategyFactory( config: GoogleConfig["config"], callbackUrl: string, - saveUserFn?: Function + saveUserFn?: SaveUserFunction ) { try { const { clientID, clientSecret } = config diff --git a/packages/backend-core/src/middleware/passport/oidc.ts b/packages/backend-core/src/middleware/passport/oidc.ts index 27c3c647b7..40bc22ec0c 100644 --- a/packages/backend-core/src/middleware/passport/oidc.ts +++ b/packages/backend-core/src/middleware/passport/oidc.ts @@ -1,5 +1,5 @@ import fetch from "node-fetch" -import { authenticateThirdParty } from "./third-party-common" +import { authenticateThirdParty, SaveUserFunction } from "./third-party-common" import { ssoCallbackUrl } from "./utils" import { Config, @@ -17,7 +17,7 @@ type JwtClaims = { email: string } -export function buildVerifyFn(saveUserFn?: Function) { +export function buildVerifyFn(saveUserFn?: SaveUserFunction) { /** * @param {*} issuer The identity provider base URL * @param {*} sub The user ID @@ -106,7 +106,7 @@ function validEmail(value: string) { */ export async function strategyFactory( config: OIDCConfiguration, - saveUserFn?: Function + saveUserFn?: SaveUserFunction ) { try { const verify = buildVerifyFn(saveUserFn) diff --git a/packages/backend-core/src/middleware/passport/third-party-common.ts b/packages/backend-core/src/middleware/passport/third-party-common.ts index 8798ce5298..451cdf6cc6 100644 --- a/packages/backend-core/src/middleware/passport/third-party-common.ts +++ b/packages/backend-core/src/middleware/passport/third-party-common.ts @@ -9,6 +9,17 @@ import fetch from "node-fetch" import { ThirdPartyUser } from "@budibase/types" const jwt = require("jsonwebtoken") +type SaveUserOpts = { + requirePassword?: boolean + hashPassword?: boolean + currentUserId?: string +} + +export type SaveUserFunction = ( + user: ThirdPartyUser, + opts: SaveUserOpts +) => Promise + /** * Common authentication logic for third parties. e.g. OAuth, OIDC. */ @@ -16,7 +27,7 @@ export async function authenticateThirdParty( thirdPartyUser: ThirdPartyUser, requireLocalAccount: boolean = true, done: Function, - saveUserFn?: Function + saveUserFn?: SaveUserFunction ) { if (!saveUserFn) { throw new Error("Save user function must be provided") @@ -81,7 +92,7 @@ export async function authenticateThirdParty( // create or sync the user try { - await saveUserFn(dbUser, false, false) + await saveUserFn(dbUser, { hashPassword: false, requirePassword: false }) } catch (err: any) { return authError(done, err) } diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index 88dc82ffd2..7192077d04 100644 --- a/packages/server/src/api/controllers/public/users.ts +++ b/packages/server/src/api/controllers/public/users.ts @@ -51,6 +51,8 @@ export async function update(ctx: BBContext, next: any) { } // disallow updating your own role - always overwrite with DB roles if (isLoggedInUser(ctx, user)) { + ctx.request.body.builder = user.builder + ctx.request.body.admin = user.admin ctx.request.body.roles = user.roles } const response = await saveGlobalUser(publicApiUserFix(ctx)) diff --git a/packages/types/package.json b/packages/types/package.json index 1038a99795..42b56e1536 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -13,6 +13,7 @@ }, "jest": {}, "devDependencies": { + "@types/json5": "^2.2.0", "@types/koa": "2.13.4", "@types/node": "14.18.20", "@types/pouchdb": "6.4.0", diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 1778d6e7c6..fbf34cb196 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -25,6 +25,7 @@ export interface ThirdPartyUser extends Document { email: string userId?: string forceResetPassword?: boolean + userGroups?: string[] } export interface User extends ThirdPartyUser { @@ -42,7 +43,6 @@ export interface User extends ThirdPartyUser { password?: string status?: string createdAt?: number // override the default createdAt behaviour - users sdk historically set this to Date.now() - userGroups?: string[] dayPassRecordedAt?: string account?: { authType: string diff --git a/packages/types/yarn.lock b/packages/types/yarn.lock index 1a057c9c72..f596c58997 100644 --- a/packages/types/yarn.lock +++ b/packages/types/yarn.lock @@ -75,6 +75,13 @@ resolved "https://registry.yarnpkg.com/@types/http-errors/-/http-errors-1.8.2.tgz#7315b4c4c54f82d13fa61c228ec5c2ea5cc9e0e1" integrity sha512-EqX+YQxINb+MeXaIqYDASb6U6FCHbWjkj4a1CKDBks3d/QiB2+PqBLyO72vLDgAO1wUI4O+9gweRcQK11bTL/w== +"@types/json5@^2.2.0": + version "2.2.0" + resolved "https://registry.yarnpkg.com/@types/json5/-/json5-2.2.0.tgz#afff29abf9182a7d4a7e39105ca051f11c603d13" + integrity sha512-NrVug5woqbvNZ0WX+Gv4R+L4TGddtmFek2u8RtccAgFZWtS9QXF2xCXY22/M4nzkaKF0q9Fc6M/5rxLDhfwc/A== + dependencies: + json5 "*" + "@types/keygrip@*": version "1.0.2" resolved "https://registry.yarnpkg.com/@types/keygrip/-/keygrip-1.0.2.tgz#513abfd256d7ad0bf1ee1873606317b33b1b2a72" @@ -447,6 +454,11 @@ inherits@2: resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.4.tgz#0fa2c64f932917c3433a0ded55363aae37416b7c" integrity sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ== +json5@*: + version "2.2.1" + resolved "https://registry.yarnpkg.com/json5/-/json5-2.2.1.tgz#655d50ed1e6f95ad1a3caababd2b0efda10b395c" + integrity sha512-1hqLFMSrGHRHxav9q9gNjJ5EXznIxGVO09xQRrwplcS8qs28pZ8s8hupZAmqDwZUmVZ2Qb2jnyPOWcDH8m8dlA== + mime-db@1.52.0: version "1.52.0" resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.52.0.tgz#bbabcdc02859f4987301c856e3387ce5ec43bf70" diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index e913ccee88..27b90cbd56 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -24,7 +24,8 @@ const MAX_USERS_UPLOAD_LIMIT = 1000 export const save = async (ctx: any) => { try { - ctx.body = await sdk.users.save(ctx.request.body) + const currentUserId = ctx.user._id + ctx.body = await sdk.users.save(ctx.request.body, { currentUserId }) } catch (err: any) { ctx.throw(err.status || 400, err) } diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index 31a99a61f1..3b732cb3d9 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -1,4 +1,4 @@ -import { InviteUsersResponse } from "@budibase/types" +import { InviteUsersResponse, User } from "@budibase/types" jest.mock("nodemailer") import { @@ -298,6 +298,23 @@ describe("/api/global/users", () => { expect(events.user.passwordForceReset).not.toBeCalled() }) + it("should not allow a user to update their own admin/builder status", async () => { + const user = (await config.api.users.getUser(config.defaultUser?._id!)) + .body as User + await config.api.users.saveUser({ + ...user, + admin: { + global: false, + }, + builder: { + global: false, + }, + }) + const userOut = (await config.api.users.getUser(user._id!)).body + expect(userOut.admin.global).toBe(true) + expect(userOut.builder.global).toBe(true) + }) + it("should be able to force reset password", async () => { const user = await config.createUser() jest.clearAllMocks() diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 539ac21300..aedbd59703 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -29,6 +29,7 @@ import { RowResponse, SearchUsersRequest, User, + ThirdPartyUser, } from "@budibase/types" import { sendEmail } from "../../utilities/email" import { EmailTemplatePurpose } from "../../constants" @@ -103,13 +104,14 @@ export const getUser = async (userId: string) => { return user } -interface SaveUserOpts { +export interface SaveUserOpts { hashPassword?: boolean requirePassword?: boolean + currentUserId?: string } const buildUser = async ( - user: User, + user: User | ThirdPartyUser, opts: SaveUserOpts = { hashPassword: true, requirePassword: true, @@ -117,7 +119,8 @@ const buildUser = async ( tenantId: string, dbUser?: any ): Promise => { - let { password, _id } = user + let fullUser = user as User + let { password, _id } = fullUser let hashedPassword if (password) { @@ -130,24 +133,24 @@ const buildUser = async ( _id = _id || dbUtils.generateGlobalUserID() - user = { + fullUser = { createdAt: Date.now(), ...dbUser, - ...user, + ...fullUser, _id, password: hashedPassword, tenantId, } // make sure the roles object is always present - if (!user.roles) { - user.roles = {} + if (!fullUser.roles) { + fullUser.roles = {} } // add the active status to a user if its not provided - if (user.status == null) { - user.status = constants.UserStatus.ACTIVE + if (fullUser.status == null) { + fullUser.status = constants.UserStatus.ACTIVE } - return user + return fullUser } const validateUniqueUser = async (email: string, tenantId: string) => { @@ -169,12 +172,16 @@ const validateUniqueUser = async (email: string, tenantId: string) => { } export const save = async ( - user: User, - opts: SaveUserOpts = { - hashPassword: true, - requirePassword: true, - } + user: User | ThirdPartyUser, + opts: SaveUserOpts = {} ): Promise => { + // default booleans to true + if (opts.hashPassword == null) { + opts.hashPassword = true + } + if (opts.requirePassword == null) { + opts.requirePassword = true + } const tenantId = tenancy.getTenantId() const db = tenancy.getGlobalDB() @@ -213,6 +220,12 @@ export const save = async ( await validateUniqueUser(email, tenantId) let builtUser = await buildUser(user, opts, tenantId, dbUser) + // don't allow a user to update its own roles/perms + if (opts.currentUserId && opts.currentUserId === dbUser?._id) { + builtUser.builder = dbUser.builder + builtUser.admin = dbUser.admin + builtUser.roles = dbUser.roles + } // make sure we set the _id field for a new user // Also if this is a new user, associate groups with them