From 76ceb6a9515f52ae202c399b7346ce1274ae2528 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 21 Apr 2021 16:42:44 +0100 Subject: [PATCH] Some re-work of the auth package, making it a bit easier to use/less likely to make a mistake. --- packages/auth/src/db/utils.js | 27 ++++++++- packages/auth/src/index.js | 44 ++++---------- packages/auth/src/middleware/authenticated.js | 29 +++++---- packages/server/src/api/index.js | 16 +++-- packages/server/src/api/routes/index.js | 2 +- packages/server/src/constants/index.js | 2 +- packages/server/src/middleware/currentapp.js | 3 +- .../src/middleware/tests/currentapp.spec.js | 60 ++++++++++++------- .../src/tests/utilities/TestConfiguration.js | 2 +- .../src/api/controllers/admin/groups.js | 3 +- .../worker/src/api/controllers/admin/index.js | 7 --- .../src/api/controllers/admin/templates.js | 18 ++++++ .../worker/src/api/controllers/admin/users.js | 5 +- packages/worker/src/api/controllers/auth.js | 5 +- packages/worker/src/api/index.js | 4 ++ .../worker/src/api/routes/admin/groups.js | 8 +-- .../worker/src/api/routes/admin/templates.js | 20 +++++++ packages/worker/src/api/routes/admin/users.js | 8 +-- packages/worker/src/api/routes/app.js | 3 +- packages/worker/src/api/routes/auth.js | 2 +- packages/worker/src/index.js | 2 +- 21 files changed, 164 insertions(+), 106 deletions(-) delete mode 100644 packages/worker/src/api/controllers/admin/index.js create mode 100644 packages/worker/src/api/controllers/admin/templates.js create mode 100644 packages/worker/src/api/routes/admin/templates.js diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index 2e2c3f1aed..4bd1f399cd 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -14,6 +14,7 @@ const DocumentTypes = { USER: "us", APP: "app", GROUP: "group", + TEMPLATE: "template", } exports.DocumentTypes = DocumentTypes @@ -53,7 +54,7 @@ exports.generateGlobalUserID = () => { /** * Gets parameters for retrieving users. */ -exports.getGlobalUserParams = (globalId = "", otherProps = {}) => { +exports.getGlobalUserParams = (globalId, otherProps = {}) => { if (!globalId) { globalId = "" } @@ -63,3 +64,27 @@ exports.getGlobalUserParams = (globalId = "", otherProps = {}) => { endkey: `${DocumentTypes.USER}${SEPARATOR}${globalId}${UNICODE_MAX}`, } } + +/** + * Generates a template ID. + * @param ownerId The owner/user of the template, this could be global or a group level. + */ +exports.generateTemplateID = ownerId => { + return `${DocumentTypes.TEMPLATE}${SEPARATOR}${ownerId}${newid()}` +} + +/** + * Gets parameters for retrieving templates. Owner ID must be specified, either global or a group level. + */ +exports.getTemplateParams = (ownerId, templateId, otherProps = {}) => { + if (!templateId) { + templateId = "" + } + const base = `${DocumentTypes.TEMPLATE}${SEPARATOR}${ownerId}` + const final = templateId ? `${base}${SEPARATOR}${templateId}` : base + return { + ...otherProps, + startkey: final, + endkey: `${final}${UNICODE_MAX}`, + } +} diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index fee83b65d8..937b1491c5 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -5,23 +5,6 @@ const JwtStrategy = require("passport-jwt").Strategy const { setDB, getDB } = require("./db") const { StaticDatabases } = require("./db/utils") const { jwt, local, authenticated } = require("./middleware") -const { Cookies, UserStatus } = require("./constants") -const { hash, compare } = require("./hashing") -const { - getAppId, - setCookie, - getCookie, - clearCookie, - isClient, - getGlobalUserByEmail, -} = require("./utils") -const { - generateGlobalUserID, - getGlobalUserParams, - generateGroupID, - getGroupParams, -} = require("./db/utils") - // Strategies passport.use(new LocalStrategy(local.options, local.authenticate)) passport.use(new JwtStrategy(jwt.options, jwt.authenticate)) @@ -45,21 +28,14 @@ module.exports = { init(pouch) { setDB(pouch) }, - passport, - Cookies, - UserStatus, - StaticDatabases, - generateGlobalUserID, - getGlobalUserParams, - generateGroupID, - getGroupParams, - hash, - compare, - getAppId, - setCookie, - getCookie, - clearCookie, - authenticated, - isClient, - getGlobalUserByEmail, + db: require("./db/utils"), + utils: { + ...require("./utils"), + ...require("./hashing") + }, + auth: { + buildAuthMiddleware: authenticated, + passport, + }, + constants: require("./constants"), } diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index 8f69a49e17..fc3a5b177e 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -1,18 +1,25 @@ const { Cookies } = require("../constants") const { getCookie } = require("../utils") -module.exports = async (ctx, next) => { - try { - // check the actual user is authenticated first - const authCookie = getCookie(ctx, Cookies.Auth) - - if (authCookie) { - ctx.isAuthenticated = true - ctx.user = authCookie +module.exports = (noAuthPatterns = []) => { + const regex = new RegExp(noAuthPatterns.join("|")) + return async (ctx, next) => { + // the path is not authenticated + if (regex.test(ctx.request.url)) { + return next() } + try { + // check the actual user is authenticated first + const authCookie = getCookie(ctx, Cookies.Auth) - await next() - } catch (err) { - ctx.throw(err.status || 403, err) + if (authCookie) { + ctx.isAuthenticated = true + ctx.user = authCookie + } + + return next() + } catch (err) { + ctx.throw(err.status || 403, err) + } } } diff --git a/packages/server/src/api/index.js b/packages/server/src/api/index.js index 201a9f1c33..369578d05e 100644 --- a/packages/server/src/api/index.js +++ b/packages/server/src/api/index.js @@ -1,14 +1,21 @@ const Router = require("@koa/router") -const { authenticated } = require("@budibase/auth") +const { buildAuthMiddleware } = require("@budibase/auth").auth const currentApp = require("../middleware/currentapp") const compress = require("koa-compress") const zlib = require("zlib") -const { mainRoutes, authRoutes, staticRoutes } = require("./routes") +const { mainRoutes, staticRoutes } = require("./routes") const pkg = require("../../package.json") const router = new Router() const env = require("../environment") +const NO_AUTH_ENDPOINTS = [ + "/health", + "/version", + "webhooks/trigger", + "webhooks/schema", +] + router .use( compress({ @@ -31,7 +38,7 @@ router }) .use("/health", ctx => (ctx.status = 200)) .use("/version", ctx => (ctx.body = pkg.version)) - .use(authenticated) + .use(buildAuthMiddleware(NO_AUTH_ENDPOINTS)) .use(currentApp) // error handling middleware @@ -53,9 +60,6 @@ router.use(async (ctx, next) => { router.get("/health", ctx => (ctx.status = 200)) -router.use(authRoutes.routes()) -router.use(authRoutes.allowedMethods()) - // authenticated routes for (let route of mainRoutes) { router.use(route.routes()) diff --git a/packages/server/src/api/routes/index.js b/packages/server/src/api/routes/index.js index 19de709ca9..0b09a78bb8 100644 --- a/packages/server/src/api/routes/index.js +++ b/packages/server/src/api/routes/index.js @@ -25,6 +25,7 @@ const backupRoutes = require("./backup") const devRoutes = require("./dev") exports.mainRoutes = [ + authRoutes, deployRoutes, layoutRoutes, screenRoutes, @@ -52,5 +53,4 @@ exports.mainRoutes = [ rowRoutes, ] -exports.authRoutes = authRoutes exports.staticRoutes = staticRoutes diff --git a/packages/server/src/constants/index.js b/packages/server/src/constants/index.js index 5b4291998e..9853676aa6 100644 --- a/packages/server/src/constants/index.js +++ b/packages/server/src/constants/index.js @@ -1,5 +1,5 @@ const { BUILTIN_ROLE_IDS } = require("../utilities/security/roles") -const { UserStatus } = require("@budibase/auth") +const { UserStatus } = require("@budibase/auth").constants exports.LOGO_URL = "https://d33wubrfki0l68.cloudfront.net/aac32159d7207b5085e74a7ef67afbb7027786c5/2b1fd/img/logo/bb-emblem.svg" diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index 0d75e808ad..f429c74267 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -1,4 +1,5 @@ -const { getAppId, setCookie, getCookie, Cookies } = require("@budibase/auth") +const { getAppId, setCookie, getCookie } = require("@budibase/auth").utils +const { Cookies } = require("@budibase/auth").constants const { getRole } = require("../utilities/security/roles") const { getGlobalUsers } = require("../utilities/workerRequests") const { BUILTIN_ROLE_IDS } = require("../utilities/security/roles") diff --git a/packages/server/src/middleware/tests/currentapp.spec.js b/packages/server/src/middleware/tests/currentapp.spec.js index 3ed17bb521..61d5bf018d 100644 --- a/packages/server/src/middleware/tests/currentapp.spec.js +++ b/packages/server/src/middleware/tests/currentapp.spec.js @@ -23,10 +23,14 @@ function mockAuthWithNoCookie() { jest.resetModules() mockWorker() jest.mock("@budibase/auth", () => ({ - getAppId: jest.fn(), - setCookie: jest.fn(), - getCookie: jest.fn(), - Cookies: {}, + utils: { + getAppId: jest.fn(), + setCookie: jest.fn(), + getCookie: jest.fn(), + }, + constants: { + Cookies: {}, + }, })) } @@ -34,15 +38,19 @@ function mockAuthWithCookie() { jest.resetModules() mockWorker() jest.mock("@budibase/auth", () => ({ - getAppId: () => { - return "app_test" + utils: { + getAppId: () => { + return "app_test" + }, + setCookie: jest.fn(), + getCookie: () => ({appId: "app_different", roleId: "PUBLIC"}), + }, + constants: { + Cookies: { + Auth: "auth", + CurrentApp: "currentapp", + }, }, - setCookie: jest.fn(), - getCookie: () => ({ appId: "app_different", roleId: "PUBLIC" }), - Cookies: { - Auth: "auth", - CurrentApp: "currentapp", - } })) } @@ -102,7 +110,7 @@ describe("Current app middleware", () => { async function checkExpected(setCookie) { config.setUser() await config.executeMiddleware() - const cookieFn = require("@budibase/auth").setCookie + const cookieFn = require("@budibase/auth").utils.setCookie if (setCookie) { expect(cookieFn).toHaveBeenCalled() } else { @@ -122,12 +130,16 @@ describe("Current app middleware", () => { it("should perform correct when no cookie exists", async () => { mockReset() jest.mock("@budibase/auth", () => ({ - getAppId: () => { - return "app_test" + utils: { + getAppId: () => { + return "app_test" + }, + setCookie: jest.fn(), + getCookie: jest.fn(), + }, + constants: { + Cookies: {}, }, - setCookie: jest.fn(), - getCookie: jest.fn(), - Cookies: {}, })) await checkExpected(true) }) @@ -135,12 +147,14 @@ describe("Current app middleware", () => { it("lastly check what occurs when cookie doesn't need updated", async () => { mockReset() jest.mock("@budibase/auth", () => ({ - getAppId: () => { - return "app_test" + utils: { + getAppId: () => { + return "app_test" + }, + setCookie: jest.fn(), + getCookie: () => ({appId: "app_test", roleId: "BASIC"}), }, - setCookie: jest.fn(), - getCookie: () => ({ appId: "app_test", roleId: "BASIC" }), - Cookies: {}, + constants: { Cookies: {} }, })) await checkExpected(false) }) diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 0f036647bf..630ea4256e 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -15,7 +15,7 @@ const { const controllers = require("./controllers") const supertest = require("supertest") const { cleanup } = require("../../utilities/fileSystem") -const { Cookies } = require("@budibase/auth") +const { Cookies } = require("@budibase/auth").constants const EMAIL = "babs@babs.com" const PASSWORD = "babs_password" diff --git a/packages/worker/src/api/controllers/admin/groups.js b/packages/worker/src/api/controllers/admin/groups.js index 86623c337a..0943a58b1f 100644 --- a/packages/worker/src/api/controllers/admin/groups.js +++ b/packages/worker/src/api/controllers/admin/groups.js @@ -1,6 +1,5 @@ const CouchDB = require("../../../db") -const { getGroupParams, StaticDatabases } = require("@budibase/auth") -const { generateGroupID } = require("@budibase/auth") +const { getGroupParams, generateGroupID, StaticDatabases } = require("@budibase/auth").db const GLOBAL_DB = StaticDatabases.GLOBAL.name diff --git a/packages/worker/src/api/controllers/admin/index.js b/packages/worker/src/api/controllers/admin/index.js deleted file mode 100644 index 60ca3b2900..0000000000 --- a/packages/worker/src/api/controllers/admin/index.js +++ /dev/null @@ -1,7 +0,0 @@ -const users = require("./users") -const groups = require("./groups") - -module.exports = { - users, - groups, -} diff --git a/packages/worker/src/api/controllers/admin/templates.js b/packages/worker/src/api/controllers/admin/templates.js new file mode 100644 index 0000000000..5a2e018702 --- /dev/null +++ b/packages/worker/src/api/controllers/admin/templates.js @@ -0,0 +1,18 @@ +const { generateTemplateID, getTemplateParams } = require("@budibase/auth").db +const { CouchDB } = require("../../../db") + +exports.save = async ctx => { + +} + +exports.fetch = async ctx => { + +} + +exports.find = async ctx => { + +} + +exports.destroy = async ctx => { + +} diff --git a/packages/worker/src/api/controllers/admin/users.js b/packages/worker/src/api/controllers/admin/users.js index 96243d49fd..95dd474e9a 100644 --- a/packages/worker/src/api/controllers/admin/users.js +++ b/packages/worker/src/api/controllers/admin/users.js @@ -1,11 +1,10 @@ const CouchDB = require("../../../db") const { - hash, generateGlobalUserID, getGlobalUserParams, StaticDatabases, - getGlobalUserByEmail, -} = require("@budibase/auth") +} = require("@budibase/auth").db +const { hash, getGlobalUserByEmail } = require("@budibase/auth").utils const { UserStatus } = require("../../../constants") const FIRST_USER_EMAIL = "test@test.com" diff --git a/packages/worker/src/api/controllers/auth.js b/packages/worker/src/api/controllers/auth.js index 03589ab457..96ab8e73f0 100644 --- a/packages/worker/src/api/controllers/auth.js +++ b/packages/worker/src/api/controllers/auth.js @@ -1,4 +1,7 @@ -const { passport, Cookies, clearCookie } = require("@budibase/auth") +const authPkg = require("@budibase/auth") +const { clearCookie } = authPkg.utils +const { Cookies } = authPkg.constants +const { passport } = authPkg.auth exports.authenticate = async (ctx, next) => { return passport.authenticate("local", async (err, user) => { diff --git a/packages/worker/src/api/index.js b/packages/worker/src/api/index.js index 0568d79a68..bc3c74aac9 100644 --- a/packages/worker/src/api/index.js +++ b/packages/worker/src/api/index.js @@ -2,6 +2,9 @@ const Router = require("@koa/router") const compress = require("koa-compress") const zlib = require("zlib") const { routes } = require("./routes") +const { buildAuthMiddleware } = require("@budibase/auth").auth + +const NO_AUTH_ENDPOINTS = ["/api/admin/users/first"] const router = new Router() @@ -19,6 +22,7 @@ router }) ) .use("/health", ctx => (ctx.status = 200)) + .use(buildAuthMiddleware(NO_AUTH_ENDPOINTS)) // error handling middleware router.use(async (ctx, next) => { diff --git a/packages/worker/src/api/routes/admin/groups.js b/packages/worker/src/api/routes/admin/groups.js index e683a01c2b..c9d9f1c92f 100644 --- a/packages/worker/src/api/routes/admin/groups.js +++ b/packages/worker/src/api/routes/admin/groups.js @@ -1,7 +1,6 @@ const Router = require("@koa/router") const controller = require("../../controllers/admin/groups") const joiValidator = require("../../../middleware/joi-validator") -const { authenticated } = require("@budibase/auth") const Joi = require("joi") const router = Router() @@ -28,11 +27,10 @@ router .post( "/api/admin/groups", buildGroupSaveValidation(), - authenticated, controller.save ) - .delete("/api/admin/groups/:id", authenticated, controller.destroy) - .get("/api/admin/groups", authenticated, controller.fetch) - .get("/api/admin/groups/:id", authenticated, controller.find) + .get("/api/admin/groups", controller.fetch) + .delete("/api/admin/groups/:id", controller.destroy) + .get("/api/admin/groups/:id", controller.find) module.exports = router diff --git a/packages/worker/src/api/routes/admin/templates.js b/packages/worker/src/api/routes/admin/templates.js new file mode 100644 index 0000000000..0632d82ec2 --- /dev/null +++ b/packages/worker/src/api/routes/admin/templates.js @@ -0,0 +1,20 @@ +const Router = require("@koa/router") +const controller = require("../../controllers/admin/templates") +const joiValidator = require("../../../middleware/joi-validator") +const Joi = require("joi") + +const router = Router() + +function buildTemplateSaveValidation() { + +} + +router + .post( + "/api/admin/template/:type", + buildTemplateSaveValidation(), + controller.save + ) + .get("/api/admin/template/:type", controller.fetch) + .delete("/api/admin/template/:type/:id", controller.destroy) + .get("/api/admin/template/:type/:id", controller.find) diff --git a/packages/worker/src/api/routes/admin/users.js b/packages/worker/src/api/routes/admin/users.js index d7d19cbf49..bd7d8bbbd5 100644 --- a/packages/worker/src/api/routes/admin/users.js +++ b/packages/worker/src/api/routes/admin/users.js @@ -1,7 +1,6 @@ const Router = require("@koa/router") const controller = require("../../controllers/admin/users") const joiValidator = require("../../../middleware/joi-validator") -const { authenticated } = require("@budibase/auth") const Joi = require("joi") const router = Router() @@ -29,12 +28,11 @@ router .post( "/api/admin/users", buildUserSaveValidation(), - authenticated, controller.userSave ) + .get("/api/admin/users", controller.userFetch) .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/:id", authenticated, controller.userFind) + .delete("/api/admin/users/:id", controller.userDelete) + .get("/api/admin/users/:id", controller.userFind) module.exports = router diff --git a/packages/worker/src/api/routes/app.js b/packages/worker/src/api/routes/app.js index 07120f63a5..86004cb674 100644 --- a/packages/worker/src/api/routes/app.js +++ b/packages/worker/src/api/routes/app.js @@ -1,9 +1,8 @@ const Router = require("@koa/router") const controller = require("../controllers/app") -const { authenticated } = require("@budibase/auth") const router = Router() -router.get("/api/apps", authenticated, controller.getApps) +router.get("/api/apps", controller.getApps) module.exports = router diff --git a/packages/worker/src/api/routes/auth.js b/packages/worker/src/api/routes/auth.js index deea678c63..5ce1b69860 100644 --- a/packages/worker/src/api/routes/auth.js +++ b/packages/worker/src/api/routes/auth.js @@ -1,5 +1,5 @@ const Router = require("@koa/router") -const { passport } = require("@budibase/auth") +const { passport } = require("@budibase/auth").auth const authController = require("../controllers/auth") const router = Router() diff --git a/packages/worker/src/index.js b/packages/worker/src/index.js index 21ad8381fe..8b67181fcc 100644 --- a/packages/worker/src/index.js +++ b/packages/worker/src/index.js @@ -5,7 +5,7 @@ require("@budibase/auth").init(CouchDB) const Koa = require("koa") const destroyable = require("server-destroy") const koaBody = require("koa-body") -const { passport } = require("@budibase/auth") +const { passport } = require("@budibase/auth").auth const logger = require("koa-pino-logger") const http = require("http") const api = require("./api")