From c0d7f2329a2d00ab57ae4f437f7e16583e3b1772 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 3 Apr 2023 16:42:30 +0100 Subject: [PATCH 1/3] Extract valid email util --- .../backend-core/src/middleware/passport/sso/oidc.ts | 11 ++--------- packages/backend-core/src/utils/index.ts | 1 + packages/backend-core/src/utils/stringUtils.ts | 8 ++++++++ 3 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 packages/backend-core/src/utils/stringUtils.ts diff --git a/packages/backend-core/src/middleware/passport/sso/oidc.ts b/packages/backend-core/src/middleware/passport/sso/oidc.ts index b6d5eb52e9..83bfde28b6 100644 --- a/packages/backend-core/src/middleware/passport/sso/oidc.ts +++ b/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. diff --git a/packages/backend-core/src/utils/index.ts b/packages/backend-core/src/utils/index.ts index 8e663bce52..318a7f13ba 100644 --- a/packages/backend-core/src/utils/index.ts +++ b/packages/backend-core/src/utils/index.ts @@ -1,2 +1,3 @@ export * from "./hashing" export * from "./utils" +export * from "./stringUtils" diff --git a/packages/backend-core/src/utils/stringUtils.ts b/packages/backend-core/src/utils/stringUtils.ts new file mode 100644 index 0000000000..fd3b4c6132 --- /dev/null +++ b/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,}))$/ + ) + ) +} From ad4f70b82600590c2844c167fac99c75bb6ead18 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 3 Apr 2023 16:45:55 +0100 Subject: [PATCH 2/3] Type emails --- .../types/src/api/web/global/scim/users.ts | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/types/src/api/web/global/scim/users.ts b/packages/types/src/api/web/global/scim/users.ts index 1b3b05d2ee..1e6aa58d0f 100644 --- a/packages/types/src/api/web/global/scim/users.ts +++ b/packages/types/src/api/web/global/scim/users.ts @@ -1,8 +1,16 @@ -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 + }[] + | undefined + export interface ScimUserResponse extends ScimResource { schemas: ["urn:ietf:params:scim:schemas:core:2.0:User"] id: string @@ -18,13 +26,7 @@ export interface ScimUserResponse extends ScimResource { givenName: string } active: BooleanString - emails: [ - { - value: string - type: "work" - primary: true - } - ] + emails: Emails } export interface ScimCreateUserRequest { @@ -35,13 +37,7 @@ export interface ScimCreateUserRequest { externalId: string userName: string active: BooleanString - emails: [ - { - primary: true - type: "work" - value: string - } - ] + emails: Emails meta: { resourceType: "User" } From e0bcc42c803b1803ecac3fb733bdbc1f15d55eea Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 3 Apr 2023 19:06:03 +0100 Subject: [PATCH 3/3] Make scim info extensible and the object unaware of the data --- .../types/src/api/web/global/scim/users.ts | 5 +- packages/types/src/documents/global/user.ts | 7 +- .../src/api/routes/global/tests/scim.spec.ts | 66 ++++++++++++++++++- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/packages/types/src/api/web/global/scim/users.ts b/packages/types/src/api/web/global/scim/users.ts index 1e6aa58d0f..53424934ff 100644 --- a/packages/types/src/api/web/global/scim/users.ts +++ b/packages/types/src/api/web/global/scim/users.ts @@ -9,7 +9,6 @@ type Emails = type: "work" primary: boolean }[] - | undefined export interface ScimUserResponse extends ScimResource { schemas: ["urn:ietf:params:scim:schemas:core:2.0:User"] @@ -26,7 +25,7 @@ export interface ScimUserResponse extends ScimResource { givenName: string } active: BooleanString - emails: Emails + emails?: Emails } export interface ScimCreateUserRequest { @@ -37,7 +36,7 @@ export interface ScimCreateUserRequest { externalId: string userName: string active: BooleanString - emails: Emails + emails?: Emails meta: { resourceType: "User" } diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index cd59482f26..9b4aadf404 100644 --- a/packages/types/src/documents/global/user.ts +++ b/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 } export enum UserStatus { diff --git a/packages/worker/src/api/routes/global/tests/scim.spec.ts b/packages/worker/src/api/routes/global/tests/scim.spec.ts index 1fe4c0450e..3dff3fb7a5 100644 --- a/packages/worker/src/api/routes/global/tests/scim.spec.ts +++ b/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)