From ce264ff4d3ac24e4d7fa7bc97706fdf83d89f605 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sun, 15 Oct 2017 14:27:20 +0200 Subject: [PATCH] Add {dn} as an available matcher in LDAP groups filter Sometimes, LDAP organization is such that groups membership cannot be computed with username only. User DN is required to retrieve groups. e.g. user Joe has a username joe and a cn of Joe Blogs, resulting in a dn of cn=Joe Blogs,ou=users,dc=example,dc=com which is needed to retrieve groups but cannot be computed from joe only. Issue was reported in issue #146 --- config.template.yml | 5 +- config.test.yml | 5 +- .../lib/configuration/ConfigurationParser.ts | 5 +- server/src/lib/ldap/Client.ts | 33 +++++++++--- server/src/lib/routes/verify/get.ts | 2 +- .../LdapConfigurationAdaptation.test.ts | 2 +- server/test/ldap/Client.test.ts | 51 +++++++++++++++++-- 7 files changed, 82 insertions(+), 21 deletions(-) diff --git a/config.template.yml b/config.template.yml index 8bef04b6..846f7a8c 100644 --- a/config.template.yml +++ b/config.template.yml @@ -33,8 +33,9 @@ ldap: # The groups filter used for retrieving groups of a given user. # {0} is a matcher replaced by username. - # 'member=cn={0},,' by default. - groups_filter: (&(member=cn={0},ou=users,dc=example,dc=com)(objectclass=groupOfNames)) + # {dn} is a matcher replaced by user DN. + # 'member={dn}' by default. + groups_filter: (&(member={dn})(objectclass=groupOfNames)) # The attribute holding the name of the group group_name_attribute: cn diff --git a/config.test.yml b/config.test.yml index 034d12ad..d2a9b7a4 100644 --- a/config.test.yml +++ b/config.test.yml @@ -33,8 +33,9 @@ ldap: # The groups filter used for retrieving groups of a given user. # {0} is a matcher replaced by username. - # 'member=cn={0},,' by default. - groups_filter: (&(member=cn={0},ou=users,dc=example,dc=com)(objectclass=groupOfNames)) + # {dn} is a matcher replaced by user DN. + # 'member={dn}' by default. + groups_filter: (&(member={dn})(objectclass=groupOfNames)) # The attribute holding the name of the group group_name_attribute: cn diff --git a/server/src/lib/configuration/ConfigurationParser.ts b/server/src/lib/configuration/ConfigurationParser.ts index 32f5acf5..bbbe9acf 100644 --- a/server/src/lib/configuration/ConfigurationParser.ts +++ b/server/src/lib/configuration/ConfigurationParser.ts @@ -29,10 +29,7 @@ function ensure_key_existence(config: object, path: string): void { function adaptLdapConfiguration(userConfig: UserLdapConfiguration): LdapConfiguration { const DEFAULT_USERS_FILTER = "cn={0}"; - const DEFAULT_GROUPS_FILTER = - userConfig.additional_users_dn - ? Util.format("member=cn={0},%s,%s", userConfig.additional_groups_dn, userConfig.base_dn) - : Util.format("member=cn={0},%s", userConfig.base_dn); + const DEFAULT_GROUPS_FILTER = "member={dn}"; const DEFAULT_GROUP_NAME_ATTRIBUTE = "cn"; const DEFAULT_MAIL_ATTRIBUTE = "mail"; diff --git a/server/src/lib/ldap/Client.ts b/server/src/lib/ldap/Client.ts index c391bbde..e71a82d0 100644 --- a/server/src/lib/ldap/Client.ts +++ b/server/src/lib/ldap/Client.ts @@ -47,18 +47,34 @@ export class Client implements IClient { }); } + private createGroupsFilter(userGroupsFilter: string, username: string): BluebirdPromise { + if (userGroupsFilter.indexOf("{0}") > 0) { + return BluebirdPromise.resolve(userGroupsFilter.replace("{0}", username)); + } + else if (userGroupsFilter.indexOf("{dn}") > 0) { + return this.searchUserDn(username) + .then(function (userDN: string) { + return BluebirdPromise.resolve(userGroupsFilter.replace("{dn}", userDN)); + }); + } + return BluebirdPromise.resolve(userGroupsFilter); + } + searchGroups(username: string): BluebirdPromise { const that = this; - const filter = that.options.groups_filter.replace("{0}", username); - const query = { - scope: "sub", - attributes: [that.options.group_name_attribute], - filter: filter - }; - return this.ldapClient.searchAsync(that.options.groups_dn, query) + return this.createGroupsFilter(this.options.groups_filter, username) + .then(function (groupsFilter: string) { + that.logger.debug("Computed groups filter is %s", groupsFilter); + const query = { + scope: "sub", + attributes: [that.options.group_name_attribute], + filter: groupsFilter + }; + return that.ldapClient.searchAsync(that.options.groups_dn, query); + }) .then(function (docs: { cn: string }[]) { const groups = docs.map((doc: any) => { return doc.cn; }); - that.logger.debug("LDAP: groups of user %s are %s", username, groups); + that.logger.debug("LDAP: groups of user %s are [%s]", username, groups.join(",")); return BluebirdPromise.resolve(groups); }); } @@ -66,6 +82,7 @@ export class Client implements IClient { searchUserDn(username: string): BluebirdPromise { const that = this; const filter = this.options.users_filter.replace("{0}", username); + this.logger.debug("Computed users filter is %s", filter); const query = { scope: "sub", sizeLimit: 1, diff --git a/server/src/lib/routes/verify/get.ts b/server/src/lib/routes/verify/get.ts index ccf21be0..87c40f89 100644 --- a/server/src/lib/routes/verify/get.ts +++ b/server/src/lib/routes/verify/get.ts @@ -45,7 +45,7 @@ function verify_filter(req: express.Request, res: express.Response): BluebirdPro const isAllowed = accessController.isAccessAllowed(domain, path, username, groups); if (!isAllowed) return BluebirdPromise.reject( - new exceptions.DomainAccessDenied(Util.format("User '%s' does not have access to '%'", + new exceptions.DomainAccessDenied(Util.format("User '%s' does not have access to '%s'", username, domain))); if (authenticationMethod == "two_factor" && !authSession.second_factor) diff --git a/server/test/configuration/LdapConfigurationAdaptation.test.ts b/server/test/configuration/LdapConfigurationAdaptation.test.ts index 59aab253..2d91a4ba 100644 --- a/server/test/configuration/LdapConfigurationAdaptation.test.ts +++ b/server/test/configuration/LdapConfigurationAdaptation.test.ts @@ -56,7 +56,7 @@ describe("test ldap configuration adaptation", function () { users_dn: "dc=example,dc=com", users_filter: "cn={0}", groups_dn: "dc=example,dc=com", - groups_filter: "member=cn={0},dc=example,dc=com", + groups_filter: "member={dn}", group_name_attribute: "cn", mail_attribute: "mail", user: "admin", diff --git a/server/test/ldap/Client.test.ts b/server/test/ldap/Client.test.ts index d69cec01..d2f9c4b1 100644 --- a/server/test/ldap/Client.test.ts +++ b/server/test/ldap/Client.test.ts @@ -31,9 +31,9 @@ describe("test authelia ldap client", function () { const ldapClient = new LdapClientStub(); factory.createStub.returns(ldapClient); - ldapClient.searchAsyncStub.returns(BluebirdPromise.resolve([ - "group1" - ])); + ldapClient.searchAsyncStub.returns(BluebirdPromise.resolve([{ + cn: "group1" + }])); const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Dovehash, Winston); return client.searchGroups("user1") @@ -42,4 +42,49 @@ describe("test authelia ldap client", function () { "member=cn=user1,ou=users,dc=example,dc=com"); }); }); + + it("should replace {dn} by user DN when searching for groups in LDAP", function () { + const USER_DN = "cn=user1,ou=users,dc=example,dc=com"; + const options: LdapConfiguration = { + url: "ldap://ldap", + users_dn: "ou=users,dc=example,dc=com", + users_filter: "cn={0}", + groups_dn: "ou=groups,dc=example,dc=com", + groups_filter: "member={dn}", + group_name_attribute: "cn", + mail_attribute: "mail", + user: "cn=admin,dc=example,dc=com", + password: "password" + }; + const factory = new LdapClientFactoryStub(); + const ldapClient = new LdapClientStub(); + + factory.createStub.returns(ldapClient); + + // Retrieve user DN + ldapClient.searchAsyncStub.withArgs("ou=users,dc=example,dc=com", { + scope: "sub", + sizeLimit: 1, + attributes: ["dn"], + filter: "cn=user1" + }).returns(BluebirdPromise.resolve([{ + dn: USER_DN + }])); + + // Retrieve groups + ldapClient.searchAsyncStub.withArgs("ou=groups,dc=example,dc=com", { + scope: "sub", + attributes: ["cn"], + filter: "member=" + USER_DN + }).returns(BluebirdPromise.resolve([{ + cn: "group1" + }])); + + const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Dovehash, Winston); + + return client.searchGroups("user1") + .then(function (groups: string[]) { + Assert.deepEqual(groups, ["group1"]); + }); + }); }); \ No newline at end of file