From 732700ab82f4b3f2ef2ca899637e2ef9df0ac016 Mon Sep 17 00:00:00 2001 From: John Redwood Date: Sat, 23 Nov 2024 00:27:02 +1100 Subject: [PATCH] fix: Updated patch for original PR26923 that filters ldap users Signed-off-by: John Redwood --- .changeset/few-colts-train.md | 5 ++ .../catalog-backend-module-ldap/report.api.md | 2 - .../src/ldap/config.ts | 8 -- .../src/ldap/read.test.ts | 21 ++--- .../src/ldap/read.ts | 77 ++++++++++--------- .../src/ldap/vendors.ts | 5 -- 6 files changed, 51 insertions(+), 67 deletions(-) create mode 100644 .changeset/few-colts-train.md diff --git a/.changeset/few-colts-train.md b/.changeset/few-colts-train.md new file mode 100644 index 0000000000..1dc6becad1 --- /dev/null +++ b/.changeset/few-colts-train.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-ldap': minor +--- + +Updated fix for ldap entity mapping which doesn't require extra config setting of dnCaseSensitive diff --git a/plugins/catalog-backend-module-ldap/report.api.md b/plugins/catalog-backend-module-ldap/report.api.md index 12dcffcd02..16546cb773 100644 --- a/plugins/catalog-backend-module-ldap/report.api.md +++ b/plugins/catalog-backend-module-ldap/report.api.md @@ -205,7 +205,6 @@ export type LdapProviderConfig = { export type LdapVendor = { dnAttributeName: string; uuidAttributeName: string; - dnCaseSensitive?: boolean; decodeStringAttribute: (entry: SearchEntry, name: string) => string[]; }; @@ -275,6 +274,5 @@ export type UserTransformer = ( export type VendorConfig = { dnAttributeName?: string; uuidAttributeName?: string; - dnCaseSensitive?: boolean; }; ``` diff --git a/plugins/catalog-backend-module-ldap/src/ldap/config.ts b/plugins/catalog-backend-module-ldap/src/ldap/config.ts index 4b53034d36..6fdf682b42 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/config.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/config.ts @@ -181,13 +181,6 @@ export type VendorConfig = { * Attribute name for the unique identifier (UUID) of an entry, */ uuidAttributeName?: string; - - /** - * Attribute to determine if we need to force the DN and members/memberOf values to be forced to same case. - * Some providers may provide lowercase members but multicase DN names which causes the group filtering to break. - * The default is off, but turning this on forces the inbound DN values and all member values to lowercase. - */ - dnCaseSensitive?: boolean; }; const defaultUserConfig = { @@ -263,7 +256,6 @@ function readVendorConfig( return { dnAttributeName: c.getOptionalString('dnAttributeName'), uuidAttributeName: c.getOptionalString('uuidAttributeName'), - dnCaseSensitive: c.getOptionalBoolean('dnCaseSensitive'), }; } 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 20917a0fcc..007678c188 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts @@ -333,7 +333,6 @@ describe('readLdapUsers', () => { it('transfers all attributes from for vendorDN case sensitivity', async () => { const vendor = DefaultLdapVendor; - vendor.dnCaseSensitive = true; client.getVendor.mockResolvedValue(vendor); client.searchStreaming.mockImplementation(async (_dn, _opts, fn) => { await fn( @@ -365,15 +364,13 @@ describe('readLdapUsers', () => { }, ]; const { users, userMemberOf } = await readLdapUsers(client, config, vendor); - // reset dnCaseSensitivity - vendor.dnCaseSensitive = false; expect(users).toEqual([ expect.objectContaining({ metadata: { name: 'uid-value', description: 'description-value', annotations: { - [LDAP_DN_ANNOTATION]: 'dn-value', + [LDAP_DN_ANNOTATION]: 'dn-VALUE', [LDAP_RDN_ANNOTATION]: 'uid-value', [LDAP_UUID_ANNOTATION]: 'uuid-value', }, @@ -389,7 +386,7 @@ describe('readLdapUsers', () => { }), ]); expect(userMemberOf).toEqual( - new Map([['dn-value', new Set(['x', 'y', 'z'])]]), + new Map([['dn-VALUE', new Set(['x', 'Y', 'z'])]]), ); }); it('fails to transfer all attributes from for due case sensitivity', async () => { @@ -1111,14 +1108,13 @@ describe('defaultUserTransformerWithCaseSensitiveDNs', () => { }); const vendor = DefaultLdapVendor; - vendor.dnCaseSensitive = true; let output = await defaultUserTransformer(vendor, config, entry); expect(output).toEqual({ apiVersion: 'backstage.io/v1beta1', kind: 'User', metadata: { annotations: { - 'backstage.io/ldap-dn': 'dn-value', + 'backstage.io/ldap-dn': 'dn-VALUE', 'backstage.io/ldap-rdn': 'uid-value', 'backstage.io/ldap-uuid': 'uuid-value', a: 2, @@ -1136,14 +1132,12 @@ describe('defaultUserTransformerWithCaseSensitiveDNs', () => { // exact same inputs again output = await defaultUserTransformer(vendor, config, entry); - // reset dnCaseSensitivity - vendor.dnCaseSensitive = false; expect(output).toEqual({ apiVersion: 'backstage.io/v1beta1', kind: 'User', metadata: { annotations: { - 'backstage.io/ldap-dn': 'dn-value', + 'backstage.io/ldap-dn': 'dn-VALUE', 'backstage.io/ldap-rdn': 'uid-value', 'backstage.io/ldap-uuid': 'uuid-value', a: 2, @@ -1192,7 +1186,6 @@ describe('defaultGroupTransformerWithCaseSensitiveDNs', () => { }); const vendor = DefaultLdapVendor; - vendor.dnCaseSensitive = true; let output = await defaultGroupTransformer(vendor, config, entry); expect(output).toEqual({ @@ -1200,7 +1193,7 @@ describe('defaultGroupTransformerWithCaseSensitiveDNs', () => { kind: 'Group', metadata: { annotations: { - 'backstage.io/ldap-dn': 'dn-value', + 'backstage.io/ldap-dn': 'dn-VALUE', 'backstage.io/ldap-rdn': 'uid-value', 'backstage.io/ldap-uuid': 'uuid-value', a: 2, @@ -1220,14 +1213,12 @@ describe('defaultGroupTransformerWithCaseSensitiveDNs', () => { // exact same inputs again output = await defaultGroupTransformer(vendor, config, entry); - // reset dnCaseSensitivity - vendor.dnCaseSensitive = false; expect(output).toEqual({ apiVersion: 'backstage.io/v1beta1', kind: 'Group', metadata: { annotations: { - 'backstage.io/ldap-dn': 'dn-value', + 'backstage.io/ldap-dn': 'dn-VALUE', 'backstage.io/ldap-rdn': 'uid-value', 'backstage.io/ldap-uuid': 'uuid-value', a: 2, diff --git a/plugins/catalog-backend-module-ldap/src/ldap/read.ts b/plugins/catalog-backend-module-ldap/src/ldap/read.ts index 2ec663d7fe..7c20d012a1 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.ts @@ -80,10 +80,7 @@ export async function defaultUserTransformer( entity.metadata.annotations![LDAP_UUID_ANNOTATION] = v; }); mapStringAttr(entry, vendor, vendor.dnAttributeName, v => { - entity.metadata.annotations![LDAP_DN_ANNOTATION] = getCaseSensitivityValue( - v, - vendor, - ); + entity.metadata.annotations![LDAP_DN_ANNOTATION] = v; }); mapStringAttr(entry, vendor, map.displayName, v => { entity.spec.profile!.displayName = v; @@ -125,8 +122,6 @@ export async function readLdapUsers( vendorConfig?.dnAttributeName ?? vendorDefaults.dnAttributeName, uuidAttributeName: vendorConfig?.uuidAttributeName ?? vendorDefaults.uuidAttributeName, - dnCaseSensitive: - vendorConfig?.dnCaseSensitive ?? vendorDefaults.dnCaseSensitive, decodeStringAttribute: vendorDefaults.decodeStringAttribute, }; const transformer = opts?.transformer ?? defaultUserTransformer; @@ -195,10 +190,7 @@ export async function defaultGroupTransformer( entity.metadata.annotations![LDAP_UUID_ANNOTATION] = v; }); mapStringAttr(entry, vendor, vendor.dnAttributeName, v => { - entity.metadata.annotations![LDAP_DN_ANNOTATION] = getCaseSensitivityValue( - v, - vendor, - ); + entity.metadata.annotations![LDAP_DN_ANNOTATION] = v; }); mapStringAttr(entry, vendor, map.type, v => { entity.spec.type = v; @@ -248,8 +240,6 @@ export async function readLdapGroups( vendorConfig?.dnAttributeName ?? vendorDefaults.dnAttributeName, uuidAttributeName: vendorConfig?.uuidAttributeName ?? vendorDefaults.uuidAttributeName, - dnCaseSensitive: - vendorConfig?.dnCaseSensitive ?? vendorDefaults.dnCaseSensitive, decodeStringAttribute: vendorDefaults.decodeStringAttribute, }; @@ -351,25 +341,11 @@ function mapReferencesAttr( const values = vendor.decodeStringAttribute(entry, attributeName); const dn = vendor.decodeStringAttribute(entry, vendor.dnAttributeName); if (values && dn && dn.length === 1) { - if (vendor.dnCaseSensitive) { - setter( - dn[0].toLocaleLowerCase('en-US'), - values.map(v => v.toLocaleLowerCase('en-US')), - ); - } else { - setter(dn[0], values); - } + setter(dn[0], values); } } } -/** Validates value exists and if required forced sensitivty value to lowercase */ -function getCaseSensitivityValue(value: string, vendor: LdapVendor) { - return value && vendor.dnCaseSensitive - ? value.toLocaleLowerCase('en-US') - : value; -} - // Inserts a number of values in a key-values mapping function ensureItems( target: Map>, @@ -390,6 +366,27 @@ function ensureItems( } } +/** + * Helper function to search a Map ignoring case of the key which is the DN where + * memberOf could potentially return in lowercase. + * @param map - The map of DN's user or group as the key generally multicased. + * @param searchValue - The DN/memberOf search criteria which could potentially not match the map DN by case. + * @returns The value/result of the search criteria forcing key/search lowercase. + */ +function getValueFromMapWithInsensitiveKey( + map: Map, + searchValue: string, +) { + for (const [key, value] of map) { + if ( + key.toLocaleLowerCase('en-US') === searchValue.toLocaleLowerCase('en-US') + ) { + return value; + } + } + return undefined; // In case no match is found, return undefined +} + /** * Takes groups and entities with empty relations, and fills in the various * relations that were returned by the readers, and forms the org hierarchy. @@ -450,10 +447,10 @@ export function resolveRelations( // express relations in different directions. Some may have a user memberOf // overlay, some don't, for example. for (const [userN, groupsN] of userMemberOf.entries()) { - const user = userMap.get(userN); + const user = getValueFromMapWithInsensitiveKey(userMap, userN); if (user) { for (const groupN of groupsN) { - const group = groupMap.get(groupN); + const group = getValueFromMapWithInsensitiveKey(groupMap, groupN); if (group) { ensureItems(newUserMemberOf, stringifyEntityRef(user), [ stringifyEntityRef(group), @@ -463,10 +460,13 @@ export function resolveRelations( } } for (const [groupN, parentsN] of groupMemberOf.entries()) { - const group = groupMap.get(groupN); + const group = getValueFromMapWithInsensitiveKey(groupMap, groupN); if (group) { for (const parentN of parentsN) { - const parentGroup = groupMap.get(parentN); + const parentGroup = getValueFromMapWithInsensitiveKey( + groupMap, + parentN, + ); if (parentGroup) { ensureItems(newGroupParents, stringifyEntityRef(group), [ stringifyEntityRef(parentGroup), @@ -479,18 +479,21 @@ export function resolveRelations( } } for (const [groupN, membersN] of groupMember.entries()) { - const group = groupMap.get(groupN); + const group = getValueFromMapWithInsensitiveKey(groupMap, groupN); if (group) { for (const memberN of membersN) { // Group members can be both users and groups in the input model, so // try both - const memberUser = userMap.get(memberN); + const memberUser = getValueFromMapWithInsensitiveKey(userMap, memberN); if (memberUser) { ensureItems(newUserMemberOf, stringifyEntityRef(memberUser), [ stringifyEntityRef(group), ]); } else { - const memberGroup = groupMap.get(memberN); + const memberGroup = getValueFromMapWithInsensitiveKey( + groupMap, + memberN, + ); if (memberGroup) { ensureItems(newGroupChildren, stringifyEntityRef(group), [ stringifyEntityRef(memberGroup), @@ -506,21 +509,21 @@ export function resolveRelations( // Write down the relations again into the actual entities for (const [userN, groupsN] of newUserMemberOf.entries()) { - const user = userMap.get(userN); + const user = getValueFromMapWithInsensitiveKey(userMap, userN); if (user) { user.spec.memberOf = Array.from(groupsN).sort(); } } for (const [groupN, parentsN] of newGroupParents.entries()) { if (parentsN.size === 1) { - const group = groupMap.get(groupN); + const group = getValueFromMapWithInsensitiveKey(groupMap, groupN); if (group) { group.spec.parent = parentsN.values().next().value; } } } for (const [groupN, childrenN] of newGroupChildren.entries()) { - const group = groupMap.get(groupN); + const group = getValueFromMapWithInsensitiveKey(groupMap, groupN); if (group) { group.spec.children = Array.from(childrenN).sort(); } diff --git a/plugins/catalog-backend-module-ldap/src/ldap/vendors.ts b/plugins/catalog-backend-module-ldap/src/ldap/vendors.ts index eac8481c3e..3afec59114 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/vendors.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/vendors.ts @@ -31,11 +31,6 @@ export type LdapVendor = { */ uuidAttributeName: string; - /** - * The attribute that determines behaviour of the (dn,members,memberOf) for entries. - */ - dnCaseSensitive?: boolean; - /** * Decode ldap entry values for a given attribute name to their string representation. *