From d6b0ec2b99a016e5a7db73b53bbd3810e60e62f8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Fri, 1 Mar 2024 10:17:13 +0000 Subject: [PATCH 1/6] :bug: Only rebuild LDAP tree when necessary --- apricot/ldap/oauth_ldap_tree.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apricot/ldap/oauth_ldap_tree.py b/apricot/ldap/oauth_ldap_tree.py index 615746f..12797aa 100644 --- a/apricot/ldap/oauth_ldap_tree.py +++ b/apricot/ldap/oauth_ldap_tree.py @@ -3,6 +3,7 @@ from ldaptor.interfaces import IConnectedLDAPEntry, ILDAPEntry from ldaptor.protocols.ldap.distinguishedname import DistinguishedName from twisted.internet import defer +from twisted.python import log from zope.interface import implementer from apricot.ldap.oauth_ldap_entry import OAuthLDAPEntry @@ -36,6 +37,7 @@ def root(self) -> OAuthLDAPEntry: not self.root_ or (time.monotonic() - self.last_update) > self.refresh_interval ): + log.msg("Rebuilding LDAP tree from OAuth data.") # Create a root node for the tree self.root_ = OAuthLDAPEntry( dn=self.oauth_client.root_dn, @@ -55,6 +57,9 @@ def root(self) -> OAuthLDAPEntry: # Add users to the users OU for user_attrs in self.oauth_client.validated_users(): users_ou.add_child(f"CN={user_attrs['cn'][0]}", user_attrs) + # Set last updated time + log.msg("Finished building LDAP tree.") + self.last_update = time.monotonic() return self.root_ def __repr__(self) -> str: From fda79c5be79b1e6eb0f862b7d26e22bfeb6ca950 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Fri, 1 Mar 2024 17:12:39 +0000 Subject: [PATCH 2/6] :bug: Restore an attribute representing the OAuth user account which is used to bind when verifying password --- apricot/ldap/oauth_ldap_entry.py | 5 +++-- apricot/models/__init__.py | 2 ++ apricot/models/ldap_oauthuser.py | 5 +++++ apricot/oauth/microsoft_entra_client.py | 3 ++- apricot/oauth/oauth_client.py | 5 +++++ 5 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 apricot/models/ldap_oauthuser.py diff --git a/apricot/ldap/oauth_ldap_entry.py b/apricot/ldap/oauth_ldap_entry.py index 8a4954e..d945ef0 100644 --- a/apricot/ldap/oauth_ldap_entry.py +++ b/apricot/ldap/oauth_ldap_entry.py @@ -75,10 +75,11 @@ def add_child( def bind(self, password: bytes) -> defer.Deferred["OAuthLDAPEntry"]: def _bind(password: bytes) -> "OAuthLDAPEntry": + oauth_username = next(iter(self.get("oauth_username", "unknown"))) s_password = password.decode("utf-8") - if self.oauth_client.verify(username=self.username, password=s_password): + if self.oauth_client.verify(username=oauth_username, password=s_password): return self - msg = f"Invalid password for user '{self.username}'." + msg = f"Invalid password for user '{oauth_username}'." raise LDAPInvalidCredentials(msg) return defer.maybeDeferred(_bind, password) diff --git a/apricot/models/__init__.py b/apricot/models/__init__.py index 430bfbb..4692eac 100644 --- a/apricot/models/__init__.py +++ b/apricot/models/__init__.py @@ -1,6 +1,7 @@ from .ldap_group_of_names import LdapGroupOfNames from .ldap_inetorgperson import LdapInetOrgPerson from .ldap_inetuser import LdapInetUser +from .ldap_oauthuser import LdapOAuthUser from .ldap_person import LdapPerson from .ldap_posix_account import LdapPosixAccount from .ldap_posix_group import LdapPosixGroup @@ -9,6 +10,7 @@ "LdapGroupOfNames", "LdapInetOrgPerson", "LdapInetUser", + "LdapOAuthUser", "LdapPerson", "LdapPosixAccount", "LdapPosixGroup", diff --git a/apricot/models/ldap_oauthuser.py b/apricot/models/ldap_oauthuser.py new file mode 100644 index 0000000..8988b14 --- /dev/null +++ b/apricot/models/ldap_oauthuser.py @@ -0,0 +1,5 @@ +from pydantic import BaseModel + + +class LdapOAuthUser(BaseModel): + oauth_username: str diff --git a/apricot/oauth/microsoft_entra_client.py b/apricot/oauth/microsoft_entra_client.py index af429c9..211e68d 100644 --- a/apricot/oauth/microsoft_entra_client.py +++ b/apricot/oauth/microsoft_entra_client.py @@ -87,11 +87,12 @@ def users(self) -> list[dict[str, Any]]: attributes = {} attributes["cn"] = user_dict.get("displayName", None) attributes["description"] = user_dict.get("id", None) - attributes["displayName"] = attributes.get("cn", None) + attributes["displayName"] = user_dict.get("displayName", None) attributes["domain"] = domain attributes["gidNumber"] = user_uid attributes["givenName"] = user_dict.get("givenName", "") attributes["homeDirectory"] = f"/home/{uid}" if uid else None + attributes["oauth_username"] = user_dict.get("userPrincipalName", None) attributes["sn"] = user_dict.get("surname", "") attributes["uid"] = uid if uid else None attributes["uidNumber"] = user_uid diff --git a/apricot/oauth/oauth_client.py b/apricot/oauth/oauth_client.py index 7e76f7d..bf7f61d 100644 --- a/apricot/oauth/oauth_client.py +++ b/apricot/oauth/oauth_client.py @@ -18,6 +18,7 @@ LdapGroupOfNames, LdapInetOrgPerson, LdapInetUser, + LdapOAuthUser, LdapPerson, LdapPosixAccount, LdapPosixGroup, @@ -207,6 +208,10 @@ def validated_users(self) -> list[LDAPAttributeDict]: posix_account = LdapPosixAccount(**user_dict) attributes.update(posix_account.model_dump()) attributes["objectclass"].append("posixAccount") + # Add 'OAuthUser' attributes + oauth_user = LdapOAuthUser(**user_dict) + attributes.update(oauth_user.model_dump()) + attributes["objectclass"].append("oauthUser") # Ensure that all values are lists as required for LDAPAttributeDict output.append( { From bd8f16697921add0c781bc22efa5a5be599ea069 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 6 Mar 2024 11:51:49 +0000 Subject: [PATCH 3/6] :alien: Fix type-hint for LDAP controls passed to server --- apricot/ldap/read_only_ldap_server.py | 20 +++++++++++--------- apricot/oauth/__init__.py | 3 ++- apricot/oauth/types.py | 1 + 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/apricot/ldap/read_only_ldap_server.py b/apricot/ldap/read_only_ldap_server.py index e745a8c..c501ee9 100644 --- a/apricot/ldap/read_only_ldap_server.py +++ b/apricot/ldap/read_only_ldap_server.py @@ -11,6 +11,8 @@ ) from twisted.internet import defer +from apricot.oauth import LDAPControlTuple + class ReadOnlyLDAPServer(LDAPServer): def getRootDSE( # noqa: N802 @@ -26,7 +28,7 @@ def getRootDSE( # noqa: N802 def handle_LDAPAddRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -39,7 +41,7 @@ def handle_LDAPAddRequest( # noqa: N802 def handle_LDAPBindRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -50,7 +52,7 @@ def handle_LDAPBindRequest( # noqa: N802 def handle_LDAPCompareRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -61,7 +63,7 @@ def handle_LDAPCompareRequest( # noqa: N802 def handle_LDAPDelRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -74,7 +76,7 @@ def handle_LDAPDelRequest( # noqa: N802 def handle_LDAPExtendedRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -85,7 +87,7 @@ def handle_LDAPExtendedRequest( # noqa: N802 def handle_LDAPModifyDNRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -98,7 +100,7 @@ def handle_LDAPModifyDNRequest( # noqa: N802 def handle_LDAPModifyRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> defer.Deferred[ILDAPEntry]: """ @@ -111,7 +113,7 @@ def handle_LDAPModifyRequest( # noqa: N802 def handle_LDAPUnbindRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[..., None] | None, ) -> None: """ @@ -122,7 +124,7 @@ def handle_LDAPUnbindRequest( # noqa: N802 def handle_LDAPSearchRequest( # noqa: N802 self, request: LDAPBindRequest, - controls: LDAPControl | None, + controls: list[LDAPControlTuple] | None, reply: Callable[[LDAPSearchResultEntry], None] | None, ) -> defer.Deferred[ILDAPEntry]: """ diff --git a/apricot/oauth/__init__.py b/apricot/oauth/__init__.py index 0b78a08..12dac22 100644 --- a/apricot/oauth/__init__.py +++ b/apricot/oauth/__init__.py @@ -1,12 +1,13 @@ from .enums import OAuthBackend from .microsoft_entra_client import MicrosoftEntraClient from .oauth_client import OAuthClient -from .types import LDAPAttributeDict +from .types import LDAPAttributeDict, LDAPControlTuple OAuthClientMap = {OAuthBackend.MICROSOFT_ENTRA: MicrosoftEntraClient} __all__ = [ "LDAPAttributeDict", + "LDAPControlTuple", "OAuthBackend", "OAuthClient", "OAuthClientMap", diff --git a/apricot/oauth/types.py b/apricot/oauth/types.py index a44c815..325a2e5 100644 --- a/apricot/oauth/types.py +++ b/apricot/oauth/types.py @@ -2,3 +2,4 @@ JSONDict = dict[str, str | list[str]] LDAPAttributeDict = dict[str, list[Any]] +LDAPControlTuple = tuple[str, bool, Any] From 1272eae58f85b0eb1fbbff70b93415981d7e1d67 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 6 Mar 2024 11:52:27 +0000 Subject: [PATCH 4/6] :loud_sound: Enable debug output from LDAPServer --- apricot/ldap/read_only_ldap_server.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apricot/ldap/read_only_ldap_server.py b/apricot/ldap/read_only_ldap_server.py index c501ee9..6bba6cc 100644 --- a/apricot/ldap/read_only_ldap_server.py +++ b/apricot/ldap/read_only_ldap_server.py @@ -15,6 +15,10 @@ class ReadOnlyLDAPServer(LDAPServer): + def __init__(self) -> None: + super().__init__() + self.debug = True + def getRootDSE( # noqa: N802 self, request: LDAPBindRequest, From ab581acbcc64f89a7762ba38aab4857b8ddf1736 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 6 Mar 2024 11:53:43 +0000 Subject: [PATCH 5/6] :bug: Make sn and givenName attributes optional --- apricot/oauth/microsoft_entra_client.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apricot/oauth/microsoft_entra_client.py b/apricot/oauth/microsoft_entra_client.py index 211e68d..10ab3d9 100644 --- a/apricot/oauth/microsoft_entra_client.py +++ b/apricot/oauth/microsoft_entra_client.py @@ -82,6 +82,8 @@ def users(self) -> list[dict[str, Any]]: sorted(user_data["value"], key=lambda user: user["createdDateTime"]), ): # Get user attributes + given_name = user_dict.get("givenName", None) + surname = user_dict.get("surname", None) uid, domain = str(user_dict.get("userPrincipalName", "@")).split("@") user_uid = self.uid_cache.get_user_uid(user_dict["id"]) attributes = {} @@ -90,10 +92,10 @@ def users(self) -> list[dict[str, Any]]: attributes["displayName"] = user_dict.get("displayName", None) attributes["domain"] = domain attributes["gidNumber"] = user_uid - attributes["givenName"] = user_dict.get("givenName", "") + attributes["givenName"] = given_name if given_name else "" attributes["homeDirectory"] = f"/home/{uid}" if uid else None attributes["oauth_username"] = user_dict.get("userPrincipalName", None) - attributes["sn"] = user_dict.get("surname", "") + attributes["sn"] = surname if surname else "" attributes["uid"] = uid if uid else None attributes["uidNumber"] = user_uid # Add group attributes From cdb3ae574d653369fc6fc7b9683e13bfdf62311f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 6 Mar 2024 12:03:34 +0000 Subject: [PATCH 6/6] :memo: Require that delegated User.Read.All permission in addition to previous permissions --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b1d1f02..db843a5 100644 --- a/README.md +++ b/README.md @@ -90,4 +90,5 @@ Do this as follows: - Ensure that the following permissions are enabled - `Microsoft Graph` > `User.Read.All` (application) - `Microsoft Graph` > `GroupMember.Read.All` (application) + - `Microsoft Graph` > `User.Read.All` (delegated) - Select this and click the `Grant admin consent` button (otherwise manual consent is needed from each user)