From eaa89c824eb19215758c7c36c6dff99e13ddda56 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 19 Apr 2021 16:26:33 +0100 Subject: [PATCH 1/6] Updating the server to remove use of the email in the user ID. --- packages/server/src/api/controllers/auth.js | 69 +------------------ packages/server/src/api/controllers/row.js | 13 ++-- packages/server/src/api/controllers/user.js | 53 +++++++------- .../server/src/api/routes/tests/auth.spec.js | 15 ++-- .../src/api/routes/tests/routing.spec.js | 5 -- .../server/src/api/routes/tests/user.spec.js | 43 +++++------- .../routes/tests/utilities/TestFunctions.js | 16 ++--- .../src/api/routes/tests/utilities/index.js | 9 +++ packages/server/src/api/routes/user.js | 4 +- .../src/automations/tests/createUser.spec.js | 1 + packages/server/src/constants/index.js | 1 + packages/server/src/db/utils.js | 16 ++--- packages/server/src/middleware/currentapp.js | 7 +- .../src/middleware/tests/currentapp.spec.js | 4 +- .../src/tests/utilities/TestConfiguration.js | 34 +++------ packages/server/src/utilities/users.js | 20 +++--- .../server/src/utilities/workerRequests.js | 21 +++--- 17 files changed, 119 insertions(+), 212 deletions(-) diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index 5cf87d5a57..2ac3d30e48 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -1,75 +1,8 @@ -const jwt = require("jsonwebtoken") const CouchDB = require("../../db") -const bcrypt = require("../../utilities/bcrypt") -const env = require("../../environment") -const { getAPIKey } = require("../../utilities/usageQuota") -const { generateUserMetadataID } = require("../../db/utils") -const { setCookie } = require("../../utilities") const { outputProcessing } = require("../../utilities/rowProcessor") const { InternalTables } = require("../../db/utils") -const { UserStatus } = require("@budibase/auth") const { getFullUser } = require("../../utilities/users") -const INVALID_ERR = "Invalid Credentials" - -exports.authenticate = async ctx => { - const appId = ctx.appId - if (!appId) ctx.throw(400, "No appId") - - const { email, password } = ctx.request.body - - if (!email) ctx.throw(400, "Email Required.") - if (!password) ctx.throw(400, "Password Required.") - - // Check the user exists in the instance DB by email - const db = new CouchDB(appId) - const app = await db.get(appId) - - let dbUser - try { - dbUser = await db.get(generateUserMetadataID(email)) - } catch (_) { - // do not want to throw a 404 - as this could be - // used to determine valid emails - ctx.throw(401, INVALID_ERR) - } - - // check that the user is currently inactive, if this is the case throw invalid - if (dbUser.status === UserStatus.INACTIVE) { - ctx.throw(401, INVALID_ERR) - } - - // authenticate - if (await bcrypt.compare(password, dbUser.password)) { - const payload = { - userId: dbUser._id, - roleId: dbUser.roleId, - version: app.version, - } - // if in prod add the user api key, unless self hosted - /* istanbul ignore next */ - if (env.isProd() && !env.SELF_HOSTED) { - const { apiKey } = await getAPIKey(ctx.appId) - payload.apiKey = apiKey - } - - const token = jwt.sign(payload, ctx.config.jwtSecret, { - expiresIn: "1 day", - }) - - setCookie(ctx, token, appId) - - delete dbUser.password - ctx.body = { - token, - ...dbUser, - appId, - } - } else { - ctx.throw(401, INVALID_ERR) - } -} - exports.fetchSelf = async ctx => { if (!ctx.user) { ctx.throw(403, "No user logged in") @@ -82,7 +15,7 @@ exports.fetchSelf = async ctx => { return } - const user = await getFullUser({ ctx, userId: userId }) + const user = await getFullUser(ctx, userId) if (appId) { const db = new CouchDB(appId) diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 2987cbeace..3ceac0d832 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -42,7 +42,7 @@ async function findRow(ctx, db, tableId, rowId) { // TODO remove special user case in future if (tableId === InternalTables.USER_METADATA) { ctx.params = { - userId: rowId, + id: rowId, } await userController.findMetadata(ctx) row = ctx.body @@ -140,12 +140,7 @@ exports.save = async function(ctx) { } if (!inputs._rev && !inputs._id) { - // TODO remove special user case in future - if (inputs.tableId === InternalTables.USER_METADATA) { - inputs._id = generateUserMetadataID(inputs.email) - } else { - inputs._id = generateRowID(inputs.tableId) - } + inputs._id = generateRowID(inputs.tableId) } // this returns the table and row incase they have been updated @@ -342,7 +337,7 @@ exports.destroy = async function(ctx) { // TODO remove special user case in future if (ctx.params.tableId === InternalTables.USER_METADATA) { ctx.params = { - userId: ctx.params.rowId, + id: ctx.params.rowId, } await userController.destroyMetadata(ctx) } else { @@ -449,7 +444,7 @@ async function bulkDelete(ctx) { updates = updates.concat( rows.map(row => { ctx.params = { - userId: row._id, + id: row._id, } return userController.destroyMetadata(ctx) }) diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index 28576194c0..eee1f3f048 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -2,7 +2,7 @@ const CouchDB = require("../../db") const { generateUserMetadataID, getUserMetadataParams, - getEmailFromUserMetadataID, + getGlobalIDFromUserMetadataID, } = require("../../db/utils") const { InternalTables } = require("../../db/utils") const { getRole } = require("../../utilities/security/roles") @@ -25,15 +25,14 @@ exports.fetchMetadata = async function(ctx) { ).rows.map(row => row.doc) const users = [] for (let user of global) { - const info = metadata.find(meta => meta._id.includes(user.email)) + // find the metadata that matches up to the global ID + const info = metadata.find(meta => meta._id.includes(user._id)) // remove these props, not for the correct DB - delete user._id - delete user._rev users.push({ ...user, ...info, // make sure the ID is always a local ID, not a global one - _id: generateUserMetadataID(user.email), + _id: generateUserMetadataID(user._id), }) } ctx.body = users @@ -43,17 +42,16 @@ exports.createMetadata = async function(ctx) { const appId = ctx.appId const db = new CouchDB(appId) const { roleId } = ctx.request.body - const email = ctx.request.body.email || ctx.user.email // check role valid const role = await getRole(appId, roleId) if (!role) ctx.throw(400, "Invalid Role") - const metadata = await saveGlobalUser(ctx, appId, email, ctx.request.body) + const globalUser = await saveGlobalUser(ctx, appId, ctx.request.body) const user = { - ...metadata, - _id: generateUserMetadataID(email), + ...globalUser, + _id: generateUserMetadataID(globalUser._id), type: "user", tableId: InternalTables.USER_METADATA, } @@ -64,7 +62,7 @@ exports.createMetadata = async function(ctx) { ctx.body = { _id: response.id, _rev: response.rev, - email, + email: ctx.request.body.email, } } @@ -72,39 +70,34 @@ exports.updateMetadata = async function(ctx) { const appId = ctx.appId const db = new CouchDB(appId) const user = ctx.request.body - let email = user.email || getEmailFromUserMetadataID(user._id) - const metadata = await saveGlobalUser(ctx, appId, email, ctx.request.body) - if (!metadata._id) { - metadata._id = generateUserMetadataID(email) + const globalUser = await saveGlobalUser( + ctx, + appId, + getGlobalIDFromUserMetadataID(user._id), + ctx.request.body + ) + const metadata = { + ...globalUser, + _id: user._id || generateUserMetadataID(globalUser._id), + _rev: ctx.request.body._rev, } - if (!metadata._rev) { - metadata._rev = ctx.request.body._rev - } - ctx.body = await db.put({ - ...metadata, - }) + ctx.body = await db.put(metadata) } exports.destroyMetadata = async function(ctx) { const db = new CouchDB(ctx.appId) - const email = - ctx.params.email || getEmailFromUserMetadataID(ctx.params.userId) - await deleteGlobalUser(ctx, email) + await deleteGlobalUser(ctx, getGlobalIDFromUserMetadataID(ctx.params.id)) try { - const dbUser = await db.get(generateUserMetadataID(email)) + const dbUser = await db.get(ctx.params.id) await db.remove(dbUser._id, dbUser._rev) } catch (err) { // error just means the global user has no config in this app } ctx.body = { - message: `User ${ctx.params.email} deleted.`, + message: `User ${ctx.params.id} deleted.`, } } exports.findMetadata = async function(ctx) { - ctx.body = await getFullUser({ - ctx, - email: ctx.params.email, - userId: ctx.params.userId, - }) + ctx.body = await getFullUser(ctx, ctx.params.id) } diff --git a/packages/server/src/api/routes/tests/auth.spec.js b/packages/server/src/api/routes/tests/auth.spec.js index ec38003c1d..5978381f86 100644 --- a/packages/server/src/api/routes/tests/auth.spec.js +++ b/packages/server/src/api/routes/tests/auth.spec.js @@ -1,13 +1,18 @@ const setup = require("./utilities") +const { generateUserMetadataID } = require("../../../db/utils") require("../../../utilities/workerRequests") jest.mock("../../../utilities/workerRequests", () => ({ getGlobalUsers: jest.fn(() => { return { - email: "test@test.com", + _id: "us_uuid1", + } + }), + saveGlobalUser: jest.fn(() => { + return { + _id: "us_uuid1", } }), - saveGlobalUser: jest.fn(), })) describe("/authenticate", () => { @@ -22,14 +27,14 @@ describe("/authenticate", () => { describe("fetch self", () => { it("should be able to fetch self", async () => { - await config.createUser("test@test.com", "p4ssw0rd") - const headers = await config.login("test@test.com", "p4ssw0rd") + const user = await config.createUser("test@test.com", "p4ssw0rd") + const headers = await config.login("test@test.com", "p4ssw0rd", { userId: user._id }) const res = await request .get(`/api/self`) .set(headers) .expect("Content-Type", /json/) .expect(200) - expect(res.body.email).toEqual("test@test.com") + expect(res.body._id).toEqual(generateUserMetadataID("us_uuid1")) }) }) }) \ No newline at end of file diff --git a/packages/server/src/api/routes/tests/routing.spec.js b/packages/server/src/api/routes/tests/routing.spec.js index 79b28e82dd..5fe0e26f15 100644 --- a/packages/server/src/api/routes/tests/routing.spec.js +++ b/packages/server/src/api/routes/tests/routing.spec.js @@ -4,11 +4,6 @@ const { checkBuilderEndpoint } = require("./utilities/TestFunctions") const { BUILTIN_ROLE_IDS } = require("../../../utilities/security/roles") const workerRequests = require("../../../utilities/workerRequests") -jest.mock("../../../utilities/workerRequests", () => ({ - getGlobalUsers: jest.fn(), - saveGlobalUser: jest.fn(), -})) - const route = "/test" describe("/routing", () => { diff --git a/packages/server/src/api/routes/tests/user.spec.js b/packages/server/src/api/routes/tests/user.spec.js index 3c4d6a09e6..e80672284d 100644 --- a/packages/server/src/api/routes/tests/user.spec.js +++ b/packages/server/src/api/routes/tests/user.spec.js @@ -7,7 +7,10 @@ const workerRequests = require("../../../utilities/workerRequests") jest.mock("../../../utilities/workerRequests", () => ({ getGlobalUsers: jest.fn(), saveGlobalUser: jest.fn(() => { - return {} + const uuid = require("uuid/v4") + return { + _id: `us_${uuid()}` + } }), deleteGlobalUser: jest.fn(), })) @@ -26,10 +29,10 @@ describe("/users", () => { beforeEach(() => { workerRequests.getGlobalUsers.mockImplementationOnce(() => ([ { - email: "brenda@brenda.com" + _id: "us_uuid1", }, { - email: "pam@pam.com" + _id: "us_uuid2", } ] )) @@ -45,8 +48,8 @@ describe("/users", () => { .expect(200) expect(res.body.length).toBe(2) - expect(res.body.find(u => u.email === "brenda@brenda.com")).toBeDefined() - expect(res.body.find(u => u.email === "pam@pam.com")).toBeDefined() + expect(res.body.find(u => u._id === `ro_ta_users_us_uuid1`)).toBeDefined() + expect(res.body.find(u => u._id === `ro_ta_users_us_uuid2`)).toBeDefined() }) it("should apply authorization to endpoint", async () => { @@ -66,10 +69,10 @@ describe("/users", () => { beforeEach(() => { workerRequests.getGlobalUsers.mockImplementationOnce(() => ([ { - email: "bill@budibase.com" + _id: "us_uuid1", }, { - email: "brandNewUser@user.com" + _id: "us_uuid2", } ] )) @@ -86,7 +89,6 @@ describe("/users", () => { it("returns a success message when a user is successfully created", async () => { const body = basicUser(BUILTIN_ROLE_IDS.POWER) - body.email = "bill@budibase.com" const res = await create(body) expect(res.res.statusMessage).toEqual("OK") @@ -95,7 +97,6 @@ describe("/users", () => { it("should apply authorization to endpoint", async () => { const body = basicUser(BUILTIN_ROLE_IDS.POWER) - body.email = "brandNewUser@user.com" await checkPermissionsEndpoint({ config, method: "POST", @@ -110,13 +111,6 @@ describe("/users", () => { const user = basicUser(null) await create(user, 400) }) - - it("should throw error if user exists already", async () => { - await config.createUser("test@test.com") - const user = basicUser(BUILTIN_ROLE_IDS.POWER) - user.email = "test@test.com" - await create(user, 409) - }) }) describe("update", () => { @@ -141,10 +135,9 @@ describe("/users", () => { describe("destroy", () => { it("should be able to delete the user", async () => { - const email = "test@test.com" - await config.createUser(email) + const user = await config.createUser() const res = await request - .delete(`/api/users/metadata/${email}`) + .delete(`/api/users/metadata/${user._id}`) .set(config.defaultHeaders()) .expect(200) .expect("Content-Type", /json/) @@ -156,21 +149,23 @@ describe("/users", () => { describe("find", () => { beforeEach(() => { jest.resetAllMocks() + workerRequests.saveGlobalUser.mockImplementationOnce(() => ({ + _id: "us_uuid1", + })) workerRequests.getGlobalUsers.mockImplementationOnce(() => ({ - email: "test@test.com", + _id: "us_uuid1", roleId: BUILTIN_ROLE_IDS.POWER, })) }) it("should be able to find the user", async () => { - const email = "test@test.com" - await config.createUser(email) + const user = await config.createUser() const res = await request - .get(`/api/users/metadata/${email}`) + .get(`/api/users/metadata/${user._id}`) .set(config.defaultHeaders()) .expect(200) .expect("Content-Type", /json/) - expect(res.body.email).toEqual(email) + expect(res.body._id).toEqual(user._id) expect(res.body.roleId).toEqual(BUILTIN_ROLE_IDS.POWER) expect(res.body.tableId).toBeDefined() }) diff --git a/packages/server/src/api/routes/tests/utilities/TestFunctions.js b/packages/server/src/api/routes/tests/utilities/TestFunctions.js index 0bcb4512a7..9ee68c283f 100644 --- a/packages/server/src/api/routes/tests/utilities/TestFunctions.js +++ b/packages/server/src/api/routes/tests/utilities/TestFunctions.js @@ -63,11 +63,9 @@ exports.checkPermissionsEndpoint = async ({ }) => { const password = "PASSWORD" await config.createUser("passUser@budibase.com", password, passRole) - const passHeader = await config.login( - "passUser@budibase.com", - password, - passRole - ) + const passHeader = await config.login("passUser@budibase.com", password, { + roleId: passRole, + }) await exports .createRequest(config.request, method, url, body) @@ -75,11 +73,9 @@ exports.checkPermissionsEndpoint = async ({ .expect(200) await config.createUser("failUser@budibase.com", password, failRole) - const failHeader = await config.login( - "failUser@budibase.com", - password, - failRole - ) + const failHeader = await config.login("failUser@budibase.com", password, { + roleId: failRole, + }) await exports .createRequest(config.request, method, url, body) diff --git a/packages/server/src/api/routes/tests/utilities/index.js b/packages/server/src/api/routes/tests/utilities/index.js index 3bd3886a31..e9361aa67d 100644 --- a/packages/server/src/api/routes/tests/utilities/index.js +++ b/packages/server/src/api/routes/tests/utilities/index.js @@ -2,6 +2,15 @@ const TestConfig = require("../../../../tests/utilities/TestConfiguration") const structures = require("../../../../tests/utilities/structures") const env = require("../../../../environment") +jest.mock("../../../../utilities/workerRequests", () => ({ + getGlobalUsers: jest.fn(), + saveGlobalUser: jest.fn(() => { + return { + _id: "us_uuid1", + } + }), +})) + exports.delay = ms => new Promise(resolve => setTimeout(resolve, ms)) let request, config diff --git a/packages/server/src/api/routes/user.js b/packages/server/src/api/routes/user.js index b0450b72cc..bafb648fc6 100644 --- a/packages/server/src/api/routes/user.js +++ b/packages/server/src/api/routes/user.js @@ -16,7 +16,7 @@ router controller.fetchMetadata ) .get( - "/api/users/metadata/:email", + "/api/users/metadata/:id", authorized(PermissionTypes.USER, PermissionLevels.READ), controller.findMetadata ) @@ -32,7 +32,7 @@ router controller.createMetadata ) .delete( - "/api/users/metadata/:email", + "/api/users/metadata/:id", authorized(PermissionTypes.USER, PermissionLevels.WRITE), usage, controller.destroyMetadata diff --git a/packages/server/src/automations/tests/createUser.spec.js b/packages/server/src/automations/tests/createUser.spec.js index f085d52712..7291b75505 100644 --- a/packages/server/src/automations/tests/createUser.spec.js +++ b/packages/server/src/automations/tests/createUser.spec.js @@ -25,6 +25,7 @@ describe("test the create user action", () => { expect(res.id).toBeDefined() expect(res.revision).toBeDefined() const userDoc = await config.getRow(InternalTables.USER_METADATA, res.id) + expect(userDoc).toBeDefined() }) it("should return an error if no inputs provided", async () => { diff --git a/packages/server/src/constants/index.js b/packages/server/src/constants/index.js index 940c1100dd..5b4291998e 100644 --- a/packages/server/src/constants/index.js +++ b/packages/server/src/constants/index.js @@ -33,6 +33,7 @@ exports.USERS_TABLE_SCHEMA = { type: "table", views: {}, name: "Users", + // TODO: ADMIN PANEL - when implemented this doesn't need to be carried out schema: { email: { type: exports.FieldTypes.STRING, diff --git a/packages/server/src/db/utils.js b/packages/server/src/db/utils.js index 5cd9e1b31f..e6bcfada5b 100644 --- a/packages/server/src/db/utils.js +++ b/packages/server/src/db/utils.js @@ -127,23 +127,23 @@ exports.generateRowID = (tableId, id = null) => { /** * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ -exports.getUserMetadataParams = (email = "", otherProps = {}) => { - return exports.getRowParams(InternalTables.USER_METADATA, email, otherProps) +exports.getUserMetadataParams = (userId = null, otherProps = {}) => { + return exports.getRowParams(InternalTables.USER_METADATA, userId, otherProps) } /** - * Generates a new user ID based on the passed in email. - * @param {string} email The email which the ID is going to be built up of. + * Generates a new user ID based on the passed in global ID. + * @param {string} globalId The ID of the global user. * @returns {string} The new user ID which the user doc can be stored under. */ -exports.generateUserMetadataID = email => { - return exports.generateRowID(InternalTables.USER_METADATA, email) +exports.generateUserMetadataID = globalId => { + return exports.generateRowID(InternalTables.USER_METADATA, globalId) } /** - * Breaks up the ID to get the email address back out of it. + * Breaks up the ID to get the global ID. */ -exports.getEmailFromUserMetadataID = id => { +exports.getGlobalIDFromUserMetadataID = id => { return id.split( `${DocumentTypes.ROW}${SEPARATOR}${InternalTables.USER_METADATA}${SEPARATOR}` )[1] diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index 6616888c57..89c46c6bff 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -1,6 +1,5 @@ const { getAppId, setCookie, getCookie, Cookies } = require("@budibase/auth") const { getRole } = require("../utilities/security/roles") -const { generateUserMetadataID } = require("../db/utils") const { getGlobalUsers } = require("../utilities/workerRequests") const { BUILTIN_ROLE_IDS } = require("../utilities/security/roles") @@ -40,14 +39,10 @@ module.exports = async (ctx, next) => { if (appId) { ctx.appId = appId if (roleId) { - const userId = ctx.user - ? generateUserMetadataID(ctx.user.email) - : undefined ctx.roleId = roleId ctx.user = { ...ctx.user, - _id: userId, - userId, + _id: ctx.user ? ctx.user.userId : null, role: await getRole(appId, roleId), } } diff --git a/packages/server/src/middleware/tests/currentapp.spec.js b/packages/server/src/middleware/tests/currentapp.spec.js index 44c1c6b7ad..3ed17bb521 100644 --- a/packages/server/src/middleware/tests/currentapp.spec.js +++ b/packages/server/src/middleware/tests/currentapp.spec.js @@ -5,7 +5,7 @@ function mockWorker() { jest.mock("../../utilities/workerRequests", () => ({ getGlobalUsers: () => { return { - email: "test@test.com", + email: "us_uuid1", roles: { "app_test": "BASIC", } @@ -59,7 +59,7 @@ class TestConfiguration { setUser() { this.ctx.user = { - email: "test@test.com", + userId: "ro_ta_user_us_uuid1", } } diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 42ff603139..e4510905b5 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -70,8 +70,7 @@ class TestConfiguration { defaultHeaders() { const user = { - userId: "us_test@test.com", - email: "test@test.com", + userId: "ro_ta_user_us_uuid1", builder: { global: true, }, @@ -106,12 +105,13 @@ class TestConfiguration { } async roleHeaders(email = EMAIL, roleId = BUILTIN_ROLE_IDS.ADMIN) { + let user try { - await this.createUser(email, PASSWORD, roleId) + user = await this.createUser(email, PASSWORD, roleId) } catch (err) { // allow errors here } - return this.login(email, PASSWORD, roleId) + return this.login(email, PASSWORD, { roleId, userId: user._id }) } async createApp(appName) { @@ -293,33 +293,19 @@ class TestConfiguration { ) } - async makeUserInactive(email) { - const user = await this._req( - null, - { - email, - }, - controllers.user.findMetadata - ) - return this._req( - { - ...user, - status: "inactive", - }, - null, - controllers.user.updateMetadata - ) - } - - async login(email, password, roleId = BUILTIN_ROLE_IDS.BUILDER) { + async login(email, password, { roleId, userId } = {}) { + if (!roleId) { + roleId = BUILTIN_ROLE_IDS.BUILDER + } if (!this.request) { throw "Server has not been opened, cannot login." } if (!email || !password) { await this.createUser() } + // have to fake this const user = { - userId: `us_${email || EMAIL}`, + userId: userId || `ro_ta_users_us_uuid1`, email: email || EMAIL, } const app = { diff --git a/packages/server/src/utilities/users.js b/packages/server/src/utilities/users.js index 319a0cfa41..4c637c1865 100644 --- a/packages/server/src/utilities/users.js +++ b/packages/server/src/utilities/users.js @@ -1,20 +1,18 @@ const CouchDB = require("../db") -const { - generateUserMetadataID, - getEmailFromUserMetadataID, -} = require("../db/utils") +const { getGlobalIDFromUserMetadataID } = require("../db/utils") const { getGlobalUsers } = require("../utilities/workerRequests") -exports.getFullUser = async ({ ctx, email, userId }) => { - if (!email) { - email = getEmailFromUserMetadataID(userId) - } - const global = await getGlobalUsers(ctx, ctx.appId, email) +exports.getFullUser = async (ctx, userId) => { + const global = await getGlobalUsers( + ctx, + ctx.appId, + getGlobalIDFromUserMetadataID(userId) + ) let metadata try { // this will throw an error if the db doesn't exist, or there is no appId const db = new CouchDB(ctx.appId) - metadata = await db.get(generateUserMetadataID(email)) + metadata = await db.get(userId) } catch (err) { // it is fine if there is no user metadata, just remove global db info delete global._id @@ -24,6 +22,6 @@ exports.getFullUser = async ({ ctx, email, userId }) => { ...global, ...metadata, // make sure the ID is always a local ID, not a global one - _id: generateUserMetadataID(email), + _id: userId, } } diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js index 93a7c26753..12dc5e9ff9 100644 --- a/packages/server/src/utilities/workerRequests.js +++ b/packages/server/src/utilities/workerRequests.js @@ -60,8 +60,8 @@ exports.getDeployedApps = async ctx => { } } -exports.deleteGlobalUser = async (ctx, email) => { - const endpoint = `/api/admin/users/${email}` +exports.deleteGlobalUser = async (ctx, globalId) => { + const endpoint = `/api/admin/users/${globalId}` const reqCfg = { method: "DELETE" } const response = await fetch( checkSlashesInUrl(env.WORKER_URL + endpoint), @@ -70,8 +70,10 @@ exports.deleteGlobalUser = async (ctx, email) => { return response.json() } -exports.getGlobalUsers = async (ctx, appId = null, email = null) => { - const endpoint = email ? `/api/admin/users/${email}` : `/api/admin/users` +exports.getGlobalUsers = async (ctx, appId = null, globalId = null) => { + const endpoint = globalId + ? `/api/admin/users/${globalId}` + : `/api/admin/users` const reqCfg = { method: "GET" } const response = await fetch( checkSlashesInUrl(env.WORKER_URL + endpoint), @@ -89,8 +91,8 @@ exports.getGlobalUsers = async (ctx, appId = null, email = null) => { return users } -exports.saveGlobalUser = async (ctx, appId, email, body) => { - const globalUser = await exports.getGlobalUsers(ctx, appId, email) +exports.saveGlobalUser = async (ctx, appId, body, globalId = null) => { + const globalUser = await exports.getGlobalUsers(ctx, appId, globalId) const roles = globalUser.roles || {} if (body.roleId) { roles[appId] = body.roleId @@ -100,9 +102,9 @@ exports.saveGlobalUser = async (ctx, appId, email, body) => { method: "POST", body: { ...globalUser, - email, password: body.password || undefined, status: body.status, + email: body.email, roles, builder: { global: true, @@ -124,5 +126,8 @@ exports.saveGlobalUser = async (ctx, appId, email, body) => { delete body.status delete body.roles delete body.builder - return body + return { + ...body, + _id: json._id, + } } From c3a1841f01fef5a697c844b49cdfd5cd71b4963a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 19 Apr 2021 17:31:47 +0100 Subject: [PATCH 2/6] Updating auth package to remove use of email address (bar logging in). --- packages/auth/package.json | 3 ++- packages/auth/src/db/utils.js | 27 ++++++++++--------- packages/auth/src/db/views.js | 20 ++++++++++++++ packages/auth/src/index.js | 4 +-- packages/auth/src/middleware/authenticated.js | 3 --- .../auth/src/middleware/passport/local.js | 12 ++++----- packages/auth/src/utils.js | 22 ++++++++++++++- packages/auth/yarn.lock | 5 ++++ 8 files changed, 69 insertions(+), 27 deletions(-) create mode 100644 packages/auth/src/db/views.js diff --git a/packages/auth/package.json b/packages/auth/package.json index 294b09321d..b4f4b1cb33 100644 --- a/packages/auth/package.json +++ b/packages/auth/package.json @@ -12,6 +12,7 @@ "passport-google-auth": "^1.0.2", "passport-google-oauth": "^2.0.0", "passport-jwt": "^4.0.0", - "passport-local": "^1.0.0" + "passport-local": "^1.0.0", + "uuid": "^8.3.2" } } diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index dc1512c995..a4c0cce0dd 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -1,3 +1,9 @@ +const uuid = require("uuid/v4") + +exports.ViewNames = { + USER_BY_EMAIL: "by_email", +} + exports.StaticDatabases = { GLOBAL: { name: "global-db", @@ -17,28 +23,23 @@ const SEPARATOR = "_" exports.SEPARATOR = SEPARATOR /** - * Generates a new user ID based on the passed in email. - * @param {string} email The email which the ID is going to be built up of. + * Generates a new global user ID. * @returns {string} The new user ID which the user doc can be stored under. */ -exports.generateUserID = email => { - return `${DocumentTypes.USER}${SEPARATOR}${email}` -} - -exports.getEmailFromUserID = userId => { - return userId.split(`${DocumentTypes.USER}${SEPARATOR}`)[1] +exports.generateUserID = () => { + return `${DocumentTypes.USER}${SEPARATOR}${uuid()}` } /** * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ -exports.getUserParams = (email = "", otherProps = {}) => { - if (!email) { - email = "" +exports.getUserParams = (globalId = "", otherProps = {}) => { + if (!globalId) { + globalId = "" } return { ...otherProps, - startkey: `${DocumentTypes.USER}${SEPARATOR}${email}`, - endkey: `${DocumentTypes.USER}${SEPARATOR}${email}${UNICODE_MAX}`, + startkey: `${DocumentTypes.USER}${SEPARATOR}${globalId}`, + endkey: `${DocumentTypes.USER}${SEPARATOR}${globalId}${UNICODE_MAX}`, } } diff --git a/packages/auth/src/db/views.js b/packages/auth/src/db/views.js new file mode 100644 index 0000000000..86c919b012 --- /dev/null +++ b/packages/auth/src/db/views.js @@ -0,0 +1,20 @@ +const { DocumentTypes, ViewNames, StaticDatabases } = require("./utils") +const { CouchDB } = require("./index") + +exports.createUserEmailView = async () => { + const db = new CouchDB(StaticDatabases.GLOBAL.name) + const designDoc = await db.get("_design/database") + const view = { + // if using variables in a map function need to inject them before use + map: `function(doc) { + if (doc._id.startsWith("${DocumentTypes.USER}")) { + emit(doc.email, doc._id) + } + }`, + } + designDoc.views = { + ...designDoc.views, + [ViewNames.USER_BY_EMAIL]: view, + } + await db.put(designDoc) +} diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index 6ac56f8f36..2dbd027061 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -17,8 +17,8 @@ const { const { generateUserID, getUserParams, - getEmailFromUserID, } = require("./db/utils") +const { getGlobalUserByEmail } = require("./utils") // Strategies passport.use(new LocalStrategy(local.options, local.authenticate)) @@ -49,7 +49,6 @@ module.exports = { StaticDatabases, generateUserID, getUserParams, - getEmailFromUserID, hash, compare, getAppId, @@ -58,4 +57,5 @@ module.exports = { clearCookie, authenticated, isClient, + getGlobalUserByEmail, } diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index af2c7d5575..8f69a49e17 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -1,6 +1,5 @@ const { Cookies } = require("../constants") const { getCookie } = require("../utils") -const { getEmailFromUserID } = require("../db/utils") module.exports = async (ctx, next) => { try { @@ -10,8 +9,6 @@ module.exports = async (ctx, next) => { if (authCookie) { ctx.isAuthenticated = true ctx.user = authCookie - // make sure email is correct from ID - ctx.user.email = getEmailFromUserID(authCookie.userId) } await next() diff --git a/packages/auth/src/middleware/passport/local.js b/packages/auth/src/middleware/passport/local.js index 6249050d05..333f381297 100644 --- a/packages/auth/src/middleware/passport/local.js +++ b/packages/auth/src/middleware/passport/local.js @@ -4,6 +4,7 @@ const database = require("../../db") const { StaticDatabases, generateUserID } = require("../../db/utils") const { compare } = require("../../hashing") const env = require("../../environment") +const { getGlobalUserByEmail } = require("../../utils") const INVALID_ERR = "Invalid Credentials" @@ -11,21 +12,18 @@ exports.options = {} /** * Passport Local Authentication Middleware. - * @param {*} username - username to login with + * @param {*} email - username to login with * @param {*} password - plain text password to log in with * @param {*} done - callback from passport to return user information and errors * @returns The authenticated user, or errors if they occur */ -exports.authenticate = async function(username, password, done) { - if (!username) return done(null, false, "Email Required.") +exports.authenticate = async function(email, password, done) { + if (!email) return done(null, false, "Email Required.") if (!password) return done(null, false, "Password Required.") - // Check the user exists in the instance DB by email - const db = new database.CouchDB(StaticDatabases.GLOBAL.name) - let dbUser try { - dbUser = await db.get(generateUserID(username)) + dbUser = await getGlobalUserByEmail(email) } catch (err) { console.error("User not found", err) return done(null, false, { message: "User not found" }) diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index 5f0a135a45..2320896150 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -1,6 +1,8 @@ -const { DocumentTypes, SEPARATOR } = require("./db/utils") +const { DocumentTypes, SEPARATOR, ViewNames, StaticDatabases } = require("./db/utils") const jwt = require("jsonwebtoken") const { options } = require("./middleware/passport/jwt") +const { createUserEmailView } = require("./db/views") +const { CouchDB } = require("./db") const APP_PREFIX = DocumentTypes.APP + SEPARATOR @@ -97,3 +99,21 @@ exports.clearCookie = (ctx, name) => { exports.isClient = ctx => { return ctx.headers["x-budibase-type"] === "client" } + +exports.getGlobalUserByEmail = async email => { + const db = new CouchDB(StaticDatabases.GLOBAL.name) + try { + let users = (await db.query( + `database/${ViewNames.USER_BY_EMAIL}`, + { + key: email + }) + ).rows + return users.length <= 1 ? users[0] : users + } catch (err) { + if (err != null && err.name === "not_found") { + await createUserEmailView() + return exports.getGlobalUserByEmail(email) + } + } +} diff --git a/packages/auth/yarn.lock b/packages/auth/yarn.lock index 6771306258..c3066ebdc1 100644 --- a/packages/auth/yarn.lock +++ b/packages/auth/yarn.lock @@ -584,6 +584,11 @@ uuid@^3.3.2: resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== +uuid@^8.3.2: + version "8.3.2" + resolved "https://registry.yarnpkg.com/uuid/-/uuid-8.3.2.tgz#80d5b5ced271bb9af6c445f21a1a04c606cefbe2" + integrity sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg== + verror@1.10.0: version "1.10.0" resolved "https://registry.yarnpkg.com/verror/-/verror-1.10.0.tgz#3a105ca17053af55d6e270c1f8288682e18da400" From b4c8bf81f7a380639dc3259e1441c0e40242dbdc Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 20 Apr 2021 17:17:44 +0100 Subject: [PATCH 3/6] Swapping over everything to use the new user ID and updating everything after some end to end testing. --- packages/auth/src/db/index.js | 8 ++- packages/auth/src/db/utils.js | 18 +++--- packages/auth/src/db/views.js | 21 ++++++- packages/auth/src/index.js | 16 ++--- .../auth/src/middleware/passport/local.js | 9 +-- packages/auth/src/utils.js | 10 +++- .../src/components/login/LoginForm.svelte | 9 +-- .../components/start/CreateAppModal.svelte | 4 +- packages/builder/src/stores/backend/auth.js | 13 ++++ packages/server/src/api/controllers/user.js | 24 +++++--- packages/server/src/api/routes/user.js | 6 ++ packages/server/src/middleware/currentapp.js | 33 +++++++---- .../server/src/utilities/workerRequests.js | 6 +- .../src/api/controllers/admin/groups.js | 6 +- .../worker/src/api/controllers/admin/users.js | 59 ++++++++++++++----- packages/worker/src/api/routes/admin/users.js | 5 +- packages/worker/src/db/utils.js | 35 ----------- 17 files changed, 163 insertions(+), 119 deletions(-) delete mode 100644 packages/worker/src/db/utils.js diff --git a/packages/auth/src/db/index.js b/packages/auth/src/db/index.js index 9ae48e68b1..f94fe4afea 100644 --- a/packages/auth/src/db/index.js +++ b/packages/auth/src/db/index.js @@ -1,5 +1,9 @@ +let Pouch + module.exports.setDB = pouch => { - module.exports.CouchDB = pouch + Pouch = pouch } -module.exports.CouchDB = null +module.exports.getDB = dbName => { + return new Pouch(dbName) +} diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index 49c1c90159..2e2c3f1aed 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -23,14 +23,6 @@ const SEPARATOR = "_" exports.SEPARATOR = SEPARATOR -/** - * Generates a new global user ID. - * @returns {string} The new user ID which the user doc can be stored under. - */ -exports.generateUserID = () => { - return `${DocumentTypes.USER}${SEPARATOR}${newid()}` -} - /** * Generates a new group ID. * @returns {string} The new group ID which the group doc can be stored under. @@ -50,10 +42,18 @@ exports.getGroupParams = (id = "", otherProps = {}) => { } } +/** + * Generates a new global user ID. + * @returns {string} The new user ID which the user doc can be stored under. + */ +exports.generateGlobalUserID = () => { + return `${DocumentTypes.USER}${SEPARATOR}${newid()}` +} + /** * Gets parameters for retrieving users. */ -exports.getUserParams = (globalId = "", otherProps = {}) => { +exports.getGlobalUserParams = (globalId = "", otherProps = {}) => { if (!globalId) { globalId = "" } diff --git a/packages/auth/src/db/views.js b/packages/auth/src/db/views.js index 86c919b012..1f1f28b917 100644 --- a/packages/auth/src/db/views.js +++ b/packages/auth/src/db/views.js @@ -1,9 +1,24 @@ const { DocumentTypes, ViewNames, StaticDatabases } = require("./utils") -const { CouchDB } = require("./index") +const { getDB } = require("./index") + +function DesignDoc() { + return { + _id: "_design/database", + // view collation information, read before writing any complex views: + // https://docs.couchdb.org/en/master/ddocs/views/collation.html#collation-specification + views: {}, + } +} exports.createUserEmailView = async () => { - const db = new CouchDB(StaticDatabases.GLOBAL.name) - const designDoc = await db.get("_design/database") + const db = getDB(StaticDatabases.GLOBAL.name) + let designDoc + try { + designDoc = await db.get("_design/database") + } catch (err) { + // no design doc, make one + designDoc = DesignDoc() + } const view = { // if using variables in a map function need to inject them before use map: `function(doc) { diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index 9ee3a11bed..fee83b65d8 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -2,7 +2,7 @@ const passport = require("koa-passport") const LocalStrategy = require("passport-local").Strategy const JwtStrategy = require("passport-jwt").Strategy // const GoogleStrategy = require("passport-google-oauth").Strategy -const database = require("./db") +const { setDB, getDB } = require("./db") const { StaticDatabases } = require("./db/utils") const { jwt, local, authenticated } = require("./middleware") const { Cookies, UserStatus } = require("./constants") @@ -13,14 +13,14 @@ const { getCookie, clearCookie, isClient, + getGlobalUserByEmail, } = require("./utils") const { - generateUserID, - getUserParams, + generateGlobalUserID, + getGlobalUserParams, generateGroupID, getGroupParams, } = require("./db/utils") -const { getGlobalUserByEmail } = require("./utils") // Strategies passport.use(new LocalStrategy(local.options, local.authenticate)) @@ -30,7 +30,7 @@ passport.use(new JwtStrategy(jwt.options, jwt.authenticate)) passport.serializeUser((user, done) => done(null, user)) passport.deserializeUser(async (user, done) => { - const db = new database.CouchDB(StaticDatabases.GLOBAL.name) + const db = getDB(StaticDatabases.GLOBAL.name) try { const user = await db.get(user._id) @@ -43,14 +43,14 @@ passport.deserializeUser(async (user, done) => { module.exports = { init(pouch) { - database.setDB(pouch) + setDB(pouch) }, passport, Cookies, UserStatus, StaticDatabases, - generateUserID, - getUserParams, + generateGlobalUserID, + getGlobalUserParams, generateGroupID, getGroupParams, hash, diff --git a/packages/auth/src/middleware/passport/local.js b/packages/auth/src/middleware/passport/local.js index 333f381297..1942d0c424 100644 --- a/packages/auth/src/middleware/passport/local.js +++ b/packages/auth/src/middleware/passport/local.js @@ -1,7 +1,5 @@ const jwt = require("jsonwebtoken") const { UserStatus } = require("../../constants") -const database = require("../../db") -const { StaticDatabases, generateUserID } = require("../../db/utils") const { compare } = require("../../hashing") const env = require("../../environment") const { getGlobalUserByEmail } = require("../../utils") @@ -21,11 +19,8 @@ exports.authenticate = async function(email, password, done) { if (!email) return done(null, false, "Email Required.") if (!password) return done(null, false, "Password Required.") - let dbUser - try { - dbUser = await getGlobalUserByEmail(email) - } catch (err) { - console.error("User not found", err) + const dbUser = await getGlobalUserByEmail(email) + if (dbUser == null) { return done(null, false, { message: "User not found" }) } diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index 2320896150..14925653c9 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -2,7 +2,7 @@ const { DocumentTypes, SEPARATOR, ViewNames, StaticDatabases } = require("./db/u const jwt = require("jsonwebtoken") const { options } = require("./middleware/passport/jwt") const { createUserEmailView } = require("./db/views") -const { CouchDB } = require("./db") +const { getDB } = require("./db") const APP_PREFIX = DocumentTypes.APP + SEPARATOR @@ -101,19 +101,23 @@ exports.isClient = ctx => { } exports.getGlobalUserByEmail = async email => { - const db = new CouchDB(StaticDatabases.GLOBAL.name) + const db = getDB(StaticDatabases.GLOBAL.name) try { let users = (await db.query( `database/${ViewNames.USER_BY_EMAIL}`, { - key: email + key: email, + include_docs: true, }) ).rows + users = users.map(user => user.doc) return users.length <= 1 ? users[0] : users } catch (err) { if (err != null && err.name === "not_found") { await createUserEmailView() return exports.getGlobalUserByEmail(email) + } else { + throw err } } } diff --git a/packages/builder/src/components/login/LoginForm.svelte b/packages/builder/src/components/login/LoginForm.svelte index 7e32efb7c5..2222ebbdf7 100644 --- a/packages/builder/src/components/login/LoginForm.svelte +++ b/packages/builder/src/components/login/LoginForm.svelte @@ -21,14 +21,7 @@ async function createTestUser() { try { - await auth.createUser({ - email: "test@test.com", - password: "test", - roles: {}, - builder: { - global: true, - }, - }) + await auth.firstUser() notifier.success("Test user created") } catch (err) { console.error(err) diff --git a/packages/builder/src/components/start/CreateAppModal.svelte b/packages/builder/src/components/start/CreateAppModal.svelte index fcd5379733..3739b86b76 100644 --- a/packages/builder/src/components/start/CreateAppModal.svelte +++ b/packages/builder/src/components/start/CreateAppModal.svelte @@ -151,8 +151,8 @@ const user = { roleId: $createAppStore.values.roleId, } - const userResp = await api.post(`/api/users/metadata`, user) - const json = await userResp.json() + const userResp = await api.post(`/api/users/metadata/self`, user) + await userResp.json() $goto(`./${appJson._id}`) } catch (error) { console.error(error) diff --git a/packages/builder/src/stores/backend/auth.js b/packages/builder/src/stores/backend/auth.js index 0d59451417..29dd6dcdd0 100644 --- a/packages/builder/src/stores/backend/auth.js +++ b/packages/builder/src/stores/backend/auth.js @@ -30,11 +30,24 @@ export function createAuthStore() { }, logout: async () => { const response = await api.post(`/api/admin/auth/logout`) + if (response.status !== 200) { + throw "Unable to create logout" + } await response.json() set({ user: null }) }, createUser: async user => { const response = await api.post(`/api/admin/users`, user) + if (response.status !== 200) { + throw "Unable to create user" + } + await response.json() + }, + firstUser: async () => { + const response = await api.post(`/api/admin/users/first`) + if (response.status !== 200) { + throw "Unable to create test user" + } await response.json() }, } diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index eee1f3f048..4b6c65736a 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -43,6 +43,10 @@ exports.createMetadata = async function(ctx) { const db = new CouchDB(appId) const { roleId } = ctx.request.body + if (ctx.request.body._id) { + return exports.updateMetadata(ctx) + } + // check role valid const role = await getRole(appId, roleId) if (!role) ctx.throw(400, "Invalid Role") @@ -66,20 +70,26 @@ exports.createMetadata = async function(ctx) { } } +exports.updateSelfMetadata = async function(ctx) { + // overwrite the ID with current users + ctx.request.body._id = ctx.user.userId + // make sure no stale rev + delete ctx.request.body._rev + await exports.updateMetadata(ctx) +} + exports.updateMetadata = async function(ctx) { const appId = ctx.appId const db = new CouchDB(appId) const user = ctx.request.body - const globalUser = await saveGlobalUser( - ctx, - appId, - getGlobalIDFromUserMetadataID(user._id), - ctx.request.body - ) + const globalUser = await saveGlobalUser(ctx, appId, { + ...user, + _id: getGlobalIDFromUserMetadataID(user._id), + }) const metadata = { ...globalUser, _id: user._id || generateUserMetadataID(globalUser._id), - _rev: ctx.request.body._rev, + _rev: user._rev, } ctx.body = await db.put(metadata) } diff --git a/packages/server/src/api/routes/user.js b/packages/server/src/api/routes/user.js index bafb648fc6..a9e4aac5a2 100644 --- a/packages/server/src/api/routes/user.js +++ b/packages/server/src/api/routes/user.js @@ -31,6 +31,12 @@ router usage, controller.createMetadata ) + .post( + "/api/users/metadata/self", + authorized(PermissionTypes.USER, PermissionLevels.WRITE), + usage, + controller.updateSelfMetadata + ) .delete( "/api/users/metadata/:id", authorized(PermissionTypes.USER, PermissionLevels.WRITE), diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index 89c46c6bff..0d75e808ad 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -2,6 +2,10 @@ const { getAppId, setCookie, getCookie, Cookies } = require("@budibase/auth") const { getRole } = require("../utilities/security/roles") const { getGlobalUsers } = require("../utilities/workerRequests") const { BUILTIN_ROLE_IDS } = require("../utilities/security/roles") +const { + getGlobalIDFromUserMetadataID, + generateUserMetadataID, +} = require("../db/utils") module.exports = async (ctx, next) => { // try to get the appID from the request @@ -26,7 +30,8 @@ module.exports = async (ctx, next) => { appCookie.roleId === BUILTIN_ROLE_IDS.PUBLIC) ) { // Different App ID means cookie needs reset, or if the same public user has logged in - const globalUser = await getGlobalUsers(ctx, requestAppId, ctx.user.email) + const globalId = getGlobalIDFromUserMetadataID(ctx.user.userId) + const globalUser = await getGlobalUsers(ctx, requestAppId, globalId) updateCookie = true appId = requestAppId if (globalUser.roles && globalUser.roles[requestAppId]) { @@ -36,18 +41,24 @@ module.exports = async (ctx, next) => { appId = appCookie.appId roleId = appCookie.roleId || BUILTIN_ROLE_IDS.PUBLIC } - if (appId) { - ctx.appId = appId - if (roleId) { - ctx.roleId = roleId - ctx.user = { - ...ctx.user, - _id: ctx.user ? ctx.user.userId : null, - role: await getRole(appId, roleId), - } + // nothing more to do + if (!appId) { + return next() + } + + ctx.appId = appId + if (roleId) { + ctx.roleId = roleId + const userId = ctx.user ? generateUserMetadataID(ctx.user.userId) : null + ctx.user = { + ...ctx.user, + // override userID with metadata one + _id: userId, + userId, + role: await getRole(appId, roleId), } } - if (updateCookie && appId) { + if (updateCookie) { setCookie(ctx, { appId, roleId }, Cookies.CurrentApp) } return next() diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js index 12dc5e9ff9..2de74aa155 100644 --- a/packages/server/src/utilities/workerRequests.js +++ b/packages/server/src/utilities/workerRequests.js @@ -91,8 +91,10 @@ exports.getGlobalUsers = async (ctx, appId = null, globalId = null) => { return users } -exports.saveGlobalUser = async (ctx, appId, body, globalId = null) => { - const globalUser = await exports.getGlobalUsers(ctx, appId, globalId) +exports.saveGlobalUser = async (ctx, appId, body) => { + const globalUser = body._id + ? await exports.getGlobalUsers(ctx, appId, body._id) + : {} const roles = globalUser.roles || {} if (body.roleId) { roles[appId] = body.roleId diff --git a/packages/worker/src/api/controllers/admin/groups.js b/packages/worker/src/api/controllers/admin/groups.js index 3642c2464d..86623c337a 100644 --- a/packages/worker/src/api/controllers/admin/groups.js +++ b/packages/worker/src/api/controllers/admin/groups.js @@ -31,15 +31,13 @@ exports.fetch = async function(ctx) { include_docs: true, }) ) - const groups = response.rows.map(row => row.doc) - ctx.body = groups + ctx.body = response.rows.map(row => row.doc) } exports.find = async function(ctx) { const db = new CouchDB(GLOBAL_DB) try { - const record = await db.get(ctx.params.id) - ctx.body = record + ctx.body = await db.get(ctx.params.id) } catch (err) { ctx.throw(err.status, err) } diff --git a/packages/worker/src/api/controllers/admin/users.js b/packages/worker/src/api/controllers/admin/users.js index 600a8e75f6..96243d49fd 100644 --- a/packages/worker/src/api/controllers/admin/users.js +++ b/packages/worker/src/api/controllers/admin/users.js @@ -1,27 +1,42 @@ const CouchDB = require("../../../db") const { hash, - generateUserID, - getUserParams, + generateGlobalUserID, + getGlobalUserParams, StaticDatabases, + getGlobalUserByEmail, } = require("@budibase/auth") const { UserStatus } = require("../../../constants") +const FIRST_USER_EMAIL = "test@test.com" +const FIRST_USER_PASSWORD = "test" const GLOBAL_DB = StaticDatabases.GLOBAL.name exports.userSave = async ctx => { const db = new CouchDB(GLOBAL_DB) const { email, password, _id } = ctx.request.body - const hashedPassword = password ? await hash(password) : null - let user = { - ...ctx.request.body, - _id: generateUserID(email), - password: hashedPassword, + + // make sure another user isn't using the same email + const dbUser = await getGlobalUserByEmail(email) + if (dbUser != null && (dbUser._id !== _id || Array.isArray(dbUser))) { + ctx.throw(400, "Email address already in use.") } - let dbUser - // in-case user existed already - if (_id) { - dbUser = await db.get(_id) + + // get the password, make sure one is defined + let hashedPassword + if (password) { + hashedPassword = await hash(password) + } else if (dbUser) { + hashedPassword = dbUser.password + } else { + ctx.throw(400, "Password must be specified.") + } + + let user = { + ...dbUser, + ...ctx.request.body, + _id: _id || generateGlobalUserID(), + password: hashedPassword, } // add the active status to a user if its not provided if (user.status == null) { @@ -29,7 +44,7 @@ exports.userSave = async ctx => { } try { const response = await db.post({ - password: hashedPassword || dbUser.password, + password: hashedPassword, ...user, }) ctx.body = { @@ -46,12 +61,24 @@ exports.userSave = async ctx => { } } +exports.firstUser = async ctx => { + ctx.request.body = { + email: FIRST_USER_EMAIL, + password: FIRST_USER_PASSWORD, + roles: {}, + builder: { + global: true, + }, + } + await exports.userSave(ctx) +} + exports.userDelete = async ctx => { const db = new CouchDB(GLOBAL_DB) - const dbUser = await db.get(generateUserID(ctx.params.email)) + const dbUser = await db.get(ctx.params.id) await db.remove(dbUser._id, dbUser._rev) ctx.body = { - message: `User ${ctx.params.email} deleted.`, + message: `User ${ctx.params.id} deleted.`, } } @@ -59,7 +86,7 @@ exports.userDelete = async ctx => { exports.userFetch = async ctx => { const db = new CouchDB(GLOBAL_DB) const response = await db.allDocs( - getUserParams(null, { + getGlobalUserParams(null, { include_docs: true, }) ) @@ -78,7 +105,7 @@ exports.userFind = async ctx => { const db = new CouchDB(GLOBAL_DB) let user try { - user = await db.get(generateUserID(ctx.params.email)) + user = await db.get(ctx.params.id) } catch (err) { // no user found, just return nothing user = {} diff --git a/packages/worker/src/api/routes/admin/users.js b/packages/worker/src/api/routes/admin/users.js index fe8e57593a..d7d19cbf49 100644 --- a/packages/worker/src/api/routes/admin/users.js +++ b/packages/worker/src/api/routes/admin/users.js @@ -32,8 +32,9 @@ router authenticated, controller.userSave ) - .delete("/api/admin/users/:email", authenticated, controller.userDelete) + .post("/api/admin/users/first", controller.firstUser) + .delete("/api/admin/users/:id", authenticated, controller.userDelete) .get("/api/admin/users", authenticated, controller.userFetch) - .get("/api/admin/users/:email", authenticated, controller.userFind) + .get("/api/admin/users/:id", authenticated, controller.userFind) module.exports = router diff --git a/packages/worker/src/db/utils.js b/packages/worker/src/db/utils.js deleted file mode 100644 index b250b895bb..0000000000 --- a/packages/worker/src/db/utils.js +++ /dev/null @@ -1,35 +0,0 @@ -exports.StaticDatabases = { - USER: { - name: "user-db", - }, -} - -const DocumentTypes = { - USER: "us", - APP: "app", -} - -exports.DocumentTypes = DocumentTypes - -const UNICODE_MAX = "\ufff0" -const SEPARATOR = "_" - -/** - * Generates a new user ID based on the passed in email. - * @param {string} email The email which the ID is going to be built up of. - * @returns {string} The new user ID which the user doc can be stored under. - */ -exports.generateUserID = email => { - return `${DocumentTypes.USER}${SEPARATOR}${email}` -} - -/** - * Gets parameters for retrieving users, this is a utility function for the getDocParams function. - */ -exports.getUserParams = (email = "", otherProps = {}) => { - return { - ...otherProps, - startkey: `${DocumentTypes.USER}${SEPARATOR}${email}`, - endkey: `${DocumentTypes.USER}${SEPARATOR}${email}${UNICODE_MAX}`, - } -} From ec3a9db38383cd2342006f7d43e330946fd70585 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 20 Apr 2021 17:27:23 +0100 Subject: [PATCH 4/6] Updating test cases now that login has changed a bit. --- packages/server/src/api/routes/tests/auth.spec.js | 2 +- packages/server/src/tests/utilities/TestConfiguration.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/auth.spec.js b/packages/server/src/api/routes/tests/auth.spec.js index 5978381f86..fa307bf96f 100644 --- a/packages/server/src/api/routes/tests/auth.spec.js +++ b/packages/server/src/api/routes/tests/auth.spec.js @@ -28,7 +28,7 @@ describe("/authenticate", () => { describe("fetch self", () => { it("should be able to fetch self", async () => { const user = await config.createUser("test@test.com", "p4ssw0rd") - const headers = await config.login("test@test.com", "p4ssw0rd", { userId: user._id }) + const headers = await config.login("test@test.com", "p4ssw0rd", { userId: "us_uuid1" }) const res = await request .get(`/api/self`) .set(headers) diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index e4510905b5..0f036647bf 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -305,7 +305,7 @@ class TestConfiguration { } // have to fake this const user = { - userId: userId || `ro_ta_users_us_uuid1`, + userId: userId || `us_uuid1`, email: email || EMAIL, } const app = { From de19e986c625998035e8ad5591bbcd41cfcac033 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 20 Apr 2021 17:33:44 +0100 Subject: [PATCH 5/6] Linting and formatting. --- packages/auth/src/utils.js | 12 ++++++++---- packages/server/src/api/controllers/row.js | 1 - 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index 14925653c9..b9514d059c 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -1,4 +1,9 @@ -const { DocumentTypes, SEPARATOR, ViewNames, StaticDatabases } = require("./db/utils") +const { + DocumentTypes, + SEPARATOR, + ViewNames, + StaticDatabases, +} = require("./db/utils") const jwt = require("jsonwebtoken") const { options } = require("./middleware/passport/jwt") const { createUserEmailView } = require("./db/views") @@ -103,9 +108,8 @@ exports.isClient = ctx => { exports.getGlobalUserByEmail = async email => { const db = getDB(StaticDatabases.GLOBAL.name) try { - let users = (await db.query( - `database/${ViewNames.USER_BY_EMAIL}`, - { + let users = ( + await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { key: email, include_docs: true, }) diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 3ceac0d832..48766129b2 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -7,7 +7,6 @@ const { DocumentTypes, SEPARATOR, InternalTables, - generateUserMetadataID, } = require("../../db/utils") const userController = require("./user") const { From 89fef344012d6f02f17b8d1708c2e0f12fe848d4 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 21 Apr 2021 11:33:12 +0100 Subject: [PATCH 6/6] Updating cypress tests to work with the new user ID system. --- .../cypress/integration/createBinding.spec.js | 6 +----- packages/builder/cypress/support/commands.js | 19 ------------------- packages/builder/package.json | 5 +++-- packages/server/src/api/controllers/dev.js | 4 ++-- packages/server/src/api/routes/dev.js | 7 ++++--- 5 files changed, 10 insertions(+), 31 deletions(-) diff --git a/packages/builder/cypress/integration/createBinding.spec.js b/packages/builder/cypress/integration/createBinding.spec.js index e35abc9dc3..b32584924d 100644 --- a/packages/builder/cypress/integration/createBinding.spec.js +++ b/packages/builder/cypress/integration/createBinding.spec.js @@ -6,12 +6,8 @@ context("Create Bindings", () => { }) it("should add a current user binding", () => { - cy.addComponent("Elements", "Paragraph").then(componentId => { + cy.addComponent("Elements", "Paragraph").then(() => { addSettingBinding("text", "Current User._id") - cy.getComponent(componentId).should( - "have.text", - `ro_ta_users_test@test.com` - ) }) }) diff --git a/packages/builder/cypress/support/commands.js b/packages/builder/cypress/support/commands.js index eb34921aff..edf5d394f6 100644 --- a/packages/builder/cypress/support/commands.js +++ b/packages/builder/cypress/support/commands.js @@ -1,28 +1,9 @@ // *********************************************** -// This example commands.js shows you how to -// create various custom commands and overwrite -// existing commands. -// // For more comprehensive examples of custom // commands please read more here: // https://on.cypress.io/custom-commands // *********************************************** // -// -// -- This is a parent command -- -// Cypress.Commands.add("login", (email, password) => { ... }) -// -// -// -- This is a child command -- -// Cypress.Commands.add("drag", { prevSubject: 'element'}, (subject, options) => { ... }) -// -// -// -- This is a dual command -- -// Cypress.Commands.add("dismiss", { prevSubject: 'optional'}, (subject, options) => { ... }) -// -// -// -- This will overwrite an existing command -- -// Cypress.Commands.overwrite("visit", (originalFn, url, options) => { ... }) Cypress.Commands.add("login", () => { cy.getCookie("budibase:auth").then(cookie => { diff --git a/packages/builder/package.json b/packages/builder/package.json index 6354a90ee9..c40baba71d 100644 --- a/packages/builder/package.json +++ b/packages/builder/package.json @@ -14,9 +14,10 @@ "cy:setup": "node ./cypress/setup.js", "cy:run": "cypress run", "cy:open": "cypress open", - "cy:run:ci": "cypress run --browser electron --record --key f308590b-6070-41af-b970-794a3823d451", + "cy:run:ci": "cypress run --record --key f308590b-6070-41af-b970-794a3823d451", "cy:test": "start-server-and-test cy:setup http://localhost:10000/builder cy:run", - "cy:ci": "start-server-and-test cy:setup http://localhost:10000/builder cy:run:ci" + "cy:ci": "start-server-and-test cy:setup http://localhost:10000/builder cy:run:ci", + "cy:debug": "start-server-and-test cy:setup http://localhost:10000/builder cy:open" }, "jest": { "globals": { diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js index b08f730c48..003f82faa8 100644 --- a/packages/server/src/api/controllers/dev.js +++ b/packages/server/src/api/controllers/dev.js @@ -4,9 +4,9 @@ const { checkSlashesInUrl } = require("../../utilities") const { request } = require("../../utilities/workerRequests") async function redirect(ctx, method) { - const { path } = ctx.params + const { devPath } = ctx.params const response = await fetch( - checkSlashesInUrl(`${env.WORKER_URL}/api/admin/${path}`), + checkSlashesInUrl(`${env.WORKER_URL}/api/admin/${devPath}`), request(ctx, { method, body: ctx.request.body, diff --git a/packages/server/src/api/routes/dev.js b/packages/server/src/api/routes/dev.js index 977cc189f3..341d48c7b5 100644 --- a/packages/server/src/api/routes/dev.js +++ b/packages/server/src/api/routes/dev.js @@ -5,9 +5,10 @@ const env = require("../../environment") const router = Router() if (env.isDev() || env.isTest()) { - router.get("/api/admin/:path", controller.redirectGet) - router.post("/api/admin/:path", controller.redirectPost) - router.delete("/api/admin/:path", controller.redirectDelete) + router + .get("/api/admin/:devPath(.*)", controller.redirectGet) + .post("/api/admin/:devPath(.*)", controller.redirectPost) + .delete("/api/admin/:devPath(.*)", controller.redirectDelete) } module.exports = router