Add validation between sso config acivation and sso enforcement

packages/builder/src/pages/builder/portal/settings/auth/index.svelte

@@ -131,24 +131,25 @@
     isEqual(providers.google?.config, originalGoogleDoc?.config)
       ? (googleSaveButtonDisabled = true)
       : (googleSaveButtonDisabled = false)
+
+    // delete the callback url which is never saved to the oidc
+    // config doc, to ensure an accurate comparison
+    delete providers.oidc?.config.configs[0].callbackURL
+
     isEqual(providers.oidc?.config, originalOidcDoc?.config)
       ? (oidcSaveButtonDisabled = true)
       : (oidcSaveButtonDisabled = false)
   }
 
-  // Create a flag so that it will only try to save completed forms
+  $: googleComplete = !!(
-  $: partialGoogle =
-    providers.google?.config?.clientID || 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
-  $: oidcComplete =
+  )
+
+  $: oidcComplete = !!(
     providers.oidc?.config?.configs[0].configUrl &&
     providers.oidc?.config?.configs[0].clientID &&
     providers.oidc?.config?.configs[0].clientSecret
+  )
 
   const onFileSelected = e => {
     let fileName = e.target.files[0].name
@@ -159,74 +160,88 @@
 
   async function toggleIsSSOEnforced() {
     const value = $organisation.isSSOEnforced
-    await organisation.save({ isSSOEnforced: !value })
+    try {
+      await organisation.save({ isSSOEnforced: !value })
+    } catch (e) {
+      notifications.error(e.message)
+    }
   }
 
-  async function save(docs) {
+  async function saveConfig(config) {
-    let calls = []
+    // Delete unsupported fields
-    // Only if the user has provided an image, upload it
+    delete config.createdAt
+    delete config.updatedAt
+    return API.saveConfig(config)
+  }
+
+  async function saveOIDCLogo() {
     if (image) {
       let data = new FormData()
       data.append("file", image)
-      calls.push(
+      await API.uploadOIDCLogo({
-        API.uploadOIDCLogo({
+        name: image.name,
-          name: image.name,
+        data,
-          data,
+      })
-        })
+    }
+  }
+
+  async function saveOIDC() {
+    if (!oidcComplete) {
+      notifications.error(
+        `Please fill in all required ${ConfigTypes.OIDC} fields`
       )
+      return
     }
-    docs.forEach(element => {
-      // Delete unsupported fields
-      delete element.createdAt
-      delete element.updatedAt
 
-      const { activated } = element.config
+    const oidc = providers.oidc
 
-      if (element.type === ConfigTypes.OIDC) {
+    // Add a UUID here so each config is distinguishable when it arrives at the login page
-        // Add a UUID here so each config is distinguishable when it arrives at the login page
+    for (let config of oidc.config.configs) {
-        for (let config of element.config.configs) {
+      if (!config.uuid) {
-          if (!config.uuid) {
+        config.uuid = Helpers.uuid()
-            config.uuid = Helpers.uuid()
-          }
-          // Callback urls shouldn't be included
-          delete config.callbackURL
-        }
-        if ((partialOidc || activated) && !oidcComplete) {
-          notifications.error(
-            `Please fill in all required ${ConfigTypes.OIDC} fields`
-          )
-        } else if (oidcComplete || !activated) {
-          calls.push(API.saveConfig(element))
-          // Turn the save button grey when clicked
-          oidcSaveButtonDisabled = true
-          originalOidcDoc = cloneDeep(providers.oidc)
-        }
       }
-      if (element.type === ConfigTypes.Google) {
+      // Callback urls shouldn't be included
-        if ((partialGoogle || activated) && !googleComplete) {
+      delete config.callbackURL
-          notifications.error(
-            `Please fill in all required ${ConfigTypes.Google} fields`
-          )
-        } else if (googleComplete || !activated) {
-          calls.push(API.saveConfig(element))
-          googleSaveButtonDisabled = true
-          originalGoogleDoc = cloneDeep(providers.google)
-        }
-      }
-    })
-    if (calls.length) {
-      Promise.all(calls)
-        .then(data => {
-          data.forEach(res => {
-            providers[res.type]._rev = res._rev
-            providers[res.type]._id = res._id
-          })
-          notifications.success(`Settings saved`)
-        })
-        .catch(() => {
-          notifications.error("Failed to update auth settings")
-        })
     }
+
+    try {
+      const res = await saveConfig(oidc)
+      providers[res.type]._rev = res._rev
+      providers[res.type]._id = res._id
+      await saveOIDCLogo()
+      notifications.success(`Settings saved`)
+    } catch (e) {
+      notifications.error(e.message)
+      return
+    }
+
+    // Turn the save button grey when clicked
+    oidcSaveButtonDisabled = true
+    originalOidcDoc = cloneDeep(providers.oidc)
+  }
+
+  async function saveGoogle() {
+    if (!googleComplete) {
+      notifications.error(
+        `Please fill in all required ${ConfigTypes.Google} fields`
+      )
+      return
+    }
+
+    const google = providers.google
+
+    try {
+      const res = await saveConfig(google)
+      providers[res.type]._rev = res._rev
+      providers[res.type]._id = res._id
+      notifications.success(`Settings saved`)
+    } catch (e) {
+      notifications.error(e.message)
+      return
+    }
+
+    googleSaveButtonDisabled = true
+    originalGoogleDoc = cloneDeep(providers.google)
   }
 
   let defaultScopes = ["profile", "email", "offline_access"]
@@ -266,7 +281,7 @@
     if (!googleDoc?._id) {
       providers.google = {
         type: ConfigTypes.Google,
-        config: { activated: true },
+        config: { activated: false },
       }
       originalGoogleDoc = cloneDeep(googleDoc)
     } else {
@@ -310,7 +325,7 @@
     if (!oidcDoc?._id) {
       providers.oidc = {
         type: ConfigTypes.OIDC,
-        config: { configs: [{ activated: true, scopes: defaultScopes }] },
+        config: { configs: [{ activated: false, scopes: defaultScopes }] },
       }
     } else {
       originalOidcDoc = cloneDeep(oidcDoc)
@@ -413,7 +428,7 @@
       <Button
         disabled={googleSaveButtonDisabled}
         cta
-        on:click={() => save([providers.google])}
+        on:click={() => saveGoogle()}
       >
         Save
       </Button>
@@ -575,11 +590,7 @@
       </div>
     </Layout>
     <div>
-      <Button
+      <Button disabled={oidcSaveButtonDisabled} cta on:click={() => saveOIDC()}>
-        disabled={oidcSaveButtonDisabled}
-        cta
-        on:click={() => save([providers.oidc])}
-      >
         Save
       </Button>
     </div>

packages/builder/src/stores/portal/organisation.js

@@ -1,6 +1,7 @@
 import { writable, get } from "svelte/store"
 import { API } from "api"
 import { auth } from "stores/portal"
+import _ from "lodash"
 
 const DEFAULT_CONFIG = {
   platformUrl: "",
@@ -26,14 +27,14 @@ export function createOrganisationStore() {
 
   async function save(config) {
     // Delete non-persisted fields
-    const storeConfig = get(store)
+    const storeConfig = _.cloneDeep(get(store))
     delete storeConfig.oidc
     delete storeConfig.google
     delete storeConfig.oidcCallbackUrl
     delete storeConfig.googleCallbackUrl
     await API.saveConfig({
       type: "settings",
-      config: { ...get(store), ...config },
+      config: { ...storeConfig, ...config },
     })
     await init()
   }

packages/types/src/documents/global/config.ts

@@ -36,6 +36,9 @@ export interface SettingsConfig extends Config {
   config: SettingsInnerConfig
 }
 
+export type SSOConfigType = ConfigType.GOOGLE | ConfigType.OIDC
+export type SSOConfig = GoogleInnerConfig | OIDCInnerConfig
+
 export interface GoogleInnerConfig {
   clientID: string
   clientSecret: string
@@ -60,6 +63,10 @@ export interface OIDCStrategyConfiguration {
   callbackURL: string
 }
 
+export interface OIDCConfigs {
+  configs: OIDCInnerConfig[]
+}
+
 export interface OIDCInnerConfig {
   configUrl: string
   clientID: string
@@ -72,9 +79,7 @@ export interface OIDCInnerConfig {
 }
 
 export interface OIDCConfig extends Config {
-  config: {
+  config: OIDCConfigs
-    configs: OIDCInnerConfig[]
-  }
 }
 
 export interface OIDCWellKnownConfig {

packages/worker/src/api/controllers/global/configs.ts

@@ -17,10 +17,15 @@ import {
   Ctx,
   GetPublicOIDCConfigResponse,
   GetPublicSettingsResponse,
+  GoogleInnerConfig,
   isGoogleConfig,
   isOIDCConfig,
   isSettingsConfig,
   isSMTPConfig,
+  OIDCConfigs,
+  SettingsInnerConfig,
+  SSOConfig,
+  SSOConfigType,
   UserCtx,
 } from "@budibase/types"
 import * as pro from "@budibase/pro"
@@ -119,6 +124,63 @@ const getEventFns = async (config: Config, existing?: Config) => {
   return fns
 }
 
+type SSOConfigs = { [key in SSOConfigType]: SSOConfig | undefined }
+
+async function getSSOConfigs(): Promise<SSOConfigs> {
+  const google = await configs.getGoogleConfig()
+  const oidc = await configs.getOIDCConfig()
+  return {
+    [ConfigType.GOOGLE]: google,
+    [ConfigType.OIDC]: oidc,
+  }
+}
+
+async function hasActivatedConfig(ssoConfigs?: SSOConfigs) {
+  if (!ssoConfigs) {
+    ssoConfigs = await getSSOConfigs()
+  }
+ return !!Object.values(ssoConfigs).find(c => c?.activated)
+}
+
+async function verifySettingsConfig(config: SettingsInnerConfig) {
+  if (config.isSSOEnforced) {
+    const valid = await hasActivatedConfig()
+    if (!valid) {
+      throw new Error(
+        "Cannot enforce SSO without an activated configuration"
+      )
+    }
+  }
+}
+
+async function verifySSOConfig(type: SSOConfigType, config: SSOConfig) {
+  const settings = await configs.getSettingsConfig()
+  if (settings.isSSOEnforced && !config.activated) {
+    // config is being saved as deactivated
+    // ensure there is at least one other activated sso config
+    const ssoConfigs = await getSSOConfigs()
+
+    // overwrite the config being updated
+    // to reflect the desired state
+    ssoConfigs[type] = config
+
+    const activated = await hasActivatedConfig(ssoConfigs)
+    if (!activated) {
+      throw new Error(
+        "Configuration cannot be deactivated while SSO is enforced"
+      )
+    }
+  }
+}
+
+async function verifyGoogleConfig(config: GoogleInnerConfig) {
+  await verifySSOConfig(ConfigType.GOOGLE, config)
+}
+
+async function verifyOIDCConfig(config: OIDCConfigs) {
+  await verifySSOConfig(ConfigType.OIDC, config.configs[0])
+}
+
 export async function save(ctx: UserCtx<Config>) {
   const body = ctx.request.body
   const type = body.type
@@ -133,10 +195,19 @@ export async function save(ctx: UserCtx<Config>) {
 
   try {
     // verify the configuration
-    switch (config.type) {
+    switch (type) {
       case ConfigType.SMTP:
         await email.verifyConfig(config)
         break
+      case ConfigType.SETTINGS:
+        await verifySettingsConfig(config)
+        break
+      case ConfigType.GOOGLE:
+        await verifyGoogleConfig(config)
+        break
+      case ConfigType.OIDC:
+        await verifyOIDCConfig(config)
+        break
     }
   } catch (err: any) {
     ctx.throw(400, err)

packages/worker/src/api/routes/global/configs.ts

@@ -34,8 +34,8 @@ function settingValidation() {
 function googleValidation() {
   // prettier-ignore
   return Joi.object({
-    clientID: Joi.when('activated', { is: true, then: Joi.string().required() }),
+    clientID: Joi.string().required(),
-    clientSecret: Joi.when('activated', { is: true, then: Joi.string().required() }),
+    clientSecret: Joi.string().required(),
     activated: Joi.boolean().required(),
   }).unknown(true)
 }
@@ -45,12 +45,12 @@ function oidcValidation() {
   return Joi.object({
     configs: Joi.array().items(
       Joi.object({
-        clientID: Joi.when('activated', { is: true, then: Joi.string().required() }),
+        clientID: Joi.string().required(),
-        clientSecret: Joi.when('activated', { is: true, then: Joi.string().required() }),
+        clientSecret: Joi.string().required(),
-        configUrl: Joi.when('activated', { is: true, then: Joi.string().required() }),
+        configUrl: Joi.string().required(),
         logo: Joi.string().allow("", null),
         name: Joi.string().allow("", null),
-        uuid: Joi.when('activated', { is: true, then: Joi.string().required() }),
+        uuid: Joi.string().required(),
         activated: Joi.boolean().required(),
         scopes: Joi.array().optional()
       })

packages/worker/src/utilities/email.ts

@@ -203,7 +203,7 @@ export async function sendEmail(
  * @param {object} config an SMTP configuration - this is based on the nodemailer API.
  * @return {Promise<boolean>} returns true if the configuration is valid.
  */
-export async function verifyConfig(config: any) {
+export async function verifyConfig(config: SMTPInnerConfig) {
   const transport = createSMTPTransport(config)
   await transport.verify()
 }