From 33b352c3ef739fb5cc94c381bad22fedcce1d39c Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 15 Jul 2021 16:20:31 +0100 Subject: [PATCH] Store OIDC config in cookie instead of URL --- packages/auth/src/constants.js | 1 + .../builder/auth/_components/OIDCButton.svelte | 5 +++-- .../builder/portal/manage/auth/index.svelte | 6 +----- .../worker/src/api/controllers/admin/auth.js | 17 ++++++++++------- packages/worker/src/api/routes/admin/auth.js | 4 ++-- .../worker/src/api/routes/tests/auth.spec.js | 10 ++++++---- .../routes/tests/utilities/TestConfiguration.js | 13 ++++++++++++- 7 files changed, 35 insertions(+), 21 deletions(-) diff --git a/packages/auth/src/constants.js b/packages/auth/src/constants.js index 0d714a0021..df7fbf5c3f 100644 --- a/packages/auth/src/constants.js +++ b/packages/auth/src/constants.js @@ -6,6 +6,7 @@ exports.UserStatus = { exports.Cookies = { CurrentApp: "budibase:currentapp", Auth: "budibase:auth", + OIDC_CONFIG: "budibase:oidc:config", } exports.GlobalRoles = { diff --git a/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte b/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte index d3a2f7c9a5..2e11454081 100644 --- a/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte +++ b/packages/builder/src/pages/builder/auth/_components/OIDCButton.svelte @@ -4,7 +4,7 @@ import Auth0Logo from "assets/auth0-logo.png" import MicrosoftLogo from "assets/microsoft-logo.png" - import { admin, oidc } from "stores/portal" + import { oidc } from "stores/portal" import { onMount } from "svelte" let show = false @@ -27,7 +27,8 @@ {#if show} window.open(`/api/admin/auth/oidc/${$oidc.uuid}`, "_blank")} + on:click={() => + window.open(`/api/admin/auth/oidc/configs/${$oidc.uuid}`, "_blank")} >
oidc icon 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 e9548b8ec0..99ae00ede5 100644 --- a/packages/builder/src/pages/builder/portal/manage/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/manage/auth/index.svelte @@ -79,10 +79,6 @@ providers.oidc?.config?.configs[0].clientID && providers.oidc?.config?.configs[0].clientSecret - $: oidcCallback = providers.oidc?.config.configs[0].uuid - ? `/api/admin/auth/oidc/callback/${providers.oidc?.config.configs[0].uuid}` - : "" - async function uploadLogo(file) { let data = new FormData() data.append("file", file) @@ -240,7 +236,7 @@ {/each}
- +

diff --git a/packages/worker/src/api/controllers/admin/auth.js b/packages/worker/src/api/controllers/admin/auth.js index c6395e4737..2a641e6194 100644 --- a/packages/worker/src/api/controllers/admin/auth.js +++ b/packages/worker/src/api/controllers/admin/auth.js @@ -4,7 +4,8 @@ const { oidc } = require("@budibase/auth/src/middleware") const { Configs, EmailTemplatePurpose } = require("../../../constants") const CouchDB = require("../../../db") const { sendEmail, isEmailConfigured } = require("../../../utilities/email") -const { clearCookie, getGlobalUserByEmail, hash } = authPkg.utils +const { setCookie, getCookie, clearCookie, getGlobalUserByEmail, hash } = + authPkg.utils const { Cookies } = authPkg.constants const { passport } = authPkg.auth const { checkResetPasswordCode } = require("../../../utilities/redis") @@ -133,9 +134,7 @@ exports.googleAuth = async (ctx, next) => { )(ctx, next) } -async function oidcStrategyFactory(ctx) { - const { configId } = ctx.params - +async function oidcStrategyFactory(ctx, configId) { const db = new CouchDB(GLOBAL_DB) const config = await authPkg.db.getScopedConfig(db, { @@ -145,7 +144,7 @@ async function oidcStrategyFactory(ctx) { const chosenConfig = config.configs.filter(c => c.uuid === configId)[0] - const callbackUrl = `${ctx.protocol}://${ctx.host}/api/admin/auth/oidc/callback/${configId}` + const callbackUrl = `${ctx.protocol}://${ctx.host}/api/admin/auth/oidc/callback` return oidc.strategyFactory(chosenConfig, callbackUrl) } @@ -155,7 +154,10 @@ async function oidcStrategyFactory(ctx) { * On a successful login, you will be redirected to the oidcAuth callback route. */ exports.oidcPreAuth = async (ctx, next) => { - const strategy = await oidcStrategyFactory(ctx) + const { configId } = ctx.params + const strategy = await oidcStrategyFactory(ctx, configId) + + setCookie(ctx, configId, Cookies.OIDC_CONFIG) return passport.authenticate(strategy, { // required 'openid' scope is added by oidc strategy factory @@ -164,7 +166,8 @@ exports.oidcPreAuth = async (ctx, next) => { } exports.oidcAuth = async (ctx, next) => { - const strategy = await oidcStrategyFactory(ctx) + const configId = getCookie(ctx, Cookies.OIDC_CONFIG) + const strategy = await oidcStrategyFactory(ctx, configId) return passport.authenticate( strategy, diff --git a/packages/worker/src/api/routes/admin/auth.js b/packages/worker/src/api/routes/admin/auth.js index 81f94fbf02..9a7ef5ebac 100644 --- a/packages/worker/src/api/routes/admin/auth.js +++ b/packages/worker/src/api/routes/admin/auth.js @@ -39,7 +39,7 @@ router .post("/api/admin/auth/logout", authController.logout) .get("/api/admin/auth/google", authController.googlePreAuth) .get("/api/admin/auth/google/callback", authController.googleAuth) - .get("/api/admin/auth/oidc/:configId", authController.oidcPreAuth) - .get("/api/admin/auth/oidc/callback/:configId", authController.oidcAuth) + .get("/api/admin/auth/oidc/configs/:configId", authController.oidcPreAuth) + .get("/api/admin/auth/oidc/callback", authController.oidcAuth) module.exports = router diff --git a/packages/worker/src/api/routes/tests/auth.spec.js b/packages/worker/src/api/routes/tests/auth.spec.js index bb8e9d0918..ceccf7edaf 100644 --- a/packages/worker/src/api/routes/tests/auth.spec.js +++ b/packages/worker/src/api/routes/tests/auth.spec.js @@ -1,4 +1,5 @@ const setup = require("./utilities") +const { Cookies } = require("@budibase/auth").constants jest.mock("nodemailer") const sendMailMock = setup.emailMock() @@ -74,13 +75,13 @@ describe("/api/admin/auth", () => { afterEach(() => { expect(strategyFactory).toBeCalledWith( chosenConfig, - `http://127.0.0.1:4003/api/admin/auth/oidc/callback/${configId}` // calculated url + `http://127.0.0.1:4003/api/admin/auth/oidc/callback` // calculated url ) }) - describe("/api/admin/auth/oidc", () => { + describe("/api/admin/auth/oidc/configs", () => { it("should load strategy and delegate to passport", async () => { - await request.get(`/api/admin/auth/oidc/${configId}`) + await request.get(`/api/admin/auth/oidc/configs/${configId}`) expect(passportSpy).toBeCalledWith(mockStrategyReturn, { scope: ["profile", "email"], @@ -91,7 +92,8 @@ describe("/api/admin/auth", () => { describe("/api/admin/auth/oidc/callback", () => { it("should load strategy and delegate to passport", async () => { - await request.get(`/api/admin/auth/oidc/callback/${configId}`) + await request.get(`/api/admin/auth/oidc/callback`) + .set(config.getOIDConfigCookie(configId)) expect(passportSpy).toBeCalledWith(mockStrategyReturn, { successRedirect: "/", failureRedirect: "/error" diff --git a/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js b/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js index d8b4dab655..812dbe51e2 100644 --- a/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js +++ b/packages/worker/src/api/routes/tests/utilities/TestConfiguration.js @@ -68,6 +68,12 @@ class TestConfiguration { } } + cookieHeader(cookies) { + return { + Cookie: [cookies], + } + } + defaultHeaders() { const user = { _id: "us_uuid1", @@ -77,7 +83,7 @@ class TestConfiguration { const authToken = jwt.sign(user, env.JWT_SECRET) return { Accept: "application/json", - Cookie: [`${Cookies.Auth}=${authToken}`], + ...this.cookieHeader([`${Cookies.Auth}=${authToken}`]), } } @@ -156,6 +162,11 @@ class TestConfiguration { ) } + getOIDConfigCookie(configId) { + const token = jwt.sign(configId, env.JWT_SECRET) + return this.cookieHeader([[`${Cookies.OIDC_CONFIG}=${token}`]]) + } + async saveOIDCConfig() { await this.deleteConfig(Configs.OIDC) const config = {