From eaad867780bbbbaae8f13bcf2cad640e27712fcf Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Apr 2021 18:31:58 +0100 Subject: [PATCH] Some work towards implementing the current app cookie, removing some old dead code and re-working some of the different middlewares involved. --- packages/auth/src/db/utils.js | 4 ++ packages/auth/src/index.js | 3 +- packages/auth/src/middleware/authenticated.js | 48 ++-------------- packages/auth/src/middleware/passport/jwt.js | 5 -- packages/auth/src/utils.js | 22 +++---- packages/server/src/api/index.js | 2 + packages/server/src/middleware/authorized.js | 37 ++++-------- packages/server/src/middleware/currentapp.js | 57 +++++++++++++++++++ .../src/middleware/tests/authorized.spec.js | 25 -------- packages/server/src/utilities/index.js | 9 ++- .../server/src/utilities/security/apikey.js | 19 ------- 11 files changed, 95 insertions(+), 136 deletions(-) create mode 100644 packages/server/src/middleware/currentapp.js delete mode 100644 packages/server/src/utilities/security/apikey.js diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index 871423df03..b928d809f9 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -22,6 +22,10 @@ exports.generateUserID = email => { return `${DocumentTypes.USER}${SEPARATOR}${email}` } +exports.getEmailFromUserID = userId => { + return userId.split(`${DocumentTypes.USER}${SEPARATOR}`)[1] +} + /** * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index b3b60b2cc9..51fa0aad18 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -7,7 +7,7 @@ const { StaticDatabases } = require("./db/utils") const { jwt, local, google, authenticated } = require("./middleware") const { Cookies, UserStatus } = require("./constants") const { hash, compare } = require("./hashing") -const { getAppId, setCookie } = require("./utils") +const { getAppId, setCookie, getCookie } = require("./utils") const { generateUserID, getUserParams, @@ -45,5 +45,6 @@ module.exports = { compare, getAppId, setCookie, + getCookie, authenticated, } diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index 3301cfbd89..5966da483e 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -1,57 +1,21 @@ -const CouchDB = require("../db") const { Cookies } = require("../constants") -const { getAppId, setCookie, getCookie } = require("../utils") -const { StaticDatabases } = require("../db/utils") - -async function setCurrentAppContext(ctx) { - let role = "PUBLIC" - - // Current app cookie - let appId = getAppId(ctx) - if (!appId) { - ctx.user = { - role, - } - return - } - - console.log("THE APP ID", appId) - - const currentAppCookie = getCookie(ctx, Cookies.CurrentApp, { decrypt: true }) - const appIdChanged = appId && currentAppCookie.appId !== appId - if (appIdChanged) { - try { - // get roles for user from global DB - const db = new CouchDB(StaticDatabases.USER) - const user = await db.get(ctx.user) - role = user.roles[appId] - } catch (err) { - // no user exists - } - } else if (currentAppCookie.appId) { - appId = currentAppCookie.appId - } - setCookie(ctx, { appId, role }, Cookies.CurrentApp, { encrypt: true }) - return appId -} +const { getCookie } = require("../utils") +const { getEmailFromUserID } = require("../db/utils") module.exports = async (ctx, next) => { try { // check the actual user is authenticated first - const authCookie = getCookie(ctx, Cookies.Auth, { decrypt: true }) + const authCookie = getCookie(ctx, Cookies.Auth) if (authCookie) { ctx.isAuthenticated = true - ctx.user = authCookie._id + ctx.user = authCookie + // make sure email is correct from ID + ctx.user.email = getEmailFromUserID(authCookie._id) } - ctx.appId = await setCurrentAppContext(ctx) - - console.log("CONTEXT", ctx) - await next() } catch (err) { - console.log(err) ctx.throw(err.status || 403, err.text) } } diff --git a/packages/auth/src/middleware/passport/jwt.js b/packages/auth/src/middleware/passport/jwt.js index a619ab994b..1d6a4e04e0 100644 --- a/packages/auth/src/middleware/passport/jwt.js +++ b/packages/auth/src/middleware/passport/jwt.js @@ -1,9 +1,4 @@ -const { Cookies } = require("../../constants") - exports.options = { - jwtFromRequest: function(ctx) { - return ctx.cookies.get(Cookies.Auth) - }, secretOrKey: process.env.JWT_SECRET, } diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index 8aa8eb97e0..5f0a135a45 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -45,17 +45,15 @@ exports.getAppId = ctx => { * Get a cookie from context, and decrypt if necessary. * @param {object} ctx The request which is to be manipulated. * @param {string} name The name of the cookie to get. - * @param {object} options options . */ -exports.getCookie = (ctx, value, options = {}) => { - const cookie = ctx.cookies.get(value) +exports.getCookie = (ctx, name) => { + const cookie = ctx.cookies.get(name) - if (!cookie) return + if (!cookie) { + return cookie + } - if (!options.decrypt) return cookie - - const payload = jwt.verify(cookie, process.env.JWT_SECRET) - return payload + return jwt.verify(cookie, options.secretOrKey) } /** @@ -71,11 +69,9 @@ exports.setCookie = (ctx, value, name = "builder") => { if (!value) { ctx.cookies.set(name) } else { - if (options.encrypt) { - value = jwt.sign(value, process.env.JWT_SECRET, { - expiresIn: "1 day", - }) - } + value = jwt.sign(value, options.secretOrKey, { + expiresIn: "1 day", + }) ctx.cookies.set(name, value, { expires, path: "/", diff --git a/packages/server/src/api/index.js b/packages/server/src/api/index.js index 7cf7027ef9..201a9f1c33 100644 --- a/packages/server/src/api/index.js +++ b/packages/server/src/api/index.js @@ -1,5 +1,6 @@ const Router = require("@koa/router") const { authenticated } = require("@budibase/auth") +const currentApp = require("../middleware/currentapp") const compress = require("koa-compress") const zlib = require("zlib") const { mainRoutes, authRoutes, staticRoutes } = require("./routes") @@ -31,6 +32,7 @@ router .use("/health", ctx => (ctx.status = 200)) .use("/version", ctx => (ctx.body = pkg.version)) .use(authenticated) + .use(currentApp) // error handling middleware router.use(async (ctx, next) => { diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index d66df3b7f7..1ef58369ac 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -1,52 +1,37 @@ -const { - BUILTIN_ROLE_IDS, - getUserPermissions, -} = require("../utilities/security/roles") +const { getUserPermissions } = require("../utilities/security/roles") const { PermissionTypes, doesHaveResourcePermission, doesHaveBasePermission, } = require("../utilities/security/permissions") -const env = require("../environment") -const { isAPIKeyValid } = require("../utilities/security/apikey") -const { AuthTypes } = require("../constants") - -const ADMIN_ROLES = [BUILTIN_ROLE_IDS.ADMIN, BUILTIN_ROLE_IDS.BUILDER] function hasResource(ctx) { return ctx.resourceId != null } -module.exports = (permType, permLevel = null) => async (ctx, next) => { - if (env.isProd() && ctx.headers["x-api-key"] && ctx.headers["x-instanceid"]) { - // api key header passed by external webhook - if (await isAPIKeyValid(ctx.headers["x-api-key"])) { - ctx.auth = { - authenticated: AuthTypes.EXTERNAL, - apiKey: ctx.headers["x-api-key"], - } - ctx.user = { - appId: ctx.headers["x-instanceid"], - } - return next() - } +const WEBHOOK_ENDPOINTS = new RegExp( + ["webhooks/trigger", "webhooks/schema"].join("|") +) - return ctx.throw(403, "API key invalid") +module.exports = (permType, permLevel = null) => async (ctx, next) => { + // webhooks don't need authentication, each webhook unique + if (WEBHOOK_ENDPOINTS.test(ctx.request.url)) { + return next() } if (!ctx.user) { return ctx.throw(403, "No user info found") } - const role = ctx.user.role - const isAdmin = ADMIN_ROLES.includes(role._id) const isAuthed = ctx.isAuthenticated const { basePermissions, permissions } = await getUserPermissions( ctx.appId, - role._id + ctx.roleId ) + // TODO: need to determine if the user has permission to build here, global cookie + // this may need to change in the future, right now only admins // can have access to builder features, this is hard coded into // our rules diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js new file mode 100644 index 0000000000..a591874e6f --- /dev/null +++ b/packages/server/src/middleware/currentapp.js @@ -0,0 +1,57 @@ +const { getAppId, setCookie, getCookie, Cookies } = require("@budibase/auth") +const { getGlobalUsers } = require("../utilities/workerRequests") +const { BUILTIN_ROLE_IDS } = require("../utilities/security/roles") + +function CurrentAppCookie(appId, roleId) { + this.appId = appId + this.roleId = roleId +} + +function finish(ctx, next, { appId, roleId, cookie = false }) { + if (appId) { + ctx.appId = appId + } + if (roleId) { + ctx.roleId = roleId + } + if (cookie && appId) { + setCookie(ctx, new CurrentAppCookie(appId, roleId)) + } + return next() +} + +module.exports = async (ctx, next) => { + // try to get the appID from the request + const requestAppId = getAppId(ctx) + // get app cookie if it exists + const appCookie = getCookie(ctx, Cookies.CurrentApp) + if (!appCookie && !requestAppId) { + return next() + } + + let updateCookie = false, + appId, + roleId + if (!ctx.user) { + // not logged in, try to set a cookie for public apps + updateCookie = true + appId = requestAppId + roleId = BUILTIN_ROLE_IDS.PUBLIC + } else if ( + requestAppId != null && + (appCookie == null || requestAppId === appCookie.appId) + ) { + const globalUser = await getGlobalUsers(ctx, requestAppId, ctx.user.email) + updateCookie = true + appId = requestAppId + roleId = globalUser.roles[requestAppId] || BUILTIN_ROLE_IDS.PUBLIC + } else if (requestAppId == null && appCookie != null) { + appId = appCookie.appId + roleId = appCookie.roleId || BUILTIN_ROLE_IDS.PUBLIC + } + return finish(ctx, next, { + appId: appId, + roleId: roleId, + cookie: updateCookie, + }) +} diff --git a/packages/server/src/middleware/tests/authorized.spec.js b/packages/server/src/middleware/tests/authorized.spec.js index 234db96d78..d1ab6bc48c 100644 --- a/packages/server/src/middleware/tests/authorized.spec.js +++ b/packages/server/src/middleware/tests/authorized.spec.js @@ -1,6 +1,5 @@ const authorizedMiddleware = require("../authorized") const env = require("../../environment") -const apiKey = require("../../utilities/security/apikey") const { AuthTypes } = require("../../constants") const { PermissionTypes, PermissionLevels } = require("../../utilities/security/permissions") jest.mock("../../environment", () => ({ @@ -12,7 +11,6 @@ jest.mock("../../environment", () => ({ } }) ) -jest.mock("../../utilities/security/apikey") class TestConfiguration { constructor(role) { @@ -92,29 +90,6 @@ describe("Authorization middleware", () => { "x-instanceid": "instance123", }) }) - - it("passes to next() if api key is valid", async () => { - apiKey.isAPIKeyValid.mockResolvedValueOnce(true) - - await config.executeMiddleware() - - expect(config.next).toHaveBeenCalled() - expect(config.ctx.auth).toEqual({ - authenticated: AuthTypes.EXTERNAL, - apiKey: config.ctx.headers["x-api-key"], - }) - expect(config.ctx.user).toEqual({ - appId: config.ctx.headers["x-instanceid"], - }) - }) - - it("throws if api key is invalid", async () => { - apiKey.isAPIKeyValid.mockResolvedValueOnce(false) - - await config.executeMiddleware() - - expect(config.throw).toHaveBeenCalledWith(403, "API key invalid") - }) }) describe("non-webhook call", () => { diff --git a/packages/server/src/utilities/index.js b/packages/server/src/utilities/index.js index 733afd2870..5b5c2bef06 100644 --- a/packages/server/src/utilities/index.js +++ b/packages/server/src/utilities/index.js @@ -49,12 +49,11 @@ exports.getAppId = ctx => { /** * Get the name of the cookie which is to be updated/retrieved - * @param {string|undefined|null} name OPTIONAL can specify the specific app if previewing etc - * @returns {string} The name of the token trying to find + * @param {string} name The name/type of cookie. + * @returns {string} The full name of the cookie to retrieve/update. */ -exports.getCookieName = (name = "builder") => { - let environment = env.isProd() ? "cloud" : "local" - return `budibase:${name}:${environment}` +exports.getCookieName = name => { + return `budibase:${name}` } /** diff --git a/packages/server/src/utilities/security/apikey.js b/packages/server/src/utilities/security/apikey.js deleted file mode 100644 index 3d5f428bb7..0000000000 --- a/packages/server/src/utilities/security/apikey.js +++ /dev/null @@ -1,19 +0,0 @@ -const { apiKeyTable } = require("../../db/dynamoClient") -const env = require("../../environment") - -/** - * This file purely exists so that we can centralise all logic pertaining to API keys, as their usage differs - * in our Cloud environment versus self hosted. - */ - -exports.isAPIKeyValid = async apiKeyId => { - if (!env.SELF_HOSTED) { - let apiKeyInfo = await apiKeyTable.get({ - primary: apiKeyId, - }) - return apiKeyInfo != null - } else { - // if the api key supplied is correct then return structure similar - return apiKeyId === env.HOSTING_KEY - } -}