diff --git a/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.js b/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.js index a43a07f0c6..fb73ed0665 100644 --- a/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.js +++ b/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.js @@ -29,7 +29,17 @@ export default class AdSettingsController { parseDomainName(account) { this.domainName = ''; - if (!account || !account.includes('@')) { + if (!account) { + return; + } + + // Service account entered as a distinguished name (e.g. cn=reader,dc=portainer,dc=io) + if (!account.includes('@')) { + this.domainSuffix = account + .split(',') + .map((part) => part.trim()) + .filter((part) => part.toLowerCase().startsWith('dc=')) + .join(','); return; } diff --git a/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.test.ts b/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.test.ts new file mode 100644 index 0000000000..4af45959f0 --- /dev/null +++ b/app/portainer/settings/authentication/ldap/ad-settings/ad-settings.controller.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'vitest'; + +import AdSettingsController from './ad-settings.controller'; + +function createController() { + return new AdSettingsController(null, null); +} + +describe('parseDomainName', () => { + it('derives the domain suffix from a UPN-format service account', () => { + const ctrl = createController(); + + ctrl.parseDomainName('reader@portainer.io'); + + expect(ctrl.domainSuffix).toBe('dc=portainer,dc=io'); + }); + + it('derives the domain suffix from a DN-format service account', () => { + const ctrl = createController(); + + ctrl.parseDomainName('cn=reader,dc=portainer,dc=io'); + + expect(ctrl.domainSuffix).toBe('dc=portainer,dc=io'); + }); + + it('trims whitespace around DN components', () => { + const ctrl = createController(); + + ctrl.parseDomainName('cn=reader, dc=portainer, dc=io'); + + expect(ctrl.domainSuffix).toBe('dc=portainer,dc=io'); + }); + + it('clears the domain suffix when a DN has no domain components', () => { + const ctrl = createController(); + + ctrl.parseDomainName('cn=reader'); + + expect(ctrl.domainSuffix).toBe(''); + }); +}); diff --git a/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.controller.js b/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.controller.js index 8b6f5997e2..95775fe922 100644 --- a/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.controller.js +++ b/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.controller.js @@ -28,8 +28,12 @@ export default class LdapUserSearchItemController { } removeGroup(index) { - this.groups.splice(index, 1); - this.onGroupsChange(this.groups); + // Invoked from the React GroupDnBuilder, i.e. outside Angular's digest, so + // wrap the mutation in $evalAsync to trigger a re-render of the ng-repeat. + this.$scope.$evalAsync(() => { + this.groups = this.groups.toSpliced(index, 1); + this.onGroupsChange(this.groups); + }); } addGroup() { diff --git a/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.html b/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.html index 8e839576c8..8fca999a86 100644 --- a/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.html +++ b/app/portainer/settings/authentication/ldap/ldap-user-search-item/ldap-user-search-item.html @@ -21,7 +21,7 @@
- )} +
); } diff --git a/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.test.ts b/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.test.ts index 4d4c3c16c0..47221ea2a1 100644 --- a/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.test.ts +++ b/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from 'vitest'; -import { parseDN, buildDN, DnEntry } from './ldap-dn-utils'; +import { + parseDN, + buildDN, + validateDnEntryValue, + DnEntry, +} from './ldap-dn-utils'; describe('parseDN', () => { it('should parse DN with OU entries', () => { @@ -79,6 +84,25 @@ describe('parseDN', () => { { type: 'ou', value: 'dept_2' }, ]); }); + + it('should preserve special characters in values', () => { + const result = parseDN( + 'ou=R&D (Eng.),ou=Sales+Mktg,dc=example,dc=com', + 'dc=example,dc=com' + ); + + expect(result).toEqual([ + { type: 'ou', value: 'R&D (Eng.)' }, + { type: 'ou', value: 'Sales+Mktg' }, + ]); + }); + + it('should round-trip values containing special characters', () => { + const dn = 'ou=R&D (Eng.),dc=example,dc=com'; + const suffix = 'dc=example,dc=com'; + + expect(buildDN(parseDN(dn, suffix), suffix)).toBe(dn); + }); }); describe('buildDN', () => { @@ -152,3 +176,63 @@ describe('buildDN', () => { expect(result).toBe(''); }); }); + +describe('validateDnEntryValue', () => { + it('should accept an empty value', () => { + expect(validateDnEntryValue('')).toBeUndefined(); + }); + + it('should accept values with allowed characters', () => { + expect(validateDnEntryValue('R&D (Eng.)')).toBeUndefined(); + expect(validateDnEntryValue('Sales_Team 01')).toBeUndefined(); + }); + + it('should accept an equals sign (escaped only in the type, not the value)', () => { + expect(validateDnEntryValue('a=b')).toBeUndefined(); + }); + + it('should accept non-ASCII characters', () => { + expect(validateDnEntryValue('Café Müller')).toBeUndefined(); + }); + + it('should accept an interior number sign', () => { + expect(validateDnEntryValue('Team#1')).toBeUndefined(); + }); + + it('should reject each reserved character', () => { + expect(validateDnEntryValue('a"b')).toContain('"'); + expect(validateDnEntryValue('a+b')).toContain('+'); + expect(validateDnEntryValue('a,b')).toContain(','); + expect(validateDnEntryValue('a;b')).toContain(';'); + expect(validateDnEntryValue('ab')).toContain('>'); + expect(validateDnEntryValue('a\\b')).toContain('\\'); + }); + + it('should list every reserved character it finds', () => { + expect(validateDnEntryValue('a,b+c')).toBe( + 'These characters are not allowed in a DN entry value: + ,' + ); + }); + + it('should reject a leading space or number sign', () => { + expect(validateDnEntryValue(' Users')).toBe( + 'A DN entry value cannot start with a space or "#".' + ); + expect(validateDnEntryValue('#Users')).toBe( + 'A DN entry value cannot start with a space or "#".' + ); + }); + + it('should reject a trailing space', () => { + expect(validateDnEntryValue('Users ')).toBe( + 'A DN entry value cannot end with a space.' + ); + }); + + it('should reject control characters', () => { + expect(validateDnEntryValue('a\tb')).toBe( + 'A DN entry value cannot contain control characters.' + ); + }); +}); diff --git a/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.ts b/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.ts index e95969fb9c..bd371734d1 100644 --- a/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.ts +++ b/app/react/portainer/settings/AuthenticationView/LDAPAuth/DnEntriesField/ldap-dn-utils.ts @@ -3,11 +3,53 @@ export interface DnEntry { value: string; } +// Characters that RFC 4514 requires to be escaped anywhere inside a +// distinguished-name attribute value, mirroring go-ldap's EscapeDN (the +// library the backend authenticates with). Portainer's DN builder concatenates +// values into the DN string without escaping, so an unescaped occurrence would +// corrupt the DN — we reject them up front with a warning instead. +// Note: '=' is intentionally absent (it only needs escaping in the type, not +// the value) and non-ASCII characters are allowed (valid UTF-8 in a DN). +const RESERVED_DN_VALUE_CHARS = ['"', '+', ',', ';', '<', '>', '\\']; + +const CONTROL_CHAR_MAX = 0x1f; +const DELETE_CHAR = 0x7f; + +export function validateDnEntryValue(value: string): string | undefined { + const reserved = RESERVED_DN_VALUE_CHARS.filter((char) => + value.includes(char) + ); + if (reserved.length > 0) { + return `These characters are not allowed in a DN entry value: ${reserved.join( + ' ' + )}`; + } + + // A leading '#' is read as a hex-encoded value, and leading/trailing spaces + // are stripped, so both positions must be escaped (RFC 4514 §2.4). + if (value.startsWith(' ') || value.startsWith('#')) { + return 'A DN entry value cannot start with a space or "#".'; + } + if (value.endsWith(' ')) { + return 'A DN entry value cannot end with a space.'; + } + + const hasControlChar = [...value].some((char) => { + const code = char.charCodeAt(0); + return code <= CONTROL_CHAR_MAX || code === DELETE_CHAR; + }); + if (hasControlChar) { + return 'A DN entry value cannot contain control characters.'; + } + + return undefined; +} + export function parseDN( dn: string | undefined, domainSuffix: string ): DnEntry[] { - const regex = /(\w+)=([a-zA-Z0-9_ -]*),?/; + const regex = /(\w+)=([^,]*),?/; const ouValues: DnEntry[] = []; let left = dn || ''; let match = left.match(regex);