From dc20ecc5ff474cb33128f15dbfee48c78f9eb8d2 Mon Sep 17 00:00:00 2001 From: Dean Date: Thu, 23 Jun 2022 14:29:19 +0100 Subject: [PATCH 1/8] Merge commit --- .vscode/settings.json | 14 ++++- packages/backend-core/package.json | 1 + packages/backend-core/src/auth.js | 53 +++++++++++++++++++ packages/backend-core/src/db/utils.ts | 16 ++++-- .../src/middleware/passport/oidc.js | 42 +++++++++------ .../src/middleware/passport/utils.js | 25 +++++++++ packages/backend-core/yarn.lock | 10 ++++ .../builder/src/builderStore/dataBinding.js | 51 +++++++++++++++++- .../server/src/api/controllers/query/index.ts | 7 ++- packages/server/src/threads/query.js | 7 +++ .../worker/src/api/controllers/global/auth.ts | 11 ++-- 11 files changed, 211 insertions(+), 26 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index d471924fe0..4838a4fd89 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,17 @@ "editor.codeActionsOnSave": { "source.fixAll": true }, - "editor.defaultFormatter": "svelte.svelte-vscode" + "editor.defaultFormatter": "svelte.svelte-vscode", + "[json]": { + "editor.defaultFormatter": "vscode.json-language-features" + }, + "[javascript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "debug.javascript.terminalOptions": { + "skipFiles": [ + "${workspaceFolder}/packages/backend-core/node_modules/**", + "/**" + ] + }, } diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index d69d57b88b..c0f5bf6e02 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -36,6 +36,7 @@ "passport-google-oauth": "2.0.0", "passport-jwt": "4.0.0", "passport-local": "1.0.0", + "passport-oauth2-refresh": "^2.1.0", "posthog-node": "1.3.0", "pouchdb": "7.3.0", "pouchdb-find": "7.2.2", diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index b13cd932c6..58c00c92c3 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -2,6 +2,9 @@ const passport = require("koa-passport") const LocalStrategy = require("passport-local").Strategy const JwtStrategy = require("passport-jwt").Strategy const { getGlobalDB } = require("./tenancy") +const refresh = require("passport-oauth2-refresh") +const { Configs } = require("./constants") +const { getScopedConfig } = require("./db/utils") const { jwt, local, @@ -34,6 +37,55 @@ passport.deserializeUser(async (user, done) => { } }) +//requestAccessStrategy +//refreshOAuthAccessToken + +//configId for google and OIDC?? +async function reUpToken(refreshToken, configId) { + const db = getGlobalDB() + console.log(refreshToken, configId) + const config = await getScopedConfig(db, { + type: Configs.OIDC, + group: {}, //ctx.query.group, this was an empty object when authentication initially + }) + + const chosenConfig = config.configs[0] //.filter((c) => c.uuid === configId)[0] + let callbackUrl = await oidc.oidcCallbackUrl(db, chosenConfig) + + //Remote Config + const enrichedConfig = await oidc.fetchOIDCStrategyConfig( + chosenConfig, + callbackUrl + ) + + const strategy = await oidc.strategyFactory(enrichedConfig, () => { + console.log("saveFn RETURN ARGS", JSON.stringify(arguments)) + }) + + try { + refresh.use(strategy, { + setRefreshOAuth2() { + return strategy._getOAuth2Client(enrichedConfig) + }, + }) + console.log("Testing") + + // By default, the strat calls itself "openidconnect" + + // refresh.requestNewAccessToken( + // 'openidconnect', + // refToken, + // (err, accessToken, refreshToken) => { + // console.log("REAUTH CB", err, accessToken, refreshToken); + // }) + } catch (err) { + console.error(err) + throw new Error("Error constructing OIDC refresh strategy", err) + } + + console.log("end") +} + module.exports = { buildAuthMiddleware: authenticated, passport, @@ -46,4 +98,5 @@ module.exports = { authError, buildCsrfMiddleware: csrf, internalApi, + reUpToken, } diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index dc7a0454c3..ac2c7df345 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -384,7 +384,10 @@ export const getScopedFullConfig = async function ( if (type === Configs.SETTINGS) { if (scopedConfig && scopedConfig.doc) { // overrides affected by environment variables - scopedConfig.doc.config.platformUrl = await getPlatformUrl() + scopedConfig.doc.config.platformUrl = await getPlatformUrl( + { tenantAware: true }, + db + ) scopedConfig.doc.config.analyticsEnabled = await events.analytics.enabled() } else { @@ -393,7 +396,7 @@ export const getScopedFullConfig = async function ( doc: { _id: generateConfigID({ type, user, workspace }), config: { - platformUrl: await getPlatformUrl(), + platformUrl: await getPlatformUrl({ tenantAware: true }, db), analyticsEnabled: await events.analytics.enabled(), }, }, @@ -404,7 +407,10 @@ export const getScopedFullConfig = async function ( return scopedConfig && scopedConfig.doc } -export const getPlatformUrl = async (opts = { tenantAware: true }) => { +export const getPlatformUrl = async ( + opts = { tenantAware: true }, + db = null +) => { let platformUrl = env.PLATFORM_URL || "http://localhost:10000" if (!env.SELF_HOSTED && env.MULTI_TENANCY && opts.tenantAware) { @@ -414,11 +420,11 @@ export const getPlatformUrl = async (opts = { tenantAware: true }) => { platformUrl = platformUrl.replace("://", `://${tenantId}.`) } } else if (env.SELF_HOSTED) { - const db = getGlobalDB() + const dbx = db ? db : getGlobalDB() // get the doc directly instead of with getScopedConfig to prevent loop let settings try { - settings = await db.get(generateConfigID({ type: Configs.SETTINGS })) + settings = await dbx.get(generateConfigID({ type: Configs.SETTINGS })) } catch (e: any) { if (e.status !== 404) { throw e diff --git a/packages/backend-core/src/middleware/passport/oidc.js b/packages/backend-core/src/middleware/passport/oidc.js index 1e93e20b1c..ef785f8aeb 100644 --- a/packages/backend-core/src/middleware/passport/oidc.js +++ b/packages/backend-core/src/middleware/passport/oidc.js @@ -1,6 +1,7 @@ const fetch = require("node-fetch") const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy const { authenticateThirdParty } = require("./third-party-common") +const { ssoCallbackUrl } = require("./utils") const buildVerifyFn = saveUserFn => { /** @@ -89,11 +90,22 @@ function validEmail(value) { * 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 (config, callbackUrl, saveUserFn) { +exports.strategyFactory = async function (config, saveUserFn) { + try { + const verify = buildVerifyFn(saveUserFn) + return new OIDCStrategy(config, verify) + } catch (err) { + console.error(err) + throw new Error("Error constructing OIDC authentication strategy", err) + } +} + +export const fetchOIDCStrategyConfig = async (config, callbackUrl) => { try { const { clientID, clientSecret, configUrl } = config if (!clientID || !clientSecret || !callbackUrl || !configUrl) { + //check for remote config and all required elements throw new Error( "Configuration invalid. Must contain clientID, clientSecret, callbackUrl and configUrl" ) @@ -109,24 +121,24 @@ exports.strategyFactory = async function (config, callbackUrl, saveUserFn) { const body = await response.json() - const verify = buildVerifyFn(saveUserFn) - return new OIDCStrategy( - { - issuer: body.issuer, - authorizationURL: body.authorization_endpoint, - tokenURL: body.token_endpoint, - userInfoURL: body.userinfo_endpoint, - clientID: clientID, - clientSecret: clientSecret, - callbackURL: callbackUrl, - }, - verify - ) + return { + issuer: body.issuer, + authorizationURL: body.authorization_endpoint, + tokenURL: body.token_endpoint, + userInfoURL: body.userinfo_endpoint, + clientID: clientID, + clientSecret: clientSecret, + callbackURL: callbackUrl, + } } catch (err) { console.error(err) - throw new Error("Error constructing OIDC authentication strategy", err) + throw new Error("Error constructing OIDC authentication configuration", err) } } +export const oidcCallbackUrl = async (db, config) => { + return ssoCallbackUrl(db, config, "oidc") +} + // expose for testing exports.buildVerifyFn = buildVerifyFn diff --git a/packages/backend-core/src/middleware/passport/utils.js b/packages/backend-core/src/middleware/passport/utils.js index cbb93bfa3b..ddc87c6cd0 100644 --- a/packages/backend-core/src/middleware/passport/utils.js +++ b/packages/backend-core/src/middleware/passport/utils.js @@ -1,3 +1,7 @@ +const { getGlobalDB, isMultiTenant, getTenantId } = require("../../tenancy") +const { getScopedConfig } = require("../../db/utils") +const { Configs } = require("../../constants") + /** * Utility to handle authentication errors. * @@ -5,6 +9,7 @@ * @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, @@ -12,3 +17,23 @@ exports.authError = function (done, message, err = null) { { message: message } ) } + +exports.ssoCallbackUrl = async (db, config, type) => { + // incase there is a callback URL from before + if (config && config.callbackURL) { + return config.callbackURL + } + + const dbx = db ? db : getGlobalDB() + const publicConfig = await getScopedConfig(dbx, { + type: Configs.SETTINGS, + }) + + let callbackUrl = `/api/global/auth` + if (isMultiTenant()) { + callbackUrl += `/${getTenantId()}` + } + callbackUrl += `/${type}/callback` + + return `${publicConfig.platformUrl}${callbackUrl}` +} diff --git a/packages/backend-core/yarn.lock b/packages/backend-core/yarn.lock index 01d1a43b64..29ba61d327 100644 --- a/packages/backend-core/yarn.lock +++ b/packages/backend-core/yarn.lock @@ -291,6 +291,11 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== +"@budibase/types@^1.0.206": + version "1.0.208" + resolved "https://registry.yarnpkg.com/@budibase/types/-/types-1.0.208.tgz#c45cb494fb5b85229e15a34c6ac1805bae5be867" + integrity sha512-zKIHg6TGK+soVxMNZNrGypP3DCrd3jhlUQEFeQ+rZR6/tCue1G74bjzydY5FjnLEsXeLH1a0hkS5HulTmvQ2bA== + "@istanbuljs/load-nyc-config@^1.0.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz#fd3db1d59ecf7cf121e80650bb86712f9b55eced" @@ -3965,6 +3970,11 @@ passport-oauth1@1.x.x: passport-strategy "1.x.x" utils-merge "1.x.x" +passport-oauth2-refresh@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/passport-oauth2-refresh/-/passport-oauth2-refresh-2.1.0.tgz#c31cd133826383f5539d16ad8ab4f35ca73ce4a4" + integrity sha512-4ML7ooCESCqiTgdDBzNUFTBcPR8zQq9iM6eppEUGMMvLdsjqRL93jKwWm4Az3OJcI+Q2eIVyI8sVRcPFvxcF/A== + passport-oauth2@1.x.x: version "1.6.1" resolved "https://registry.yarnpkg.com/passport-oauth2/-/passport-oauth2-1.6.1.tgz#c5aee8f849ce8bd436c7f81d904a3cd1666f181b" diff --git a/packages/builder/src/builderStore/dataBinding.js b/packages/builder/src/builderStore/dataBinding.js index 8cbc629291..da3269ba81 100644 --- a/packages/builder/src/builderStore/dataBinding.js +++ b/packages/builder/src/builderStore/dataBinding.js @@ -49,6 +49,43 @@ export const getBindableProperties = (asset, componentId) => { ] } +/** + * Gets all rest bindable data fields + */ +export const getRestBindings = () => { + const userBindings = getUserBindings() + const oauthBindings = getAuthBindings() + return [...userBindings, ...oauthBindings] +} + +/** + * Utility - coverting a map of readable bindings to runtime + */ +export const readableToRuntimeMap = (bindings, ctx) => { + if (!bindings || !ctx) { + return {} + } + return Object.keys(ctx).reduce((acc, key) => { + let parsedQuery = readableToRuntimeBinding(bindings, ctx[key]) + acc[key] = parsedQuery + return acc + }, {}) +} + +/** + * Utility - coverting a map of runtime bindings to readable + */ +export const runtimeToReadableMap = (bindings, ctx) => { + if (!bindings || !ctx) { + return {} + } + return Object.keys(ctx).reduce((acc, key) => { + let parsedQuery = runtimeToReadableBinding(bindings, ctx[key]) + acc[key] = parsedQuery + return acc + }, {}) +} + /** * Gets the bindable properties exposed by a certain component. */ @@ -298,10 +335,22 @@ const getUserBindings = () => { providerId: "user", }) }) - return bindings } +const getAuthBindings = () => { + return [ + { + type: "context", + runtimeBinding: `${makePropSafe("user")}.${makePropSafe( + "oauth2" + )}.${makePropSafe("accessToken")}`, + readableBinding: "OAuthToken", + providerId: "user", + }, + ] +} + /** * Gets all device bindings that are globally available. */ diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 2abd83140a..73b508f028 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -8,6 +8,8 @@ import { QUERY_THREAD_TIMEOUT } from "../../../environment" import { getAppDB } from "@budibase/backend-core/context" import { quotas } from "@budibase/pro" import { events } from "@budibase/backend-core" +import { getCookie } from "@budibase/backend-core/utils" +import { Cookies } from "@budibase/backend-core/constants" const Runner = new Thread(ThreadType.QUERY, { timeoutMs: QUERY_THREAD_TIMEOUT || 10000, @@ -119,6 +121,10 @@ export async function preview(ctx: any) { // this stops dynamic variables from calling the same query const { fields, parameters, queryVerb, transformer, queryId } = query + //check for oAuth elements here? + const configId = getCookie(ctx, Cookies.OIDC_CONFIG) + console.log(configId) + try { const runFn = () => Runner.run({ @@ -130,7 +136,6 @@ export async function preview(ctx: any) { transformer, queryId, }) - const { rows, keys, info, extra } = await quotas.addQuery(runFn) await events.query.previewed(datasource, query) ctx.body = { diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js index ec9d9a6fa6..6e6822d6f5 100644 --- a/packages/server/src/threads/query.js +++ b/packages/server/src/threads/query.js @@ -4,6 +4,7 @@ const ScriptRunner = require("../utilities/scriptRunner") const { integrations } = require("../integrations") const { processStringSync } = require("@budibase/string-templates") const { doInAppContext, getAppDB } = require("@budibase/backend-core/context") +// const { reUpToken } = require("@budibase/backend-core/auth") const { isSQL } = require("../integrations/utils") const { enrichQueryFields, @@ -30,6 +31,11 @@ class QueryRunner { async execute() { let { datasource, fields, queryVerb, transformer } = this + + // if(this.ctx.user.oauth2?.refreshToken){ + // reUpToken(this.ctx.user.oauth2?.refreshToken) + // } + const Integration = integrations[datasource.source] if (!Integration) { throw "Integration type does not exist." @@ -79,6 +85,7 @@ class QueryRunner { this.cachedVariables.length > 0 && !this.hasRerun ) { + // return { info } this.hasRerun = true // invalidate the cache value await threadUtils.invalidateDynamicVariables(this.cachedVariables) diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index 896a7c48de..bd399a29fc 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -224,7 +224,7 @@ export const googleAuth = async (ctx: any, next: any) => { )(ctx, next) } -async function oidcStrategyFactory(ctx: any, configId: any) { +export const oidcStrategyFactory = async (ctx: any, configId: any) => { const db = getGlobalDB() const config = await core.db.getScopedConfig(db, { type: Configs.OIDC, @@ -234,7 +234,12 @@ async function oidcStrategyFactory(ctx: any, configId: any) { const chosenConfig = config.configs.filter((c: any) => c.uuid === configId)[0] let callbackUrl = await exports.oidcCallbackUrl(chosenConfig) - return oidc.strategyFactory(chosenConfig, callbackUrl, users.save) + //Remote Config + const enrichedConfig = await oidc.fetchOIDCStrategyConfig( + chosenConfig, + callbackUrl + ) + return oidc.strategyFactory(enrichedConfig, users.save) } /** @@ -249,7 +254,7 @@ export const oidcPreAuth = async (ctx: any, next: any) => { return passport.authenticate(strategy, { // required 'openid' scope is added by oidc strategy factory - scope: ["profile", "email"], + scope: ["profile", "email", "offline_access"], //auth0 offline_access scope required for the refresh token behaviour. })(ctx, next) } From 2ea4a9d22562c6798f4ffcfc4399e1a4cf451806 Mon Sep 17 00:00:00 2001 From: Dean Date: Sun, 3 Jul 2022 21:13:15 +0100 Subject: [PATCH 2/8] Refresh the OAuth tokens automatically when making rest calls. Fix to remove the password from the api token authentication. --- packages/backend-core/src/auth.js | 144 +++++++++++++----- .../src/middleware/authenticated.js | 2 + .../src/middleware/passport/google.js | 18 ++- .../src/middleware/passport/local.js | 16 +- .../src/middleware/passport/oidc.js | 11 +- .../integration/appPublishWorkflow.spec.js | 12 +- .../server/src/api/controllers/query/index.ts | 28 +++- packages/server/src/module.d.ts | 1 + packages/server/src/threads/query.js | 64 ++++++-- .../worker/src/api/controllers/global/auth.ts | 8 +- 10 files changed, 233 insertions(+), 71 deletions(-) diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index 58c00c92c3..9566a646f1 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -37,53 +37,118 @@ passport.deserializeUser(async (user, done) => { } }) -//requestAccessStrategy -//refreshOAuthAccessToken - -//configId for google and OIDC?? -async function reUpToken(refreshToken, configId) { - const db = getGlobalDB() - console.log(refreshToken, configId) - const config = await getScopedConfig(db, { - type: Configs.OIDC, - group: {}, //ctx.query.group, this was an empty object when authentication initially - }) - - const chosenConfig = config.configs[0] //.filter((c) => c.uuid === configId)[0] - let callbackUrl = await oidc.oidcCallbackUrl(db, chosenConfig) - - //Remote Config - const enrichedConfig = await oidc.fetchOIDCStrategyConfig( - chosenConfig, - callbackUrl - ) - - const strategy = await oidc.strategyFactory(enrichedConfig, () => { - console.log("saveFn RETURN ARGS", JSON.stringify(arguments)) - }) +async function refreshOIDCAccessToken(db, chosenConfig, refreshToken) { + const callbackUrl = await oidc.getCallbackUrl(db, chosenConfig) + let enrichedConfig + let strategy try { - refresh.use(strategy, { - setRefreshOAuth2() { - return strategy._getOAuth2Client(enrichedConfig) - }, - }) - console.log("Testing") + enrichedConfig = await oidc.fetchStrategyConfig(chosenConfig, callbackUrl) + if (!enrichedConfig) { + throw new Error("OIDC Config contents invalid") + } + strategy = await oidc.strategyFactory(enrichedConfig) + } catch (err) { + console.error(err) + throw new Error("Could not refresh OAuth Token") + } - // By default, the strat calls itself "openidconnect" + refresh.use(strategy, { + setRefreshOAuth2() { + return strategy._getOAuth2Client(enrichedConfig) + }, + }) - // refresh.requestNewAccessToken( - // 'openidconnect', - // refToken, - // (err, accessToken, refreshToken) => { - // console.log("REAUTH CB", err, accessToken, refreshToken); - // }) + return new Promise(resolve => { + refresh.requestNewAccessToken( + Configs.OIDC, + refreshToken, + (err, accessToken, refreshToken, params) => { + resolve({ err, accessToken, refreshToken, params }) + } + ) + }) +} + +async function refreshGoogleAccessToken(db, config, refreshToken) { + let callbackUrl = await google.getCallbackUrl(db, config) + const googleConfig = await google.fetchStrategyConfig(config) + + let strategy + try { + strategy = await google.strategyFactory(googleConfig, callbackUrl) } catch (err) { console.error(err) throw new Error("Error constructing OIDC refresh strategy", err) } - console.log("end") + refresh.use(strategy) + + return new Promise(resolve => { + refresh.requestNewAccessToken( + Configs.GOOGLE, + refreshToken, + (err, accessToken, refreshToken, params) => { + resolve({ err, accessToken, refreshToken, params }) + } + ) + }) +} + +async function refreshOAuthToken(refreshToken, configType, configId) { + const db = getGlobalDB() + + const config = await getScopedConfig(db, { + type: configType, + group: {}, + }) + + let chosenConfig = {} + let refreshResponse + if (configType === Configs.OIDC) { + // configId - retrieved from cookie. + chosenConfig = config.configs.filter(c => c.uuid === configId)[0] + if (!chosenConfig) { + throw new Error("Invalid OIDC configuration") + } + refreshResponse = await refreshOIDCAccessToken( + db, + chosenConfig, + refreshToken + ) + } else { + chosenConfig = config + refreshResponse = await refreshGoogleAccessToken( + db, + chosenConfig, + refreshToken + ) + } + + console.log(JSON.stringify(refreshResponse)) + return refreshResponse +} + +async function updateUserOAuth(userId, oAuthConfig) { + const details = { ...oAuthConfig } + try { + const db = getGlobalDB() + const dbUser = db.get(userId) + + //Do not overwrite the refresh token if a valid one is not provided. + if (typeof details.refreshToken !== "string") { + delete details.refreshToken + } + + dbUser.oAuth2 = { + ...dbUser.oAuth2, + ...details, + } + + await db.put(dbUser) + } catch (e) { + console.error("Could not update OAuth details for current user", e) + } } module.exports = { @@ -98,5 +163,6 @@ module.exports = { authError, buildCsrfMiddleware: csrf, internalApi, - reUpToken, + refreshOAuthToken, + updateUserOAuth, } diff --git a/packages/backend-core/src/middleware/authenticated.js b/packages/backend-core/src/middleware/authenticated.js index 9c35336dda..cca80cc511 100644 --- a/packages/backend-core/src/middleware/authenticated.js +++ b/packages/backend-core/src/middleware/authenticated.js @@ -128,6 +128,8 @@ module.exports = ( } if (!user && tenantId) { user = { tenantId } + } else { + delete user.password } // be explicit if (authenticated !== true) { diff --git a/packages/backend-core/src/middleware/passport/google.js b/packages/backend-core/src/middleware/passport/google.js index 858029ca80..167e719203 100644 --- a/packages/backend-core/src/middleware/passport/google.js +++ b/packages/backend-core/src/middleware/passport/google.js @@ -1,6 +1,8 @@ const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy - +const { ssoCallbackUrl } = require("./utils") const { authenticateThirdParty } = require("./third-party-common") +const { Configs } = require("../../../constants") +const environment = require("../../environment") const buildVerifyFn = saveUserFn => { return (accessToken, refreshToken, profile, done) => { @@ -57,5 +59,19 @@ exports.strategyFactory = async function (config, callbackUrl, saveUserFn) { ) } } + +exports.fetchStrategyConfig = async function (googleConfig) { + return ( + googleConfig || { + clientID: environment.GOOGLE_CLIENT_ID, + clientSecret: environment.GOOGLE_CLIENT_SECRET, + } + ) +} + +exports.getCallbackUrl = async function (db, config) { + return ssoCallbackUrl(db, config, Configs.GOOGLE) +} + // expose for testing exports.buildVerifyFn = buildVerifyFn diff --git a/packages/backend-core/src/middleware/passport/local.js b/packages/backend-core/src/middleware/passport/local.js index 445893b1df..3f224e7abd 100644 --- a/packages/backend-core/src/middleware/passport/local.js +++ b/packages/backend-core/src/middleware/passport/local.js @@ -6,7 +6,7 @@ const users = require("../../users") const { authError } = require("./utils") const { newid } = require("../../hashing") const { createASession } = require("../../security/sessions") -const { getTenantId } = require("../../tenancy") +const { getTenantId, getGlobalDB } = require("../../tenancy") const INVALID_ERR = "Invalid credentials" const SSO_NO_PASSWORD = "SSO user does not have a password set" @@ -55,6 +55,20 @@ exports.authenticate = async function (ctx, email, password, done) { if (await compare(password, dbUser.password)) { const sessionId = newid() const tenantId = getTenantId() + + if (dbUser.provider || dbUser.providerType || dbUser.pictureUrl) { + delete dbUser.provider + delete dbUser.providerType + delete dbUser.pictureUrl + + try { + const db = getGlobalDB() + await db.put(dbUser) + } catch (err) { + console.error("OAuth elements could not be purged") + } + } + await createASession(dbUser._id, { sessionId, tenantId }) dbUser.token = jwt.sign( diff --git a/packages/backend-core/src/middleware/passport/oidc.js b/packages/backend-core/src/middleware/passport/oidc.js index ef785f8aeb..570ce641cf 100644 --- a/packages/backend-core/src/middleware/passport/oidc.js +++ b/packages/backend-core/src/middleware/passport/oidc.js @@ -2,6 +2,7 @@ const fetch = require("node-fetch") const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy const { authenticateThirdParty } = require("./third-party-common") const { ssoCallbackUrl } = require("./utils") +const { Configs } = require("../../../constants") const buildVerifyFn = saveUserFn => { /** @@ -93,14 +94,16 @@ function validEmail(value) { exports.strategyFactory = async function (config, saveUserFn) { try { const verify = buildVerifyFn(saveUserFn) - return new OIDCStrategy(config, verify) + const strategy = new OIDCStrategy(config, verify) + strategy.name = "oidc" + return strategy } catch (err) { console.error(err) throw new Error("Error constructing OIDC authentication strategy", err) } } -export const fetchOIDCStrategyConfig = async (config, callbackUrl) => { +exports.fetchStrategyConfig = async function (config, callbackUrl) { try { const { clientID, clientSecret, configUrl } = config @@ -136,8 +139,8 @@ export const fetchOIDCStrategyConfig = async (config, callbackUrl) => { } } -export const oidcCallbackUrl = async (db, config) => { - return ssoCallbackUrl(db, config, "oidc") +exports.getCallbackUrl = async function (db, config) { + return ssoCallbackUrl(db, config, Configs.OIDC) } // expose for testing diff --git a/packages/builder/cypress/integration/appPublishWorkflow.spec.js b/packages/builder/cypress/integration/appPublishWorkflow.spec.js index a431051075..e65c01c1b6 100644 --- a/packages/builder/cypress/integration/appPublishWorkflow.spec.js +++ b/packages/builder/cypress/integration/appPublishWorkflow.spec.js @@ -85,12 +85,12 @@ filterTests(['all'], () => { cy.get(interact.APP_TABLE_APP_NAME).click({ force: true }) }) - cy.get(interact.DEPLOYMENT_TOP_NAV).click() - cy.get(interact.PUBLISH_POPOVER_ACTION).click({ force: true }) - cy.get(interact.UNPUBLISH_MODAL) - .within(() => { - cy.get(interact.CONFIRM_WRAP_BUTTON).click({ force: true }) - }) + cy.get(interact.DEPLOYMENT_TOP_GLOBE).should("exist").click({ force: true }) + + cy.get("[data-cy='publish-popover-menu']") + .within(() => { + cy.get(interact.PUBLISH_POPOVER_ACTION).click({ force: true }) + }) cy.get(interact.UNPUBLISH_MODAL).should("be.visible") .within(() => { diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 73b508f028..528079be83 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -9,7 +9,7 @@ import { getAppDB } from "@budibase/backend-core/context" import { quotas } from "@budibase/pro" import { events } from "@budibase/backend-core" import { getCookie } from "@budibase/backend-core/utils" -import { Cookies } from "@budibase/backend-core/constants" +import { Cookies, Configs } from "@budibase/backend-core/constants" const Runner = new Thread(ThreadType.QUERY, { timeoutMs: QUERY_THREAD_TIMEOUT || 10000, @@ -112,6 +112,21 @@ export async function find(ctx: any) { ctx.body = query } +//Required to discern between OIDC OAuth config entries +function getOAuthConfigCookieId(ctx: any) { + if (ctx.user.providerType === Configs.OIDC) { + return getCookie(ctx, Cookies.OIDC_CONFIG) + } +} + +function getAuthConfig(ctx: any) { + const authCookie = getCookie(ctx, Cookies.Auth) + let authConfigCtx: any = {} + authConfigCtx["configId"] = getOAuthConfigCookieId(ctx) + authConfigCtx["sessionId"] = authCookie ? authCookie.sessionId : null + return authConfigCtx +} + export async function preview(ctx: any) { const db = getAppDB() @@ -121,9 +136,7 @@ export async function preview(ctx: any) { // this stops dynamic variables from calling the same query const { fields, parameters, queryVerb, transformer, queryId } = query - //check for oAuth elements here? - const configId = getCookie(ctx, Cookies.OIDC_CONFIG) - console.log(configId) + const authConfigCtx: any = getAuthConfig(ctx) try { const runFn = () => @@ -135,6 +148,10 @@ export async function preview(ctx: any) { parameters, transformer, queryId, + ctx: { + user: ctx.user, + auth: { ...authConfigCtx }, + }, }) const { rows, keys, info, extra } = await quotas.addQuery(runFn) await events.query.previewed(datasource, query) @@ -177,6 +194,9 @@ async function execute(ctx: any, opts = { rowsOnly: false }) { parameters: enrichedParameters, transformer: query.transformer, queryId: ctx.params.queryId, + ctx: { + user: ctx.user, + }, }) const { rows, pagination, extra } = await quotas.addQuery(runFn) diff --git a/packages/server/src/module.d.ts b/packages/server/src/module.d.ts index 466bea6576..fb523cf90d 100644 --- a/packages/server/src/module.d.ts +++ b/packages/server/src/module.d.ts @@ -8,3 +8,4 @@ declare module "@budibase/backend-core/constants" declare module "@budibase/backend-core/auth" declare module "@budibase/backend-core/sessions" declare module "@budibase/backend-core/encryption" +declare module "@budibase/backend-core/utils" diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js index 6e6822d6f5..7808297f89 100644 --- a/packages/server/src/threads/query.js +++ b/packages/server/src/threads/query.js @@ -4,7 +4,12 @@ const ScriptRunner = require("../utilities/scriptRunner") const { integrations } = require("../integrations") const { processStringSync } = require("@budibase/string-templates") const { doInAppContext, getAppDB } = require("@budibase/backend-core/context") -// const { reUpToken } = require("@budibase/backend-core/auth") +const { + refreshOAuthToken, + updateUserOAuth, +} = require("@budibase/backend-core/auth") +const { getGlobalIDFromUserMetadataID } = require("../db/utils") + const { isSQL } = require("../integrations/utils") const { enrichQueryFields, @@ -22,20 +27,19 @@ class QueryRunner { this.queryId = input.queryId this.noRecursiveQuery = flags.noRecursiveQuery this.cachedVariables = [] + // Additional context items for enrichment + this.ctx = input.ctx // allows the response from a query to be stored throughout this // execution so that if it needs to be re-used for another variable // it can be this.queryResponse = {} this.hasRerun = false + this.hasRefreshedOAuth = false } async execute() { let { datasource, fields, queryVerb, transformer } = this - // if(this.ctx.user.oauth2?.refreshToken){ - // reUpToken(this.ctx.user.oauth2?.refreshToken) - // } - const Integration = integrations[datasource.source] if (!Integration) { throw "Integration type does not exist." @@ -79,15 +83,24 @@ class QueryRunner { } // if the request fails we retry once, invalidating the cached value - if ( - info && - info.code >= 400 && - this.cachedVariables.length > 0 && - !this.hasRerun - ) { - // return { info } + if (info && info.code >= 400 && !this.hasRerun) { + if ( + this.ctx.user?.provider && + info.code === 401 && + !this.hasRefreshedOAuth + ) { + // Attempt to refresh the access token from the provider + this.hasRefreshedOAuth = true + const authResponse = await this.refreshOAuth2(this.ctx) + + if (!authResponse || authResponse.err) { + // In this event the user may have oAuth issues that + // could require re-authenticating with their provider. + throw new Error("OAuth2 access token could not be refreshed") + } + } + this.hasRerun = true - // invalidate the cache value await threadUtils.invalidateDynamicVariables(this.cachedVariables) return this.execute() } @@ -133,6 +146,31 @@ class QueryRunner { ).execute() } + async refreshOAuth2(ctx) { + const { oauth2, providerType, _id } = ctx.user + const { configId } = ctx.auth + + if (!providerType || !oauth2?.refreshToken) { + console.error("No refresh token found for authenticated user") + return + } + + const resp = await refreshOAuthToken( + oauth2.refreshToken, + providerType, + configId + ) + + // Refresh session flow. Should be in same location as refreshOAuthToken + // There are several other properties available in 'resp' + if (!resp.error) { + const globalUserId = getGlobalIDFromUserMetadataID(_id) + await updateUserOAuth(globalUserId, resp) + } + + return resp + } + async getDynamicVariable(variable) { let { parameters } = this const queryId = variable.queryId, diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index bd399a29fc..2b738b4900 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -70,7 +70,7 @@ async function authInternal(ctx: any, user: any, err = null, info = null) { export const authenticate = async (ctx: any, next: any) => { return passport.authenticate( "local", - async (err: any, user: User, info: any) => { + async (err: any, user: any, info: any) => { await authInternal(ctx, user, err, info) await context.identity.doInUserContext(user, async () => { await events.auth.login("local") @@ -197,7 +197,9 @@ export const googlePreAuth = async (ctx: any, next: any) => { const strategy = await google.strategyFactory(config, callbackUrl, users.save) return passport.authenticate(strategy, { - scope: ["profile", "email"], + scope: ["profile", "email", "https://www.googleapis.com/auth/spreadsheets"], + accessType: "offline", + prompt: "consent", })(ctx, next) } @@ -235,7 +237,7 @@ export const oidcStrategyFactory = async (ctx: any, configId: any) => { let callbackUrl = await exports.oidcCallbackUrl(chosenConfig) //Remote Config - const enrichedConfig = await oidc.fetchOIDCStrategyConfig( + const enrichedConfig = await oidc.fetchStrategyConfig( chosenConfig, callbackUrl ) From 79f54d5873d1d4e235449eac25dbf57e9a1279e7 Mon Sep 17 00:00:00 2001 From: Dean Date: Sun, 3 Jul 2022 22:14:18 +0100 Subject: [PATCH 3/8] Fix for oauth user db update --- packages/backend-core/src/auth.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index 9566a646f1..5a02446212 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -130,18 +130,22 @@ async function refreshOAuthToken(refreshToken, configType, configId) { } async function updateUserOAuth(userId, oAuthConfig) { - const details = { ...oAuthConfig } + const details = { + accessToken: oAuthConfig.accessToken, + refreshToken: oAuthConfig.refreshToken, + } + try { const db = getGlobalDB() - const dbUser = db.get(userId) + const dbUser = await db.get(userId) //Do not overwrite the refresh token if a valid one is not provided. if (typeof details.refreshToken !== "string") { delete details.refreshToken } - dbUser.oAuth2 = { - ...dbUser.oAuth2, + dbUser.oauth2 = { + ...dbUser.oauth2, ...details, } From 69b424dee6604e8f79ba32b837e0a26ca5f1e5fd Mon Sep 17 00:00:00 2001 From: Dean Date: Sun, 3 Jul 2022 22:17:29 +0100 Subject: [PATCH 4/8] Removed debugging line --- packages/backend-core/src/auth.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index 5a02446212..88a90e2126 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -125,7 +125,6 @@ async function refreshOAuthToken(refreshToken, configType, configId) { ) } - console.log(JSON.stringify(refreshResponse)) return refreshResponse } From 5a97b714034a3dcaf5b25c08974bf6cdaf4bc0f0 Mon Sep 17 00:00:00 2001 From: Dean Date: Sun, 3 Jul 2022 22:39:16 +0100 Subject: [PATCH 5/8] OIDC config test fix --- .../backend-core/src/middleware/passport/tests/oidc.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/middleware/passport/tests/oidc.spec.js b/packages/backend-core/src/middleware/passport/tests/oidc.spec.js index c5e9fe0034..c00ab2ea7d 100644 --- a/packages/backend-core/src/middleware/passport/tests/oidc.spec.js +++ b/packages/backend-core/src/middleware/passport/tests/oidc.spec.js @@ -48,8 +48,8 @@ describe("oidc", () => { it("should create successfully create an oidc strategy", async () => { const oidc = require("../oidc") - - await oidc.strategyFactory(oidcConfig, callbackUrl) + const enrichedConfig = await oidc.fetchStrategyConfig(oidcConfig, callbackUrl) + await oidc.strategyFactory(enrichedConfig, callbackUrl) expect(mockFetch).toHaveBeenCalledWith(oidcConfig.configUrl) From 709e8600b0fc7c54354d0ad284d9ff66ffe1c2bc Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 4 Jul 2022 09:04:55 +0100 Subject: [PATCH 6/8] Auth test fix for oidc strategy mocks --- .../worker/src/api/routes/tests/auth.spec.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/worker/src/api/routes/tests/auth.spec.js b/packages/worker/src/api/routes/tests/auth.spec.js index 03b00d8312..165ecd0f4a 100644 --- a/packages/worker/src/api/routes/tests/auth.spec.js +++ b/packages/worker/src/api/routes/tests/auth.spec.js @@ -77,27 +77,30 @@ describe("/api/global/auth", () => { describe("oidc", () => { const auth = require("@budibase/backend-core/auth") - // mock the oidc strategy implementation and return value - let strategyFactory = jest.fn() - let mockStrategyReturn = jest.fn() - strategyFactory.mockReturnValue(mockStrategyReturn) - auth.oidc.strategyFactory = strategyFactory - const passportSpy = jest.spyOn(auth.passport, "authenticate") let oidcConf let chosenConfig let configId + // mock the oidc strategy implementation and return value + let strategyFactory = jest.fn() + let mockStrategyReturn = jest.fn() + let mockStrategyConfig = jest.fn() + auth.oidc.fetchStrategyConfig = mockStrategyConfig + + strategyFactory.mockReturnValue(mockStrategyReturn) + auth.oidc.strategyFactory = strategyFactory + beforeEach(async () => { oidcConf = await config.saveOIDCConfig() chosenConfig = oidcConf.config.configs[0] configId = chosenConfig.uuid + mockStrategyConfig.mockReturnValue(chosenConfig) }) afterEach(() => { expect(strategyFactory).toBeCalledWith( chosenConfig, - `http://localhost:10000/api/global/auth/${TENANT_ID}/oidc/callback`, expect.any(Function) ) }) @@ -107,7 +110,7 @@ describe("/api/global/auth", () => { await request.get(`/api/global/auth/${TENANT_ID}/oidc/configs/${configId}`) expect(passportSpy).toBeCalledWith(mockStrategyReturn, { - scope: ["profile", "email"], + scope: ["profile", "email", "offline_access"] }) expect(passportSpy.mock.calls.length).toBe(1); }) From 4ecae7fa5b47602e293f56f55699657364b8e25f Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 4 Jul 2022 12:54:26 +0100 Subject: [PATCH 7/8] Code review updates --- packages/backend-core/src/auth.js | 5 ++- packages/backend-core/src/db/utils.ts | 18 ++++----- .../src/middleware/authenticated.js | 1 - packages/backend-core/src/middleware/index.js | 3 +- .../src/middleware/passport/google.js | 10 ----- .../src/middleware/passport/local.js | 15 +------- .../src/middleware/passport/oidc.js | 4 +- .../src/middleware/passport/utils.js | 6 +-- .../worker/src/api/controllers/global/auth.ts | 37 +++---------------- 9 files changed, 22 insertions(+), 77 deletions(-) diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index 88a90e2126..b6d6a2027f 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -15,6 +15,7 @@ const { tenancy, appTenancy, authError, + ssoCallbackUrl, csrf, internalApi, } = require("./middleware") @@ -72,11 +73,10 @@ async function refreshOIDCAccessToken(db, chosenConfig, refreshToken) { async function refreshGoogleAccessToken(db, config, refreshToken) { let callbackUrl = await google.getCallbackUrl(db, config) - const googleConfig = await google.fetchStrategyConfig(config) let strategy try { - strategy = await google.strategyFactory(googleConfig, callbackUrl) + strategy = await google.strategyFactory(config, callbackUrl) } catch (err) { console.error(err) throw new Error("Error constructing OIDC refresh strategy", err) @@ -168,4 +168,5 @@ module.exports = { internalApi, refreshOAuthToken, updateUserOAuth, + ssoCallbackUrl, } diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index 3ad7f9b4f6..c23b56197b 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -386,10 +386,9 @@ export const getScopedFullConfig = async function ( if (type === Configs.SETTINGS) { if (scopedConfig && scopedConfig.doc) { // overrides affected by environment variables - scopedConfig.doc.config.platformUrl = await getPlatformUrl( - { tenantAware: true }, - db - ) + scopedConfig.doc.config.platformUrl = await getPlatformUrl({ + tenantAware: true, + }) scopedConfig.doc.config.analyticsEnabled = await events.analytics.enabled() } else { @@ -398,7 +397,7 @@ export const getScopedFullConfig = async function ( doc: { _id: generateConfigID({ type, user, workspace }), config: { - platformUrl: await getPlatformUrl({ tenantAware: true }, db), + platformUrl: await getPlatformUrl({ tenantAware: true }), analyticsEnabled: await events.analytics.enabled(), }, }, @@ -409,10 +408,7 @@ export const getScopedFullConfig = async function ( return scopedConfig && scopedConfig.doc } -export const getPlatformUrl = async ( - opts = { tenantAware: true }, - db = null -) => { +export const getPlatformUrl = async (opts = { tenantAware: true }) => { let platformUrl = env.PLATFORM_URL || "http://localhost:10000" if (!env.SELF_HOSTED && env.MULTI_TENANCY && opts.tenantAware) { @@ -422,11 +418,11 @@ export const getPlatformUrl = async ( platformUrl = platformUrl.replace("://", `://${tenantId}.`) } } else if (env.SELF_HOSTED) { - const dbx = db ? db : getGlobalDB() + const db = getGlobalDB() // get the doc directly instead of with getScopedConfig to prevent loop let settings try { - settings = await dbx.get(generateConfigID({ type: Configs.SETTINGS })) + settings = await db.get(generateConfigID({ type: Configs.SETTINGS })) } catch (e: any) { if (e.status !== 404) { throw e diff --git a/packages/backend-core/src/middleware/authenticated.js b/packages/backend-core/src/middleware/authenticated.js index cca80cc511..4e6e0b7ba2 100644 --- a/packages/backend-core/src/middleware/authenticated.js +++ b/packages/backend-core/src/middleware/authenticated.js @@ -94,7 +94,6 @@ module.exports = ( user = await getUser(userId, session.tenantId) } user.csrfToken = session.csrfToken - delete user.password authenticated = true } catch (err) { error = err diff --git a/packages/backend-core/src/middleware/index.js b/packages/backend-core/src/middleware/index.js index 6c4c0d8883..1721d56a3c 100644 --- a/packages/backend-core/src/middleware/index.js +++ b/packages/backend-core/src/middleware/index.js @@ -2,7 +2,7 @@ const jwt = require("./passport/jwt") const local = require("./passport/local") const google = require("./passport/google") const oidc = require("./passport/oidc") -const { authError } = require("./passport/utils") +const { authError, ssoCallbackUrl } = require("./passport/utils") const authenticated = require("./authenticated") const auditLog = require("./auditLog") const tenancy = require("./tenancy") @@ -20,6 +20,7 @@ module.exports = { tenancy, authError, internalApi, + ssoCallbackUrl, datasource: { google: datasourceGoogle, }, diff --git a/packages/backend-core/src/middleware/passport/google.js b/packages/backend-core/src/middleware/passport/google.js index 167e719203..7419974cd7 100644 --- a/packages/backend-core/src/middleware/passport/google.js +++ b/packages/backend-core/src/middleware/passport/google.js @@ -2,7 +2,6 @@ const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy const { ssoCallbackUrl } = require("./utils") const { authenticateThirdParty } = require("./third-party-common") const { Configs } = require("../../../constants") -const environment = require("../../environment") const buildVerifyFn = saveUserFn => { return (accessToken, refreshToken, profile, done) => { @@ -60,15 +59,6 @@ exports.strategyFactory = async function (config, callbackUrl, saveUserFn) { } } -exports.fetchStrategyConfig = async function (googleConfig) { - return ( - googleConfig || { - clientID: environment.GOOGLE_CLIENT_ID, - clientSecret: environment.GOOGLE_CLIENT_SECRET, - } - ) -} - exports.getCallbackUrl = async function (db, config) { return ssoCallbackUrl(db, config, Configs.GOOGLE) } diff --git a/packages/backend-core/src/middleware/passport/local.js b/packages/backend-core/src/middleware/passport/local.js index 3f224e7abd..b955d29102 100644 --- a/packages/backend-core/src/middleware/passport/local.js +++ b/packages/backend-core/src/middleware/passport/local.js @@ -6,7 +6,7 @@ const users = require("../../users") const { authError } = require("./utils") const { newid } = require("../../hashing") const { createASession } = require("../../security/sessions") -const { getTenantId, getGlobalDB } = require("../../tenancy") +const { getTenantId } = require("../../tenancy") const INVALID_ERR = "Invalid credentials" const SSO_NO_PASSWORD = "SSO user does not have a password set" @@ -56,19 +56,6 @@ exports.authenticate = async function (ctx, email, password, done) { const sessionId = newid() const tenantId = getTenantId() - if (dbUser.provider || dbUser.providerType || dbUser.pictureUrl) { - delete dbUser.provider - delete dbUser.providerType - delete dbUser.pictureUrl - - try { - const db = getGlobalDB() - await db.put(dbUser) - } catch (err) { - console.error("OAuth elements could not be purged") - } - } - await createASession(dbUser._id, { sessionId, tenantId }) dbUser.token = jwt.sign( diff --git a/packages/backend-core/src/middleware/passport/oidc.js b/packages/backend-core/src/middleware/passport/oidc.js index 570ce641cf..20dbd4669b 100644 --- a/packages/backend-core/src/middleware/passport/oidc.js +++ b/packages/backend-core/src/middleware/passport/oidc.js @@ -103,9 +103,9 @@ exports.strategyFactory = async function (config, saveUserFn) { } } -exports.fetchStrategyConfig = async function (config, callbackUrl) { +exports.fetchStrategyConfig = async function (enrichedConfig, callbackUrl) { try { - const { clientID, clientSecret, configUrl } = config + const { clientID, clientSecret, configUrl } = enrichedConfig if (!clientID || !clientSecret || !callbackUrl || !configUrl) { //check for remote config and all required elements diff --git a/packages/backend-core/src/middleware/passport/utils.js b/packages/backend-core/src/middleware/passport/utils.js index ddc87c6cd0..217130cd6d 100644 --- a/packages/backend-core/src/middleware/passport/utils.js +++ b/packages/backend-core/src/middleware/passport/utils.js @@ -1,4 +1,4 @@ -const { getGlobalDB, isMultiTenant, getTenantId } = require("../../tenancy") +const { isMultiTenant, getTenantId } = require("../../tenancy") const { getScopedConfig } = require("../../db/utils") const { Configs } = require("../../constants") @@ -23,9 +23,7 @@ exports.ssoCallbackUrl = async (db, config, type) => { if (config && config.callbackURL) { return config.callbackURL } - - const dbx = db ? db : getGlobalDB() - const publicConfig = await getScopedConfig(dbx, { + const publicConfig = await getScopedConfig(db, { type: Configs.SETTINGS, }) diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index 2b738b4900..97b6c4c02f 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -1,49 +1,22 @@ const core = require("@budibase/backend-core") -const { getScopedConfig } = require("@budibase/backend-core/db") -const { google } = require("@budibase/backend-core/middleware") -const { oidc } = require("@budibase/backend-core/middleware") const { Configs, EmailTemplatePurpose } = require("../../../constants") const { sendEmail, isEmailConfigured } = require("../../../utilities/email") const { setCookie, getCookie, clearCookie, hash, platformLogout } = core.utils const { Cookies, Headers } = core.constants -const { passport } = core.auth +const { passport, ssoCallbackUrl, google, oidc } = core.auth const { checkResetPasswordCode } = require("../../../utilities/redis") -const { - getGlobalDB, - getTenantId, - isMultiTenant, -} = require("@budibase/backend-core/tenancy") +const { getGlobalDB } = require("@budibase/backend-core/tenancy") const env = require("../../../environment") import { events, users as usersCore, context } from "@budibase/backend-core" import { users } from "../../../sdk" import { User } from "@budibase/types" -const ssoCallbackUrl = async (config: any, type: any) => { - // incase there is a callback URL from before - if (config && config.callbackURL) { - return config.callbackURL - } - - const db = getGlobalDB() - const publicConfig = await getScopedConfig(db, { - type: Configs.SETTINGS, - }) - - let callbackUrl = `/api/global/auth` - if (isMultiTenant()) { - callbackUrl += `/${getTenantId()}` - } - callbackUrl += `/${type}/callback` - - return `${publicConfig.platformUrl}${callbackUrl}` -} - export const googleCallbackUrl = async (config: any) => { - return ssoCallbackUrl(config, "google") + return ssoCallbackUrl(getGlobalDB(), config, "google") } export const oidcCallbackUrl = async (config: any) => { - return ssoCallbackUrl(config, "oidc") + return ssoCallbackUrl(getGlobalDB(), config, "oidc") } async function authInternal(ctx: any, user: any, err = null, info = null) { @@ -70,7 +43,7 @@ async function authInternal(ctx: any, user: any, err = null, info = null) { export const authenticate = async (ctx: any, next: any) => { return passport.authenticate( "local", - async (err: any, user: any, info: any) => { + async (err: any, user: User, info: any) => { await authInternal(ctx, user, err, info) await context.identity.doInUserContext(user, async () => { await events.auth.login("local") From d7a572d9ee219a3909cdbe46fdee182e4a54d33c Mon Sep 17 00:00:00 2001 From: Dean Date: Mon, 4 Jul 2022 13:37:56 +0100 Subject: [PATCH 8/8] Code review update, removing sheets scope from authentication --- packages/worker/src/api/controllers/global/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index 97b6c4c02f..dc96554cb2 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -170,7 +170,7 @@ export const googlePreAuth = async (ctx: any, next: any) => { const strategy = await google.strategyFactory(config, callbackUrl, users.save) return passport.authenticate(strategy, { - scope: ["profile", "email", "https://www.googleapis.com/auth/spreadsheets"], + scope: ["profile", "email"], accessType: "offline", prompt: "consent", })(ctx, next)