From 3305400c83885d181bf550f07bb96a551c5d6257 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 21 Jul 2021 11:10:26 +0100 Subject: [PATCH] Fixing saving of oidc and google auth, neither should require the callbackURL property with the tenancy update. --- .../auth/src/middleware/passport/google.js | 10 +++++----- .../middleware/passport/tests/google.spec.js | 4 +++- .../builder/portal/manage/auth/index.svelte | 19 +++++++++++-------- .../worker/src/api/controllers/admin/auth.js | 16 +++++++++++----- .../worker/src/api/routes/admin/configs.js | 1 - .../tests/utilities/TestConfiguration.js | 1 - 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/packages/auth/src/middleware/passport/google.js b/packages/auth/src/middleware/passport/google.js index 68fe885512..07d6816c0b 100644 --- a/packages/auth/src/middleware/passport/google.js +++ b/packages/auth/src/middleware/passport/google.js @@ -27,13 +27,13 @@ async function authenticate(accessToken, refreshToken, profile, done) { * from couchDB rather than environment variables, using this factory is necessary for dynamically configuring passport. * @returns Dynamically configured Passport Google Strategy */ -exports.strategyFactory = async function (config) { +exports.strategyFactory = async function (config, callbackUrl) { try { - const { clientID, clientSecret, callbackURL } = config + const { clientID, clientSecret } = config - if (!clientID || !clientSecret || !callbackURL) { + if (!clientID || !clientSecret) { throw new Error( - "Configuration invalid. Must contain google clientID, clientSecret and callbackURL" + "Configuration invalid. Must contain google clientID and clientSecret" ) } @@ -41,7 +41,7 @@ exports.strategyFactory = async function (config) { { clientID: config.clientID, clientSecret: config.clientSecret, - callbackURL: config.callbackURL, + callbackURL: callbackUrl, }, authenticate ) diff --git a/packages/auth/src/middleware/passport/tests/google.spec.js b/packages/auth/src/middleware/passport/tests/google.spec.js index 30e582a68f..0e2d3d96ef 100644 --- a/packages/auth/src/middleware/passport/tests/google.spec.js +++ b/packages/auth/src/middleware/passport/tests/google.spec.js @@ -2,6 +2,8 @@ const { data } = require("./utilities/mock-data") +const TENANT_ID = "default" + const googleConfig = { callbackURL: "http://somecallbackurl", clientID: data.clientID, @@ -27,7 +29,7 @@ describe("google", () => { it("should create successfully create a google strategy", async () => { const google = require("../google") - await google.strategyFactory(googleConfig) + await google.strategyFactory(googleConfig, `/api/admin/auth/${TENANT_ID}/google/callback`) const expectedOptions = { clientID: googleConfig.clientID, 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 92e7bc52d2..904c27ecb0 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -114,16 +114,14 @@ // Create a flag so that it will only try to save completed forms $: partialGoogle = providers.google?.config?.clientID || - providers.google?.config?.clientSecret || - providers.google?.config?.callbackURL + providers.google?.config?.clientSecret $: partialOidc = providers.oidc?.config?.configs[0].configUrl || providers.oidc?.config?.configs[0].clientID || providers.oidc?.config?.configs[0].clientSecret $: googleComplete = providers.google?.config?.clientID && - providers.google?.config?.clientSecret && - providers.google?.config?.callbackURL + providers.google?.config?.clientSecret $: oidcComplete = providers.oidc?.config?.configs[0].configUrl && providers.oidc?.config?.configs[0].clientID && @@ -153,10 +151,14 @@ let calls = [] docs.forEach(element => { if (element.type === ConfigTypes.OIDC) { - //Add a UUID here so each config is distinguishable when it arrives at the login page. - element.config.configs.forEach(config => { - !config.uuid && (config.uuid = uuid()) - }) + //Add a UUID here so each config is distinguishable when it arrives at the login page + for (let config of element.config.configs) { + if (!config.uuid) { + config.uuid = uuid() + } + // callback urls shouldn't be included + delete config.callbackURL + } if (partialOidc) { if (!oidcComplete) { notifications.error( @@ -177,6 +179,7 @@ `Please fill in all required ${ConfigTypes.Google} fields` ) } else { + delete element.config.callbackURL calls.push(api.post(`/api/admin/configs`, element)) googleSaveButtonDisabled = true originalGoogleDoc = cloneDeep(providers.google) diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index a48cb0da59..c6c1641ab5 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -101,12 +101,15 @@ exports.logout = async ctx => { * On a successful login, you will be redirected to the googleAuth callback route. */ exports.googlePreAuth = async (ctx, next) => { - const db = getGlobalDB(ctx.params.tenantId) + const tenantId = ctx.params.tenantId + const db = getGlobalDB(tenantId) + const callbackUrl = `/api/admin/auth/${tenantId}/google/callback` + const config = await authPkg.db.getScopedConfig(db, { type: Configs.GOOGLE, workspace: ctx.query.workspace, }) - const strategy = await google.strategyFactory(config) + const strategy = await google.strategyFactory(config, callbackUrl) return passport.authenticate(strategy, { scope: ["profile", "email"], @@ -114,13 +117,15 @@ exports.googlePreAuth = async (ctx, next) => { } exports.googleAuth = async (ctx, next) => { - const db = getGlobalDB(ctx.params.tenantId) + const tenantId = ctx.params.tenantId + const db = getGlobalDB(tenantId) + const callbackUrl = `/api/admin/auth/${tenantId}/google/callback` const config = await authPkg.db.getScopedConfig(db, { type: Configs.GOOGLE, workspace: ctx.query.workspace, }) - const strategy = await google.strategyFactory(config) + const strategy = await google.strategyFactory(config, callbackUrl) return passport.authenticate( strategy, @@ -134,6 +139,7 @@ exports.googleAuth = async (ctx, next) => { } async function oidcStrategyFactory(ctx, configId) { + const tenantId = ctx.params.tenantId const db = getGlobalDB(ctx.params.tenantId) const config = await authPkg.db.getScopedConfig(db, { type: Configs.OIDC, @@ -142,7 +148,7 @@ async function oidcStrategyFactory(ctx, configId) { const chosenConfig = config.configs.filter(c => c.uuid === configId)[0] - const callbackUrl = `${ctx.protocol}://${ctx.host}/api/admin/auth/oidc/callback` + const callbackUrl = `${ctx.protocol}://${ctx.host}/api/admin/auth/${tenantId}/oidc/callback` return oidc.strategyFactory(chosenConfig, callbackUrl) } diff --git a/packages/worker/src/api/routes/admin/configs.js b/packages/worker/src/api/routes/admin/configs.js index 11c4868d88..840201cbd0 100644 --- a/packages/worker/src/api/routes/admin/configs.js +++ b/packages/worker/src/api/routes/admin/configs.js @@ -37,7 +37,6 @@ function googleValidation() { return Joi.object({ clientID: Joi.string().required(), clientSecret: Joi.string().required(), - callbackURL: Joi.string().required(), activated: Joi.boolean().required(), }).unknown(true) } diff --git a/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js b/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js index f7c4d4a90d..224b61cf0d 100644 --- a/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js +++ b/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js @@ -155,7 +155,6 @@ class TestConfiguration { { type: Configs.GOOGLE, config: { - callbackURL: "http://somecallbackurl", clientID: "clientId", clientSecret: "clientSecret", },