Remove loop for get account during user bulk import (#10203)

This commit is contained in:
Rory Powell 2023-04-04 10:14:20 +01:00 committed by GitHub
parent fbbb36b809
commit 5eecb6e686
3 changed files with 52 additions and 29 deletions

View File

@ -126,9 +126,8 @@ describe("/api/global/auth", () => {
it("should prevent user from logging in", async () => {
user = await config.createUser()
const account = structures.accounts.ssoAccount() as CloudAccount
mocks.accounts.getAccount.mockReturnValueOnce(
Promise.resolve(account)
)
account.email = user.email
mocks.accounts.getAccountByTenantId.mockResolvedValueOnce(account)
await testSSOUser()
})
@ -186,9 +185,8 @@ describe("/api/global/auth", () => {
it("should prevent user from generating password reset email", async () => {
user = await config.createUser(structures.users.user())
const account = structures.accounts.ssoAccount() as CloudAccount
mocks.accounts.getAccount.mockReturnValueOnce(
Promise.resolve(account)
)
account.email = user.email
mocks.accounts.getAccountByTenantId.mockResolvedValueOnce(account)
await testSSOUser()
})

View File

@ -1,6 +1,6 @@
import { structures } from "../../../tests"
import { mocks } from "@budibase/backend-core/tests"
import { env } from "@budibase/backend-core"
import { env, context } from "@budibase/backend-core"
import * as users from "../users"
import { CloudAccount } from "@budibase/types"
import { isPreventPasswordActions } from "../users"
@ -16,32 +16,50 @@ describe("users", () => {
describe("isPreventPasswordActions", () => {
it("returns false for non sso user", async () => {
const user = structures.users.user()
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(false)
await context.doInTenant(structures.tenant.id(), async () => {
const user = structures.users.user()
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(false)
})
})
it("returns true for sso account user", async () => {
const user = structures.users.user()
mocks.accounts.getAccount.mockReturnValue(
Promise.resolve(structures.accounts.ssoAccount() as CloudAccount)
)
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
await context.doInTenant(structures.tenant.id(), async () => {
const user = structures.users.user()
const account = structures.accounts.ssoAccount() as CloudAccount
account.email = user.email
mocks.accounts.getAccountByTenantId.mockResolvedValueOnce(account)
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
})
})
it("returns false when account doesn't match user email", async () => {
await context.doInTenant(structures.tenant.id(), async () => {
const user = structures.users.user()
const account = structures.accounts.ssoAccount() as CloudAccount
mocks.accounts.getAccountByTenantId.mockResolvedValueOnce(account)
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(false)
})
})
it("returns true for sso user", async () => {
const user = structures.users.ssoUser()
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
await context.doInTenant(structures.tenant.id(), async () => {
const user = structures.users.ssoUser()
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
})
})
describe("enforced sso", () => {
it("returns true for all users when sso is enforced", async () => {
const user = structures.users.user()
pro.features.isSSOEnforced.mockReturnValue(Promise.resolve(true))
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
await context.doInTenant(structures.tenant.id(), async () => {
const user = structures.users.user()
pro.features.isSSOEnforced.mockResolvedValueOnce(true)
const result = await users.isPreventPasswordActions(user)
expect(result).toBe(true)
})
})
})

View File

@ -31,6 +31,7 @@ import {
SearchUsersRequest,
User,
SaveUserOpts,
Account,
} from "@budibase/types"
import { sendEmail } from "../../utilities/email"
import { EmailTemplatePurpose } from "../../constants"
@ -127,7 +128,8 @@ const buildUser = async (
requirePassword: true,
},
tenantId: string,
dbUser?: any
dbUser?: any,
account?: Account
): Promise<User> => {
let { password, _id } = user
@ -138,7 +140,7 @@ const buildUser = async (
let hashedPassword
if (password) {
if (await isPreventPasswordActions(user)) {
if (await isPreventPasswordActions(user, account)) {
throw new HTTPError("Password change is disabled for this user", 400)
}
hashedPassword = opts.hashPassword ? await utils.hash(password) : password
@ -209,7 +211,7 @@ const validateUniqueUser = async (email: string, tenantId: string) => {
}
}
export async function isPreventPasswordActions(user: User) {
export async function isPreventPasswordActions(user: User, account?: Account) {
// when in maintenance mode we allow sso users with the admin role
// to perform any password action - this prevents lockout
if (coreEnv.ENABLE_SSO_MAINTENANCE_MODE && user.admin?.global) {
@ -227,8 +229,10 @@ export async function isPreventPasswordActions(user: User) {
}
// Check account sso
const account = await accountSdk.api.getAccount(user.email)
return !!(account && isSSOAccount(account))
if (!account) {
account = await accountSdk.api.getAccountByTenantId(tenancy.getTenantId())
}
return !!(account && account.email === user.email && isSSOAccount(account))
}
export const save = async (
@ -439,6 +443,7 @@ export const bulkCreate = async (
newUsers.push(newUser)
}
const account = await accountSdk.api.getAccountByTenantId(tenantId)
// create the promises array that will be called by bulkDocs
newUsers.forEach((user: any) => {
usersToSave.push(
@ -448,7 +453,9 @@ export const bulkCreate = async (
hashPassword: true,
requirePassword: user.requirePassword,
},
tenantId
tenantId,
undefined, // no dbUser
account
)
)
})