From e95af51cde8b006f57a09133c97ba074d15601c7 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Tue, 13 Oct 2020 21:33:56 +0100 Subject: [PATCH] simplify user authentication, remove anon user, fix login cookie issues --- .../cypress/integration/createTable.spec.js | 1 - .../src/components/settings/tabs/Users.svelte | 1 + .../src/pages/[application]/_reset.svelte | 7 +++- packages/server/src/api/controllers/auth.js | 2 +- packages/server/src/api/controllers/static.js | 30 +++---------- packages/server/src/constants/index.js | 7 ++++ .../server/src/middleware/authenticated.js | 42 +++++++------------ packages/server/src/middleware/authorized.js | 12 +++--- packages/server/src/utilities/accessLevels.js | 1 - .../src/Test/TestApp.svelte | 2 +- 10 files changed, 43 insertions(+), 62 deletions(-) create mode 100644 packages/server/src/constants/index.js diff --git a/packages/builder/cypress/integration/createTable.spec.js b/packages/builder/cypress/integration/createTable.spec.js index 36aed6e058..0f57d56f3e 100644 --- a/packages/builder/cypress/integration/createTable.spec.js +++ b/packages/builder/cypress/integration/createTable.spec.js @@ -27,7 +27,6 @@ context("Create a Table", () => { cy.get(".actions input") .first() .type("updated") - cy.get("select").select("Text") cy.contains("Save Column").click() cy.contains("nameupdated").should("have.text", "nameupdated") }) diff --git a/packages/builder/src/components/settings/tabs/Users.svelte b/packages/builder/src/components/settings/tabs/Users.svelte index d24dcc7a6b..a2cb34e8d0 100644 --- a/packages/builder/src/components/settings/tabs/Users.svelte +++ b/packages/builder/src/components/settings/tabs/Users.svelte @@ -58,6 +58,7 @@ diff --git a/packages/builder/src/pages/[application]/_reset.svelte b/packages/builder/src/pages/[application]/_reset.svelte index 7174bd14d7..b7199d27fe 100644 --- a/packages/builder/src/pages/[application]/_reset.svelte +++ b/packages/builder/src/pages/[application]/_reset.svelte @@ -68,7 +68,10 @@ window.open(`/${application}`)}> + on:click={() => { + document.cookie = 'budibase:token=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;' + window.open(`/${application}`) + }}> @@ -77,7 +80,7 @@ {#await promise}
- {:then result} + {:then results} {:catch error}

Something went wrong: {error.message}

diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index ac436399a9..0fed3337f8 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -41,7 +41,7 @@ exports.authenticate = async ctx => { dbUser = await instanceDb.get(generateUserID(username)) } catch (_) { // do not want to throw a 404 - as this could be - // used to dtermine valid usernames + // used to determine valid usernames ctx.throw(401, "Invalid Credentials") } diff --git a/packages/server/src/api/controllers/static.js b/packages/server/src/api/controllers/static.js index 5aaa9ab125..2abb0069ab 100644 --- a/packages/server/src/api/controllers/static.js +++ b/packages/server/src/api/controllers/static.js @@ -1,6 +1,5 @@ const send = require("koa-send") const { resolve, join } = require("../../utilities/centralPath") -const jwt = require("jsonwebtoken") const fetch = require("node-fetch") const fs = require("fs-extra") const uuid = require("uuid") @@ -13,8 +12,8 @@ const { } = require("../../utilities/budibaseDir") const CouchDB = require("../../db") const setBuilderToken = require("../../utilities/builder/setBuilderToken") -const { ANON_LEVEL_ID } = require("../../utilities/accessLevels") const fileProcessor = require("../../utilities/fileProcessor") +const { AuthTypes } = require("../../constants") exports.serveBuilder = async function(ctx) { let builderPath = resolve(__dirname, "../../../builder") @@ -136,7 +135,8 @@ exports.performLocalFileProcessing = async function(ctx) { } exports.serveApp = async function(ctx) { - const mainOrAuth = ctx.auth.authenticated ? "main" : "unauthenticated" + const mainOrAuth = + ctx.auth.authenticated === AuthTypes.APP ? "main" : "unauthenticated" // default to homedir const appPath = resolve( @@ -146,26 +146,7 @@ exports.serveApp = async function(ctx) { mainOrAuth ) - let appId = ctx.params.appId - if (process.env.CLOUD) { - appId = ctx.subdomains[1] - } - - // only set the appId cookie for /appId .. we COULD check for valid appIds - // but would like to avoid that DB hit - const looksLikeAppId = /^(app_)?[0-9a-f]{32}$/.test(appId) - if (looksLikeAppId && !ctx.auth.authenticated) { - const anonUser = { - userId: "ANON", - accessLevelId: ANON_LEVEL_ID, - appId, - } - const anonToken = jwt.sign(anonUser, ctx.config.jwtSecret) - ctx.cookies.set("budibase:token", anonToken, { - path: "/", - httpOnly: false, - }) - } + const appId = ctx.user.appId if (process.env.CLOUD) { const S3_URL = `https://${appId}.app.budi.live/assets/${appId}/${mainOrAuth}/${ctx.file || @@ -200,7 +181,8 @@ exports.serveAttachment = async function(ctx) { exports.serveAppAsset = async function(ctx) { // default to homedir - const mainOrAuth = ctx.auth.authenticated ? "main" : "unauthenticated" + const mainOrAuth = + ctx.auth.authenticated === AuthTypes.APP ? "main" : "unauthenticated" const appPath = resolve( budibaseAppsDir(), diff --git a/packages/server/src/constants/index.js b/packages/server/src/constants/index.js new file mode 100644 index 0000000000..cae1cde3e8 --- /dev/null +++ b/packages/server/src/constants/index.js @@ -0,0 +1,7 @@ +const AuthTypes = { + APP: "app", + BUILDER: "builder", + EXTERNAL: "external", +} + +exports.AuthTypes = AuthTypes diff --git a/packages/server/src/middleware/authenticated.js b/packages/server/src/middleware/authenticated.js index 1203ea0033..3a2c2ec9a9 100644 --- a/packages/server/src/middleware/authenticated.js +++ b/packages/server/src/middleware/authenticated.js @@ -7,6 +7,8 @@ const { BUILDER_LEVEL_ID, ANON_LEVEL_ID, } = require("../utilities/accessLevels") +const environment = require("../environment") +const { AuthTypes } = require("../constants") module.exports = async (ctx, next) => { if (ctx.path === "/_builder") { @@ -17,36 +19,28 @@ module.exports = async (ctx, next) => { const appToken = ctx.cookies.get("budibase:token") const builderToken = ctx.cookies.get("builder:token") - if (builderToken) { - try { - const jwtPayload = jwt.verify(builderToken, ctx.config.jwtSecret) - ctx.auth = { - apiKey: jwtPayload.apiKey, - authenticated: jwtPayload.accessLevelId === BUILDER_LEVEL_ID, - } - ctx.user = { - ...jwtPayload, - accessLevel: await getAccessLevel( - jwtPayload.instanceId, - jwtPayload.accessLevelId - ), - } - } catch (_) { - // empty: do nothing - } - - await next() - return + let token + // if running locally in the builder itself + if (!environment.CLOUD && !appToken) { + token = builderToken + ctx.auth.authenticated = AuthTypes.BUILDER + } else { + token = appToken + ctx.auth.authenticated = AuthTypes.APP } - if (!appToken) { + if (!token) { ctx.auth.authenticated = false + ctx.user = { + appId: process.env.CLOUD ? ctx.subdomains[1] : ctx.params.appId, + } await next() return } try { - const jwtPayload = jwt.verify(appToken, ctx.config.jwtSecret) + const jwtPayload = jwt.verify(token, ctx.config.jwtSecret) + ctx.auth.apiKey = jwtPayload.apiKey ctx.user = { ...jwtPayload, accessLevel: await getAccessLevel( @@ -54,10 +48,6 @@ module.exports = async (ctx, next) => { jwtPayload.accessLevelId ), } - ctx.auth = { - authenticated: ctx.user.accessLevelId !== ANON_LEVEL_ID, - apiKey: jwtPayload.apiKey, - } } catch (err) { ctx.throw(err.status || STATUS_CODES.FORBIDDEN, err.text) } diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index bd09029471..238b2cab3d 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -7,6 +7,7 @@ const { } = require("../utilities/accessLevels") const environment = require("../environment") const { apiKeyTable } = require("../db/dynamoClient") +const { AuthTypes } = require("../constants") module.exports = (permName, getItemId) => async (ctx, next) => { if ( @@ -21,8 +22,7 @@ module.exports = (permName, getItemId) => async (ctx, next) => { if (apiKeyInfo) { ctx.auth = { - authenticated: true, - external: true, + authenticated: AuthTypes.EXTERNAL, apiKey: ctx.headers["x-api-key"], } ctx.user = { @@ -42,6 +42,10 @@ module.exports = (permName, getItemId) => async (ctx, next) => { ctx.throw(403, "User not found") } + if (ctx.user.accessLevel._id === ADMIN_LEVEL_ID) { + return next() + } + if (ctx.user.accessLevel._id === BUILDER_LEVEL_ID) { return next() } @@ -53,10 +57,6 @@ module.exports = (permName, getItemId) => async (ctx, next) => { const permissionId = ({ name, itemId }) => name + (itemId ? `-${itemId}` : "") - if (ctx.user.accessLevel._id === ADMIN_LEVEL_ID) { - return next() - } - const thisPermissionId = permissionId({ name: permName, itemId: getItemId && getItemId(ctx), diff --git a/packages/server/src/utilities/accessLevels.js b/packages/server/src/utilities/accessLevels.js index 201007527c..ada3f36880 100644 --- a/packages/server/src/utilities/accessLevels.js +++ b/packages/server/src/utilities/accessLevels.js @@ -21,7 +21,6 @@ module.exports.PRETTY_ACCESS_LEVELS = { [module.exports.ADMIN_LEVEL_ID]: "Admin", [module.exports.POWERUSER_LEVEL_ID]: "Power user", [module.exports.BUILDER_LEVEL_ID]: "Builder", - [module.exports.ANON_LEVEL_ID]: "Anonymous", } module.exports.adminPermissions = [ { diff --git a/packages/standard-components/src/Test/TestApp.svelte b/packages/standard-components/src/Test/TestApp.svelte index 13504e1e15..1ad2cc484e 100644 --- a/packages/standard-components/src/Test/TestApp.svelte +++ b/packages/standard-components/src/Test/TestApp.svelte @@ -35,7 +35,7 @@ {#await _appPromise} loading -{:then _bb} +{:then [object Object]}
{/await}