paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 35 Packages Projects Releases Wiki Activity

deep clone data read out of config, to avoid pollution / mutation

Browse Source 
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2022-01-08 16:30:45 +01:00
parent f5343e7c1a
commit 2b19fd2e94
5 changed files with 271 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/fuzzy-llamas-collect.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-ldap': patch
---
Make sure to avoid accidental data sharing / mutation of `set` values
plugins/catalog-backend-module-ldap/src/ldap/config.test.ts
+84
View File
@@ -209,4 +209,88 @@ describe('readLdapConfig', () => {
const expected = '(|(cn=foo bar)(cn=bar))';
expect(actual[0].users.options.filter).toEqual(expected);
});
it('supports a dot nested set structure', () => {
const config = {
providers: [
{
target: 'target',
users: {
dn: 'udn',
options: {
filter: 'f',
},
set: {
'metadata.annotations': {
a: 'b',
},
},
},
groups: {
dn: 'gdn',
options: {
filter: 'f',
},
set: {
x: { a: 'b' },
},
},
},
],
};
const actual = readLdapConfig(new ConfigReader(config));
expect(actual[0].users.set).toEqual({ 'metadata.annotations': { a: 'b' } });
});
it('throws on attempts to modify the set structure', () => {
const config = {
providers: [
{
target: 'target',
users: {
dn: 'udn',
options: {
filter: 'f',
},
set: {
x: { a: 'b' },
},
},
groups: {
dn: 'gdn',
options: {
filter: 'f',
},
set: {
x: { a: 'b' },
},
},
},
],
};
const actual = readLdapConfig(new ConfigReader(config));
expect(() => {
(actual[0].users.set as any).y = 2;
}).toThrowErrorMatchingInlineSnapshot(
`"Cannot add property y, object is not extensible"`,
);
expect(() => {
(actual[0].users.set as any).x.b = 2;
}).toThrowErrorMatchingInlineSnapshot(
`"Cannot add property b, object is not extensible"`,
);
expect(() => {
(actual[0].groups.set as any).y = 2;
}).toThrowErrorMatchingInlineSnapshot(
`"Cannot add property y, object is not extensible"`,
);
expect(() => {
(actual[0].groups.set as any).x.b = 2;
}).toThrowErrorMatchingInlineSnapshot(
`"Cannot add property b, object is not extensible"`,
);
});
});
plugins/catalog-backend-module-ldap/src/ldap/config.ts
+11 -2
View File
@@ -166,6 +166,15 @@ const defaultConfig = {
* @param config The root of the LDAP config hierarchy
*/
export function readLdapConfig(config: Config): LdapProviderConfig[] {
function freeze<T>(data: T): T {
return JSON.parse(JSON.stringify(data), (_key, value) => {
if (typeof value === 'object' && value !== null) {
Object.freeze(value);
}
return value;
});
}
function readBindConfig(
c: Config | undefined,
): LdapProviderConfig['bind'] | undefined {
@@ -217,7 +226,7 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
if (!c) {
return undefined;
}
return Object.fromEntries(c.keys().map(path => [path, c.get(path)]));
return c.get();
}
function readUserMapConfig(
@@ -297,6 +306,6 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
// Replace arrays instead of merging, otherwise default behavior
return Array.isArray(from) ? from : undefined;
});
return merged as LdapProviderConfig;
return freeze(merged) as LdapProviderConfig;
});
}
plugins/catalog-backend-module-ldap/src/ldap/read.test.ts
+168 -1
View File
@@ -25,7 +25,13 @@ import {
LDAP_RDN_ANNOTATION,
LDAP_UUID_ANNOTATION,
} from './constants';
import { readLdapGroups, readLdapUsers, resolveRelations } from './read';
import {
defaultGroupTransformer,
defaultUserTransformer,
readLdapGroups,
readLdapUsers,
resolveRelations,
} from './read';
import { ActiveDirectoryVendor, DefaultLdapVendor } from './vendors';
function user(data: RecursivePartial<UserEntity>): UserEntity {
@@ -264,6 +270,7 @@ describe('readLdapGroups', () => {
new Map([['dn-value', new Set(['x', 'y', 'z'])]]),
);
});
it('transfers all attributes from Microsoft Active Directory', async () => {
client.getVendor.mockResolvedValue(ActiveDirectoryVendor);
client.searchStreaming.mockImplementation(async (_dn, _opts, fn) => {
@@ -358,6 +365,7 @@ describe('resolveRelations', () => {
expect(parent.spec.children).toEqual(['child']);
expect(child.spec.parent).toEqual('parent');
});
it('matches by UUID', () => {
const parent = group({
metadata: {
@@ -539,3 +547,162 @@ describe('resolveRelations', () => {
});
});
});
describe('defaultUserTransformer', () => {
it('can set things safely', async () => {
const config: UserConfig = {
dn: 'ddd',
options: {},
map: {
rdn: 'uid',
name: 'uid',
displayName: 'cn',
email: 'mail',
memberOf: 'memberOf',
},
set: {
'metadata.annotations.a': 1,
'metadata.annotations': { a: 2, b: 3 },
},
};
const entry = searchEntry({
uid: ['uid-value'],
description: ['description-value'],
cn: ['cn-value'],
mail: ['mail-value'],
avatarUrl: ['avatarUrl-value'],
memberOf: ['x', 'y', 'z'],
entryDN: ['dn-value'],
entryUUID: ['uuid-value'],
});
let output = await defaultUserTransformer(DefaultLdapVendor, config, entry);
expect(output).toEqual({
apiVersion: 'backstage.io/v1beta1',
kind: 'User',
metadata: {
annotations: {
'backstage.io/ldap-dn': 'dn-value',
'backstage.io/ldap-rdn': 'uid-value',
'backstage.io/ldap-uuid': 'uuid-value',
a: 2,
b: 3,
},
name: 'uid-value',
},
spec: {
memberOf: [],
profile: { displayName: 'cn-value', email: 'mail-value' },
},
});
(output!.metadata.annotations as any).c = 7;
// exact same inputs again
output = await defaultUserTransformer(DefaultLdapVendor, config, entry);
expect(output).toEqual({
apiVersion: 'backstage.io/v1beta1',
kind: 'User',
metadata: {
annotations: {
'backstage.io/ldap-dn': 'dn-value',
'backstage.io/ldap-rdn': 'uid-value',
'backstage.io/ldap-uuid': 'uuid-value',
a: 2,
b: 3,
},
name: 'uid-value',
},
spec: {
memberOf: [],
profile: { displayName: 'cn-value', email: 'mail-value' },
},
});
});
});
describe('defaultGroupTransformer', () => {
it('can set things safely', async () => {
const config: GroupConfig = {
dn: 'ddd',
options: {},
map: {
rdn: 'uid',
name: 'uid',
displayName: 'cn',
email: 'mail',
description: 'description',
type: 'type',
members: 'members',
memberOf: 'memberOf',
},
set: {
'metadata.annotations.a': 1,
'metadata.annotations': { a: 2, b: 3 },
},
};
const entry = searchEntry({
uid: ['uid-value'],
description: ['description-value'],
cn: ['cn-value'],
mail: ['mail-value'],
avatarUrl: ['avatarUrl-value'],
memberOf: ['x', 'y', 'z'],
entryDN: ['dn-value'],
entryUUID: ['uuid-value'],
});
let output = await defaultGroupTransformer(
DefaultLdapVendor,
config,
entry,
);
expect(output).toEqual({
apiVersion: 'backstage.io/v1beta1',
kind: 'Group',
metadata: {
annotations: {
'backstage.io/ldap-dn': 'dn-value',
'backstage.io/ldap-rdn': 'uid-value',
'backstage.io/ldap-uuid': 'uuid-value',
a: 2,
b: 3,
},
description: 'description-value',
name: 'uid-value',
},
spec: {
type: 'unknown',
children: [],
profile: { displayName: 'cn-value', email: 'mail-value' },
},
});
(output!.metadata.annotations as any).c = 7;
// exact same inputs again
output = await defaultGroupTransformer(DefaultLdapVendor, config, entry);
expect(output).toEqual({
apiVersion: 'backstage.io/v1beta1',
kind: 'Group',
metadata: {
annotations: {
'backstage.io/ldap-dn': 'dn-value',
'backstage.io/ldap-rdn': 'uid-value',
'backstage.io/ldap-uuid': 'uuid-value',
a: 2,
b: 3,
},
description: 'description-value',
name: 'uid-value',
},
spec: {
type: 'unknown',
children: [],
profile: { displayName: 'cn-value', email: 'mail-value' },
},
});
});
});
plugins/catalog-backend-module-ldap/src/ldap/read.ts
+3 -2
View File
@@ -17,6 +17,7 @@
import { GroupEntity, UserEntity } from '@backstage/catalog-model';
import { SearchEntry } from 'ldapjs';
import lodashSet from 'lodash/set';
import cloneDeep from 'lodash/cloneDeep';
import { buildOrgHierarchy } from './org';
import { LdapClient } from './client';
import { GroupConfig, UserConfig } from './config';
@@ -52,7 +53,7 @@ export async function defaultUserTransformer(
if (set) {
for (const [path, value] of Object.entries(set)) {
lodashSet(entity, path, value);
lodashSet(entity, path, cloneDeep(value));
}
}
@@ -146,7 +147,7 @@ export async function defaultGroupTransformer(
if (set) {
for (const [path, value] of Object.entries(set)) {
lodashSet(entity, path, value);
lodashSet(entity, path, cloneDeep(value));
}
}