feat(groups): add LDAP link manager and deprecate old API endpoints

nejch · JohnVillalovos · commit 3a61f601adae · 2022-11-17T14:46:04.000-08:00
diff --git a/docs/gl_objects/groups.rst b/docs/gl_objects/groups.rst
@@ -326,11 +326,22 @@ LDAP group links
 
 Add an LDAP group link to an existing GitLab group::
 
-    group.add_ldap_group_link(ldap_group_cn, gitlab.const.AccessLevel.DEVELOPER, 'ldapmain')
+    ldap_link = group.ldap_group_links.create({
+        'provider': 'ldapmain',
+        'group_access': gitlab.const.AccessLevel.DEVELOPER,
+        'cn: 'ldap_group_cn'
+    })
+
+List a group's LDAP group links:
+
+    group.ldap_group_links.list()
 
 Remove a link::
 
-    group.delete_ldap_group_link(ldap_group_cn, 'ldapmain')
+    ldap_link.delete()
+    # or by explicitly providing the CN or filter
+    group.ldap_group_links.delete(provider='ldapmain', cn='ldap_group_cn')
+    group.ldap_group_links.delete(provider='ldapmain', filter='(cn=Common Name)')
 
 Sync the LDAP groups::
 
diff --git a/gitlab/v4/objects/groups.py b/gitlab/v4/objects/groups.py
@@ -5,9 +5,16 @@
 import gitlab
 from gitlab import cli
 from gitlab import exceptions as exc
-from gitlab import types
+from gitlab import types, utils
 from gitlab.base import RESTManager, RESTObject
-from gitlab.mixins import CRUDMixin, ListMixin, ObjectDeleteMixin, SaveMixin
+from gitlab.mixins import (
+    CreateMixin,
+    CRUDMixin,
+    DeleteMixin,
+    ListMixin,
+    ObjectDeleteMixin,
+    SaveMixin,
+)
 from gitlab.types import RequiredOptional
 
 from .access_requests import GroupAccessRequestManager  # noqa: F401
@@ -47,6 +54,8 @@
     "GroupManager",
     "GroupDescendantGroup",
     "GroupDescendantGroupManager",
+    "GroupLDAPGroupLink",
+    "GroupLDAPGroupLinkManager",
     "GroupSubgroup",
     "GroupSubgroupManager",
 ]
@@ -74,6 +83,7 @@ class Group(SaveMixin, ObjectDeleteMixin, RESTObject):
     issues_statistics: GroupIssuesStatisticsManager
     iterations: GroupIterationManager
     labels: GroupLabelManager
+    ldap_group_links: "GroupLDAPGroupLinkManager"
     members: GroupMemberManager
     members_all: GroupMemberAllManager
     mergerequests: GroupMergeRequestManager
@@ -168,6 +178,13 @@ def add_ldap_group_link(
             GitlabAuthenticationError: If authentication is not correct
             GitlabCreateError: If the server cannot perform the request
         """
+        utils.warn(
+            message=(
+                "The add_ldap_group_link() method is deprecated and will be removed "
+                "in a future version. Use ldap_group_links.create() instead."
+            ),
+            category=DeprecationWarning,
+        )
         path = f"/groups/{self.encoded_id}/ldap_group_links"
         data = {"cn": cn, "group_access": group_access, "provider": provider}
         self.manager.gitlab.http_post(path, post_data=data, **kwargs)
@@ -188,29 +205,19 @@ def delete_ldap_group_link(
             GitlabAuthenticationError: If authentication is not correct
             GitlabDeleteError: If the server cannot perform the request
         """
+        utils.warn(
+            message=(
+                "The delete_ldap_group_link() method is deprecated and will be "
+                "removed in a future version. Use ldap_group_links.delete() instead."
+            ),
+            category=DeprecationWarning,
+        )
         path = f"/groups/{self.encoded_id}/ldap_group_links"
         if provider is not None:
             path += f"/{provider}"
         path += f"/{cn}"
         self.manager.gitlab.http_delete(path, **kwargs)
 
-    @cli.register_custom_action("Group")
-    @exc.on_http_error(exc.GitlabGetError)
-    def list_ldap_group_links(
-        self, **kwargs: Any
-    ) -> Union[gitlab.GitlabList, List[Dict[str, Any]]]:
-        """List LDAP group links.
-
-        Args:
-            **kwargs: Extra options to send to the server (e.g. sudo)
-
-        Raises:
-            GitlabAuthenticationError: If authentication is not correct
-            GitlabGetError: If the server cannot perform the request
-        """
-        path = f"/groups/{self.encoded_id}/ldap_group_links"
-        return self.manager.gitlab.http_list(path, **kwargs)
-
     @cli.register_custom_action("Group")
     @exc.on_http_error(exc.GitlabCreateError)
     def ldap_sync(self, **kwargs: Any) -> None:
@@ -416,3 +423,44 @@ class GroupDescendantGroupManager(GroupSubgroupManager):
 
     _path = "/groups/{group_id}/descendant_groups"
     _obj_cls: Type[GroupDescendantGroup] = GroupDescendantGroup
+
+
+class GroupLDAPGroupLink(RESTObject):
+    _repr_attr = "provider"
+
+    def _get_link_attrs(self) -> Dict[str, str]:
+        # https://docs.gitlab.com/ee/api/groups.html#add-ldap-group-link-with-cn-or-filter
+        # https://docs.gitlab.com/ee/api/groups.html#delete-ldap-group-link-with-cn-or-filter
+        # We can tell what attribute to use based on the data returned
+        data = {"provider": self.provider}
+        if self.cn:
+            data["cn"] = self.cn
+        else:
+            data["filter"] = self.filter
+
+        return data
+
+    def delete(self, **kwargs: Any) -> None:
+        """Delete the LDAP group link from the server.
+
+        Args:
+            **kwargs: Extra options to send to the server (e.g. sudo)
+
+        Raises:
+            GitlabAuthenticationError: If authentication is not correct
+            GitlabDeleteError: If the server cannot perform the request
+        """
+        if TYPE_CHECKING:
+            assert isinstance(self.manager, DeleteMixin)
+        self.manager.delete(
+            self.encoded_id, query_data=self._get_link_attrs(), **kwargs
+        )
+
+
+class GroupLDAPGroupLinkManager(ListMixin, CreateMixin, DeleteMixin, RESTManager):
+    _path = "/groups/{group_id}/ldap_group_links"
+    _obj_cls: Type[GroupLDAPGroupLink] = GroupLDAPGroupLink
+    _from_parent_attrs = {"group_id": "id"}
+    _create_attrs = RequiredOptional(
+        required=("provider", "group_access"), exclusive=("cn", "filter")
+    )
diff --git a/tests/functional/api/test_groups.py b/tests/functional/api/test_groups.py
@@ -139,15 +139,37 @@ def test_group_labels(group):
 
 
 @pytest.mark.gitlab_premium
-@pytest.mark.xfail(reason="/ldap/groups endpoint is gone")
-def test_group_ldap_links(gl, group):
+@pytest.mark.xfail(reason="/ldap/groups endpoint not documented")
+def test_ldap_groups(gl):
+    assert isinstance(gl.ldapgroups.list(), list)
+
+
+@pytest.mark.gitlab_premium
+def test_group_ldap_links(group):
     ldap_cn = "common-name"
     ldap_provider = "ldap-provider"
-    assert gl.ldapgroups.list()
 
-    group.add_ldap_group_link(ldap_cn, 30, ldap_provider)
-    group.ldap_sync()
-    group.delete_ldap_group_link(ldap_cn)
+    ldap_cn_link = group.ldap_group_links.create(
+        {"provider": ldap_provider, "group_access": 30, "cn": ldap_cn}
+    )
+    ldap_filter_link = group.ldap_group_links.create(
+        {"provider": ldap_provider, "group_access": 30, "filter": "(cn=Common Name)"}
+    )
+
+    ldap_links = group.ldap_group_links.list()
+
+    assert ldap_cn_link.cn == ldap_links[0].cn
+    assert ldap_filter_link.filter == ldap_links[1].filter
+
+    with pytest.raises(gitlab.GitlabCreateError):
+        # todo - can we configure dummy LDAP in the container?
+        group.ldap_sync()
+
+    ldap_filter_link.delete()
+    group.ldap_group_links.delete(provider=ldap_provider, cn=ldap_cn)
+
+    with pytest.raises(gitlab.GitlabListError, match="No linked LDAP groups found"):
+        group.ldap_group_links.list()
 
 
 def test_group_notification_settings(group):
diff --git a/tests/unit/objects/test_groups.py b/tests/unit/objects/test_groups.py
@@ -8,7 +8,7 @@
 import responses
 
 import gitlab
-from gitlab.v4.objects import GroupDescendantGroup, GroupSubgroup
+from gitlab.v4.objects import GroupDescendantGroup, GroupLDAPGroupLink, GroupSubgroup
 from gitlab.v4.objects.projects import GroupProject, SharedProject
 
 content = {"name": "name", "id": 1, "path": "path"}
@@ -283,9 +283,9 @@ def test_list_group_descendant_groups(group, resp_list_subgroups_descendant_grou
 
 
 def test_list_ldap_group_links(group, resp_list_ldap_group_links):
-    ldap_group_links = group.list_ldap_group_links()
-    assert isinstance(ldap_group_links, list)
-    assert ldap_group_links[0]["provider"] == ldap_group_links_content[0]["provider"]
+    ldap_group_links = group.ldap_group_links.list()
+    assert isinstance(ldap_group_links[0], GroupLDAPGroupLink)
+    assert ldap_group_links[0].provider == ldap_group_links_content[0]["provider"]
 
 
 @pytest.mark.skip("GitLab API endpoint not implemented")
