From a0f2de65e78c36da1e139b072a586c6ca3b12c7a Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 5 Jul 2021 14:24:13 +0100 Subject: [PATCH 01/57] Front End form for OIDC configuration --- .../portal/manage/auth/_logos/OIDC.svelte | 20 +++ .../builder/portal/manage/auth/index.svelte | 138 ++++++++++++++---- 2 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 packages/builder/src/pages/builder/portal/manage/auth/_logos/OIDC.svelte diff --git a/packages/builder/src/pages/builder/portal/manage/auth/_logos/OIDC.svelte b/packages/builder/src/pages/builder/portal/manage/auth/_logos/OIDC.svelte new file mode 100644 index 0000000000..f56d716ae0 --- /dev/null +++ b/packages/builder/src/pages/builder/portal/manage/auth/_logos/OIDC.svelte @@ -0,0 +1,20 @@ + + + + + + + + + + + diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 86137fddf8..ce30e5f2f1 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -1,5 +1,6 @@ @@ -74,7 +128,7 @@ below. - {#if google} + {#if providers.google} @@ -89,17 +143,50 @@ - {#each ConfigFields.Google as field} + {#each GoogleConfigFields.Google as field}
- - + +
{/each} -
- -
{/if} + {#if providers.oidc} + + + + + + OpenID Connect + + + + To allow users to authenticate using OIDC, fill out the fields below. + + + + {#each OIDCConfigFields.Oidc as field} +
+ + +
+ {/each} + + To customize your login button, fill out the fields below. + +
+ +
{/each} +
- To customize your login button, fill out the fields below. + To customize your login button, fill out the fields below.
- - +
+
+ -
+ onFileSelected(e)} + bind:this={fileinput} + /> {/if}
- +
@@ -201,4 +251,8 @@ align-items: center; gap: var(--spacing-s); } + + input { + display: none; + } diff --git a/packages/worker/src/api/controllers/admin/configs.js b/packages/worker/src/api/controllers/admin/configs.js index ffd85e98e9..2f62af18bb 100644 --- a/packages/worker/src/api/controllers/admin/configs.js +++ b/packages/worker/src/api/controllers/admin/configs.js @@ -146,7 +146,7 @@ exports.upload = async function (ctx) { } } const url = `/${bucket}/${key}` - cfgStructure.config[`${name}Url`] = url + cfgStructure.config[`${name}`] = url // write back to db with url updated await db.put(cfgStructure) @@ -192,8 +192,6 @@ exports.configChecklist = async function (ctx) { const oidcConfig = await getScopedFullConfig(db, { type: Configs.OIDC, }) - - // They have set up an admin user const users = await db.allDocs( getGlobalUserParams(null, { diff --git a/packages/worker/src/api/routes/admin/configs.js b/packages/worker/src/api/routes/admin/configs.js index 83eec20cf3..2e926b61f0 100644 --- a/packages/worker/src/api/routes/admin/configs.js +++ b/packages/worker/src/api/routes/admin/configs.js @@ -79,7 +79,7 @@ function buildUploadValidation() { // prettier-ignore return joiValidator.params(Joi.object({ type: Joi.string().valid(...Object.values(Configs)).required(), - name: Joi.string().valid(...Object.values(ConfigUploads)).required(), + name: Joi.string().required(), }).required()) } diff --git a/packages/worker/src/constants/index.js b/packages/worker/src/constants/index.js index 70c61cd6b0..ae52af9d7f 100644 --- a/packages/worker/src/constants/index.js +++ b/packages/worker/src/constants/index.js @@ -16,7 +16,7 @@ exports.Configs = Configs exports.ConfigUploads = { LOGO: "logo", - OIDC_LOGO: "oidc_logo" + OIDC_LOGO: "oidc_logo", } const TemplateTypes = { From d6c66812208c107fb4205f023acd95aa2d2f1a12 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Wed, 7 Jul 2021 13:45:33 +0100 Subject: [PATCH 08/57] Fallback to ID token to retrieve email when not available in passport profile (oidc userinfo) --- packages/auth/src/middleware/passport/oidc.js | 49 ++++++++++++++++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/packages/auth/src/middleware/passport/oidc.js b/packages/auth/src/middleware/passport/oidc.js index d9a86ce574..f28aa030f9 100644 --- a/packages/auth/src/middleware/passport/oidc.js +++ b/packages/auth/src/middleware/passport/oidc.js @@ -9,7 +9,40 @@ const { ViewNames, } = require("../../db/utils") -// async function authenticate(token, tokenSecret, profile, done) { +/** + * Attempt to parse the users email address. + * + * It is not guaranteed that the email will be returned by the user info endpoint (e.g. github connected account used in azure ad). + * Fallback to the id token where possible. + * + * @param {*} profile The structured profile created by passport using the user info endpoint + * @param {*} jwtClaims The raw claims returned in the id token + */ +function getEmail(profile, jwtClaims) { + if (profile._json.email) { + return profile._json.email + } + + if (jwtClaims.email) { + return jwtClaims.email + } + + return null; +} + +/** + * + * @param {*} issuer The identity provider base URL + * @param {*} sub The user ID + * @param {*} profile The user profile information. Created by passport from the /userinfo response + * @param {*} jwtClaims The parsed id_token claims + * @param {*} accessToken The access_token for contacting the identity provider - may or may not be a JWT + * @param {*} refreshToken The refresh_token for obtaining a new access_token - usually not a JWT + * @param {*} idToken The id_token - always a JWT + * @param {*} params The response body from requesting an access_token + * @param {*} done The passport callback: err, user, info + * @returns + */ async function authenticate( issuer, sub, @@ -40,8 +73,13 @@ async function authenticate( } // check if an account with the OIDC email address exists locally + const email = getEmail(profile, jwtClaims) + if (!email) { + return done(null, false, { message: "No email address found" }) + } + const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { - key: profile._json.email, + key: email, include_docs: true, }) @@ -61,12 +99,7 @@ async function authenticate( dbUser = user dbUser._rev = response.rev } else { - return done( - new Error( - "email does not yet exist. You must set up your local budibase account first." - ), - false - ) + return done(null, false, { message: "Email does not yet exist. You must set up your local budibase account first." }) } } From cff2a9ad7d254c83cabc28b79301a6aea5e85b3c Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 7 Jul 2021 16:18:18 +0100 Subject: [PATCH 09/57] Fix for icon upload issue --- packages/auth/src/constants.js | 2 +- packages/bbui/src/Form/Core/Picker.svelte | 3 +- .../builder/portal/manage/auth/index.svelte | 37 ++++++++++++------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/packages/auth/src/constants.js b/packages/auth/src/constants.js index ca0e69b34a..54736bed67 100644 --- a/packages/auth/src/constants.js +++ b/packages/auth/src/constants.js @@ -21,5 +21,5 @@ exports.Configs = { SMTP: "smtp", GOOGLE: "google", OIDC: "oidc", - OIDC_LOGOS:"oidc_logos" + OIDC_LOGOS: "oidc_logos", } diff --git a/packages/bbui/src/Form/Core/Picker.svelte b/packages/bbui/src/Form/Core/Picker.svelte index b7f53fb861..cd8f784b14 100644 --- a/packages/bbui/src/Form/Core/Picker.svelte +++ b/packages/bbui/src/Form/Core/Picker.svelte @@ -14,7 +14,6 @@ export let isPlaceholder = false export let placeholderOption = null export let options = [] - export let callbackOptionValue = null export let isOptionSelected = () => false export let onSelectOption = () => {} export let getOptionLabel = option => option @@ -47,7 +46,7 @@ > {#if fieldIcon} - test + OpenID Icon {/if} diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index b5fd01b361..1b74e9aaac 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -1,5 +1,5 @@ @@ -215,12 +225,11 @@
- Date: Wed, 7 Jul 2021 17:39:26 +0100 Subject: [PATCH 10/57] Add OIDC icon to login page --- .../auth/_components/OidcButton.svelte | 39 +++++++++++++++++++ .../src/pages/builder/auth/login.svelte | 2 + .../src/api/controllers/admin/configs.js | 1 + 3 files changed, 42 insertions(+) create mode 100644 packages/builder/src/pages/builder/auth/_components/OidcButton.svelte diff --git a/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte b/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte new file mode 100644 index 0000000000..46990b759e --- /dev/null +++ b/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte @@ -0,0 +1,39 @@ + + + {#if show} + window.open("/api/admin/auth/oidc", "_blank")} + > +
+ oidc icon +

Sign in with OIDC

+
+ + {/if} + + + \ No newline at end of file diff --git a/packages/builder/src/pages/builder/auth/login.svelte b/packages/builder/src/pages/builder/auth/login.svelte index 9fb984c73e..264962f3fa 100644 --- a/packages/builder/src/pages/builder/auth/login.svelte +++ b/packages/builder/src/pages/builder/auth/login.svelte @@ -12,6 +12,7 @@ import { goto, params } from "@roxi/routify" import { auth, organisation } from "stores/portal" import GoogleButton from "./_components/GoogleButton.svelte" + import OidcButton from "./_components/OidcButton.svelte" import Logo from "assets/bb-emblem.svg" import { onMount } from "svelte" @@ -61,6 +62,7 @@ Sign in to {company} + Sign in with email diff --git a/packages/worker/src/api/controllers/admin/configs.js b/packages/worker/src/api/controllers/admin/configs.js index 2f62af18bb..d83be15667 100644 --- a/packages/worker/src/api/controllers/admin/configs.js +++ b/packages/worker/src/api/controllers/admin/configs.js @@ -205,6 +205,7 @@ exports.configChecklist = async function (ctx) { smtp: !!smtpConfig, adminUser, oauth: !!oauthConfig, + oidc: !!oidcConfig, } } catch (err) { ctx.throw(err.status, err) From 25f1368227c47ecd6dfcd875543e886fe3546a6f Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 11:12:34 +0100 Subject: [PATCH 11/57] Merge google/oidc user authentication and surface user relevant error messages during authentication --- .../auth/src/middleware/passport/google.js | 81 +++--------- packages/auth/src/middleware/passport/jwt.js | 3 +- .../auth/src/middleware/passport/local.js | 11 +- packages/auth/src/middleware/passport/oidc.js | 91 +++---------- .../middleware/passport/third-party-common.js | 124 ++++++++++++++++++ .../auth/src/middleware/passport/utils.js | 14 ++ .../worker/src/api/controllers/admin/auth.js | 9 +- 7 files changed, 184 insertions(+), 149 deletions(-) create mode 100644 packages/auth/src/middleware/passport/third-party-common.js create mode 100644 packages/auth/src/middleware/passport/utils.js diff --git a/packages/auth/src/middleware/passport/google.js b/packages/auth/src/middleware/passport/google.js index f84da95e73..a20912d973 100644 --- a/packages/auth/src/middleware/passport/google.js +++ b/packages/auth/src/middleware/passport/google.js @@ -1,75 +1,24 @@ -const env = require("../../environment") -const jwt = require("jsonwebtoken") -const database = require("../../db") const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy -const { - StaticDatabases, - generateGlobalUserID, - ViewNames, -} = require("../../db/utils") -async function authenticate(token, tokenSecret, profile, done) { - // Check the user exists in the instance DB by email - const db = database.getDB(StaticDatabases.GLOBAL.name) +const { authenticateThirdParty } = require("./third-party-common") - let dbUser - - const userId = generateGlobalUserID(profile.id) - - try { - // use the google profile id - dbUser = await db.get(userId) - } catch (err) { - const user = { - _id: userId, - provider: profile.provider, - roles: {}, - ...profile._json, - } - - // check if an account with the google email address exists locally - const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { - key: profile._json.email, - include_docs: true, - }) - - // Google user already exists by email - if (users.rows.length > 0) { - const existing = users.rows[0].doc - - // remove the local account to avoid conflicts - await db.remove(existing._id, existing._rev) - - // merge with existing account - user.roles = existing.roles - user.builder = existing.builder - user.admin = existing.admin - - const response = await db.post(user) - dbUser = user - dbUser._rev = response.rev - } else { - return done( - new Error( - "email does not yet exist. You must set up your local budibase account first." - ), - false - ) +async function authenticate(accessToken, refreshToken, profile, done) { + const thirdPartyUser = { + provider: profile.provider, // should always be 'google' + providerType: "google", + userId: profile.id, + profile: profile, + email: profile._json.email, + oauth2: { + accessToken: accessToken, + refreshToken: refreshToken } } - // authenticate - const payload = { - userId: dbUser._id, - builder: dbUser.builder, - email: dbUser.email, - } - - dbUser.token = jwt.sign(payload, env.JWT_SECRET, { - expiresIn: "1 day", - }) - - return done(null, dbUser) + return authenticateThirdParty( + thirdPartyUser, + true, // require local accounts to exist + done) } /** diff --git a/packages/auth/src/middleware/passport/jwt.js b/packages/auth/src/middleware/passport/jwt.js index ed7d179482..690c2ac8a1 100644 --- a/packages/auth/src/middleware/passport/jwt.js +++ b/packages/auth/src/middleware/passport/jwt.js @@ -1,5 +1,6 @@ const { Cookies } = require("../../constants") const env = require("../../environment") +const { authError } = require("./utils") exports.options = { secretOrKey: env.JWT_SECRET, @@ -12,6 +13,6 @@ exports.authenticate = async function (jwt, done) { try { return done(null, jwt) } catch (err) { - return done(new Error("JWT invalid."), false) + return authError(done, "JWT invalid", err) } } diff --git a/packages/auth/src/middleware/passport/local.js b/packages/auth/src/middleware/passport/local.js index 0f5cb82606..01d1e00934 100644 --- a/packages/auth/src/middleware/passport/local.js +++ b/packages/auth/src/middleware/passport/local.js @@ -3,6 +3,7 @@ const { UserStatus } = require("../../constants") const { compare } = require("../../hashing") const env = require("../../environment") const { getGlobalUserByEmail } = require("../../utils") +const { authError } = require("./utils") const INVALID_ERR = "Invalid Credentials" @@ -16,17 +17,17 @@ exports.options = {} * @returns The authenticated user, or errors if they occur */ exports.authenticate = async function (email, password, done) { - if (!email) return done(null, false, "Email Required.") - if (!password) return done(null, false, "Password Required.") + if (!email) return authError(done, "Email Required") + if (!password) return authError(done, "Password Required") const dbUser = await getGlobalUserByEmail(email) if (dbUser == null) { - return done(null, false, { message: "User not found" }) + return authError(done, "User not found") } // check that the user is currently inactive, if this is the case throw invalid if (dbUser.status === UserStatus.INACTIVE) { - return done(null, false, { message: INVALID_ERR }) + return authError(done, INVALID_ERR) } // authenticate @@ -43,6 +44,6 @@ exports.authenticate = async function (email, password, done) { return done(null, dbUser) } else { - done(new Error(INVALID_ERR), false) + return authError(done, INVALID_ERR) } } diff --git a/packages/auth/src/middleware/passport/oidc.js b/packages/auth/src/middleware/passport/oidc.js index f28aa030f9..e695e085bf 100644 --- a/packages/auth/src/middleware/passport/oidc.js +++ b/packages/auth/src/middleware/passport/oidc.js @@ -1,28 +1,18 @@ -const env = require("../../environment") -const jwt = require("jsonwebtoken") -const database = require("../../db") const fetch = require("node-fetch") const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy -const { - StaticDatabases, - generateGlobalUserID, - ViewNames, -} = require("../../db/utils") +const { authenticateThirdParty } = require("./third-party-common") /** - * Attempt to parse the users email address. - * - * It is not guaranteed that the email will be returned by the user info endpoint (e.g. github connected account used in azure ad). - * Fallback to the id token where possible. - * * @param {*} profile The structured profile created by passport using the user info endpoint - * @param {*} jwtClaims The raw claims returned in the id token + * @param {*} jwtClaims The claims returned in the id token */ function getEmail(profile, jwtClaims) { + // profile not guaranteed to contain email e.g. github connected azure ad account if (profile._json.email) { return profile._json.email } + // fallback to id token if (jwtClaims.email) { return jwtClaims.email } @@ -31,7 +21,6 @@ function getEmail(profile, jwtClaims) { } /** - * * @param {*} issuer The identity provider base URL * @param {*} sub The user ID * @param {*} profile The user profile information. Created by passport from the /userinfo response @@ -54,67 +43,23 @@ async function authenticate( params, done ) { - // Check the user exists in the instance DB by email - const db = database.getDB(StaticDatabases.GLOBAL.name) - - let dbUser - - const userId = generateGlobalUserID(profile.id) - - try { - // use the OIDC profile id - dbUser = await db.get(userId) - } catch (err) { - const user = { - _id: userId, - provider: profile.provider, - roles: {}, - ...profile._json, - } - - // check if an account with the OIDC email address exists locally - const email = getEmail(profile, jwtClaims) - if (!email) { - return done(null, false, { message: "No email address found" }) - } - - const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { - key: email, - include_docs: true, - }) - - // OIDC user already exists by email - if (users.rows.length > 0) { - const existing = users.rows[0].doc - - // remove the local account to avoid conflicts - await db.remove(existing._id, existing._rev) - - // merge with existing account - user.roles = existing.roles - user.builder = existing.builder - user.admin = existing.admin - - const response = await db.post(user) - dbUser = user - dbUser._rev = response.rev - } else { - return done(null, false, { message: "Email does not yet exist. You must set up your local budibase account first." }) + const thirdPartyUser = { + // store the issuer info to enable sync in future + provider: issuer, + providerType: "oidc", + userId: profile.id, + profile: profile, + email: getEmail(profile, jwtClaims), + oauth2: { + accessToken: accessToken, + refreshToken: refreshToken } } - // authenticate - const payload = { - userId: dbUser._id, - builder: dbUser.builder, - email: dbUser.email, - } - - dbUser.token = jwt.sign(payload, env.JWT_SECRET, { - expiresIn: "1 day", - }) - - return done(null, dbUser) + return authenticateThirdParty( + thirdPartyUser, + false, // don't require local accounts to exist + done) } /** diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js new file mode 100644 index 0000000000..55212e3304 --- /dev/null +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -0,0 +1,124 @@ +const env = require("../../environment") +const jwt = require("jsonwebtoken") +const database = require("../../db") +const { + StaticDatabases, + generateGlobalUserID, + ViewNames, +} = require("../../db/utils") +const { authError } = require("./utils") + +/** + * Common authentication logic for third parties. e.g. OAuth, OIDC. + */ +exports.authenticateThirdParty = async function ( + thirdPartyUser, + requireLocalAccount = true, + done + ) { + if (!thirdPartyUser.provider) return authError(done, "third party user provider required") + if (!thirdPartyUser.userId) return authError(done, "third party user id required") + if (!thirdPartyUser.email) return authError(done, "third party user email required") + + const db = database.getDB(StaticDatabases.GLOBAL.name) + + let dbUser + + // use the third party id + const userId = generateGlobalUserID(thirdPartyUser.userId) + + try { + dbUser = await db.get(userId) + } catch (err) { + // abort when not 404 error + if (!err.status || err.status !== 404) { + return authError(done, "Unexpected error when retrieving existing user", err) + } + + // check user already exists by email + const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { + key: thirdPartyUser.email, + include_docs: true, + }) + let userExists = users.rows.length > 0 + + if (requireLocalAccount && !userExists) { + return authError(done, "Email does not yet exist. You must set up your local budibase account first.") + } + + // create the user to save + let user + if (userExists) { + const existing = users.rows[0].doc + user = constructMergedUser(userId, existing, thirdPartyUser) + + // remove the local account to avoid conflicts + await db.remove(existing._id, existing._rev) + } else { + user = constructNewUser(userId, thirdPartyUser) + } + + // save the user + const response = await db.post(user) + dbUser = user + dbUser._rev = response.rev + } + + // authenticate + const payload = { + userId: dbUser._id, + builder: dbUser.builder, + email: dbUser.email, + } + + dbUser.token = jwt.sign(payload, env.JWT_SECRET, { + expiresIn: "1 day", + }) + + return done(null, dbUser) +} + +/** + * @returns a user object constructed from existing and third party information + */ +function constructMergedUser(userId, existing, thirdPartyUser) { + const user = constructNewUser(userId, thirdPartyUser) + + // merge with existing account + user.roles = existing.roles + user.builder = existing.builder + user.admin = existing.admin + + return user +} + +/** + * @returns a user object constructed from third party information + */ +function constructNewUser(userId, thirdPartyUser) { + const user = { + _id: userId, + provider: thirdPartyUser.provider, + providerType: thirdPartyUser.providerType, + roles: {} + } + + // persist profile information + // @reviewers: Historically stored at the root level of the user + // Nest to prevent conflicts with future fields + // Is this okay to change? + if (thirdPartyUser.profile) { + user.thirdPartyProfile = { + ...thirdPartyUser.profile._json + } + } + + // persist oauth tokens for future use + if (thirdPartyUser.oauth2) { + user.oauth2 = { + ...thirdPartyUser.oauth2 + } + } + + return user +} diff --git a/packages/auth/src/middleware/passport/utils.js b/packages/auth/src/middleware/passport/utils.js new file mode 100644 index 0000000000..1f7b4437bd --- /dev/null +++ b/packages/auth/src/middleware/passport/utils.js @@ -0,0 +1,14 @@ +/** + * Utility to handle authentication errors. + * + * @param {*} done The passport callback. + * @param {*} message Message that will be returned in the response body + * @param {*} err (Optional) error that will be logged + */ +exports.authError = function (done, message, err = null) { + return done( + err, + null, // never return a user + { message: message } + ) +} \ No newline at end of file diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index 01717bffe0..e6d553bc62 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -13,6 +13,7 @@ const GLOBAL_DB = authPkg.StaticDatabases.GLOBAL.name function authInternal(ctx, user, err = null, info = null) { if (err) { + console.error("Authentication error", err) return ctx.throw(403, info? info : "Unauthorized") } @@ -32,8 +33,8 @@ function authInternal(ctx, user, err = null, info = null) { } exports.authenticate = async (ctx, next) => { - return passport.authenticate("local", async (err, user) => { - authInternal(ctx, user, err) + return passport.authenticate("local", async (err, user, info) => { + authInternal(ctx, user, err, info) delete user.token @@ -123,8 +124,8 @@ exports.googleAuth = async (ctx, next) => { return passport.authenticate( strategy, { successRedirect: "/", failureRedirect: "/error" }, - async (err, user) => { - authInternal(ctx, user, err) + async (err, user, info) => { + authInternal(ctx, user, err, info) ctx.redirect("/") } From 99de767781b7032a01400006f94c47c56935f700 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Thu, 8 Jul 2021 11:15:22 +0100 Subject: [PATCH 12/57] fix issue where oidc config form was not loading due to oidc_logos being undefined --- .../src/pages/builder/portal/manage/auth/index.svelte | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 1b74e9aaac..3f4b39bd4e 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -135,9 +135,11 @@ } //Get the list of user uploaded logos and push it to the dropdown options. - //This needs to be done before the config callso they're available when the dropdown renders + //This needs to be done before the config call so they're available when the dropdown renders const res = await api.get(`/api/admin/configs/oidc_logos`) const configSettings = await res.json() + + if (configSettings.config) { const logoKeys = Object.keys(configSettings.config) logoKeys.map(logoKey => { @@ -148,7 +150,7 @@ icon: logoUrl, }) }) - + } const oidcResponse = await api.get(`/api/admin/configs/${ConfigTypes.OIDC}`) const oidcDoc = await oidcResponse.json() From bf94125d3319f7853862de2dac50622311945817 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 11:54:16 +0100 Subject: [PATCH 13/57] Save email from third party user --- packages/auth/src/middleware/passport/third-party-common.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js index 55212e3304..731aacf75f 100644 --- a/packages/auth/src/middleware/passport/third-party-common.js +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -40,7 +40,7 @@ exports.authenticateThirdParty = async function ( key: thirdPartyUser.email, include_docs: true, }) - let userExists = users.rows.length > 0 + const userExists = users.rows.length > 0 if (requireLocalAccount && !userExists) { return authError(done, "Email does not yet exist. You must set up your local budibase account first.") @@ -100,6 +100,7 @@ function constructNewUser(userId, thirdPartyUser) { _id: userId, provider: thirdPartyUser.provider, providerType: thirdPartyUser.providerType, + email: thirdPartyUser.email, roles: {} } From cd1e5c80875897cd5d2ed4a01b523b1c07da1e0e Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Thu, 8 Jul 2021 12:36:09 +0100 Subject: [PATCH 14/57] fixing conflict with OIDCButton --- .../auth/_components/OidcButton.svelte | 39 ------------------- .../builder/portal/manage/auth/index.svelte | 20 +++++----- 2 files changed, 10 insertions(+), 49 deletions(-) delete mode 100644 packages/builder/src/pages/builder/auth/_components/OidcButton.svelte diff --git a/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte b/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte deleted file mode 100644 index 46990b759e..0000000000 --- a/packages/builder/src/pages/builder/auth/_components/OidcButton.svelte +++ /dev/null @@ -1,39 +0,0 @@ - - - {#if show} - window.open("/api/admin/auth/oidc", "_blank")} - > -
- oidc icon -

Sign in with OIDC

-
- - {/if} - - - \ No newline at end of file diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 3f4b39bd4e..3837699e6a 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -85,7 +85,7 @@ let fileName = e.target.files[0].name image = e.target.files[0] providers.oidc.config["iconName"] = fileName - iconDropdownOptions.unshift({label: fileName, value: fileName}) + iconDropdownOptions.unshift({ label: fileName, value: fileName }) } const providers = { google, oidc } @@ -140,17 +140,17 @@ const configSettings = await res.json() if (configSettings.config) { - const logoKeys = Object.keys(configSettings.config) + const logoKeys = Object.keys(configSettings.config) - logoKeys.map(logoKey => { - const logoUrl = configSettings.config[logoKey] - iconDropdownOptions.unshift({ - label: logoKey, - value: logoKey, - icon: logoUrl, + logoKeys.map(logoKey => { + const logoUrl = configSettings.config[logoKey] + iconDropdownOptions.unshift({ + label: logoKey, + value: logoKey, + icon: logoUrl, + }) }) - }) - } + } const oidcResponse = await api.get(`/api/admin/configs/${ConfigTypes.OIDC}`) const oidcDoc = await oidcResponse.json() From ec93d154627c9ec84ec6004bd1bd669278dddca7 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 13:04:04 +0100 Subject: [PATCH 15/57] Integrate with configuration ui / support for email usernames --- packages/auth/src/middleware/passport/oidc.js | 61 +++++++++++-------- .../worker/src/api/controllers/admin/auth.js | 10 ++- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/packages/auth/src/middleware/passport/oidc.js b/packages/auth/src/middleware/passport/oidc.js index e695e085bf..69bf58243b 100644 --- a/packages/auth/src/middleware/passport/oidc.js +++ b/packages/auth/src/middleware/passport/oidc.js @@ -2,24 +2,6 @@ const fetch = require("node-fetch") const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy const { authenticateThirdParty } = require("./third-party-common") -/** - * @param {*} profile The structured profile created by passport using the user info endpoint - * @param {*} jwtClaims The claims returned in the id token - */ -function getEmail(profile, jwtClaims) { - // profile not guaranteed to contain email e.g. github connected azure ad account - if (profile._json.email) { - return profile._json.email - } - - // fallback to id token - if (jwtClaims.email) { - return jwtClaims.email - } - - return null; -} - /** * @param {*} issuer The identity provider base URL * @param {*} sub The user ID @@ -62,25 +44,52 @@ async function authenticate( done) } +/** + * @param {*} profile The structured profile created by passport using the user info endpoint + * @param {*} jwtClaims The claims returned in the id token + */ +function getEmail(profile, jwtClaims) { + // profile not guaranteed to contain email e.g. github connected azure ad account + if (profile._json.email) { + return profile._json.email + } + + // fallback to id token email + if (jwtClaims.email) { + return jwtClaims.email + } + + // fallback to id token preferred username + const username = jwtClaims.preferred_username + if (username && validEmail(username)) { + return username + } + + return null; +} + +function validEmail(value) { + return ( + (value && !!value.match(/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/)) + ) +} + /** * Create an instance of the oidc passport strategy. This wrapper fetches the configuration * from couchDB rather than environment variables, using this factory is necessary for dynamically configuring passport. * @returns Dynamically configured Passport OIDC Strategy */ -exports.strategyFactory = async function (callbackUrl) { +exports.strategyFactory = async function (config, callbackUrl) { try { - const configurationUrl = - "https://login.microsoftonline.com/2668c0dd-7ed2-4db3-b387-05b6f9204a70/v2.0/.well-known/openid-configuration" - const clientSecret = "g-ty~2iW.bo.88xj_QI6~hdc-H8mP2Xbnd" - const clientId = "bed2017b-2f53-42a9-8ef9-e58918935e07" + const { clientId, clientSecret, configUrl } = config - if (!clientId || !clientSecret || !callbackUrl || !configurationUrl) { + if (!clientId || !clientSecret || !callbackUrl || !configUrl) { throw new Error( - "Configuration invalid. Must contain clientID, clientSecret, callbackUrl and configurationUrl" + "Configuration invalid. Must contain clientID, clientSecret, callbackUrl and configUrl" ) } - const response = await fetch(configurationUrl) + const response = await fetch(configUrl) if (!response.ok) { throw new Error(`Unexpected response when fetching openid-configuration: ${response.statusText}`) diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index e6d553bc62..d1d8622d52 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -133,8 +133,16 @@ exports.googleAuth = async (ctx, next) => { } async function oidcStrategyFactory(ctx) { + const db = new CouchDB(GLOBAL_DB) + + const config = await authPkg.db.getScopedConfig(db, { + type: Configs.OIDC, + group: ctx.query.group, + }) + const callbackUrl = `${ctx.protocol}://${ctx.host}/api/admin/auth/oidc/callback` - return oidc.strategyFactory(callbackUrl) + + return oidc.strategyFactory(config, callbackUrl) } /** From c16cfc328f00fcffb6bd30b2cf5e6e86f8a07430 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 13:12:25 +0100 Subject: [PATCH 16/57] Linting --- .../auth/src/middleware/passport/google.js | 9 +- packages/auth/src/middleware/passport/oidc.js | 24 +-- .../middleware/passport/third-party-common.js | 140 ++++++++++-------- .../auth/src/middleware/passport/utils.js | 18 +-- .../builder/portal/manage/auth/index.svelte | 20 +-- .../worker/src/api/controllers/admin/auth.js | 4 +- 6 files changed, 115 insertions(+), 100 deletions(-) diff --git a/packages/auth/src/middleware/passport/google.js b/packages/auth/src/middleware/passport/google.js index a20912d973..4c21063a1e 100644 --- a/packages/auth/src/middleware/passport/google.js +++ b/packages/auth/src/middleware/passport/google.js @@ -11,14 +11,15 @@ async function authenticate(accessToken, refreshToken, profile, done) { email: profile._json.email, oauth2: { accessToken: accessToken, - refreshToken: refreshToken - } + refreshToken: refreshToken, + }, } return authenticateThirdParty( - thirdPartyUser, + thirdPartyUser, true, // require local accounts to exist - done) + done + ) } /** diff --git a/packages/auth/src/middleware/passport/oidc.js b/packages/auth/src/middleware/passport/oidc.js index 69bf58243b..356d73c022 100644 --- a/packages/auth/src/middleware/passport/oidc.js +++ b/packages/auth/src/middleware/passport/oidc.js @@ -12,7 +12,6 @@ const { authenticateThirdParty } = require("./third-party-common") * @param {*} idToken The id_token - always a JWT * @param {*} params The response body from requesting an access_token * @param {*} done The passport callback: err, user, info - * @returns */ async function authenticate( issuer, @@ -27,21 +26,22 @@ async function authenticate( ) { const thirdPartyUser = { // store the issuer info to enable sync in future - provider: issuer, + provider: issuer, providerType: "oidc", userId: profile.id, profile: profile, email: getEmail(profile, jwtClaims), oauth2: { accessToken: accessToken, - refreshToken: refreshToken - } + refreshToken: refreshToken, + }, } return authenticateThirdParty( - thirdPartyUser, + thirdPartyUser, false, // don't require local accounts to exist - done) + done + ) } /** @@ -65,12 +65,15 @@ function getEmail(profile, jwtClaims) { return username } - return null; + return null } function validEmail(value) { return ( - (value && !!value.match(/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/)) + value && + !!value.match( + /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ + ) ) } @@ -92,7 +95,9 @@ exports.strategyFactory = async function (config, callbackUrl) { const response = await fetch(configUrl) if (!response.ok) { - throw new Error(`Unexpected response when fetching openid-configuration: ${response.statusText}`) + throw new Error( + `Unexpected response when fetching openid-configuration: ${response.statusText}` + ) } const body = await response.json() @@ -110,7 +115,6 @@ exports.strategyFactory = async function (config, callbackUrl) { }, authenticate ) - } catch (err) { console.error(err) throw new Error("Error constructing OIDC authentication strategy", err) diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js index 731aacf75f..64f5f1ec3b 100644 --- a/packages/auth/src/middleware/passport/third-party-common.js +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -9,73 +9,83 @@ const { const { authError } = require("./utils") /** - * Common authentication logic for third parties. e.g. OAuth, OIDC. + * Common authentication logic for third parties. e.g. OAuth, OIDC. */ exports.authenticateThirdParty = async function ( - thirdPartyUser, - requireLocalAccount = true, - done - ) { - if (!thirdPartyUser.provider) return authError(done, "third party user provider required") - if (!thirdPartyUser.userId) return authError(done, "third party user id required") - if (!thirdPartyUser.email) return authError(done, "third party user email required") - - const db = database.getDB(StaticDatabases.GLOBAL.name) - - let dbUser - - // use the third party id - const userId = generateGlobalUserID(thirdPartyUser.userId) - - try { - dbUser = await db.get(userId) - } catch (err) { - // abort when not 404 error - if (!err.status || err.status !== 404) { - return authError(done, "Unexpected error when retrieving existing user", err) - } - - // check user already exists by email - const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { - key: thirdPartyUser.email, - include_docs: true, - }) - const userExists = users.rows.length > 0 + thirdPartyUser, + requireLocalAccount = true, + done +) { + if (!thirdPartyUser.provider) + return authError(done, "third party user provider required") + if (!thirdPartyUser.userId) + return authError(done, "third party user id required") + if (!thirdPartyUser.email) + return authError(done, "third party user email required") - if (requireLocalAccount && !userExists) { - return authError(done, "Email does not yet exist. You must set up your local budibase account first.") - } + const db = database.getDB(StaticDatabases.GLOBAL.name) - // create the user to save - let user - if (userExists) { - const existing = users.rows[0].doc - user = constructMergedUser(userId, existing, thirdPartyUser) - - // remove the local account to avoid conflicts - await db.remove(existing._id, existing._rev) - } else { - user = constructNewUser(userId, thirdPartyUser) - } + let dbUser - // save the user - const response = await db.post(user) - dbUser = user - dbUser._rev = response.rev + // use the third party id + const userId = generateGlobalUserID(thirdPartyUser.userId) + + try { + dbUser = await db.get(userId) + } catch (err) { + // abort when not 404 error + if (!err.status || err.status !== 404) { + return authError( + done, + "Unexpected error when retrieving existing user", + err + ) } - - // authenticate - const payload = { - userId: dbUser._id, - builder: dbUser.builder, - email: dbUser.email, - } - - dbUser.token = jwt.sign(payload, env.JWT_SECRET, { - expiresIn: "1 day", + + // check user already exists by email + const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { + key: thirdPartyUser.email, + include_docs: true, }) - - return done(null, dbUser) + const userExists = users.rows.length > 0 + + if (requireLocalAccount && !userExists) { + return authError( + done, + "Email does not yet exist. You must set up your local budibase account first." + ) + } + + // create the user to save + let user + if (userExists) { + const existing = users.rows[0].doc + user = constructMergedUser(userId, existing, thirdPartyUser) + + // remove the local account to avoid conflicts + await db.remove(existing._id, existing._rev) + } else { + user = constructNewUser(userId, thirdPartyUser) + } + + // save the user + const response = await db.post(user) + dbUser = user + dbUser._rev = response.rev + } + + // authenticate + const payload = { + userId: dbUser._id, + builder: dbUser.builder, + email: dbUser.email, + } + + dbUser.token = jwt.sign(payload, env.JWT_SECRET, { + expiresIn: "1 day", + }) + + return done(null, dbUser) } /** @@ -101,23 +111,23 @@ function constructNewUser(userId, thirdPartyUser) { provider: thirdPartyUser.provider, providerType: thirdPartyUser.providerType, email: thirdPartyUser.email, - roles: {} + roles: {}, } // persist profile information // @reviewers: Historically stored at the root level of the user // Nest to prevent conflicts with future fields - // Is this okay to change? + // Is this okay to change? if (thirdPartyUser.profile) { user.thirdPartyProfile = { - ...thirdPartyUser.profile._json + ...thirdPartyUser.profile._json, } } - // persist oauth tokens for future use + // persist oauth tokens for future use if (thirdPartyUser.oauth2) { user.oauth2 = { - ...thirdPartyUser.oauth2 + ...thirdPartyUser.oauth2, } } diff --git a/packages/auth/src/middleware/passport/utils.js b/packages/auth/src/middleware/passport/utils.js index 1f7b4437bd..cbb93bfa3b 100644 --- a/packages/auth/src/middleware/passport/utils.js +++ b/packages/auth/src/middleware/passport/utils.js @@ -1,14 +1,14 @@ /** - * Utility to handle authentication errors. - * - * @param {*} done The passport callback. + * Utility to handle authentication errors. + * + * @param {*} done The passport callback. * @param {*} message Message that will be returned in the response body * @param {*} err (Optional) error that will be logged */ exports.authError = function (done, message, err = null) { - return done( - err, - null, // never return a user - { message: message } - ) -} \ No newline at end of file + return done( + err, + null, // never return a user + { message: message } + ) +} diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 3f4b39bd4e..3837699e6a 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -85,7 +85,7 @@ let fileName = e.target.files[0].name image = e.target.files[0] providers.oidc.config["iconName"] = fileName - iconDropdownOptions.unshift({label: fileName, value: fileName}) + iconDropdownOptions.unshift({ label: fileName, value: fileName }) } const providers = { google, oidc } @@ -140,17 +140,17 @@ const configSettings = await res.json() if (configSettings.config) { - const logoKeys = Object.keys(configSettings.config) + const logoKeys = Object.keys(configSettings.config) - logoKeys.map(logoKey => { - const logoUrl = configSettings.config[logoKey] - iconDropdownOptions.unshift({ - label: logoKey, - value: logoKey, - icon: logoUrl, + logoKeys.map(logoKey => { + const logoUrl = configSettings.config[logoKey] + iconDropdownOptions.unshift({ + label: logoKey, + value: logoKey, + icon: logoUrl, + }) }) - }) - } + } const oidcResponse = await api.get(`/api/admin/configs/${ConfigTypes.OIDC}`) const oidcDoc = await oidcResponse.json() diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index d1d8622d52..417fdfdc26 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -14,14 +14,14 @@ const GLOBAL_DB = authPkg.StaticDatabases.GLOBAL.name function authInternal(ctx, user, err = null, info = null) { if (err) { console.error("Authentication error", err) - return ctx.throw(403, info? info : "Unauthorized") + return ctx.throw(403, info ? info : "Unauthorized") } const expires = new Date() expires.setDate(expires.getDate() + 1) if (!user) { - return ctx.throw(403, info? info : "Unauthorized") + return ctx.throw(403, info ? info : "Unauthorized") } ctx.cookies.set(Cookies.Auth, user.token, { From 1c18b4c6ac5664004d0a1a8b9e70c20f31383278 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 15:21:54 +0100 Subject: [PATCH 17/57] remove duplicate scope definition scope can be defined both within the strategy declaration or when invoking passport --- packages/auth/src/middleware/passport/oidc.js | 3 +-- packages/worker/src/api/controllers/admin/auth.js | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/middleware/passport/oidc.js b/packages/auth/src/middleware/passport/oidc.js index 356d73c022..6b39a0b20e 100644 --- a/packages/auth/src/middleware/passport/oidc.js +++ b/packages/auth/src/middleware/passport/oidc.js @@ -110,8 +110,7 @@ exports.strategyFactory = async function (config, callbackUrl) { userInfoURL: body.userinfo_endpoint, clientID: clientId, clientSecret: clientSecret, - callbackURL: callbackUrl, - scope: "profile email", + callbackURL: callbackUrl }, authenticate ) diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index 417fdfdc26..b5c60c764c 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -153,6 +153,7 @@ exports.oidcPreAuth = async (ctx, next) => { const strategy = await oidcStrategyFactory(ctx) return passport.authenticate(strategy, { + // required 'openid' scope is added by oidc strategy factory scope: ["profile", "email"], })(ctx, next) } From faf711e092e178aaac8e5b4363c5d71820f6b632 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 16:11:48 +0100 Subject: [PATCH 18/57] sync third party profile on every login --- .../middleware/passport/third-party-common.js | 74 ++++++++++++------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js index 64f5f1ec3b..b8cb7375d2 100644 --- a/packages/auth/src/middleware/passport/third-party-common.js +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -30,6 +30,7 @@ exports.authenticateThirdParty = async function ( // use the third party id const userId = generateGlobalUserID(thirdPartyUser.userId) + // try to load by id try { dbUser = await db.get(userId) } catch (err) { @@ -41,39 +42,45 @@ exports.authenticateThirdParty = async function ( err ) } + } - // check user already exists by email + // fallback to loading by email + if (!dbUser) { const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, { key: thirdPartyUser.email, include_docs: true, }) - const userExists = users.rows.length > 0 - if (requireLocalAccount && !userExists) { + if (users.rows.length > 0) { + dbUser = users.rows[0].doc + } + } + + // exit early if there is still no user and auto creation is disabled + if (!dbUser && requireLocalAccount ) { + if (requireLocalAccount) { return authError( done, "Email does not yet exist. You must set up your local budibase account first." ) } - - // create the user to save - let user - if (userExists) { - const existing = users.rows[0].doc - user = constructMergedUser(userId, existing, thirdPartyUser) - - // remove the local account to avoid conflicts - await db.remove(existing._id, existing._rev) - } else { - user = constructNewUser(userId, thirdPartyUser) - } - - // save the user - const response = await db.post(user) - dbUser = user - dbUser._rev = response.rev } + let user + // first time creation + if (!dbUser) { + user = constructNewUser(userId, thirdPartyUser) + } else { + // existing user + user = constructMergedUser(userId, dbUser, thirdPartyUser) + await db.remove(dbUser._id, dbUser._rev) + } + + // create or sync the user + const response = await db.post(user) + dbUser = user + dbUser._rev = response.rev + // authenticate const payload = { userId: dbUser._id, @@ -92,9 +99,10 @@ exports.authenticateThirdParty = async function ( * @returns a user object constructed from existing and third party information */ function constructMergedUser(userId, existing, thirdPartyUser) { + // sync third party fields const user = constructNewUser(userId, thirdPartyUser) - // merge with existing account + // merge existing fields user.roles = existing.roles user.builder = existing.builder user.admin = existing.admin @@ -114,13 +122,27 @@ function constructNewUser(userId, thirdPartyUser) { roles: {}, } - // persist profile information - // @reviewers: Historically stored at the root level of the user - // Nest to prevent conflicts with future fields - // Is this okay to change? if (thirdPartyUser.profile) { + const profile = thirdPartyUser.profile + + if (profile.name) { + const name = profile.name + // first name + if (name.givenName) { + user.firstName = name.givenName + } + // last name + if (name.familyName) { + user.lastName = name.familyName + } + } + + // profile + // @reviewers: Historically stored at the root level of the user + // Nest to prevent conflicts with future fields + // Is this okay to change? user.thirdPartyProfile = { - ...thirdPartyUser.profile._json, + ...profile._json, } } From 20b70a0445c511af6094b798da4950de0a955f1c Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 16:49:07 +0100 Subject: [PATCH 19/57] Always maintain original user id. No longer remove old user during sync --- .../middleware/passport/third-party-common.js | 48 +++++++------------ 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js index b8cb7375d2..c11465ec3b 100644 --- a/packages/auth/src/middleware/passport/third-party-common.js +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -66,19 +66,19 @@ exports.authenticateThirdParty = async function ( } } - let user // first time creation if (!dbUser) { - user = constructNewUser(userId, thirdPartyUser) - } else { - // existing user - user = constructMergedUser(userId, dbUser, thirdPartyUser) - await db.remove(dbUser._id, dbUser._rev) + // setup a blank user using the third party id + dbUser = { + _id: userId, + roles: {}, + } } + dbUser = syncUser(dbUser, thirdPartyUser) + // create or sync the user - const response = await db.post(user) - dbUser = user + const response = await db.post(dbUser) dbUser._rev = response.rev // authenticate @@ -96,31 +96,15 @@ exports.authenticateThirdParty = async function ( } /** - * @returns a user object constructed from existing and third party information + * @returns a user that has been sync'd with third party information */ -function constructMergedUser(userId, existing, thirdPartyUser) { - // sync third party fields - const user = constructNewUser(userId, thirdPartyUser) +function syncUser(user, thirdPartyUser) { + // provider + user.provider = thirdPartyUser.provider + user.providerType = thirdPartyUser.providerType - // merge existing fields - user.roles = existing.roles - user.builder = existing.builder - user.admin = existing.admin - - return user -} - -/** - * @returns a user object constructed from third party information - */ -function constructNewUser(userId, thirdPartyUser) { - const user = { - _id: userId, - provider: thirdPartyUser.provider, - providerType: thirdPartyUser.providerType, - email: thirdPartyUser.email, - roles: {}, - } + // email + user.email = thirdPartyUser.email if (thirdPartyUser.profile) { const profile = thirdPartyUser.profile @@ -146,7 +130,7 @@ function constructNewUser(userId, thirdPartyUser) { } } - // persist oauth tokens for future use + // oauth tokens for future use if (thirdPartyUser.oauth2) { user.oauth2 = { ...thirdPartyUser.oauth2, From a98ae5b3674cbce5fe4a8a4b940ecd195c315aa9 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Fri, 9 Jul 2021 09:37:52 +0100 Subject: [PATCH 20/57] Remove review comment --- packages/auth/src/middleware/passport/third-party-common.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/auth/src/middleware/passport/third-party-common.js b/packages/auth/src/middleware/passport/third-party-common.js index 82083236ca..c25af011bd 100644 --- a/packages/auth/src/middleware/passport/third-party-common.js +++ b/packages/auth/src/middleware/passport/third-party-common.js @@ -125,9 +125,6 @@ function syncUser(user, thirdPartyUser) { } // profile - // @reviewers: Historically stored at the root level of the user - // Nest to prevent conflicts with future fields - // Is this okay to change? user.thirdPartyProfile = { ...profile._json, } From bcfbca0f085aeb033efb464b23a4823c896d003c Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 9 Jul 2021 09:49:16 +0100 Subject: [PATCH 21/57] Add oidc icon and name to public api for login page --- .../src/api/controllers/admin/configs.js | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/worker/src/api/controllers/admin/configs.js b/packages/worker/src/api/controllers/admin/configs.js index d83be15667..8774dd488d 100644 --- a/packages/worker/src/api/controllers/admin/configs.js +++ b/packages/worker/src/api/controllers/admin/configs.js @@ -102,13 +102,23 @@ exports.publicSettings = async function (ctx) { const db = new CouchDB(GLOBAL_DB) try { // Find the config with the most granular scope based on context - const config = await getScopedFullConfig(db, { + const publicConfig = await getScopedFullConfig(db, { type: Configs.SETTINGS, }) - if (!config) { + + // Pull out the OIDC icon and name because the other properties shouldn't + // be made public + const oidcConfig = await getScopedFullConfig(db, { + type: Configs.OIDC, + }) + if (!publicConfig) { ctx.body = {} } else { - ctx.body = config + ctx.body = publicConfig + if (oidcConfig.config) { + publicConfig.config.oidcIcon = oidcConfig.config.iconName + publicConfig.config.oidcName = oidcConfig.config.name + } } } catch (err) { ctx.throw(err.status, err) @@ -122,12 +132,8 @@ exports.upload = async function (ctx) { const file = ctx.request.files.file const { type, name } = ctx.params - const fileExtension = [...file.name.split(".")].pop() - // filenames converted to UUIDs so they are unique - const processedFileName = `${name}.${fileExtension}` - const bucket = ObjectStoreBuckets.GLOBAL - const key = `${type}/${processedFileName}` + const key = `${type}/${name}` await upload({ bucket, filename: key, From 005e0a2deb7b12bac8f3e2b0906ff0760f10a723 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 9 Jul 2021 09:49:34 +0100 Subject: [PATCH 22/57] Update login page to support user based oidc icon and name --- .../auth/_components/OIDCButton.svelte | 32 +++++++++++++++---- .../src/pages/builder/auth/login.svelte | 2 +- .../builder/portal/manage/auth/index.svelte | 3 +- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte b/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte index fbd67a6437..e03faa0208 100644 --- a/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte +++ b/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte @@ -1,14 +1,33 @@ {#if show} - window.open("/api/admin/auth/oidc", "_blank")}> + window.open("/api/admin/auth/oidc", "_blank")} + >
-

Sign in with OIDC

+ oidc icon +

{`Sign in with ${oidcName || 'OIDC'}`}

 {/if} @@ -22,11 +41,12 @@ padding-top: var(--spacing-xs); padding-bottom: var(--spacing-xs); } - /* .inner img { + .inner img { width: 18px; margin: 3px 10px 3px 3px; - } */ + } .inner p { margin: 0; } + diff --git a/packages/builder/src/pages/builder/auth/login.svelte b/packages/builder/src/pages/builder/auth/login.svelte index 3850431a0f..d51134d13c 100644 --- a/packages/builder/src/pages/builder/auth/login.svelte +++ b/packages/builder/src/pages/builder/auth/login.svelte @@ -62,7 +62,7 @@ Sign in to {company} - + Sign in with email diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 3837699e6a..051b7bc2f8 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -16,7 +16,6 @@ Input, Body, Select, - Dropzone, } from "@budibase/bbui" import { onMount } from "svelte" import api from "builderStore/api" @@ -55,7 +54,7 @@ let iconDropdownOptions = [ { label: "Azure AD", - value: "Active Directory", + value: "AD", icon: MicrosoftLogo, }, { label: "Oracle", value: "Oracle", icon: OracleLogo }, From 7138070e4c84c661b7a5c218180fe7640f2d4c81 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Fri, 9 Jul 2021 14:18:03 +0100 Subject: [PATCH 23/57] Fix bug where OIDC icon was not being displayed due to misconfiguration --- packages/bbui/src/Form/Core/Picker.svelte | 1 - .../auth/_components/OIDCButton.svelte | 1 - .../builder/portal/manage/auth/index.svelte | 6 ++--- .../src/api/controllers/admin/configs.js | 23 +++++++++++++------ .../worker/src/api/routes/admin/configs.js | 12 ++++------ 5 files changed, 23 insertions(+), 20 deletions(-) diff --git a/packages/bbui/src/Form/Core/Picker.svelte b/packages/bbui/src/Form/Core/Picker.svelte index cd8f784b14..4705f6f0d8 100644 --- a/packages/bbui/src/Form/Core/Picker.svelte +++ b/packages/bbui/src/Form/Core/Picker.svelte @@ -31,7 +31,6 @@ } open = true } - console.log(fieldIcon) +
+ + To allow users to authenticate using their Google accounts, fill out the @@ -263,17 +299,39 @@ {/each} +
+
+ + + + +
+
{/if} {#if providers.oidc} - +
OpenID Connect - - +
+
+ +
+
+
To allow users to authenticate using OIDC, fill out the fields below. @@ -313,15 +371,31 @@ bind:this={fileinput} /> +
+
+ + + + +
+
{/if} -
- -
diff --git a/packages/worker/src/api/controllers/admin/configs.js b/packages/worker/src/api/controllers/admin/configs.js index fe7c97f1e9..7dfb5b75be 100644 --- a/packages/worker/src/api/controllers/admin/configs.js +++ b/packages/worker/src/api/controllers/admin/configs.js @@ -149,9 +149,16 @@ exports.publicSettings = async function (ctx) { config = publicConfig } - config.config.oidc = !!oidcConfig - config.config.google = !!googleConfig - + config.config.google = !googleConfig + ? !!googleConfig + : !googleConfig.config.activated + ? false + : true + config.config.oidc = !oidcConfig + ? !!oidcConfig + : !oidcConfig.config.configs[0].activated + ? false + : true ctx.body = config } catch (err) { ctx.throw(err.status, err) diff --git a/packages/worker/src/api/routes/admin/configs.js b/packages/worker/src/api/routes/admin/configs.js index 66be9dd13b..6873d82757 100644 --- a/packages/worker/src/api/routes/admin/configs.js +++ b/packages/worker/src/api/routes/admin/configs.js @@ -38,6 +38,7 @@ function googleValidation() { clientID: Joi.string().required(), clientSecret: Joi.string().required(), callbackURL: Joi.string().required(), + activated: Joi.boolean().required(), }).unknown(true) } @@ -52,6 +53,7 @@ function oidcValidation() { logo: Joi.string().allow("", null), name: Joi.string().allow("", null), uuid: Joi.string().required(), + activated: Joi.boolean().required(), }) ).required(true) }).unknown(true) @@ -100,7 +102,7 @@ router buildConfigSaveValidation(), controller.save ) - .delete("/api/admin/configs/:id", adminOnly, controller.destroy) + .delete("/api/admin/configs/:id/:rev", adminOnly, controller.destroy) .get("/api/admin/configs", controller.fetch) .get("/api/admin/configs/checklist", controller.configChecklist) .get( From 30558ad3bde1a9e6d76eb61d160791c03ebb9c7a Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 20 Jul 2021 10:55:39 +0100 Subject: [PATCH 56/57] only enable form save button when config has changed --- .../builder/portal/manage/auth/index.svelte | 49 +++++++------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 7f132b5638..78c0a5a52c 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -6,6 +6,7 @@ import OktaLogo from "assets/okta-logo.png" import OneLoginLogo from "assets/onelogin-logo.png" import OidcLogoPng from "assets/oidc-logo.png" + import { isEqual, cloneDeep } from "lodash/fp" import { Button, @@ -91,18 +92,22 @@ let google let oidc const providers = { google, oidc } + + // control the state of the save button depending on whether form has changed + let originalGoogleDoc + let originalOidcDoc let googleSaveButtonDisabled let oidcSaveButtonDisabled - - $: googleSaveButtonDisabled = - providers.google?.config && !providers.google?.config.activated && true - $: oidcSaveButtonDisabled = - providers.oidc?.config.configs[0] && - !providers.oidc?.config.configs[0].activated && - true + $: { + isEqual(providers.google, originalGoogleDoc) + ? (googleSaveButtonDisabled = true) + : (googleSaveButtonDisabled = false) + isEqual(providers.oidc, originalOidcDoc) + ? (oidcSaveButtonDisabled = true) + : (oidcSaveButtonDisabled = false) + } // Create a flag so that it will only try to save completed forms - $: partialGoogle = providers.google?.config?.clientID || providers.google?.config?.clientSecret || @@ -138,22 +143,6 @@ iconDropdownOptions.unshift({ label: fileName, value: fileName }) } - const toggleGoogle = ({ detail }) => { - // Only save on de-activation, as the user can save themselves when toggle is active - if (!detail) { - providers.google.config.activated = detail - save([providers.google]) - } - } - - const toggleOidc = ({ detail }) => { - // Only save on de-activation, as the user can save themselves when toggle is active - if (!detail) { - providers.oidc.config.configs[0].activated = detail - save([providers.oidc]) - } - } - async function save(docs) { // only if the user has provided an image, upload it. image && uploadLogo(image) @@ -225,6 +214,7 @@ config: { activated: true }, } } else { + originalGoogleDoc = googleDoc providers.google = googleDoc } @@ -247,13 +237,15 @@ } const oidcResponse = await api.get(`/api/admin/configs/${ConfigTypes.OIDC}`) const oidcDoc = await oidcResponse.json() - + console.log("teaste") if (!oidcDoc._id) { providers.oidc = { type: ConfigTypes.OIDC, config: { configs: [{ activated: true }] }, } } else { + console.log("hello") + originalOidcDoc = cloneDeep(oidcDoc) providers.oidc = oidcDoc } }) @@ -303,11 +295,7 @@
- +
@@ -377,7 +365,6 @@ From 1c6d455094480b060dd0c1c5bd50aca0ed7373b8 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 20 Jul 2021 12:30:11 +0100 Subject: [PATCH 57/57] fix save button --- .../builder/portal/manage/auth/index.svelte | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte index 78c0a5a52c..70abf7d376 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -99,10 +99,10 @@ let googleSaveButtonDisabled let oidcSaveButtonDisabled $: { - isEqual(providers.google, originalGoogleDoc) + isEqual(providers.google?.config, originalGoogleDoc?.config) ? (googleSaveButtonDisabled = true) : (googleSaveButtonDisabled = false) - isEqual(providers.oidc, originalOidcDoc) + isEqual(providers.oidc?.config, originalOidcDoc?.config) ? (oidcSaveButtonDisabled = true) : (oidcSaveButtonDisabled = false) } @@ -147,7 +147,6 @@ // only if the user has provided an image, upload it. image && uploadLogo(image) let calls = [] - let completed = [] docs.forEach(element => { if (element.type === ConfigTypes.OIDC) { //Add a UUID here so each config is distinguishable when it arrives at the login page. @@ -161,7 +160,9 @@ ) } else { calls.push(api.post(`/api/admin/configs`, element)) - completed.push(ConfigTypes.OIDC) + // turn the save button grey when clicked + oidcSaveButtonDisabled = true + originalOidcDoc = cloneDeep(providers.oidc) } } } @@ -173,7 +174,8 @@ ) } else { calls.push(api.post(`/api/admin/configs`, element)) - completed.push(ConfigTypes.Google) + googleSaveButtonDisabled = true + originalGoogleDoc = cloneDeep(providers.google) } } } @@ -213,8 +215,9 @@ type: ConfigTypes.Google, config: { activated: true }, } + originalGoogleDoc = cloneDeep(googleDoc) } else { - originalGoogleDoc = googleDoc + originalGoogleDoc = cloneDeep(googleDoc) providers.google = googleDoc } @@ -237,8 +240,9 @@ } const oidcResponse = await api.get(`/api/admin/configs/${ConfigTypes.OIDC}`) const oidcDoc = await oidcResponse.json() - console.log("teaste") if (!oidcDoc._id) { + console.log("hi") + providers.oidc = { type: ConfigTypes.OIDC, config: { configs: [{ activated: true }] },