From bf94125d3319f7853862de2dac50622311945817 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 11:54:16 +0100 Subject: [PATCH 1/3] 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 ec93d154627c9ec84ec6004bd1bd669278dddca7 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 8 Jul 2021 13:04:04 +0100 Subject: [PATCH 2/3] 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 3/3] 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, {