Merge pull request #10201 from Budibase/budi-6822-allow-ad-users-without-emails-set

BUDI-6822 - Allow AD users without emails set

packages/backend-core/src/middleware/passport/sso/oidc.ts

@@ -1,6 +1,7 @@
 import fetch from "node-fetch"
 import * as sso from "./sso"
 import { ssoCallbackUrl } from "../utils"
+import { validEmail } from "../../../utils"
 import {
   ConfigType,
   OIDCInnerConfig,
@@ -11,6 +12,7 @@ import {
   JwtClaims,
   SaveSSOUserFunction,
 } from "@budibase/types"
+
 const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy
 
 export function buildVerifyFn(saveUserFn: SaveSSOUserFunction) {
@@ -86,15 +88,6 @@ function getEmail(profile: SSOProfile, jwtClaims: JwtClaims) {
   )
 }
 
-function validEmail(value: string) {
-  return (
-    value &&
-    !!value.match(
-      /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
-    )
-  )
-}
-
 /**
  * Create an instance of the oidc passport strategy. This wrapper fetches the configuration
  * from couchDB rather than environment variables, using this factory is necessary for dynamically configuring passport.

packages/backend-core/src/utils/index.ts

@@ -1,2 +1,3 @@
 export * from "./hashing"
 export * from "./utils"
+export * from "./stringUtils"

packages/backend-core/src/utils/stringUtils.ts

@@ -0,0 +1,8 @@
+export function validEmail(value: string) {
+  return (
+    value &&
+    !!value.match(
+      /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
+    )
+  )
+}

packages/types/src/api/web/global/scim/users.ts

@@ -1,8 +1,15 @@
-import { ScimResource, ScimMeta, ScimPatchOperation } from "scim-patch"
+import { ScimResource, ScimMeta } from "scim-patch"
 import { ScimListResponse } from "./shared"
 
 type BooleanString = boolean | "True" | "true" | "False" | "false"
 
+type Emails =
+  | {
+      value: string
+      type: "work"
+      primary: boolean
+    }[]
+
 export interface ScimUserResponse extends ScimResource {
   schemas: ["urn:ietf:params:scim:schemas:core:2.0:User"]
   id: string
@@ -18,13 +25,7 @@ export interface ScimUserResponse extends ScimResource {
     givenName: string
   }
   active: BooleanString
-  emails: [
-    {
-      value: string
-      type: "work"
-      primary: true
-    }
-  ]
+  emails?: Emails
 }
 
 export interface ScimCreateUserRequest {
@@ -35,13 +36,7 @@ export interface ScimCreateUserRequest {
   externalId: string
   userName: string
   active: BooleanString
-  emails: [
-    {
-      primary: true
-      type: "work"
-      value: string
-    }
-  ]
+  emails?: Emails
   meta: {
     resourceType: "User"
   }

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

@@ -53,12 +53,7 @@ export interface User extends Document {
   dayPassRecordedAt?: string
   userGroups?: string[]
   onboardedAt?: string
-  scimInfo?: {
-    isSync: boolean
-    userName: string
-    externalId: string
-    displayName?: string
-  }
+  scimInfo?: { isSync: true } & Record<string, any>
 }
 
 export enum UserStatus {

packages/worker/src/api/routes/global/tests/scim.spec.ts

@@ -2,6 +2,7 @@ import tk from "timekeeper"
 import _ from "lodash"
 import { mocks, structures } from "@budibase/backend-core/tests"
 import {
+  ScimCreateUserRequest,
   ScimGroupResponse,
   ScimUpdateRequest,
   ScimUserResponse,
@@ -176,7 +177,9 @@ describe("scim", () => {
           const response = await getScimUsers({
             params: {
               filter: encodeURI(
-                `emails[type eq "work"].value eq "${userToFetch?.emails[0].value}"`
+                `emails[type eq "work"].value eq "${
+                  userToFetch?.emails![0].value
+                }"`
               ),
             },
           })
@@ -259,6 +262,61 @@ describe("scim", () => {
 
           expect(events.user.created).toBeCalledTimes(1)
         })
+
+        it("if the username is an email, the user name will be used as email", async () => {
+          const email = structures.generator.email()
+
+          const body: ScimCreateUserRequest = structures.scim.createUserRequest(
+            { username: email }
+          )
+          delete body.emails
+
+          await postScimUser({ body })
+
+          const user = await config.getUser(email)
+          expect(user).toBeDefined()
+          expect(user.email).toEqual(email)
+        })
+
+        it("if multiple emails are provided, the first primary one is used as email", async () => {
+          const email = structures.generator.email()
+
+          const body: ScimCreateUserRequest = {
+            ...structures.scim.createUserRequest(),
+            emails: [
+              {
+                primary: false,
+                type: "work",
+                value: structures.generator.email(),
+              },
+              {
+                primary: true,
+                type: "work",
+                value: email,
+              },
+              {
+                primary: true,
+                type: "work",
+                value: structures.generator.email(),
+              },
+            ],
+          }
+
+          await postScimUser({ body })
+
+          const user = await config.getUser(email)
+          expect(user).toBeDefined()
+          expect(user.email).toEqual(email)
+        })
+
+        it("if no email is provided and the user name is not an email, an exception is thrown", async () => {
+          const body: ScimCreateUserRequest = structures.scim.createUserRequest(
+            { username: structures.generator.name() }
+          )
+          delete body.emails
+
+          await postScimUser({ body }, { expect: 500 })
+        })
       })
     })
 
@@ -392,21 +450,23 @@ describe("scim", () => {
       )
 
       it("supports updating unmapped fields", async () => {
+        const value = structures.generator.letter()
         const body: ScimUpdateRequest = {
           schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
           Operations: [
             {
               op: "Add",
               path: "preferredLanguage",
-              value: structures.generator.letter(),
+              value,
             },
           ],
         }
 
         const response = await patchScimUser({ id: user.id, body })
 
-        const expectedScimUser: ScimUserResponse = {
+        const expectedScimUser = {
           ...user,
+          preferredLanguage: value,
         }
         expect(response).toEqual(expectedScimUser)