From 0d96e68c1e697a89a5afb6d26bebdabde0adc254 Mon Sep 17 00:00:00 2001
From: Jens Langhammer <jens.langhammer@beryju.org>
Date: Sun, 19 Jun 2022 21:24:57 +0200
Subject: [PATCH] core: add limit of 20 to group recursion

closes #3116

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
---
 authentik/core/models.py            |  5 +++-
 authentik/core/tests/test_groups.py | 36 +++++++++++++++++++----------
 website/docs/releases/v2022.7.md    |  5 ++++
 website/docs/user-group/group.md    |  2 ++
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/authentik/core/models.py b/authentik/core/models.py
index 9871dd325..b278a7845 100644
--- a/authentik/core/models.py
+++ b/authentik/core/models.py
@@ -106,7 +106,10 @@ class Group(models.Model):
 
             SELECT authentik_core_group.*, parents.relative_depth - 1
             FROM authentik_core_group,parents
-            WHERE authentik_core_group.parent_id = parents.group_uuid
+            WHERE (
+                authentik_core_group.parent_id = parents.group_uuid and
+                parents.relative_depth > -20
+            )
         )
         SELECT group_uuid
         FROM parents
diff --git a/authentik/core/tests/test_groups.py b/authentik/core/tests/test_groups.py
index 9d66341f9..c4b314430 100644
--- a/authentik/core/tests/test_groups.py
+++ b/authentik/core/tests/test_groups.py
@@ -2,6 +2,7 @@
 from django.test.testcases import TestCase
 
 from authentik.core.models import Group, User
+from authentik.lib.generators import generate_id
 
 
 class TestGroups(TestCase):
@@ -9,32 +10,43 @@ class TestGroups(TestCase):
 
     def test_group_membership_simple(self):
         """Test simple membership"""
-        user = User.objects.create(username="user")
-        user2 = User.objects.create(username="user2")
-        group = Group.objects.create(name="group")
+        user = User.objects.create(username=generate_id())
+        user2 = User.objects.create(username=generate_id())
+        group = Group.objects.create(name=generate_id())
         group.users.add(user)
         self.assertTrue(group.is_member(user))
         self.assertFalse(group.is_member(user2))
 
     def test_group_membership_parent(self):
         """Test parent membership"""
-        user = User.objects.create(username="user")
-        user2 = User.objects.create(username="user2")
-        first = Group.objects.create(name="first")
-        second = Group.objects.create(name="second", parent=first)
+        user = User.objects.create(username=generate_id())
+        user2 = User.objects.create(username=generate_id())
+        first = Group.objects.create(name=generate_id())
+        second = Group.objects.create(name=generate_id(), parent=first)
         second.users.add(user)
         self.assertTrue(first.is_member(user))
         self.assertFalse(first.is_member(user2))
 
     def test_group_membership_parent_extra(self):
         """Test parent membership"""
-        user = User.objects.create(username="user")
-        user2 = User.objects.create(username="user2")
-        first = Group.objects.create(name="first")
-        second = Group.objects.create(name="second", parent=first)
-        third = Group.objects.create(name="third", parent=second)
+        user = User.objects.create(username=generate_id())
+        user2 = User.objects.create(username=generate_id())
+        first = Group.objects.create(name=generate_id())
+        second = Group.objects.create(name=generate_id(), parent=first)
+        third = Group.objects.create(name=generate_id(), parent=second)
         second.users.add(user)
         self.assertTrue(first.is_member(user))
         self.assertFalse(first.is_member(user2))
         self.assertFalse(third.is_member(user))
         self.assertFalse(third.is_member(user2))
+
+    def test_group_membership_recursive(self):
+        """Test group membership (recursive)"""
+        user = User.objects.create(username=generate_id())
+        group = Group.objects.create(name=generate_id())
+        group2 = Group.objects.create(name=generate_id(), parent=group)
+        group.users.add(user)
+        group.parent = group2
+        group.save()
+        self.assertTrue(group.is_member(user))
+        self.assertTrue(group2.is_member(user))
diff --git a/website/docs/releases/v2022.7.md b/website/docs/releases/v2022.7.md
index a2119029d..80f9d9e57 100644
--- a/website/docs/releases/v2022.7.md
+++ b/website/docs/releases/v2022.7.md
@@ -9,6 +9,11 @@ slug: "2022.7"
 
     Instead, create an OAuth Source with the certificate configured as JWKS Data, and enable the source in the provider.
 
+-   Maximum Limit of group recursion
+
+    In earlier versions, cyclic group relations can lead to a deadlock when one of groups in the relationship are bound to an application/flow/etc.
+    This is now limited to 20 levels of recursion.
+
 ## New features
 
 -   User paths
diff --git a/website/docs/user-group/group.md b/website/docs/user-group/group.md
index 0e6707b96..a251ce18c 100644
--- a/website/docs/user-group/group.md
+++ b/website/docs/user-group/group.md
@@ -8,6 +8,8 @@ Groups can be children of another group. Members of children groups are effectiv
 
 When you bind a group to an application or flow, any members of any child group of the selected group will have access.
 
+Recursion is limited to 20 levels to prevent deadlocks.
+
 ## Attributes
 
 Attributes of groups are recursively merged, for all groups the user is a _direct_ member of.