diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 86fd4f6799..288a0462e7 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -214,6 +214,7 @@ jobs: echo "pro_commit=$pro_commit" echo "pro_commit=$pro_commit" >> "$GITHUB_OUTPUT" echo "base_commit=$base_commit" + echo "base_commit=$base_commit" >> "$GITHUB_OUTPUT" base_commit_excluding_merges=$(git log --no-merges -n 1 --format=format:%H $base_commit) echo "base_commit_excluding_merges=$base_commit_excluding_merges" @@ -230,7 +231,7 @@ jobs: base_commit_excluding_merges='${{ steps.get_pro_commits.outputs.base_commit_excluding_merges }}' pro_commit='${{ steps.get_pro_commits.outputs.pro_commit }}' - any_commit=$(git log --no-merges $base_commit...$pro_commit) + any_commit=$(git log --no-merges $base_commit_excluding_merges...$pro_commit) if [ -n "$any_commit" ]; then echo $any_commit diff --git a/packages/backend-core/src/users/db.ts b/packages/backend-core/src/users/db.ts index 37547573bd..4865ebb5bc 100644 --- a/packages/backend-core/src/users/db.ts +++ b/packages/backend-core/src/users/db.ts @@ -221,7 +221,7 @@ export class UserDB { const tenantId = getTenantId() const db = getGlobalDB() - let { email, _id, userGroups = [], roles } = user + const { email, _id, userGroups = [], roles } = user if (!email && !_id) { throw new Error("_id or email is required") @@ -231,11 +231,10 @@ export class UserDB { if (_id) { // try to get existing user from db try { - dbUser = (await db.get(_id)) as User - if (email && dbUser.email !== email) { - throw "Email address cannot be changed" + dbUser = await usersCore.getById(_id) + if (email && dbUser.email !== email && !opts.allowChangingEmail) { + throw new Error("Email address cannot be changed") } - email = dbUser.email } catch (e: any) { if (e.status === 404) { // do nothing, save this new user with the id specified - required for SSO auth @@ -271,13 +270,13 @@ export class UserDB { // make sure we set the _id field for a new user // Also if this is a new user, associate groups with them - let groupPromises = [] + const groupPromises = [] if (!_id) { - _id = builtUser._id! - if (userGroups.length > 0) { for (let groupId of userGroups) { - groupPromises.push(UserDB.groups.addUsers(groupId, [_id!])) + groupPromises.push( + UserDB.groups.addUsers(groupId, [builtUser._id!]) + ) } } } @@ -288,6 +287,11 @@ export class UserDB { builtUser._rev = response.rev await eventHelpers.handleSaveEvents(builtUser, dbUser) + if (dbUser && builtUser.email !== dbUser.email) { + // Remove the plaform email reference if the email changed + await platform.users.removeUser({ email: dbUser.email } as User) + } + await platform.users.addUser( tenantId, builtUser._id!, diff --git a/packages/backend-core/src/users/test/db.spec.ts b/packages/backend-core/src/users/test/db.spec.ts new file mode 100644 index 0000000000..3e29d6673c --- /dev/null +++ b/packages/backend-core/src/users/test/db.spec.ts @@ -0,0 +1,188 @@ +import { User, UserStatus } from "@budibase/types" +import { DBTestConfiguration, generator, structures } from "../../../tests" +import { UserDB } from "../db" +import { searchExistingEmails } from "../lookup" + +const db = UserDB + +const config = new DBTestConfiguration() + +const quotas = { + addUsers: jest + .fn() + .mockImplementation( + (_change: number, _creatorsChange: number, cb?: () => Promise) => + cb && cb() + ), + removeUsers: jest + .fn() + .mockImplementation( + (_change: number, _creatorsChange: number, cb?: () => Promise) => + cb && cb() + ), +} +const groups = { + addUsers: jest.fn(), + getBulk: jest.fn(), + getGroupBuilderAppIds: jest.fn(), +} +const features = { isSSOEnforced: jest.fn(), isAppBuildersEnabled: jest.fn() } + +describe("UserDB", () => { + beforeAll(() => { + db.init(quotas, groups, features) + }) + + describe("save", () => { + describe("create", () => { + it("creating a new user will persist it", async () => { + const email = generator.email({}) + const user: User = structures.users.user({ + email, + tenantId: config.getTenantId(), + }) + + await config.doInTenant(async () => { + const saveUserResponse = await db.save(user) + + const persistedUser = await db.getUserByEmail(email) + expect(persistedUser).toEqual({ + ...user, + _id: saveUserResponse._id, + _rev: expect.stringMatching(/^1-\w+/), + password: expect.not.stringMatching(user.password!), + status: UserStatus.ACTIVE, + createdAt: Date.now(), + updatedAt: new Date().toISOString(), + }) + }) + }) + + it("the same email cannot be used twice in the same tenant", async () => { + const email = generator.email({}) + const user: User = structures.users.user({ + email, + tenantId: config.getTenantId(), + }) + + await config.doInTenant(() => db.save(user)) + + await config.doInTenant(() => + expect(db.save(user)).rejects.toThrow( + `Email already in use: '${email}'` + ) + ) + }) + + it("the same email cannot be used twice in different tenants", async () => { + const email = generator.email({}) + const user: User = structures.users.user({ + email, + tenantId: config.getTenantId(), + }) + + await config.doInTenant(() => db.save(user)) + + config.newTenant() + await config.doInTenant(() => + expect(db.save(user)).rejects.toThrow( + `Email already in use: '${email}'` + ) + ) + }) + }) + + describe("update", () => { + let user: User + + beforeEach(async () => { + user = await config.doInTenant(() => + db.save( + structures.users.user({ + email: generator.email({}), + tenantId: config.getTenantId(), + }) + ) + ) + }) + + it("can update user properties", async () => { + await config.doInTenant(async () => { + const updatedName = generator.first() + user.firstName = updatedName + + await db.save(user) + + const persistedUser = await db.getUserByEmail(user.email) + expect(persistedUser).toEqual( + expect.objectContaining({ + _id: user._id, + email: user.email, + firstName: updatedName, + lastName: user.lastName, + }) + ) + }) + }) + + it("email cannot be updated by default", async () => { + await config.doInTenant(async () => { + await expect( + db.save({ ...user, email: generator.email({}) }) + ).rejects.toThrow("Email address cannot be changed") + }) + }) + + it("email can be updated if specified", async () => { + await config.doInTenant(async () => { + const newEmail = generator.email({}) + + await db.save( + { ...user, email: newEmail }, + { allowChangingEmail: true } + ) + + const persistedUser = await db.getUserByEmail(newEmail) + expect(persistedUser).toEqual( + expect.objectContaining({ + _id: user._id, + email: newEmail, + lastName: user.lastName, + _rev: expect.stringMatching(/^2-\w+/), + }) + ) + }) + }) + + it("updating emails frees previous emails", async () => { + await config.doInTenant(async () => { + const previousEmail = user.email + const newEmail = generator.email({}) + expect(await searchExistingEmails([previousEmail, newEmail])).toEqual( + [previousEmail] + ) + + await db.save( + { ...user, email: newEmail }, + { allowChangingEmail: true } + ) + + expect(await searchExistingEmails([previousEmail, newEmail])).toEqual( + [newEmail] + ) + + await db.save( + structures.users.user({ + email: previousEmail, + tenantId: config.getTenantId(), + }) + ) + + expect(await searchExistingEmails([previousEmail, newEmail])).toEqual( + [previousEmail, newEmail] + ) + }) + }) + }) + }) +}) diff --git a/packages/pro b/packages/pro index e8f2c5a147..dbb78c8737 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit e8f2c5a14780e1f61ec3896821ba5f93d486eb72 +Subproject commit dbb78c8737c291871500bc655e30f331f6ffccbf diff --git a/packages/server/__mocks__/@sendgrid/mail.ts b/packages/server/__mocks__/@sendgrid/mail.ts deleted file mode 100644 index 8613ae4b16..0000000000 --- a/packages/server/__mocks__/@sendgrid/mail.ts +++ /dev/null @@ -1,23 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -module SendgridMock { - class Email { - constructor() { - // @ts-ignore - this.apiKey = null - } - - setApiKey(apiKey: any) { - // @ts-ignore - this.apiKey = apiKey - } - - async send(msg: any) { - if (msg.to === "invalid@example.com") { - throw "Invalid" - } - return msg - } - } - - module.exports = new Email() -} diff --git a/packages/server/src/automations/tests/openai.spec.ts b/packages/server/src/automations/tests/openai.spec.ts index 44b410a0f1..6bebe16a49 100644 --- a/packages/server/src/automations/tests/openai.spec.ts +++ b/packages/server/src/automations/tests/openai.spec.ts @@ -43,9 +43,7 @@ describe("test the openai action", () => { it("should present the correct error message when the OPENAI_API_KEY variable isn't set", async () => { await config.withCoreEnv({ OPENAI_API_KEY: "" }, async () => { - let res = await runStep("OPENAI", { - prompt: OPENAI_PROMPT, - }) + let res = await runStep("OPENAI", { prompt: OPENAI_PROMPT }) expect(res.response).toEqual( "OpenAI API Key not configured - please add the OPENAI_API_KEY environment variable." ) @@ -54,17 +52,13 @@ describe("test the openai action", () => { }) it("should be able to receive a response from ChatGPT given a prompt", async () => { - const res = await runStep("OPENAI", { - prompt: OPENAI_PROMPT, - }) + const res = await runStep("OPENAI", { prompt: OPENAI_PROMPT }) expect(res.response).toEqual("This is a test") expect(res.success).toBeTruthy() }) it("should present the correct error message when a prompt is not provided", async () => { - const res = await runStep("OPENAI", { - prompt: null, - }) + const res = await runStep("OPENAI", { prompt: null }) expect(res.response).toEqual( "Budibase OpenAI Automation Failed: No prompt supplied" ) diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index c8203392e6..b643bd50b0 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -75,7 +75,6 @@ const environment = { AUTOMATION_MAX_ITERATIONS: parseIntSafe(process.env.AUTOMATION_MAX_ITERATIONS) || DEFAULTS.AUTOMATION_MAX_ITERATIONS, - SENDGRID_API_KEY: process.env.SENDGRID_API_KEY, DYNAMO_ENDPOINT: process.env.DYNAMO_ENDPOINT, QUERY_THREAD_TIMEOUT: QUERY_THREAD_TIMEOUT, AUTOMATION_THREAD_TIMEOUT: diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 325d911f07..793bfa8c6a 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -290,7 +290,7 @@ export default class TestConfiguration { * that can be called to reset the environment variables to their original values. */ setCoreEnv(newEnvVars: Partial): () => void { - const oldEnv = cloneDeep(env) + const oldEnv = cloneDeep(coreEnv) let key: keyof typeof newEnvVars for (key in newEnvVars) { diff --git a/packages/types/src/sdk/user.ts b/packages/types/src/sdk/user.ts index 3f6f69d2d1..c0e2f80297 100644 --- a/packages/types/src/sdk/user.ts +++ b/packages/types/src/sdk/user.ts @@ -3,4 +3,5 @@ export interface SaveUserOpts { requirePassword?: boolean currentUserId?: string skipPasswordValidation?: boolean + allowChangingEmail?: boolean } diff --git a/packages/worker/src/api/routes/global/tests/scim.spec.ts b/packages/worker/src/api/routes/global/tests/scim.spec.ts index 81dcc4b7c9..7f23bc9460 100644 --- a/packages/worker/src/api/routes/global/tests/scim.spec.ts +++ b/packages/worker/src/api/routes/global/tests/scim.spec.ts @@ -574,6 +574,41 @@ describe("scim", () => { expect(events.user.updated).toHaveBeenCalledTimes(1) }) + + it("an existing user's email can be updated", async () => { + const newEmail = structures.generator.email() + const body: ScimUpdateRequest = { + schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations: [ + { + op: "Replace", + path: 'emails[type eq "work"].value', + value: newEmail, + }, + ], + } + + const response = await patchScimUser({ id: user.id, body }) + + const expectedScimUser: ScimUserResponse = { + ...user, + emails: [ + { + value: newEmail, + type: "work", + primary: true, + }, + ], + } + expect(response).toEqual(expectedScimUser) + + const persistedUser = await config.api.scimUsersAPI.find(user.id) + expect(persistedUser).toEqual(expectedScimUser) + + expect((await config.api.users.getUser(user.id)).body).toEqual( + expect.objectContaining({ _id: user.id, email: newEmail }) + ) + }) }) describe("DELETE /api/global/scim/v2/users/:id", () => {