From 82434ee7b5f8fdd414c8ed7e624f52ee4d43b5f7 Mon Sep 17 00:00:00 2001 From: Jente Sondervorst Date: Mon, 17 Jun 2024 22:37:31 +0200 Subject: [PATCH] Review comments Closes: #25256 Signed-off-by: Jente Sondervorst --- .../catalog-backend-module-ldap/api-report.md | 14 +- .../catalog-backend-module-ldap/config.d.ts | 512 ++++++++++++------ .../src/ldap/config.test.ts | 182 ++++--- .../src/ldap/config.ts | 145 ++--- .../src/ldap/index.ts | 2 - .../src/ldap/read.test.ts | 153 +++--- .../src/ldap/read.ts | 25 +- 7 files changed, 589 insertions(+), 444 deletions(-) diff --git a/plugins/catalog-backend-module-ldap/api-report.md b/plugins/catalog-backend-module-ldap/api-report.md index 7bc53eb7b2..9013f75450 100644 --- a/plugins/catalog-backend-module-ldap/api-report.md +++ b/plugins/catalog-backend-module-ldap/api-report.md @@ -68,9 +68,6 @@ export type GroupConfig = { }; }; -// @public -export type GroupConfigList = GroupConfig | GroupConfig[] | undefined; - // @public export type GroupTransformer = ( vendor: LdapVendor, @@ -200,8 +197,8 @@ export type LdapProviderConfig = { target: string; tls?: TLSConfig; bind?: BindConfig; - users: UserConfigList; - groups: GroupConfigList; + users: UserConfig[]; + groups: GroupConfig[]; schedule?: TaskScheduleDefinition; }; @@ -226,8 +223,8 @@ export function readLdapLegacyConfig(config: Config): LdapProviderConfig[]; // @public export function readLdapOrg( client: LdapClient, - userConfig: UserConfigList, - groupConfig: GroupConfigList, + userConfig: UserConfig[], + groupConfig: GroupConfig[], options: { groupTransformer?: GroupTransformer; userTransformer?: UserTransformer; @@ -266,9 +263,6 @@ export type UserConfig = { }; }; -// @public -export type UserConfigList = UserConfig | UserConfig[] | undefined; - // @public export type UserTransformer = ( vendor: LdapVendor, diff --git a/plugins/catalog-backend-module-ldap/config.d.ts b/plugins/catalog-backend-module-ldap/config.d.ts index a585471033..65914fdf74 100644 --- a/plugins/catalog-backend-module-ldap/config.d.ts +++ b/plugins/catalog-backend-module-ldap/config.d.ts @@ -63,180 +63,352 @@ export interface Config { /** * The settings that govern the reading and interpretation of users. */ - users: { - /** - * The DN under which users are stored. - * - * E.g. "ou=people,ou=example,dc=example,dc=net" - */ - dn: string; - /** - * The search options to use. The default is scope "one" and - * attributes "*" and "+". - * - * It is common to want to specify a filter, to narrow down the set - * of matching items. - */ - options: { - scope?: 'base' | 'one' | 'sub'; - filter?: string; - attributes?: string | string[]; - sizeLimit?: number; - timeLimit?: number; - derefAliases?: number; - typesOnly?: boolean; - paged?: - | boolean - | { - pageSize?: number; - pagePause?: boolean; - }; - }; - /** - * JSON paths (on a.b.c form) and hard coded values to set on those - * paths. - * - * This can be useful for example if you want to hard code a - * namespace or similar on the generated entities. - */ - set?: { [key: string]: JsonValue }; - /** - * Mappings from well known entity fields, to LDAP attribute names - */ - map?: { - /** - * The name of the attribute that holds the relative - * distinguished name of each entry. Defaults to "uid". - */ - rdn?: string; - /** - * The name of the attribute that shall be used for the value of - * the metadata.name field of the entity. Defaults to "uid". - */ - name?: string; - /** - * The name of the attribute that shall be used for the value of - * the metadata.description field of the entity. - */ - description?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.displayName field of the entity. Defaults to - * "cn". - */ - displayName?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.email field of the entity. Defaults to - * "mail". - */ - email?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.picture field of the entity. - */ - picture?: string; - /** - * The name of the attribute that shall be used for the values of - * the spec.memberOf field of the entity. Defaults to "memberOf". - */ - memberOf?: string; - }; - }; + users?: + | { + /** + * The DN under which users are stored. + * + * E.g. "ou=people,ou=example,dc=example,dc=net" + */ + dn: string; + /** + * The search options to use. The default is scope "one" and + * attributes "*" and "+". + * + * It is common to want to specify a filter, to narrow down the set + * of matching items. + */ + options: { + scope?: 'base' | 'one' | 'sub'; + filter?: string; + attributes?: string | string[]; + sizeLimit?: number; + timeLimit?: number; + derefAliases?: number; + typesOnly?: boolean; + paged?: + | boolean + | { + pageSize?: number; + pagePause?: boolean; + }; + }; + /** + * JSON paths (on a.b.c form) and hard coded values to set on those + * paths. + * + * This can be useful for example if you want to hard code a + * namespace or similar on the generated entities. + */ + set?: { [key: string]: JsonValue }; + /** + * Mappings from well known entity fields, to LDAP attribute names + */ + map?: { + /** + * The name of the attribute that holds the relative + * distinguished name of each entry. Defaults to "uid". + */ + rdn?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.name field of the entity. Defaults to "uid". + */ + name?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.description field of the entity. + */ + description?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.displayName field of the entity. Defaults to + * "cn". + */ + displayName?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.email field of the entity. Defaults to + * "mail". + */ + email?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.picture field of the entity. + */ + picture?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.memberOf field of the entity. Defaults to "memberOf". + */ + memberOf?: string; + }; + } + | Array<{ + /** + * The DN under which users are stored. + * + * E.g. "ou=people,ou=example,dc=example,dc=net" + */ + dn: string; + /** + * The search options to use. The default is scope "one" and + * attributes "*" and "+". + * + * It is common to want to specify a filter, to narrow down the set + * of matching items. + */ + options: { + scope?: 'base' | 'one' | 'sub'; + filter?: string; + attributes?: string | string[]; + sizeLimit?: number; + timeLimit?: number; + derefAliases?: number; + typesOnly?: boolean; + paged?: + | boolean + | { + pageSize?: number; + pagePause?: boolean; + }; + }; + /** + * JSON paths (on a.b.c form) and hard coded values to set on those + * paths. + * + * This can be useful for example if you want to hard code a + * namespace or similar on the generated entities. + */ + set?: { [key: string]: JsonValue }; + /** + * Mappings from well known entity fields, to LDAP attribute names + */ + map?: { + /** + * The name of the attribute that holds the relative + * distinguished name of each entry. Defaults to "uid". + */ + rdn?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.name field of the entity. Defaults to "uid". + */ + name?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.description field of the entity. + */ + description?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.displayName field of the entity. Defaults to + * "cn". + */ + displayName?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.email field of the entity. Defaults to + * "mail". + */ + email?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.picture field of the entity. + */ + picture?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.memberOf field of the entity. Defaults to "memberOf". + */ + memberOf?: string; + }; + }>; /** * The settings that govern the reading and interpretation of groups. */ - groups: { - /** - * The DN under which groups are stored. - * - * E.g. "ou=people,ou=example,dc=example,dc=net" - */ - dn: string; - /** - * The search options to use. The default is scope "one" and - * attributes "*" and "+". - * - * It is common to want to specify a filter, to narrow down the set - * of matching items. - */ - options: { - scope?: 'base' | 'one' | 'sub'; - filter?: string; - attributes?: string | string[]; - sizeLimit?: number; - timeLimit?: number; - derefAliases?: number; - typesOnly?: boolean; - paged?: - | boolean - | { - pageSize?: number; - pagePause?: boolean; - }; - }; - /** - * JSON paths (on a.b.c form) and hard coded values to set on those - * paths. - * - * This can be useful for example if you want to hard code a - * namespace or similar on the generated entities. - */ - set?: { [key: string]: JsonValue }; - /** - * Mappings from well known entity fields, to LDAP attribute names - */ - map?: { - /** - * The name of the attribute that holds the relative - * distinguished name of each entry. Defaults to "cn". - */ - rdn?: string; - /** - * The name of the attribute that shall be used for the value of - * the metadata.name field of the entity. Defaults to "cn". - */ - name?: string; - /** - * The name of the attribute that shall be used for the value of - * the metadata.description field of the entity. Defaults to - * "description". - */ - description?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.type field of the entity. Defaults to "groupType". - */ - type?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.displayName field of the entity. Defaults to - * "cn". - */ - displayName?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.email field of the entity. - */ - email?: string; - /** - * The name of the attribute that shall be used for the value of - * the spec.profile.picture field of the entity. - */ - picture?: string; - /** - * The name of the attribute that shall be used for the values of - * the spec.parent field of the entity. Defaults to "memberOf". - */ - memberOf?: string; - /** - * The name of the attribute that shall be used for the values of - * the spec.children field of the entity. Defaults to "member". - */ - members?: string; - }; - }; + groups?: + | { + /** + * The DN under which groups are stored. + * + * E.g. "ou=people,ou=example,dc=example,dc=net" + */ + dn: string; + /** + * The search options to use. The default is scope "one" and + * attributes "*" and "+". + * + * It is common to want to specify a filter, to narrow down the set + * of matching items. + */ + options: { + scope?: 'base' | 'one' | 'sub'; + filter?: string; + attributes?: string | string[]; + sizeLimit?: number; + timeLimit?: number; + derefAliases?: number; + typesOnly?: boolean; + paged?: + | boolean + | { + pageSize?: number; + pagePause?: boolean; + }; + }; + /** + * JSON paths (on a.b.c form) and hard coded values to set on those + * paths. + * + * This can be useful for example if you want to hard code a + * namespace or similar on the generated entities. + */ + set?: { [key: string]: JsonValue }; + /** + * Mappings from well known entity fields, to LDAP attribute names + */ + map?: { + /** + * The name of the attribute that holds the relative + * distinguished name of each entry. Defaults to "cn". + */ + rdn?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.name field of the entity. Defaults to "cn". + */ + name?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.description field of the entity. Defaults to + * "description". + */ + description?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.type field of the entity. Defaults to "groupType". + */ + type?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.displayName field of the entity. Defaults to + * "cn". + */ + displayName?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.email field of the entity. + */ + email?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.picture field of the entity. + */ + picture?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.parent field of the entity. Defaults to "memberOf". + */ + memberOf?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.children field of the entity. Defaults to "member". + */ + members?: string; + }; + } + | Array<{ + /** + * The DN under which groups are stored. + * + * E.g. "ou=people,ou=example,dc=example,dc=net" + */ + dn: string; + /** + * The search options to use. The default is scope "one" and + * attributes "*" and "+". + * + * It is common to want to specify a filter, to narrow down the set + * of matching items. + */ + options: { + scope?: 'base' | 'one' | 'sub'; + filter?: string; + attributes?: string | string[]; + sizeLimit?: number; + timeLimit?: number; + derefAliases?: number; + typesOnly?: boolean; + paged?: + | boolean + | { + pageSize?: number; + pagePause?: boolean; + }; + }; + /** + * JSON paths (on a.b.c form) and hard coded values to set on those + * paths. + * + * This can be useful for example if you want to hard code a + * namespace or similar on the generated entities. + */ + set?: { [key: string]: JsonValue }; + /** + * Mappings from well known entity fields, to LDAP attribute names + */ + map?: { + /** + * The name of the attribute that holds the relative + * distinguished name of each entry. Defaults to "cn". + */ + rdn?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.name field of the entity. Defaults to "cn". + */ + name?: string; + /** + * The name of the attribute that shall be used for the value of + * the metadata.description field of the entity. Defaults to + * "description". + */ + description?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.type field of the entity. Defaults to "groupType". + */ + type?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.displayName field of the entity. Defaults to + * "cn". + */ + displayName?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.email field of the entity. + */ + email?: string; + /** + * The name of the attribute that shall be used for the value of + * the spec.profile.picture field of the entity. + */ + picture?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.parent field of the entity. Defaults to "memberOf". + */ + memberOf?: string; + /** + * The name of the attribute that shall be used for the values of + * the spec.children field of the entity. Defaults to "member". + */ + members?: string; + }; + }>; }>; }; diff --git a/plugins/catalog-backend-module-ldap/src/ldap/config.test.ts b/plugins/catalog-backend-module-ldap/src/ldap/config.test.ts index 8cd41e0b70..553bf3c525 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/config.test.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/config.test.ts @@ -15,7 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; -import { readProviderConfigs, UserConfig } from './config'; +import { readProviderConfigs } from './config'; describe('readLdapConfig', () => { it('applies all of the defaults', () => { @@ -42,38 +42,42 @@ describe('readLdapConfig', () => { id: 'default', target: 'target', bind: undefined, - users: { - dn: 'udn', - options: { - scope: 'one', - attributes: ['*', '+'], + users: [ + { + dn: 'udn', + options: { + scope: 'one', + attributes: ['*', '+'], + }, + set: undefined, + map: { + rdn: 'uid', + name: 'uid', + displayName: 'cn', + email: 'mail', + memberOf: 'memberOf', + }, }, - set: undefined, - map: { - rdn: 'uid', - name: 'uid', - displayName: 'cn', - email: 'mail', - memberOf: 'memberOf', + ], + groups: [ + { + dn: 'gdn', + options: { + scope: 'one', + attributes: ['*', '+'], + }, + set: undefined, + map: { + rdn: 'cn', + name: 'cn', + description: 'description', + type: 'groupType', + displayName: 'cn', + memberOf: 'memberOf', + members: 'member', + }, }, - }, - groups: { - dn: 'gdn', - options: { - scope: 'one', - attributes: ['*', '+'], - }, - set: undefined, - map: { - rdn: 'cn', - name: 'cn', - description: 'description', - type: 'groupType', - displayName: 'cn', - memberOf: 'memberOf', - members: 'member', - }, - }, + ], }, ]; expect(actual).toEqual(expected); @@ -159,57 +163,61 @@ describe('readLdapConfig', () => { keys: '/tmp/keys.pem', certs: '/tmp/certs.pem', }, - users: { - dn: 'udn', - options: { - scope: 'base', - attributes: ['*'], - filter: 'f', - paged: true, - timeLimit: 42, - sizeLimit: 100, - derefAliases: 0, - typesOnly: false, - }, - set: { p: 'v' }, - map: { - rdn: 'u', - name: 'v', - description: 'd', - displayName: 'c', - email: 'm', - picture: 'p', - memberOf: 'm', - }, - }, - groups: { - dn: 'gdn', - options: { - scope: 'base', - attributes: ['*'], - filter: 'f', - paged: { - pageSize: 7, - pagePause: true, + users: [ + { + dn: 'udn', + options: { + scope: 'base', + attributes: ['*'], + filter: 'f', + paged: true, + timeLimit: 42, + sizeLimit: 100, + derefAliases: 0, + typesOnly: false, + }, + set: { p: 'v' }, + map: { + rdn: 'u', + name: 'v', + description: 'd', + displayName: 'c', + email: 'm', + picture: 'p', + memberOf: 'm', }, - timeLimit: 42, - sizeLimit: 100, - derefAliases: 1, - typesOnly: true, }, - set: { p: 'v' }, - map: { - rdn: 'u', - name: 'v', - description: 'd', - type: 't', - displayName: 'c', - email: 'm', - picture: 'p', - memberOf: 'm', - members: 'n', + ], + groups: [ + { + dn: 'gdn', + options: { + scope: 'base', + attributes: ['*'], + filter: 'f', + paged: { + pageSize: 7, + pagePause: true, + }, + timeLimit: 42, + sizeLimit: 100, + derefAliases: 1, + typesOnly: true, + }, + set: { p: 'v' }, + map: { + rdn: 'u', + name: 'v', + description: 'd', + type: 't', + displayName: 'c', + email: 'm', + picture: 'p', + memberOf: 'm', + members: 'n', + }, }, - }, + ], }, ]; expect(actual).toEqual(expected); @@ -247,7 +255,7 @@ describe('readLdapConfig', () => { const actual = readProviderConfigs(new ConfigReader(config)); const expected = '(|(cn=foo bar)(cn=bar))'; - expect((actual[0].users!! as UserConfig).options.filter).toEqual(expected); + expect(actual[0].users[0].options.filter).toEqual(expected); }); it('supports a dot nested set structure', () => { @@ -284,7 +292,7 @@ describe('readLdapConfig', () => { }; const actual = readProviderConfigs(new ConfigReader(config)); - expect((actual[0].users!! as UserConfig).set).toEqual({ + expect(actual[0].users[0].set).toEqual({ 'metadata.annotations': { a: 'b' }, }); }); @@ -322,23 +330,23 @@ describe('readLdapConfig', () => { const actual = readProviderConfigs(new ConfigReader(config)); expect(() => { - ((actual[0].users!! as UserConfig).set as any).y = 2; + (actual[0].users[0].set as any).y = 2; }).toThrowErrorMatchingInlineSnapshot( `"Cannot add property y, object is not extensible"`, ); expect(() => { - ((actual[0].users!! as UserConfig).set as any).x.b = 2; + (actual[0].users[0].set as any).x.b = 2; }).toThrowErrorMatchingInlineSnapshot( `"Cannot add property b, object is not extensible"`, ); expect(() => { - ((actual[0].users!! as UserConfig).set as any).y = 2; + (actual[0].users[0].set as any).y = 2; }).toThrowErrorMatchingInlineSnapshot( `"Cannot add property y, object is not extensible"`, ); expect(() => { - ((actual[0].users!! as UserConfig).set as any).x.b = 2; + (actual[0].users[0].set as any).x.b = 2; }).toThrowErrorMatchingInlineSnapshot( `"Cannot add property b, object is not extensible"`, ); @@ -392,7 +400,7 @@ describe('readLdapConfig', () => { }; const actual = readProviderConfigs(new ConfigReader(config)); - expect(actual[0].users).toBeUndefined(); - expect(actual[0].groups).toBeUndefined(); + expect(actual[0].users).toHaveLength(0); + expect(actual[0].groups).toHaveLength(0); }); }); diff --git a/plugins/catalog-backend-module-ldap/src/ldap/config.ts b/plugins/catalog-backend-module-ldap/src/ldap/config.ts index d31de61605..9295639cae 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/config.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/config.ts @@ -42,9 +42,9 @@ export type LdapProviderConfig = { // command is not issued. bind?: BindConfig; // The settings that govern the reading and interpretation of users - users: UserConfigList; + users: UserConfig[]; // The settings that govern the reading and interpretation of groups - groups: GroupConfigList; + groups: GroupConfig[]; // Schedule configuration for refresh tasks. schedule?: TaskScheduleDefinition; }; @@ -76,13 +76,6 @@ export type BindConfig = { secret: string; }; -/** - * The settings that govern the reading and interpretation of users. - * - * @public - */ -export type UserConfigList = UserConfig | UserConfig[] | undefined; - /** * The settings that govern the reading and interpretation of users. * @@ -123,13 +116,6 @@ export type UserConfig = { }; }; -/** - * The settings that govern the reading and interpretation of groups. - * - * @public - */ -export type GroupConfigList = GroupConfig | GroupConfig[] | undefined; - /** * The settings that govern the reading and interpretation of groups. * @@ -175,34 +161,33 @@ export type GroupConfig = { }; }; -const defaultConfig = { - users: { - options: { - scope: 'one', - attributes: ['*', '+'], - }, - map: { - rdn: 'uid', - name: 'uid', - displayName: 'cn', - email: 'mail', - memberOf: 'memberOf', - }, +const defaultUserConfig = { + options: { + scope: 'one', + attributes: ['*', '+'], }, - groups: { - options: { - scope: 'one', - attributes: ['*', '+'], - }, - map: { - rdn: 'cn', - name: 'cn', - description: 'description', - displayName: 'cn', - type: 'groupType', - memberOf: 'memberOf', - members: 'member', - }, + map: { + rdn: 'uid', + name: 'uid', + displayName: 'cn', + email: 'mail', + memberOf: 'memberOf', + }, +}; + +const defaultGroupConfig = { + options: { + scope: 'one', + attributes: ['*', '+'], + }, + map: { + rdn: 'cn', + name: 'cn', + description: 'description', + displayName: 'cn', + type: 'groupType', + memberOf: 'memberOf', + members: 'member', }, }; @@ -326,18 +311,15 @@ function readUserConfig( c: Config | Config[] | undefined, ): RecursivePartial { if (!c) { - return undefined; + return []; } if (Array.isArray(c)) { - return c.map(it => { - return { - dn: it.getString('dn'), - options: readOptionsConfig(it.getOptionalConfig('options')), - set: readSetConfig(it.getOptionalConfig('set')), - map: readUserMapConfig(it.getOptionalConfig('map')), - }; - }); + return c.map(it => readSingleUserConfig(it)); } + return [readSingleUserConfig(c)]; +} + +function readSingleUserConfig(c: Config): RecursivePartial { return { dn: c.getString('dn'), options: readOptionsConfig(c.getOptionalConfig('options')), @@ -350,18 +332,15 @@ function readGroupConfig( c: Config | Config[] | undefined, ): RecursivePartial { if (!c) { - return undefined; + return []; } if (Array.isArray(c)) { - return c.map(it => { - return { - dn: it.getString('dn'), - options: readOptionsConfig(it.getOptionalConfig('options')), - set: readSetConfig(it.getOptionalConfig('set')), - map: readGroupMapConfig(it.getOptionalConfig('map')), - }; - }); + return c.map(it => readSingleGroupConfig(it)); } + return [readSingleGroupConfig(c)]; +} + +function readSingleGroupConfig(c: Config): RecursivePartial { return { dn: c.getString('dn'), options: readOptionsConfig(c.getOptionalConfig('options')), @@ -390,14 +369,15 @@ export function readLdapLegacyConfig(config: Config): LdapProviderConfig[] { target: trimEnd(c.getString('target'), '/'), tls: readTlsConfig(c.getOptionalConfig('tls')), bind: readBindConfig(c.getOptionalConfig('bind')), - users: readUserConfig(c.getConfig('users')), - groups: readGroupConfig(c.getConfig('groups')), + users: readUserConfig(c.getConfig('users')).map(it => { + return mergeWith({}, defaultUserConfig, it, replaceArraysIfPresent); + }), + groups: readGroupConfig(c.getConfig('groups')).map(it => { + return mergeWith({}, defaultGroupConfig, it, replaceArraysIfPresent); + }), }; - const merged = mergeWith({}, defaultConfig, newConfig, (_into, from) => { - // Replace arrays instead of merging, otherwise default behavior - return Array.isArray(from) ? from : undefined; - }); - return freeze(merged) as LdapProviderConfig; + + return freeze(newConfig) as LdapProviderConfig; }); } @@ -433,29 +413,24 @@ export function readProviderConfigs(config: Config): LdapProviderConfig[] { isUserList ? c.getOptionalConfigArray('users') : c.getOptionalConfig('users'), - ), + ).map(it => { + return mergeWith({}, defaultUserConfig, it, replaceArraysIfPresent); + }), groups: readGroupConfig( isGroupList ? c.getOptionalConfigArray('groups') : c.getOptionalConfig('groups'), - ), + ).map(it => { + return mergeWith({}, defaultGroupConfig, it, replaceArraysIfPresent); + }), schedule, }; - const merged = mergeWith( - {}, - defaultConfig, - newConfig, - (_value, srcValue, key, object, _source) => { - // Remove users and groups from default if they are not set in the new config - if ((key === 'users' || key === 'groups') && !srcValue) { - return (object[key] = srcValue); - } - // Replace arrays instead of merging, otherwise default behavior - return Array.isArray(srcValue) ? srcValue : undefined; - }, - ); - - return freeze(merged) as LdapProviderConfig; + return freeze(newConfig) as LdapProviderConfig; }); } + +function replaceArraysIfPresent(_into: any, from: any) { + // Replace arrays instead of merging, otherwise default behavior + return Array.isArray(from) ? from : undefined; +} diff --git a/plugins/catalog-backend-module-ldap/src/ldap/index.ts b/plugins/catalog-backend-module-ldap/src/ldap/index.ts index 81cab40b75..cfab5a4444 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/index.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/index.ts @@ -20,9 +20,7 @@ export { readProviderConfigs, readLdapLegacyConfig } from './config'; export type { LdapProviderConfig, GroupConfig, - GroupConfigList, UserConfig, - UserConfigList, BindConfig, TLSConfig, } from './config'; diff --git a/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts b/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts index 33c52477aa..1c3dfb179a 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts @@ -18,12 +18,7 @@ import { GroupEntity, UserEntity } from '@backstage/catalog-model'; import { SearchEntry } from 'ldapjs'; import merge from 'lodash/merge'; import { LdapClient } from './client'; -import { - GroupConfig, - GroupConfigList, - UserConfig, - UserConfigList, -} from './config'; +import { GroupConfig, UserConfig } from './config'; import { LDAP_DN_ANNOTATION, LDAP_RDN_ANNOTATION, @@ -104,19 +99,21 @@ describe('readLdapUsers', () => { }), ); }); - const config: UserConfig = { - dn: 'ddd', - options: {}, - map: { - rdn: 'uid', - name: 'uid', - description: 'description', - displayName: 'cn', - email: 'mail', - picture: 'avatarUrl', - memberOf: 'memberOf', + const config: UserConfig[] = [ + { + dn: 'ddd', + options: {}, + map: { + rdn: 'uid', + name: 'uid', + description: 'description', + displayName: 'cn', + email: 'mail', + picture: 'avatarUrl', + memberOf: 'memberOf', + }, }, - }; + ]; const { users, userMemberOf } = await readLdapUsers(client, config); expect(users).toEqual([ expect.objectContaining({ @@ -165,19 +162,21 @@ describe('readLdapUsers', () => { }), ); }); - const config: UserConfig = { - dn: 'ddd', - options: {}, - map: { - rdn: 'uid', - name: 'uid', - description: 'description', - displayName: 'cn', - email: 'mail', - picture: 'avatarUrl', - memberOf: 'memberOf', + const config: UserConfig[] = [ + { + dn: 'ddd', + options: {}, + map: { + rdn: 'uid', + name: 'uid', + description: 'description', + displayName: 'cn', + email: 'mail', + picture: 'avatarUrl', + memberOf: 'memberOf', + }, }, - }; + ]; const { users, userMemberOf } = await readLdapUsers(client, config); expect(users).toEqual([ expect.objectContaining({ @@ -221,19 +220,21 @@ describe('readLdapUsers', () => { }), ); }); - const config: UserConfig = { - dn: 'ddd', - options: {}, - map: { - rdn: 'uid', - name: 'uid', - description: 'description', - displayName: 'cn', - email: 'mail', - picture: 'avatarUrl', - memberOf: 'memberOf', + const config: UserConfig[] = [ + { + dn: 'ddd', + options: {}, + map: { + rdn: 'uid', + name: 'uid', + description: 'description', + displayName: 'cn', + email: 'mail', + picture: 'avatarUrl', + memberOf: 'memberOf', + }, }, - }; + ]; const { users, userMemberOf } = await readLdapUsers(client, config); expect(users).toEqual([ expect.objectContaining({ @@ -276,7 +277,7 @@ describe('readLdapUsers', () => { }), ); }); - const config: UserConfigList = [ + const config: UserConfig[] = [ { dn: 'ddd', options: {}, @@ -308,7 +309,7 @@ describe('readLdapUsers', () => { expect(users).toHaveLength(2); }); it('can process no UserConfigs', async () => { - const config: UserConfigList = undefined; + const config: UserConfig[] = []; const { users } = await readLdapUsers(client, config); expect(users).toHaveLength(0); }); @@ -339,21 +340,23 @@ describe('readLdapGroups', () => { }), ); }); - const config: GroupConfig = { - dn: 'ddd', - options: {}, - map: { - rdn: 'cn', - name: 'cn', - description: 'description', - displayName: 'cn', - email: 'mail', - picture: 'avatarUrl', - type: 'tt', - memberOf: 'memberOf', - members: 'member', + const config: GroupConfig[] = [ + { + dn: 'ddd', + options: {}, + map: { + rdn: 'cn', + name: 'cn', + description: 'description', + displayName: 'cn', + email: 'mail', + picture: 'avatarUrl', + type: 'tt', + memberOf: 'memberOf', + members: 'member', + }, }, - }; + ]; const { groups, groupMember, groupMemberOf } = await readLdapGroups( client, config, @@ -410,21 +413,23 @@ describe('readLdapGroups', () => { }), ); }); - const config: GroupConfig = { - dn: 'ddd', - options: {}, - map: { - rdn: 'cn', - name: 'cn', - description: 'description', - displayName: 'cn', - email: 'mail', - picture: 'avatarUrl', - type: 'tt', - memberOf: 'memberOf', - members: 'member', + const config: GroupConfig[] = [ + { + dn: 'ddd', + options: {}, + map: { + rdn: 'cn', + name: 'cn', + description: 'description', + displayName: 'cn', + email: 'mail', + picture: 'avatarUrl', + type: 'tt', + memberOf: 'memberOf', + members: 'member', + }, }, - }; + ]; const { groups, groupMember, groupMemberOf } = await readLdapGroups( client, config, @@ -476,7 +481,7 @@ describe('readLdapGroups', () => { }), ); }); - const config: GroupConfigList = [ + const config: GroupConfig[] = [ { dn: 'ddd', options: {}, @@ -513,7 +518,7 @@ describe('readLdapGroups', () => { }); it('can process no GroupConfigs', async () => { - const config: GroupConfigList = undefined; + const config: GroupConfig[] = []; const { groups } = await readLdapGroups(client, config); expect(groups).toHaveLength(0); }); diff --git a/plugins/catalog-backend-module-ldap/src/ldap/read.ts b/plugins/catalog-backend-module-ldap/src/ldap/read.ts index 5dadcaede2..9e6e11a0bc 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.ts @@ -24,12 +24,7 @@ import lodashSet from 'lodash/set'; import cloneDeep from 'lodash/cloneDeep'; import { buildOrgHierarchy } from './org'; import { LdapClient } from './client'; -import { - GroupConfig, - GroupConfigList, - UserConfig, - UserConfigList, -} from './config'; +import { GroupConfig, UserConfig } from './config'; import { LDAP_DN_ANNOTATION, LDAP_RDN_ANNOTATION, @@ -109,23 +104,22 @@ export async function defaultUserTransformer( */ export async function readLdapUsers( client: LdapClient, - config: UserConfigList, + config: UserConfig[], opts?: { transformer?: UserTransformer }, ): Promise<{ users: UserEntity[]; // With all relations empty userMemberOf: Map>; // DN -> DN or UUID of groups }> { - if (!config) { + if (config.length === 0) { return { users: [], userMemberOf: new Map() }; } - const configs = Array.isArray(config) ? config : [config]; const entities: UserEntity[] = []; const userMemberOf: Map> = new Map(); const vendor = await client.getVendor(); const transformer = opts?.transformer ?? defaultUserTransformer; - for (const cfg of configs) { + for (const cfg of config) { const { dn, options, map } = cfg; await client.searchStreaming(dn, options, async user => { const entity = await transformer(vendor, cfg, user); @@ -216,7 +210,7 @@ export async function defaultGroupTransformer( */ export async function readLdapGroups( client: LdapClient, - config: GroupConfigList, + config: GroupConfig[], opts?: { transformer?: GroupTransformer; }, @@ -225,10 +219,9 @@ export async function readLdapGroups( groupMemberOf: Map>; // DN -> DN or UUID of groups groupMember: Map>; // DN -> DN or UUID of groups & users }> { - if (!config) { + if (config.length === 0) { return { groups: [], groupMemberOf: new Map(), groupMember: new Map() }; } - const configs = Array.isArray(config) ? config : [config]; const groups: GroupEntity[] = []; const groupMemberOf: Map> = new Map(); const groupMember: Map> = new Map(); @@ -236,7 +229,7 @@ export async function readLdapGroups( const vendor = await client.getVendor(); const transformer = opts?.transformer ?? defaultGroupTransformer; - for (const cfg of configs) { + for (const cfg of config) { const { dn, map, options } = cfg; await client.searchStreaming(dn, options, async entry => { @@ -280,8 +273,8 @@ export async function readLdapGroups( */ export async function readLdapOrg( client: LdapClient, - userConfig: UserConfigList, - groupConfig: GroupConfigList, + userConfig: UserConfig[], + groupConfig: GroupConfig[], options: { groupTransformer?: GroupTransformer; userTransformer?: UserTransformer;