diff --git a/CHANGES b/CHANGES index be7828599..748d5aa7b 100644 --- a/CHANGES +++ b/CHANGES @@ -2741,4 +2741,5 @@ * OIDC avatar support via picture claim * backupcleaner: fix bug where preserved backups were removed incorrectly * directoryserver: cloudflare warning +* oidc/ldap: fix display name parsing to send anything after first name as the last name diff --git a/src/directoryserver.js b/src/directoryserver.js index ac2da55a5..4502eff45 100644 --- a/src/directoryserver.js +++ b/src/directoryserver.js @@ -218,10 +218,9 @@ async function userSearch(req, res, next) { const dn = ldap.parseDN(`cn=${user.id},ou=users,dc=cloudron`); const displayName = user.displayName || user.username || ''; // displayName can be empty and username can be null - const nameParts = displayName.split(' '); - const firstName = nameParts[0]; - const lastName = nameParts.length > 1 ? nameParts[nameParts.length - 1] : ''; // choose last part, if it exists + const { firstName, lastName, middleName } = users.parseDisplayName(displayName); + // https://datatracker.ietf.org/doc/html/rfc2798 const obj = { dn: dn.toString(), attributes: { @@ -234,6 +233,8 @@ async function userSearch(req, res, next) { mailAlternateAddress: user.fallbackEmail, displayname: displayName, givenName: firstName, + sn: lastName, + middleName: middleName, username: user.username, samaccountname: user.username, // to support ActiveDirectory clients memberof: allGroups.filter(function (g) { return g.userIds.indexOf(user.id) !== -1; }).map(function (g) { return `cn=${g.name},ou=groups,dc=cloudron`; }) @@ -242,10 +243,6 @@ async function userSearch(req, res, next) { if (user.twoFactorAuthenticationEnabled) obj.attributes.twoFactorAuthenticationEnabled = true; - // http://www.zytrax.com/books/ldap/ape/core-schema.html#sn has 'name' as SUP which is a DirectoryString - // which is required to have atleast one character if present - if (lastName.length !== 0) obj.attributes.sn = lastName; - // ensure all filter values are also lowercase const lowerCaseFilter = safe(function () { return ldap.parseFilter(req.filter.toString().toLowerCase()); }, null); if (!lowerCaseFilter) return next(new ldap.OperationsError(safe.error.message)); diff --git a/src/ldapserver.js b/src/ldapserver.js index 8f79b1c57..b929caf13 100644 --- a/src/ldapserver.js +++ b/src/ldapserver.js @@ -161,10 +161,9 @@ async function userSearch(req, res, next) { const dn = ldap.parseDN('cn=' + user.id + ',ou=users,dc=cloudron'); const displayName = user.displayName || user.username || ''; // displayName can be empty and username can be null - const nameParts = displayName.split(' '); - const firstName = nameParts[0]; - const lastName = nameParts.length > 1 ? nameParts[nameParts.length - 1] : ''; // choose last part, if it exists + const { firstName, lastName, middleName } = users.parseDisplayName(displayName); + // https://datatracker.ietf.org/doc/html/rfc2798 const obj = { dn: dn.toString(), attributes: { @@ -177,17 +176,14 @@ async function userSearch(req, res, next) { mailAlternateAddress: user.fallbackEmail, displayname: displayName, givenName: firstName, + sn: lastName, + middleName: middleName, username: user.username, samaccountname: user.username, // to support ActiveDirectory clients memberof: allGroups.filter(function (g) { return g.userIds.indexOf(user.id) !== -1; }).map(function (g) { return `cn=${g.name},ou=groups,dc=cloudron`; }) } }; - // http://www.zytrax.com/books/ldap/ape/core-schema.html#sn has 'name' as SUP which is a DirectoryString - // which is required to have atleast one character if present - if (lastName.length !== 0) obj.attributes.sn = lastName; - - // ensure all filter values are also lowercase const lowerCaseFilter = safe(function () { return ldap.parseFilter(req.filter.toString().toLowerCase()); }, null); if (!lowerCaseFilter) return next(new ldap.OperationsError(safe.error.message)); diff --git a/src/oidc.js b/src/oidc.js index 54d99b3a4..068187aae 100644 --- a/src/oidc.js +++ b/src/oidc.js @@ -648,17 +648,17 @@ async function claims(userId/*, use, scope*/) { if (error) return { error: 'user not found' }; const displayName = user.displayName || user.username || ''; // displayName can be empty and username can be null - const nameParts = displayName.split(' '); - const firstName = nameParts[0]; - const lastName = nameParts.length > 1 ? nameParts[nameParts.length - 1] : ''; // choose last part, if it exists + const { firstName, lastName, middleName } = users.parseDisplayName(displayName); const { fqdn:dashboardFqdn } = await dashboard.getLocation(); + // https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims const claims = { sub: user.username, // it is essential to always return a sub claim email: user.email, email_verified: true, family_name: lastName, + middle_name: middleName, given_name: firstName, locale: 'en-US', name: user.displayName, diff --git a/src/routes/test/users-test.js b/src/routes/test/users-test.js index 744416296..4568a1fb4 100644 --- a/src/routes/test/users-test.js +++ b/src/routes/test/users-test.js @@ -638,4 +638,3 @@ describe('Users API', function () { }); }); }); - diff --git a/src/test/users-test.js b/src/test/users-test.js index 02acd35a5..5e13db915 100644 --- a/src/test/users-test.js +++ b/src/test/users-test.js @@ -623,4 +623,18 @@ describe('User', function () { expect(result.length).to.be(0); // should have been removed by mandatory 2fa setting change }); }); + + describe('parseDisplayName', function () { + it('parses names', function () { + const names = [ + { in: 'James', out: { firstName: 'James', lastName: '', middleName: '' } }, + { in: 'James Anderson', out: { firstName: 'James', lastName: 'Anderson', middleName: '' } }, + { in: 'James Philip Anderson', out: { firstName: 'James', lastName: 'Philip Anderson', middleName: '' } }, + ]; + + for (const name of names) { + expect(users.parseDisplayName(name.in)).to.eql(name.out); + } + }); + }); }); diff --git a/src/users.js b/src/users.js index ff396ce3c..c73878ecd 100644 --- a/src/users.js +++ b/src/users.js @@ -56,6 +56,8 @@ exports = module.exports = { resetSource, + parseDisplayName, + AP_MAIL: 'mail', AP_WEBADMIN: 'webadmin', @@ -981,3 +983,15 @@ async function setProfileConfig(profileConfig) { async function resetSource() { await database.query('UPDATE users SET source = ?', [ '' ]); } + +function parseDisplayName(displayName) { + assert.strictEqual(typeof displayName, 'string'); + + const middleName = ''; + const idx = displayName.indexOf(' '); + if (idx === -1) return { firstName: displayName, lastName: '', middleName }; + + const firstName = displayName.substring(0, idx); + const lastName = displayName.substring(idx+1); + return { firstName, lastName, middleName }; +}