From c9fb48da6a9ab16f519bd5bc8d390a55d387deab Mon Sep 17 00:00:00 2001 From: Aditi Yashwant Chikkali Date: Thu, 5 Mar 2026 16:51:34 -0500 Subject: [PATCH 1/3] Improve LDAP user info retrieval: separate regular and operational attribute searches, add robust attribute extraction and cleaning utilities --- src/nsls2api/services/ldap_service.py | 70 +++++++++++++++++++++------ 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/src/nsls2api/services/ldap_service.py b/src/nsls2api/services/ldap_service.py index fcd93981..cfc78355 100644 --- a/src/nsls2api/services/ldap_service.py +++ b/src/nsls2api/services/ldap_service.py @@ -7,16 +7,25 @@ def to_hex(val): - if isinstance(val, bytes): return binascii.hexlify(val).decode() return None +OPERATIONAL_ATTRIBUTES = [ + 'manager', + 'objectGUID', + 'objectSid', + 'memberOf', + 'whenCreated', + 'whenChanged', +] + def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password): conn = None try: server = Server(ldap_server) conn = Connection(server, user=ldap_bind_user, password=bind_password, auto_bind=True) + search_filter = f"(&(objectclass=person)(userPrincipalName={upn}))" conn.search(ldap_base_dn, search_filter, attributes=['sAMAccountName']) @@ -27,31 +36,47 @@ def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password) entry = conn.entries[0] username = entry.sAMAccountName.value if 'sAMAccountName' in entry else None if username is None: + logger.warning("sAMAccountName not found.") return None search_filter = f"(&(objectclass=posixaccount)(sAMAccountName={username}))" + + # Search 1: Get regular attributes conn.search(ldap_base_dn, search_filter, attributes=['*']) - if not conn.entries: - logger.warning("no posix entries found for the given username.") return None entry = conn.entries[0] - user = dict() - for attribute in entry.entry_attributes: - value = entry[attribute].value - if attribute in ("objectGUID", "objectSid"): - user[attribute] = value # keep as bytes - else: - user[attribute] = str(value) + user = _extract_attributes(entry) + + # Search 2: Get operational attributes and merge + conn.search(ldap_base_dn, search_filter, attributes=OPERATIONAL_ATTRIBUTES) + if conn.entries: + entry = conn.entries[0] + operational = _extract_attributes(entry) + user.update(operational) + return user + except Exception as e: - logger.error(f"LDAP Error: {e}") + logger.error(f"LDAP Error: {e}", exc_info=True) return None finally: if conn is not None: conn.unbind() +def _extract_attributes(entry): + data = {} + for attribute in entry.entry_attributes: + value = entry[attribute].value + if isinstance(value, bytes): + data[attribute] = value + elif isinstance(value, list): + data[attribute] = value + else: + data[attribute] = str(value) if value is not None else None + return data + def filetime_to_str(filetime): try: if filetime is None or int(filetime) == 0 or int(filetime) == 9223372036854775807: @@ -63,11 +88,15 @@ def filetime_to_str(filetime): def generalized_time_to_str(gt): try: - if not gt: return "" - dt = datetime.strptime(gt.split(".")[0], "%Y%m%d%H%M%S") + if not gt: + return "" + if isinstance(gt, datetime): + return gt.strftime("%Y-%m-%d %H:%M:%S UTC") + gt_str = str(gt) + dt = datetime.strptime(gt_str.split(".")[0], "%Y%m%d%H%M%S") return dt.strftime("%Y-%m-%d %H:%M:%S UTC") except Exception: - return str(gt) + return str(gt) if gt else "" def decode_uac(uac): flags = [] @@ -87,11 +116,20 @@ def clean_groups(groups_val): if not groups_val: return [] if isinstance(groups_val, list): - return groups_val + return [str(g) for g in groups_val] elif isinstance(groups_val, str): return [g.strip() for g in groups_val.replace("\n", ",").split(",") if g.strip()] return [] + def clean_object_class(obj_class_val): + if not obj_class_val: + return [] + if isinstance(obj_class_val, list): + return [str(s).strip() for s in obj_class_val] + elif isinstance(obj_class_val, str): + return [s.strip() for s in obj_class_val.replace(",", " ").split() if s.strip()] + return [] + return { "dn": dn or user_info.get("distinguishedName"), "status": status, @@ -142,6 +180,6 @@ def clean_groups(groups_val): "codePage": user_info.get("codePage"), "countryCode": user_info.get("countryCode"), "instanceType": user_info.get("instanceType"), - "objectClass": [s.strip() for s in user_info.get("objectClass", "").split() if s.strip()] + "objectClass": clean_object_class(user_info.get("objectClass")) } } \ No newline at end of file From 4c1403b9a3b0c49b42a04cc0c7eeccc0429ca88c Mon Sep 17 00:00:00 2001 From: Aditi Yashwant Chikkali Date: Mon, 9 Mar 2026 10:12:21 -0400 Subject: [PATCH 2/3] refactored search stratergy code --- src/nsls2api/services/ldap_service.py | 60 ++++++++++++++++++++------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/src/nsls2api/services/ldap_service.py b/src/nsls2api/services/ldap_service.py index cfc78355..e1c971bc 100644 --- a/src/nsls2api/services/ldap_service.py +++ b/src/nsls2api/services/ldap_service.py @@ -18,6 +18,11 @@ def to_hex(val): 'memberOf', 'whenCreated', 'whenChanged', + 'logonCount', + 'lastLogon', + 'lastLogoff', + 'street', + 'badPasswordTime', ] def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password): @@ -26,40 +31,63 @@ def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password) server = Server(ldap_server) conn = Connection(server, user=ldap_bind_user, password=bind_password, auto_bind=True) + search_filter = f"(&(objectclass=person)(userPrincipalName={upn}))" + # Search 1: Get sAMAccountName for the given UPN conn.search(ldap_base_dn, search_filter, attributes=['sAMAccountName']) if not conn.entries: - logger.warning("No entries found for the given UPN.") + logger.warning(f"No entries found for the given UPN: {upn}") return None entry = conn.entries[0] username = entry.sAMAccountName.value if 'sAMAccountName' in entry else None if username is None: - logger.warning("sAMAccountName not found.") + logger.warning(f"sAMAccountName not found for upn: {upn}") return None - search_filter = f"(&(objectclass=posixaccount)(sAMAccountName={username}))" + posix_filter = f"(&(objectclass=posixaccount)(sAMAccountName={username}))" + user={} - # Search 1: Get regular attributes - conn.search(ldap_base_dn, search_filter, attributes=['*']) + # Search 2: Get regular attributes + conn.search(ldap_base_dn, posix_filter, attributes=['*']) if not conn.entries: + logger.warning(f"No posix entries found for username: {username}, will still search operational attributes") + return None + else: + entry = conn.entries[0] + user = _extract_attributes(entry) + + missing_ops = [attr for attr in OPERATIONAL_ATTRIBUTES if not user.get(attr)] + + if missing_ops: + # Search 3: Only performed if operational attributes are missing + logger.warning(f"Operational attributes missing after second search for {username}: {missing_ops}, performing explicit search") + conn.search(ldap_base_dn, posix_filter, attributes=missing_ops) + if not conn.entries: + logger.warning(f"No operational attributes found for username: {username}") + else: + entry = conn.entries[0] + operational = _extract_attributes(entry) + + for attr, value in operational.items(): + if attr not in user or user[attr] is None: + user[attr] = value + + # Log if still missing after explicit search + still_missing = [attr for attr in OPERATIONAL_ATTRIBUTES if not user.get(attr)] + if still_missing: + logger.warning(f"Operational attributes still missing after explicit search for {username}: {still_missing}") + + if not user: + logger.error(f"No data found for UPN: {upn}") return None - entry = conn.entries[0] - user = _extract_attributes(entry) - - # Search 2: Get operational attributes and merge - conn.search(ldap_base_dn, search_filter, attributes=OPERATIONAL_ATTRIBUTES) - if conn.entries: - entry = conn.entries[0] - operational = _extract_attributes(entry) - user.update(operational) - + return user except Exception as e: - logger.error(f"LDAP Error: {e}", exc_info=True) + logger.error(f"LDAP Error for UPN {upn}: {e}", exc_info=True) return None finally: if conn is not None: From e19b81e1eef1c2567cfaad09f49c87b62f5cc8db Mon Sep 17 00:00:00 2001 From: Aditi Yashwant Chikkali Date: Tue, 10 Mar 2026 09:02:11 -0400 Subject: [PATCH 3/3] added explicit search and logging for missing attributes --- src/nsls2api/services/ldap_service.py | 91 +++++++++++++-------------- 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/src/nsls2api/services/ldap_service.py b/src/nsls2api/services/ldap_service.py index e1c971bc..17cae616 100644 --- a/src/nsls2api/services/ldap_service.py +++ b/src/nsls2api/services/ldap_service.py @@ -15,75 +15,71 @@ def to_hex(val): 'manager', 'objectGUID', 'objectSid', - 'memberOf', 'whenCreated', 'whenChanged', 'logonCount', 'lastLogon', - 'lastLogoff', + 'sAMAccountType', 'street', 'badPasswordTime', + 'memberOf' ] + def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password): - conn = None + conn = None try: server = Server(ldap_server) conn = Connection(server, user=ldap_bind_user, password=bind_password, auto_bind=True) - - - search_filter = f"(&(objectclass=person)(userPrincipalName={upn}))" - # Search 1: Get sAMAccountName for the given UPN - conn.search(ldap_base_dn, search_filter, attributes=['sAMAccountName']) + + username = upn.split("@")[0] + posix_filter = f"(&(objectclass=posixaccount)(sAMAccountName={username}))" + + # SEARCH 1: Get all regular attributes with ['*'] + conn.search(ldap_base_dn, posix_filter, attributes=['*']) if not conn.entries: - logger.warning(f"No entries found for the given UPN: {upn}") + logger.warning(f"No posixaccount entries found for username: {username}") return None entry = conn.entries[0] - username = entry.sAMAccountName.value if 'sAMAccountName' in entry else None - if username is None: - logger.warning(f"sAMAccountName not found for upn: {upn}") - return None + user = _extract_attributes(entry) - posix_filter = f"(&(objectclass=posixaccount)(sAMAccountName={username}))" - user={} - - # Search 2: Get regular attributes - conn.search(ldap_base_dn, posix_filter, attributes=['*']) - if not conn.entries: - logger.warning(f"No posix entries found for username: {username}, will still search operational attributes") - return None - else: - entry = conn.entries[0] - user = _extract_attributes(entry) + # SEARCH 2: Always fetch operational attributes explicitly + logger.info(f"Search 2 - fetching operational attributes for: {username}") + conn.search(ldap_base_dn, posix_filter, attributes=OPERATIONAL_ATTRIBUTES) + + if conn.entries: + operational = _extract_attributes(conn.entries[0]) + + for attr, value in operational.items(): + if attr not in user or user[attr] is None or user[attr] == "": + user[attr] = value + # SEARCH 3: Check for still-missing operational attributes missing_ops = [attr for attr in OPERATIONAL_ATTRIBUTES if not user.get(attr)] - if missing_ops: - # Search 3: Only performed if operational attributes are missing - logger.warning(f"Operational attributes missing after second search for {username}: {missing_ops}, performing explicit search") - conn.search(ldap_base_dn, posix_filter, attributes=missing_ops) - if not conn.entries: - logger.warning(f"No operational attributes found for username: {username}") - else: - entry = conn.entries[0] - operational = _extract_attributes(entry) - - for attr, value in operational.items(): - if attr not in user or user[attr] is None: - user[attr] = value - - # Log if still missing after explicit search - still_missing = [attr for attr in OPERATIONAL_ATTRIBUTES if not user.get(attr)] - if still_missing: - logger.warning(f"Operational attributes still missing after explicit search for {username}: {still_missing}") - - if not user: - logger.error(f"No data found for UPN: {upn}") - return None + if missing_ops: + logger.warning(f"Still missing after search 2 for {username}: {missing_ops}") + + # Fetching each missing attribute individually hoping to bypass proxy cache since each is a unique query + for attr in missing_ops: + logger.info(f"Search 3 - fetching individual attribute '{attr}' for {username}") + conn.search(ldap_base_dn, posix_filter, attributes=[attr]) + if conn.entries: + val = _extract_attributes(conn.entries[0]) + if val.get(attr): + user[attr] = val[attr] + + # FINAL VALIDATION: Log what we have and what's still missing + final_missing = [attr for attr in OPERATIONAL_ATTRIBUTES if not user.get(attr)] + if final_missing: + logger.warning(f"Final missing attributes for {username}: {final_missing} — user may not have these set in LDAP") + else: + logger.info(f"All operational attributes present for {username}") + + logger.info(f"Final user dict has {len(user)} attributes for {username}") - return user except Exception as e: @@ -93,6 +89,7 @@ def get_user_info(upn, ldap_server, ldap_base_dn, ldap_bind_user, bind_password) if conn is not None: conn.unbind() + def _extract_attributes(entry): data = {} for attribute in entry.entry_attributes: