From 4383c337d4085ed485372d83876d50f9bd50ef6c Mon Sep 17 00:00:00 2001 From: Melchior NOGUES Date: Wed, 27 Jul 2022 11:35:47 +0200 Subject: [PATCH 1/3] fix: ldap type ad search user group when nested groups --- powerdnsadmin/models/user.py | 92 ++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/powerdnsadmin/models/user.py b/powerdnsadmin/models/user.py index 2f8b87c..cb857c8 100644 --- a/powerdnsadmin/models/user.py +++ b/powerdnsadmin/models/user.py @@ -5,6 +5,7 @@ import bcrypt import pyotp import ldap import ldap.filter +from collections import OrderedDict from flask import current_app from flask_login import AnonymousUserMixin from sqlalchemy import orm @@ -282,53 +283,62 @@ class User(db.Model): LDAP_USER_GROUP)) return False elif LDAP_TYPE == 'ad': - ldap_admin_group_filter, ldap_operator_group, ldap_user_group = "", "", "" - if LDAP_ADMIN_GROUP: - ldap_admin_group_filter = "(memberOf:1.2.840.113556.1.4.1941:={0})".format(LDAP_ADMIN_GROUP) - if LDAP_OPERATOR_GROUP: - ldap_operator_group = "(memberOf:1.2.840.113556.1.4.1941:={0})".format(LDAP_OPERATOR_GROUP) - if LDAP_USER_GROUP: - ldap_user_group = "(memberOf:1.2.840.113556.1.4.1941:={0})".format(LDAP_USER_GROUP) - searchFilter = "(&({0}={1})(|{2}{3}{4}))".format(LDAP_FILTER_USERNAME, self.username, - ldap_admin_group_filter, - ldap_operator_group, ldap_user_group) - ldap_result = self.ldap_search(searchFilter, LDAP_BASE_DN) - user_ad_member_of = ldap_result[0][0][1].get( - 'memberOf') + ldap_group_security_roles = OrderedDict( + Administrator=LDAP_ADMIN_GROUP, + Operator=LDAP_OPERATOR_GROUP, + User=LDAP_USER_GROUP, + ) + user_dn = ldap_result[0][0][0] + sf_groups = "" - if not user_ad_member_of: + for group in ldap_group_security_roles.values(): + if not group: + continue + + sf_groups += f"(distinguishedName={group})" + + sf_member_user = f"(member:1.2.840.113556.1.4.1941:={user_dn})" + search_filter = f"(&(|{sf_groups}){sf_member_user})" + current_app.logger.debug(f"LDAP groupSearchFilter '{search_filter}'") + + ldap_user_groups = [ + group[0][0] + for group in self.ldap_search( + search_filter, + LDAP_BASE_DN + ) + ] + + if not ldap_user_groups: current_app.logger.error( - 'User {0} does not belong to any group while LDAP_GROUP_SECURITY_ENABLED is ON' - .format(self.username)) + f"User '{self.username}' " + "does not belong to any group " + "while LDAP_GROUP_SECURITY_ENABLED is ON" + ) return False - user_ad_member_of = [g.decode("utf-8") for g in user_ad_member_of] + current_app.logger.debug( + "LDAP User security groups " + f"for user '{self.username}': " + " ".join(ldap_user_groups) + ) - if (LDAP_ADMIN_GROUP in user_ad_member_of): - role_name = 'Administrator' + for role, ldap_group in ldap_group_security_roles.items(): + # Continue when groups is not defined or + # user is'nt member of LDAP group + if not ldap_group or not ldap_group in ldap_user_groups: + continue + + role_name = role current_app.logger.info( - 'User {0} is part of the "{1}" group that allows admin access to PowerDNS-Admin' - .format(self.username, - LDAP_ADMIN_GROUP)) - elif (LDAP_OPERATOR_GROUP in user_ad_member_of): - role_name = 'Operator' - current_app.logger.info( - 'User {0} is part of the "{1}" group that allows operator access to PowerDNS-Admin' - .format(self.username, - LDAP_OPERATOR_GROUP)) - elif (LDAP_USER_GROUP in user_ad_member_of): - current_app.logger.info( - 'User {0} is part of the "{1}" group that allows user access to PowerDNS-Admin' - .format(self.username, - LDAP_USER_GROUP)) - else: - current_app.logger.error( - 'User {0} is not part of the "{1}", "{2}" or "{3}" groups that allow access to PowerDNS-Admin' - .format(self.username, - LDAP_ADMIN_GROUP, - LDAP_OPERATOR_GROUP, - LDAP_USER_GROUP)) - return False + f"User '{self.username}' member of " + f"the '{ldap_group}' group that allows " + f"'{role}' access to to PowerDNS-Admin" + ) + + # Stop loop on first found + break + else: current_app.logger.error('Invalid LDAP type') return False From f44ff7d26149f14524afebec28aa41e9893c7ced Mon Sep 17 00:00:00 2001 From: Nigel Kukard Date: Sat, 18 Mar 2023 19:14:58 +0000 Subject: [PATCH 2/3] fix: fixed session clearing and let logout_user take care of cleanup It seems when logging in and logging out, then logging back in, setting the session timeout to 5 minutes, then waiting for expiry can cause a situation when using SQLA-based sessions which results in a NULL field in the database and causes a persistent 500 Internal Server Error. As per issue 1439 here is a fix found by @raunz. Resolves #1439. Tested for about 8 hours and tons and tons of expired sessions, could not reproduce with the fix applied. --- powerdnsadmin/routes/index.py | 1 - 1 file changed, 1 deletion(-) diff --git a/powerdnsadmin/routes/index.py b/powerdnsadmin/routes/index.py index a21ad31..19fd277 100644 --- a/powerdnsadmin/routes/index.py +++ b/powerdnsadmin/routes/index.py @@ -528,7 +528,6 @@ def clear_session(): session.pop('google_token', None) session.pop('authentication_type', None) session.pop('remote_user', None) - session.clear() logout_user() From 138532fb95ed803a26299ad4da43be245d05519b Mon Sep 17 00:00:00 2001 From: Nigel Kukard Date: Sat, 18 Mar 2023 20:27:02 +0000 Subject: [PATCH 3/3] fix: allow the specification of any combination of groups in LDAP group security configuration Previous behavior required the specification of all three group security groups before the "Save Settings" button would be enabled. This adds a check into users.py which checks that the group is set before searching and removes the javascript preventing the specification of any combination of groups. Tested: - Tested all combinations on AD after MR 1238 - Tested all combinations on OpenLDAP - Tested enabling the Group Security with no groups set which correctly prevents login Resolves #1462 --- powerdnsadmin/models/user.py | 25 ++++++------------- .../admin_setting_authentication.html | 6 ----- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/powerdnsadmin/models/user.py b/powerdnsadmin/models/user.py index 24b43e9..e989aa0 100644 --- a/powerdnsadmin/models/user.py +++ b/powerdnsadmin/models/user.py @@ -255,33 +255,24 @@ class User(db.Model): if LDAP_TYPE == 'ldap': groupSearchFilter = "(&({0}={1}){2})".format(LDAP_FILTER_GROUPNAME, ldap_username, LDAP_FILTER_GROUP) current_app.logger.debug('Ldap groupSearchFilter {0}'.format(groupSearchFilter)) - if (self.ldap_search(groupSearchFilter, - LDAP_ADMIN_GROUP)): + if (LDAP_ADMIN_GROUP and self.ldap_search(groupSearchFilter, LDAP_ADMIN_GROUP)): role_name = 'Administrator' current_app.logger.info( 'User {0} is part of the "{1}" group that allows admin access to PowerDNS-Admin' - .format(self.username, - LDAP_ADMIN_GROUP)) - elif (self.ldap_search(groupSearchFilter, - LDAP_OPERATOR_GROUP)): + .format(self.username, LDAP_ADMIN_GROUP)) + elif (LDAP_OPERATOR_GROUP and self.ldap_search(groupSearchFilter, LDAP_OPERATOR_GROUP)): role_name = 'Operator' current_app.logger.info( 'User {0} is part of the "{1}" group that allows operator access to PowerDNS-Admin' - .format(self.username, - LDAP_OPERATOR_GROUP)) - elif (self.ldap_search(groupSearchFilter, - LDAP_USER_GROUP)): + .format(self.username, LDAP_OPERATOR_GROUP)) + elif (LDAP_USER_GROUP and self.ldap_search(groupSearchFilter, LDAP_USER_GROUP)): current_app.logger.info( 'User {0} is part of the "{1}" group that allows user access to PowerDNS-Admin' - .format(self.username, - LDAP_USER_GROUP)) + .format(self.username, LDAP_USER_GROUP)) else: current_app.logger.error( - 'User {0} is not part of the "{1}", "{2}" or "{3}" groups that allow access to PowerDNS-Admin' - .format(self.username, - LDAP_ADMIN_GROUP, - LDAP_OPERATOR_GROUP, - LDAP_USER_GROUP)) + 'User {0} is not part of any security groups that allow access to PowerDNS-Admin' + .format(self.username)) return False elif LDAP_TYPE == 'ad': ldap_group_security_roles = OrderedDict( diff --git a/powerdnsadmin/templates/admin_setting_authentication.html b/powerdnsadmin/templates/admin_setting_authentication.html index a4c0288..c545958 100644 --- a/powerdnsadmin/templates/admin_setting_authentication.html +++ b/powerdnsadmin/templates/admin_setting_authentication.html @@ -1772,12 +1772,6 @@ $('#ldap_filter_username').prop('required', true); $('#ldap_filter_groupname').prop('required', true); - if ($('#ldap_sg_on').is(":checked")) { - $('#ldap_admin_group').prop('required', true); - $('#ldap_operator_group').prop('required', true); - $('#ldap_user_group').prop('required', true); - } - if ($('#autoprovisioning_on').is(":checked")) { $('#autoprovisioning_attribute').prop('required', true); $('#urn_value').prop('required', true);