From 2c46109e7d26626785aa525d5071aaff789174de Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Mon, 27 Feb 2023 13:42:51 +0000 Subject: [PATCH] Enforceable SSO (#9787) * Add ENFORCEABLE_SSO feature flag * First draft of enforce sso configuration / show single sign on url * Reading and writing isSSOEnforced + integration with login page * Enable CI + lint * Set correct base branch for CI * Test fix for expected string changed * Use tenant aware platform url as SSO link * Bring in latest pro changes * Lint * Add useEnforceableSSO mock helper function * Update configs.spec.ts with coverage for public settings * Update users.spec.ts with additional tests for isPreventPasswordActions * Lint * Update refresh OAuthToken to use correct enum and add case statement --- .github/workflows/budibase_ci.yml | 3 +- packages/backend-core/src/auth/auth.ts | 35 +++-- packages/backend-core/src/environment.ts | 12 +- .../tests/utilities/mocks/licenses.ts | 4 + .../src/pages/builder/auth/login.svelte | 122 ++++++++------- .../builder/portal/settings/auth/index.svelte | 72 ++++++++- .../portal/settings/organisation.svelte | 2 + .../builder/src/stores/portal/licensing.js | 4 + .../builder/src/stores/portal/organisation.js | 6 +- packages/frontend-core/src/constants.js | 1 + packages/types/src/documents/global/config.ts | 1 + packages/types/src/sdk/licensing/feature.ts | 1 + .../worker/src/api/controllers/global/auth.ts | 4 +- .../src/api/controllers/global/configs.ts | 18 ++- .../src/api/routes/global/tests/auth.spec.ts | 4 +- .../api/routes/global/tests/configs.spec.ts | 146 +++++++++++------- packages/worker/src/environment.ts | 12 +- packages/worker/src/index.ts | 10 +- packages/worker/src/sdk/auth/auth.ts | 4 +- .../worker/src/sdk/users/tests/users.spec.ts | 38 ++++- packages/worker/src/sdk/users/users.ts | 20 ++- packages/worker/src/tests/api/configs.ts | 8 + .../worker/src/tests/structures/configs.ts | 29 ++-- 23 files changed, 365 insertions(+), 191 deletions(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index e0263546ff..838142898d 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -10,7 +10,8 @@ on: pull_request: branches: - master - - develop + - develop + - configs-refactor-and-server-test-fixes workflow_dispatch: env: diff --git a/packages/backend-core/src/auth/auth.ts b/packages/backend-core/src/auth/auth.ts index 435b6433b2..dd28344b1f 100644 --- a/packages/backend-core/src/auth/auth.ts +++ b/packages/backend-core/src/auth/auth.ts @@ -20,6 +20,7 @@ import { GoogleInnerConfig, OIDCInnerConfig, PlatformLogoutOpts, + SSOProviderType, User, } from "@budibase/types" import { logAlert } from "../logging" @@ -149,26 +150,26 @@ interface RefreshResponse { export async function refreshOAuthToken( refreshToken: string, - configType: ConfigType, + providerType: SSOProviderType, configId?: string ): Promise { - if (configType === ConfigType.OIDC && configId) { - const config = await configs.getOIDCConfigById(configId) - if (!config) { - return { err: { data: "OIDC configuration not found" } } - } - return refreshOIDCAccessToken(config, refreshToken) + switch (providerType) { + case SSOProviderType.OIDC: + if (!configId) { + return { err: { data: "OIDC config id not provided" } } + } + const oidcConfig = await configs.getOIDCConfigById(configId) + if (!oidcConfig) { + return { err: { data: "OIDC configuration not found" } } + } + return refreshOIDCAccessToken(oidcConfig, refreshToken) + case SSOProviderType.GOOGLE: + let googleConfig = await configs.getGoogleConfig() + if (!googleConfig) { + return { err: { data: "Google configuration not found" } } + } + return refreshGoogleAccessToken(googleConfig, refreshToken) } - - if (configType === ConfigType.GOOGLE) { - const config = await configs.getGoogleConfig() - if (!config) { - return { err: { data: "Google configuration not found" } } - } - return refreshGoogleAccessToken(config, refreshToken) - } - - throw new Error(`Unsupported configType=${configType}`) } // TODO: Refactor to use user save function instead to prevent the need for diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index d565606133..0579d709d7 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -28,6 +28,8 @@ const DefaultBucketName = { PLUGINS: "plugins", } +const selfHosted = !!parseInt(process.env.SELF_HOSTED || "") + const environment = { isTest, isJest, @@ -58,7 +60,7 @@ const environment = { process.env.ACCOUNT_PORTAL_URL || "https://account.budibase.app", ACCOUNT_PORTAL_API_KEY: process.env.ACCOUNT_PORTAL_API_KEY || "", DISABLE_ACCOUNT_PORTAL: process.env.DISABLE_ACCOUNT_PORTAL, - SELF_HOSTED: !!parseInt(process.env.SELF_HOSTED || ""), + SELF_HOSTED: selfHosted, COOKIE_DOMAIN: process.env.COOKIE_DOMAIN, PLATFORM_URL: process.env.PLATFORM_URL || "", POSTHOG_TOKEN: process.env.POSTHOG_TOKEN, @@ -91,6 +93,14 @@ const environment = { SMTP_HOST: process.env.SMTP_HOST, SMTP_PORT: parseInt(process.env.SMTP_PORT || ""), SMTP_FROM_ADDRESS: process.env.SMTP_FROM_ADDRESS, + /** + * Enable to allow an admin user to login using a password. + * This can be useful to prevent lockout when configuring SSO. + * However, this should be turned OFF by default for security purposes. + */ + ENABLE_SSO_MAINTENANCE_MODE: selfHosted + ? process.env.ENABLE_SSO_MAINTENANCE_MODE + : false, _set(key: any, value: any) { process.env[key] = value // @ts-ignore diff --git a/packages/backend-core/tests/utilities/mocks/licenses.ts b/packages/backend-core/tests/utilities/mocks/licenses.ts index e374612f5f..109fcdf9c6 100644 --- a/packages/backend-core/tests/utilities/mocks/licenses.ts +++ b/packages/backend-core/tests/utilities/mocks/licenses.ts @@ -70,6 +70,10 @@ export const useBackups = () => { return useFeature(Feature.APP_BACKUPS) } +export const useEnforceableSSO = () => { + return useFeature(Feature.ENFORCEABLE_SSO) +} + export const useGroups = () => { return useFeature(Feature.USER_GROUPS) } diff --git a/packages/builder/src/pages/builder/auth/login.svelte b/packages/builder/src/pages/builder/auth/login.svelte index 80122c23a5..06e09e4fee 100644 --- a/packages/builder/src/pages/builder/auth/login.svelte +++ b/packages/builder/src/pages/builder/auth/login.svelte @@ -79,67 +79,71 @@ - {/if} - - { - formData = { - ...formData, - username: e.detail, - } - }} - validate={() => { - let fieldError = { - username: !formData.username - ? "Please enter a valid email" - : undefined, - } - errors = handleError({ ...errors, ...fieldError }) - }} - error={errors.username} - /> - { - formData = { - ...formData, - password: e.detail, - } - }} - validate={() => { - let fieldError = { - password: !formData.password - ? "Please enter your password" - : undefined, - } - errors = handleError({ ...errors, ...fieldError }) - }} - error={errors.password} - /> - - - - - - - + {#if !$organisation.isSSOEnforced} + + + { + formData = { + ...formData, + username: e.detail, + } + }} + validate={() => { + let fieldError = { + username: !formData.username + ? "Please enter a valid email" + : undefined, + } + errors = handleError({ ...errors, ...fieldError }) + }} + error={errors.username} + /> + { + formData = { + ...formData, + password: e.detail, + } + }} + validate={() => { + let fieldError = { + password: !formData.password + ? "Please enter your password" + : undefined, + } + errors = handleError({ ...errors, ...fieldError }) + }} + error={errors.password} + /> + + {/if} + {#if !$organisation.isSSOEnforced} + + + + + + + {/if} {#if cloud} diff --git a/packages/builder/src/pages/builder/portal/settings/auth/index.svelte b/packages/builder/src/pages/builder/portal/settings/auth/index.svelte index 608ca271c0..e5196a1cbf 100644 --- a/packages/builder/src/pages/builder/portal/settings/auth/index.svelte +++ b/packages/builder/src/pages/builder/portal/settings/auth/index.svelte @@ -22,10 +22,11 @@ Tags, Icon, Helpers, + Link, } from "@budibase/bbui" import { onMount } from "svelte" import { API } from "api" - import { organisation, admin } from "stores/portal" + import { organisation, admin, licensing } from "stores/portal" const ConfigTypes = { Google: "google", @@ -34,6 +35,8 @@ const HasSpacesRegex = /[\\"\s]/ + $: enforcedSSO = $organisation.isSSOEnforced + // Some older google configs contain a manually specified value - retain the functionality to edit the field // When there is no value or we are in the cloud - prohibit editing the field, must use platform url to change $: googleCallbackUrl = undefined @@ -154,6 +157,11 @@ iconDropdownOptions.unshift({ label: fileName, value: fileName }) } + async function toggleIsSSOEnforced() { + const value = $organisation.isSSOEnforced + await organisation.save({ isSSOEnforced: !value }) + } + async function save(docs) { let calls = [] // Only if the user has provided an image, upload it @@ -316,6 +324,49 @@ Authentication Add additional authentication methods from the options below + + + Single Sign-On URL + + Use the following link to access your configured identity provider. + + +
+ {$organisation.platformUrl} +
+ +
+
+ + + + +
+
+
+ Enforce Single Sign-On +
+ {#if !$licensing.enforceableSSO} + + Business plan + + {/if} +
+ {#if $licensing.enforceableSSO} + + {/if} +
+ + Require SSO authentication for all users. It is recommended to read the + help documentation before enabling this feature. + + {#if providers.google} @@ -546,7 +597,24 @@ input[type="file"] { display: none; } - + .sso-link-icon { + padding-top: 4px; + margin-left: 3px; + } + .sso-link { + margin-top: 12px; + display: flex; + flex-direction: row; + align-items: center; + } + .enforce-sso-title { + margin-right: 10px; + } + .enforce-sso-heading-container { + display: flex; + flex-direction: row; + align-items: start; + } .provider-title { display: flex; flex-direction: row; diff --git a/packages/builder/src/pages/builder/portal/settings/organisation.svelte b/packages/builder/src/pages/builder/portal/settings/organisation.svelte index 8e4d6e738c..bba046ce10 100644 --- a/packages/builder/src/pages/builder/portal/settings/organisation.svelte +++ b/packages/builder/src/pages/builder/portal/settings/organisation.svelte @@ -24,6 +24,7 @@ } const values = writable({ + isSSOEnforced: $organisation.isSSOEnforced, company: $organisation.company, platformUrl: $organisation.platformUrl, analyticsEnabled: $organisation.analyticsEnabled, @@ -54,6 +55,7 @@ } const config = { + isSSOEnforced: $values.isSSOEnforced, company: $values.company ?? "", platformUrl: $values.platformUrl ?? "", analyticsEnabled: $values.analyticsEnabled, diff --git a/packages/builder/src/stores/portal/licensing.js b/packages/builder/src/stores/portal/licensing.js index 6f5c80e03c..42c150c893 100644 --- a/packages/builder/src/stores/portal/licensing.js +++ b/packages/builder/src/stores/portal/licensing.js @@ -63,6 +63,9 @@ export const createLicensingStore = () => { const environmentVariablesEnabled = license.features.includes( Constants.Features.ENVIRONMENT_VARIABLES ) + const enforceableSSO = license.features.includes( + Constants.Features.ENFORCEABLE_SSO + ) store.update(state => { return { @@ -72,6 +75,7 @@ export const createLicensingStore = () => { groupsEnabled, backupsEnabled, environmentVariablesEnabled, + enforceableSSO, } }) }, diff --git a/packages/builder/src/stores/portal/organisation.js b/packages/builder/src/stores/portal/organisation.js index 9709578fa2..c8b62bb2bc 100644 --- a/packages/builder/src/stores/portal/organisation.js +++ b/packages/builder/src/stores/portal/organisation.js @@ -11,6 +11,7 @@ const DEFAULT_CONFIG = { google: undefined, oidcCallbackUrl: "", googleCallbackUrl: "", + isSSOEnforced: false, } export function createOrganisationStore() { @@ -19,8 +20,8 @@ export function createOrganisationStore() { async function init() { const tenantId = get(auth).tenantId - const tenant = await API.getTenantConfig(tenantId) - set({ ...DEFAULT_CONFIG, ...tenant.config, _rev: tenant._rev }) + const settingsConfigDoc = await API.getTenantConfig(tenantId) + set({ ...DEFAULT_CONFIG, ...settingsConfigDoc.config }) } async function save(config) { @@ -33,7 +34,6 @@ export function createOrganisationStore() { await API.saveConfig({ type: "settings", config: { ...get(store), ...config }, - _rev: get(store)._rev, }) await init() } diff --git a/packages/frontend-core/src/constants.js b/packages/frontend-core/src/constants.js index 3a16013df2..ec0b447159 100644 --- a/packages/frontend-core/src/constants.js +++ b/packages/frontend-core/src/constants.js @@ -115,6 +115,7 @@ export const Features = { USER_GROUPS: "userGroups", BACKUPS: "appBackups", ENVIRONMENT_VARIABLES: "environmentVariables", + ENFORCEABLE_SSO: "enforceableSSO", } // Role IDs diff --git a/packages/types/src/documents/global/config.ts b/packages/types/src/documents/global/config.ts index e2323d2b19..f4bba21e0f 100644 --- a/packages/types/src/documents/global/config.ts +++ b/packages/types/src/documents/global/config.ts @@ -29,6 +29,7 @@ export interface SettingsInnerConfig { logoUrlEtag?: string uniqueTenantId?: string analyticsEnabled?: boolean + isSSOEnforced?: boolean } export interface SettingsConfig extends Config { diff --git a/packages/types/src/sdk/licensing/feature.ts b/packages/types/src/sdk/licensing/feature.ts index a39bcab18b..f28c3c0709 100644 --- a/packages/types/src/sdk/licensing/feature.ts +++ b/packages/types/src/sdk/licensing/feature.ts @@ -2,4 +2,5 @@ export enum Feature { USER_GROUPS = "userGroups", APP_BACKUPS = "appBackups", ENVIRONMENT_VARIABLES = "environmentVariables", + ENFORCEABLE_SSO = "enforceableSSO", } diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index ab017a4184..9336ba0d5b 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -61,8 +61,8 @@ export const login = async (ctx: Ctx, next: any) => { const email = ctx.request.body.username const user = await userSdk.getUserByEmail(email) - if (user && (await userSdk.isPreventSSOPasswords(user))) { - ctx.throw(400, "SSO user cannot login using password") + if (user && (await userSdk.isPreventPasswordActions(user))) { + ctx.throw(400, "Password login is disabled for this user") } return passport.authenticate( diff --git a/packages/worker/src/api/controllers/global/configs.ts b/packages/worker/src/api/controllers/global/configs.ts index b076416989..1ea0f52c96 100644 --- a/packages/worker/src/api/controllers/global/configs.ts +++ b/packages/worker/src/api/controllers/global/configs.ts @@ -23,6 +23,7 @@ import { isSMTPConfig, UserCtx, } from "@budibase/types" +import * as pro from "@budibase/pro" const getEventFns = async (config: Config, existing?: Config) => { const fns = [] @@ -126,10 +127,10 @@ export async function save(ctx: UserCtx) { const existingConfig = await configs.getConfig(type) let eventFns = await getEventFns(ctx.request.body, existingConfig) - // Config does not exist yet - if (!existingConfig) { - body._id = configs.generateConfigID(type) + if (existingConfig) { + body._rev = existingConfig._rev } + try { // verify the configuration switch (config.type) { @@ -142,6 +143,7 @@ export async function save(ctx: UserCtx) { } try { + body._id = configs.generateConfigID(type) const response = await configs.save(body) await cache.bustCache(cache.CacheKey.CHECKLIST) await cache.bustCache(cache.CacheKey.ANALYTICS_ENABLED) @@ -203,7 +205,8 @@ export async function publicSettings( ) { try { // settings - const config = await configs.getSettingsConfig() + const configDoc = await configs.getSettingsConfigDoc() + const config = configDoc.config // enrich the logo url - empty url means deleted if (config.logoUrl && config.logoUrl !== "") { config.logoUrl = objectStore.getGlobalFileUrl( @@ -224,13 +227,18 @@ export async function publicSettings( const oidc = oidcConfig?.activated || false const _oidcCallbackUrl = await oidcCallbackUrl() + // sso enforced + const isSSOEnforced = await pro.features.isSSOEnforced({ config }) + ctx.body = { type: ConfigType.SETTINGS, - _id: configs.generateConfigID(ConfigType.SETTINGS), + _id: configDoc._id, + _rev: configDoc._rev, config: { ...config, google, oidc, + isSSOEnforced, oidcCallbackUrl: _oidcCallbackUrl, googleCallbackUrl: _googleCallbackUrl, }, diff --git a/packages/worker/src/api/routes/global/tests/auth.spec.ts b/packages/worker/src/api/routes/global/tests/auth.spec.ts index 84f8ce1b0a..b6be1fd7af 100644 --- a/packages/worker/src/api/routes/global/tests/auth.spec.ts +++ b/packages/worker/src/api/routes/global/tests/auth.spec.ts @@ -110,7 +110,7 @@ describe("/api/global/auth", () => { ) expect(response.body).toEqual({ - message: "SSO user cannot login using password", + message: "Password login is disabled for this user", status: 400, }) } @@ -175,7 +175,7 @@ describe("/api/global/auth", () => { ) expect(res.body).toEqual({ - message: "SSO user cannot reset password", + message: "Password reset is disabled for this user", status: 400, error: { code: "http", diff --git a/packages/worker/src/api/routes/global/tests/configs.spec.ts b/packages/worker/src/api/routes/global/tests/configs.spec.ts index 39ad74d295..892fe8a67b 100644 --- a/packages/worker/src/api/routes/global/tests/configs.spec.ts +++ b/packages/worker/src/api/routes/global/tests/configs.spec.ts @@ -2,7 +2,8 @@ jest.mock("nodemailer") import { TestConfiguration, structures, mocks } from "../../../../tests" mocks.email.mock() -import { Config, events } from "@budibase/backend-core" +import { events } from "@budibase/backend-core" +import { GetPublicSettingsResponse, Config, ConfigType } from "@budibase/types" describe("configs", () => { const config = new TestConfiguration() @@ -19,22 +20,29 @@ describe("configs", () => { await config.afterAll() }) - describe("post /api/global/configs", () => { - const saveConfig = async (conf: any, _id?: string, _rev?: string) => { - const data = { - ...conf, - _id, - _rev, - } - - const res = await config.api.configs.saveConfig(data) - - return { - ...data, - ...res.body, - } + const saveConfig = async (conf: Config, _id?: string, _rev?: string) => { + const data = { + ...conf, + _id, + _rev, } + const res = await config.api.configs.saveConfig(data) + return { + ...data, + ...res.body, + } + } + const saveSettingsConfig = async ( + conf?: any, + _id?: string, + _rev?: string + ) => { + const settingsConfig = structures.configs.settings(conf) + return saveConfig(settingsConfig, _id, _rev) + } + + describe("POST /api/global/configs", () => { describe("google", () => { const saveGoogleConfig = async ( conf?: any, @@ -49,20 +57,20 @@ describe("configs", () => { it("should create activated google config", async () => { await saveGoogleConfig() expect(events.auth.SSOCreated).toBeCalledTimes(1) - expect(events.auth.SSOCreated).toBeCalledWith(Config.GOOGLE) + expect(events.auth.SSOCreated).toBeCalledWith(ConfigType.GOOGLE) expect(events.auth.SSODeactivated).not.toBeCalled() expect(events.auth.SSOActivated).toBeCalledTimes(1) - expect(events.auth.SSOActivated).toBeCalledWith(Config.GOOGLE) - await config.deleteConfig(Config.GOOGLE) + expect(events.auth.SSOActivated).toBeCalledWith(ConfigType.GOOGLE) + await config.deleteConfig(ConfigType.GOOGLE) }) it("should create deactivated google config", async () => { await saveGoogleConfig({ activated: false }) expect(events.auth.SSOCreated).toBeCalledTimes(1) - expect(events.auth.SSOCreated).toBeCalledWith(Config.GOOGLE) + expect(events.auth.SSOCreated).toBeCalledWith(ConfigType.GOOGLE) expect(events.auth.SSOActivated).not.toBeCalled() expect(events.auth.SSODeactivated).not.toBeCalled() - await config.deleteConfig(Config.GOOGLE) + await config.deleteConfig(ConfigType.GOOGLE) }) }) @@ -76,11 +84,11 @@ describe("configs", () => { googleConf._rev ) expect(events.auth.SSOUpdated).toBeCalledTimes(1) - expect(events.auth.SSOUpdated).toBeCalledWith(Config.GOOGLE) + expect(events.auth.SSOUpdated).toBeCalledWith(ConfigType.GOOGLE) expect(events.auth.SSOActivated).not.toBeCalled() expect(events.auth.SSODeactivated).toBeCalledTimes(1) - expect(events.auth.SSODeactivated).toBeCalledWith(Config.GOOGLE) - await config.deleteConfig(Config.GOOGLE) + expect(events.auth.SSODeactivated).toBeCalledWith(ConfigType.GOOGLE) + await config.deleteConfig(ConfigType.GOOGLE) }) it("should update google config to activated", async () => { @@ -92,11 +100,11 @@ describe("configs", () => { googleConf._rev ) expect(events.auth.SSOUpdated).toBeCalledTimes(1) - expect(events.auth.SSOUpdated).toBeCalledWith(Config.GOOGLE) + expect(events.auth.SSOUpdated).toBeCalledWith(ConfigType.GOOGLE) expect(events.auth.SSODeactivated).not.toBeCalled() expect(events.auth.SSOActivated).toBeCalledTimes(1) - expect(events.auth.SSOActivated).toBeCalledWith(Config.GOOGLE) - await config.deleteConfig(Config.GOOGLE) + expect(events.auth.SSOActivated).toBeCalledWith(ConfigType.GOOGLE) + await config.deleteConfig(ConfigType.GOOGLE) }) }) }) @@ -115,20 +123,20 @@ describe("configs", () => { it("should create activated OIDC config", async () => { await saveOIDCConfig() expect(events.auth.SSOCreated).toBeCalledTimes(1) - expect(events.auth.SSOCreated).toBeCalledWith(Config.OIDC) + expect(events.auth.SSOCreated).toBeCalledWith(ConfigType.OIDC) expect(events.auth.SSODeactivated).not.toBeCalled() expect(events.auth.SSOActivated).toBeCalledTimes(1) - expect(events.auth.SSOActivated).toBeCalledWith(Config.OIDC) - await config.deleteConfig(Config.OIDC) + expect(events.auth.SSOActivated).toBeCalledWith(ConfigType.OIDC) + await config.deleteConfig(ConfigType.OIDC) }) it("should create deactivated OIDC config", async () => { await saveOIDCConfig({ activated: false }) expect(events.auth.SSOCreated).toBeCalledTimes(1) - expect(events.auth.SSOCreated).toBeCalledWith(Config.OIDC) + expect(events.auth.SSOCreated).toBeCalledWith(ConfigType.OIDC) expect(events.auth.SSOActivated).not.toBeCalled() expect(events.auth.SSODeactivated).not.toBeCalled() - await config.deleteConfig(Config.OIDC) + await config.deleteConfig(ConfigType.OIDC) }) }) @@ -142,11 +150,11 @@ describe("configs", () => { oidcConf._rev ) expect(events.auth.SSOUpdated).toBeCalledTimes(1) - expect(events.auth.SSOUpdated).toBeCalledWith(Config.OIDC) + expect(events.auth.SSOUpdated).toBeCalledWith(ConfigType.OIDC) expect(events.auth.SSOActivated).not.toBeCalled() expect(events.auth.SSODeactivated).toBeCalledTimes(1) - expect(events.auth.SSODeactivated).toBeCalledWith(Config.OIDC) - await config.deleteConfig(Config.OIDC) + expect(events.auth.SSODeactivated).toBeCalledWith(ConfigType.OIDC) + await config.deleteConfig(ConfigType.OIDC) }) it("should update OIDC config to activated", async () => { @@ -158,11 +166,11 @@ describe("configs", () => { oidcConf._rev ) expect(events.auth.SSOUpdated).toBeCalledTimes(1) - expect(events.auth.SSOUpdated).toBeCalledWith(Config.OIDC) + expect(events.auth.SSOUpdated).toBeCalledWith(ConfigType.OIDC) expect(events.auth.SSODeactivated).not.toBeCalled() expect(events.auth.SSOActivated).toBeCalledTimes(1) - expect(events.auth.SSOActivated).toBeCalledWith(Config.OIDC) - await config.deleteConfig(Config.OIDC) + expect(events.auth.SSOActivated).toBeCalledWith(ConfigType.OIDC) + await config.deleteConfig(ConfigType.OIDC) }) }) }) @@ -179,11 +187,11 @@ describe("configs", () => { describe("create", () => { it("should create SMTP config", async () => { - await config.deleteConfig(Config.SMTP) + await config.deleteConfig(ConfigType.SMTP) await saveSMTPConfig() expect(events.email.SMTPUpdated).not.toBeCalled() expect(events.email.SMTPCreated).toBeCalledTimes(1) - await config.deleteConfig(Config.SMTP) + await config.deleteConfig(ConfigType.SMTP) }) }) @@ -194,24 +202,15 @@ describe("configs", () => { await saveSMTPConfig(smtpConf.config, smtpConf._id, smtpConf._rev) expect(events.email.SMTPCreated).not.toBeCalled() expect(events.email.SMTPUpdated).toBeCalledTimes(1) - await config.deleteConfig(Config.SMTP) + await config.deleteConfig(ConfigType.SMTP) }) }) }) describe("settings", () => { - const saveSettingsConfig = async ( - conf?: any, - _id?: string, - _rev?: string - ) => { - const settingsConfig = structures.configs.settings(conf) - return saveConfig(settingsConfig, _id, _rev) - } - describe("create", () => { it("should create settings config with default settings", async () => { - await config.deleteConfig(Config.SETTINGS) + await config.deleteConfig(ConfigType.SETTINGS) await saveSettingsConfig() @@ -222,7 +221,7 @@ describe("configs", () => { it("should create settings config with non-default settings", async () => { config.selfHosted() - await config.deleteConfig(Config.SETTINGS) + await config.deleteConfig(ConfigType.SETTINGS) const conf = { company: "acme", logoUrl: "http://example.com", @@ -241,7 +240,7 @@ describe("configs", () => { describe("update", () => { it("should update settings config", async () => { config.selfHosted() - await config.deleteConfig(Config.SETTINGS) + await config.deleteConfig(ConfigType.SETTINGS) const settingsConfig = await saveSettingsConfig() settingsConfig.config.company = "acme" settingsConfig.config.logoUrl = "http://example.com" @@ -262,14 +261,43 @@ describe("configs", () => { }) }) - it("should return the correct checklist status based on the state of the budibase installation", async () => { - await config.saveSmtpConfig() + describe("GET /api/global/configs/checklist", () => { + it("should return the correct checklist", async () => { + await config.saveSmtpConfig() - const res = await config.api.configs.getConfigChecklist() - const checklist = res.body + const res = await config.api.configs.getConfigChecklist() + const checklist = res.body - expect(checklist.apps.checked).toBeFalsy() - expect(checklist.smtp.checked).toBeTruthy() - expect(checklist.adminUser.checked).toBeTruthy() + expect(checklist.apps.checked).toBeFalsy() + expect(checklist.smtp.checked).toBeTruthy() + expect(checklist.adminUser.checked).toBeTruthy() + }) + }) + + describe("GET /api/global/configs/public", () => { + it("should return the expected public settings", async () => { + await saveSettingsConfig() + + const res = await config.api.configs.getPublicSettings() + const body = res.body as GetPublicSettingsResponse + + const expected = { + _id: "config_settings", + type: "settings", + config: { + company: "Budibase", + logoUrl: "", + analyticsEnabled: false, + google: true, + googleCallbackUrl: `http://localhost:10000/api/global/auth/${config.tenantId}/google/callback`, + isSSOEnforced: false, + oidc: false, + oidcCallbackUrl: `http://localhost:10000/api/global/auth/${config.tenantId}/oidc/callback`, + platformUrl: "http://localhost:10000", + }, + } + delete body._rev + expect(body).toEqual(expected) + }) }) }) diff --git a/packages/worker/src/environment.ts b/packages/worker/src/environment.ts index 71fd89f276..cd5360f7f7 100644 --- a/packages/worker/src/environment.ts +++ b/packages/worker/src/environment.ts @@ -26,8 +26,6 @@ function parseIntSafe(number: any) { } } -const selfHosted = !!parseInt(process.env.SELF_HOSTED || "") - const environment = { // auth MINIO_ACCESS_KEY: process.env.MINIO_ACCESS_KEY, @@ -51,7 +49,7 @@ const environment = { CLUSTER_PORT: process.env.CLUSTER_PORT, // flags NODE_ENV: process.env.NODE_ENV, - SELF_HOSTED: selfHosted, + SELF_HOSTED: !!parseInt(process.env.SELF_HOSTED || ""), LOG_LEVEL: process.env.LOG_LEVEL, MULTI_TENANCY: process.env.MULTI_TENANCY, DISABLE_ACCOUNT_PORTAL: process.env.DISABLE_ACCOUNT_PORTAL, @@ -71,14 +69,6 @@ const environment = { * Mock the email service in use - links to ethereal hosted emails are logged instead. */ ENABLE_EMAIL_TEST_MODE: process.env.ENABLE_EMAIL_TEST_MODE, - /** - * Enable to allow an admin user to login using a password. - * This can be useful to prevent lockout when configuring SSO. - * However, this should be turned OFF by default for security purposes. - */ - ENABLE_SSO_MAINTENANCE_MODE: selfHosted - ? process.env.ENABLE_SSO_MAINTENANCE_MODE - : false, _set(key: any, value: any) { process.env[key] = value // @ts-ignore diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 1e3ff3cbdf..6e3a88bf20 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -13,7 +13,13 @@ import { Event } from "@sentry/types/dist/event" import Application from "koa" import { bootstrap } from "global-agent" import * as db from "./db" -import { auth, logging, events, middleware } from "@budibase/backend-core" +import { + auth, + logging, + events, + middleware, + env as coreEnv, +} from "@budibase/backend-core" db.init() import Koa from "koa" import koaBody from "koa-body" @@ -25,7 +31,7 @@ const koaSession = require("koa-session") const logger = require("koa-pino-logger") import destroyable from "server-destroy" -if (env.ENABLE_SSO_MAINTENANCE_MODE) { +if (coreEnv.ENABLE_SSO_MAINTENANCE_MODE) { console.warn( "Warning: ENABLE_SSO_MAINTENANCE_MODE is set. It is recommended this flag is disabled if maintenance is not in progress" ) diff --git a/packages/worker/src/sdk/auth/auth.ts b/packages/worker/src/sdk/auth/auth.ts index 15a4f3c7e7..8e9cff18dd 100644 --- a/packages/worker/src/sdk/auth/auth.ts +++ b/packages/worker/src/sdk/auth/auth.ts @@ -58,8 +58,8 @@ export const reset = async (email: string) => { } // exit if user has sso - if (await userSdk.isPreventSSOPasswords(user)) { - throw new HTTPError("SSO user cannot reset password", 400) + if (await userSdk.isPreventPasswordActions(user)) { + throw new HTTPError("Password reset is disabled for this user", 400) } // send password reset diff --git a/packages/worker/src/sdk/users/tests/users.spec.ts b/packages/worker/src/sdk/users/tests/users.spec.ts index 41d9298997..77f02eec7a 100644 --- a/packages/worker/src/sdk/users/tests/users.spec.ts +++ b/packages/worker/src/sdk/users/tests/users.spec.ts @@ -1,26 +1,50 @@ import { structures } from "../../../tests" -import * as users from "../users" -import env from "../../../environment" import { mocks } from "@budibase/backend-core/tests" +import { env } from "@budibase/backend-core" +import * as users from "../users" import { CloudAccount } from "@budibase/types" +import { isPreventPasswordActions } from "../users" + +jest.mock("@budibase/pro") +import * as _pro from "@budibase/pro" +const pro = jest.mocked(_pro, true) describe("users", () => { - describe("isPreventSSOPasswords", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + describe("isPreventPasswordActions", () => { + it("returns false for non sso user", async () => { + const user = structures.users.user() + const result = await users.isPreventPasswordActions(user) + expect(result).toBe(false) + }) + it("returns true for sso account user", async () => { const user = structures.users.user() mocks.accounts.getAccount.mockReturnValue( Promise.resolve(structures.accounts.ssoAccount() as CloudAccount) ) - const result = await users.isPreventSSOPasswords(user) + const result = await users.isPreventPasswordActions(user) expect(result).toBe(true) }) it("returns true for sso user", async () => { const user = structures.users.ssoUser() - const result = await users.isPreventSSOPasswords(user) + const result = await users.isPreventPasswordActions(user) expect(result).toBe(true) }) + describe("enforced sso", () => { + it("returns true for all users when sso is enforced", async () => { + const user = structures.users.user() + pro.features.isSSOEnforced.mockReturnValue(Promise.resolve(true)) + const result = await users.isPreventPasswordActions(user) + expect(result).toBe(true) + }) + }) + describe("sso maintenance mode", () => { beforeEach(() => { env._set("ENABLE_SSO_MAINTENANCE_MODE", true) @@ -33,7 +57,7 @@ describe("users", () => { describe("non-admin user", () => { it("returns true", async () => { const user = structures.users.ssoUser() - const result = await users.isPreventSSOPasswords(user) + const result = await users.isPreventPasswordActions(user) expect(result).toBe(true) }) }) @@ -43,7 +67,7 @@ describe("users", () => { const user = structures.users.ssoUser({ user: structures.users.adminUser(), }) - const result = await users.isPreventSSOPasswords(user) + const result = await users.isPreventPasswordActions(user) expect(result).toBe(false) }) }) diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 7d4a2f04f0..b0ca22e44a 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -14,6 +14,7 @@ import { users as usersCore, utils, ViewName, + env as coreEnv, } from "@budibase/backend-core" import { AccountMetadata, @@ -34,7 +35,7 @@ import { } from "@budibase/types" import { sendEmail } from "../../utilities/email" import { EmailTemplatePurpose } from "../../constants" -import { groups as groupsSdk } from "@budibase/pro" +import * as pro from "@budibase/pro" import * as accountSdk from "../accounts" const PAGE_LIMIT = 8 @@ -122,8 +123,8 @@ const buildUser = async ( let hashedPassword if (password) { - if (await isPreventSSOPasswords(user)) { - throw new HTTPError("SSO user cannot set password", 400) + if (await isPreventPasswordActions(user)) { + throw new HTTPError("Password change is disabled for this user", 400) } hashedPassword = opts.hashPassword ? await utils.hash(password) : password } else if (dbUser) { @@ -188,13 +189,18 @@ const validateUniqueUser = async (email: string, tenantId: string) => { } } -export async function isPreventSSOPasswords(user: User) { +export async function isPreventPasswordActions(user: User) { // when in maintenance mode we allow sso users with the admin role // to perform any password action - this prevents lockout - if (env.ENABLE_SSO_MAINTENANCE_MODE && user.admin?.global) { + if (coreEnv.ENABLE_SSO_MAINTENANCE_MODE && user.admin?.global) { return false } + // SSO is enforced for all users + if (await pro.features.isSSOEnforced()) { + return true + } + // Check local sso if (isSSOUser(user)) { return true @@ -278,7 +284,7 @@ export const save = async ( if (userGroups.length > 0) { for (let groupId of userGroups) { - groupPromises.push(groupsSdk.addUsers(groupId, [_id])) + groupPromises.push(pro.groups.addUsers(groupId, [_id])) } } } @@ -456,7 +462,7 @@ export const bulkCreate = async ( const groupPromises = [] const createdUserIds = saved.map(user => user._id) for (let groupId of groups) { - groupPromises.push(groupsSdk.addUsers(groupId, createdUserIds)) + groupPromises.push(pro.groups.addUsers(groupId, createdUserIds)) } await Promise.all(groupPromises) } diff --git a/packages/worker/src/tests/api/configs.ts b/packages/worker/src/tests/api/configs.ts index 76a6f31415..74cef2bf8b 100644 --- a/packages/worker/src/tests/api/configs.ts +++ b/packages/worker/src/tests/api/configs.ts @@ -14,6 +14,14 @@ export class ConfigAPI extends TestAPI { .expect("Content-Type", /json/) } + getPublicSettings = () => { + return this.request + .get(`/api/global/configs/public`) + .set(this.config.defaultHeaders()) + .expect(200) + .expect("Content-Type", /json/) + } + saveConfig = (data: any) => { return this.request .post(`/api/global/configs`) diff --git a/packages/worker/src/tests/structures/configs.ts b/packages/worker/src/tests/structures/configs.ts index 9b5b29f652..d50f5ebc72 100644 --- a/packages/worker/src/tests/structures/configs.ts +++ b/packages/worker/src/tests/structures/configs.ts @@ -1,9 +1,15 @@ -import { Config } from "../../constants" import { utils } from "@budibase/backend-core" +import { + SettingsConfig, + ConfigType, + SMTPConfig, + GoogleConfig, + OIDCConfig, +} from "@budibase/types" -export function oidc(conf?: any) { +export function oidc(conf?: any): OIDCConfig { return { - type: Config.OIDC, + type: ConfigType.OIDC, config: { configs: [ { @@ -21,9 +27,9 @@ export function oidc(conf?: any) { } } -export function google(conf?: any) { +export function google(conf?: any): GoogleConfig { return { - type: Config.GOOGLE, + type: ConfigType.GOOGLE, config: { clientID: "clientId", clientSecret: "clientSecret", @@ -33,9 +39,9 @@ export function google(conf?: any) { } } -export function smtp(conf?: any) { +export function smtp(conf?: any): SMTPConfig { return { - type: Config.SMTP, + type: ConfigType.SMTP, config: { port: 12345, host: "smtptesthost.com", @@ -47,12 +53,13 @@ export function smtp(conf?: any) { } } -export function smtpEthereal() { +export function smtpEthereal(): SMTPConfig { return { - type: Config.SMTP, + type: ConfigType.SMTP, config: { port: 587, host: "smtp.ethereal.email", + from: "testfrom@test.com", secure: false, auth: { user: "wyatt.zulauf29@ethereal.email", @@ -63,9 +70,9 @@ export function smtpEthereal() { } } -export function settings(conf?: any) { +export function settings(conf?: any): SettingsConfig { return { - type: Config.SETTINGS, + type: ConfigType.SETTINGS, config: { platformUrl: "http://localhost:10000", logoUrl: "",