providers/ldap: Remove search group (#10639)

* remove search_group Signed-off-by: Jens Langhammer <jens@goauthentik.io> * make api operations cleaerer Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix migration Signed-off-by: Jens Langhammer <jens@goauthentik.io> * actually use get Signed-off-by: Jens Langhammer <jens@goauthentik.io> * use correct api client for ldap Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix tests Signed-off-by: Jens Langhammer <jens@goauthentik.io> * fix migration Signed-off-by: Jens Langhammer <jens@goauthentik.io> * unrelated: fix migration warning Signed-off-by: Jens Langhammer <jens@goauthentik.io> * add docs Signed-off-by: Jens Langhammer <jens@goauthentik.io> * update docs Signed-off-by: Jens Langhammer <jens@goauthentik.io> * unrelated: fix styling issue in dark mode Signed-off-by: Jens Langhammer <jens@goauthentik.io> * unrelated-ish fix button order in wizard Signed-off-by: Jens Langhammer <jens@goauthentik.io> * unrelated: fix missing css import Signed-off-by: Jens Langhammer <jens@goauthentik.io> * Optimised images with calibre/image-actions * Update index.md Co-authored-by: Tana M Berry <tanamarieberry@yahoo.com> Signed-off-by: Jens L. <jens@beryju.org> * Update index.md Co-authored-by: Tana M Berry <tanamarieberry@yahoo.com> Signed-off-by: Jens L. <jens@beryju.org> * Apply suggestions from code review Co-authored-by: Tana M Berry <tanamarieberry@yahoo.com> Signed-off-by: Jens L. <jens@beryju.org> * update release notes based on new template Signed-off-by: Jens Langhammer <jens@goauthentik.io> --------- Signed-off-by: Jens Langhammer <jens@goauthentik.io> Signed-off-by: Jens L. <jens@beryju.org> Co-authored-by: authentik-automation[bot] <135050075+authentik-automation[bot]@users.noreply.github.com> Co-authored-by: Tana M Berry <tanamarieberry@yahoo.com>
goauthentik · Aug 14, 2024 · 8f53d0b · 8f53d0b
1 parent 3815803
commit 8f53d0b
Show file tree

Hide file tree

Showing 33 changed files with 238 additions and 204 deletions.
diff --git a/authentik/providers/ldap/api.py b/authentik/providers/ldap/api.py
@@ -2,15 +2,25 @@
 
 from django.db.models import QuerySet
 from django.db.models.query import Q
+from django.shortcuts import get_object_or_404
 from django_filters.filters import BooleanFilter
 from django_filters.filterset import FilterSet
-from rest_framework.fields import CharField, ListField, SerializerMethodField
+from drf_spectacular.types import OpenApiTypes
+from drf_spectacular.utils import OpenApiParameter, extend_schema
+from rest_framework.decorators import action
+from rest_framework.fields import BooleanField, CharField, ListField, SerializerMethodField
 from rest_framework.mixins import ListModelMixin
+from rest_framework.request import Request
+from rest_framework.response import Response
 from rest_framework.viewsets import GenericViewSet, ModelViewSet
 
 from authentik.core.api.providers import ProviderSerializer
 from authentik.core.api.used_by import UsedByMixin
-from authentik.core.api.utils import ModelSerializer
+from authentik.core.api.utils import ModelSerializer, PassiveSerializer
+from authentik.core.models import Application
+from authentik.policies.api.exec import PolicyTestResultSerializer
+from authentik.policies.engine import PolicyEngine
+from authentik.policies.types import PolicyResult
 from authentik.providers.ldap.models import LDAPProvider
 
 
@@ -23,7 +33,6 @@ class Meta:
         model = LDAPProvider
         fields = ProviderSerializer.Meta.fields + [
             "base_dn",
-            "search_group",
             "certificate",
             "tls_server_name",
             "uid_start_number",
@@ -55,8 +64,6 @@ class Meta:
             "name": ["iexact"],
             "authorization_flow__slug": ["iexact"],
             "base_dn": ["iexact"],
-            "search_group__group_uuid": ["iexact"],
-            "search_group__name": ["iexact"],
             "certificate__kp_uuid": ["iexact"],
             "certificate__name": ["iexact"],
             "tls_server_name": ["iexact"],
@@ -95,7 +102,6 @@ class Meta:
             "base_dn",
             "bind_flow_slug",
             "application_slug",
-            "search_group",
             "certificate",
             "tls_server_name",
             "uid_start_number",
@@ -116,3 +122,33 @@ class LDAPOutpostConfigViewSet(ListModelMixin, GenericViewSet):
     ordering = ["name"]
     search_fields = ["name"]
     filterset_fields = ["name"]
+
+    class LDAPCheckAccessSerializer(PassiveSerializer):
+        has_search_permission = BooleanField(required=False)
+        access = PolicyTestResultSerializer()
+
+    @extend_schema(
+        request=None,
+        parameters=[OpenApiParameter("app_slug", OpenApiTypes.STR)],
+        responses={
+            200: LDAPCheckAccessSerializer(),
+        },
+        operation_id="outposts_ldap_access_check",
+    )
+    @action(detail=True)
+    def check_access(self, request: Request, pk) -> Response:
+        """Check access to a single application by slug"""
+        provider = get_object_or_404(LDAPProvider, pk=pk)
+        application = get_object_or_404(Application, slug=request.query_params["app_slug"])
+        engine = PolicyEngine(application, request.user, request)
+        engine.use_cache = False
+        engine.build()
+        result = engine.result
+        access_response = PolicyResult(result.passing)
+        response = self.LDAPCheckAccessSerializer(
+            instance={
+                "has_search_permission": request.user.has_perm("search_full_directory", provider),
+                "access": access_response,
+            }
+        )
+        return Response(response.data)
diff --git a/authentik/providers/ldap/migrations/0004_alter_ldapprovider_options_and_more.py b/authentik/providers/ldap/migrations/0004_alter_ldapprovider_options_and_more.py
@@ -0,0 +1,52 @@
+# Generated by Django 5.0.7 on 2024-07-25 14:59
+from django.apps.registry import Apps
+
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+
+from django.db import migrations
+from django.contrib.auth.management import create_permissions
+
+
+def migrate_search_group(apps: Apps, schema_editor: BaseDatabaseSchemaEditor):
+    from guardian.shortcuts import assign_perm
+    from authentik.core.models import User
+    from django.apps import apps as real_apps
+
+    db_alias = schema_editor.connection.alias
+
+    # Permissions are only created _after_ migrations are run
+    # - https://github.com/django/django/blob/43cdfa8b20e567a801b7d0a09ec67ddd062d5ea4/django/contrib/auth/apps.py#L19
+    # - https://stackoverflow.com/a/72029063/1870445
+    create_permissions(real_apps.get_app_config("authentik_providers_ldap"), using=db_alias)
+
+    LDAPProvider = apps.get_model("authentik_providers_ldap", "ldapprovider")
+
+    for provider in LDAPProvider.objects.using(db_alias).all():
+        for user_pk in (
+            provider.search_group.users.using(db_alias).all().values_list("pk", flat=True)
+        ):
+            # We need the correct user model instance to assign the permission
+            assign_perm("search_full_directory", User.objects.get(pk=user_pk), provider)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("authentik_providers_ldap", "0003_ldapprovider_mfa_support_and_more"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="ldapprovider",
+            options={
+                "permissions": [("search_full_directory", "Search full LDAP directory")],
+                "verbose_name": "LDAP Provider",
+                "verbose_name_plural": "LDAP Providers",
+            },
+        ),
+        migrations.RunPython(migrate_search_group),
+        migrations.RemoveField(
+            model_name="ldapprovider",
+            name="search_group",
+        ),
+    ]
diff --git a/authentik/providers/ldap/models.py b/authentik/providers/ldap/models.py
@@ -7,7 +7,7 @@
 from django.utils.translation import gettext_lazy as _
 from rest_framework.serializers import Serializer
 
-from authentik.core.models import BackchannelProvider, Group
+from authentik.core.models import BackchannelProvider
 from authentik.crypto.models import CertificateKeyPair
 from authentik.outposts.models import OutpostModel
 
@@ -27,17 +27,6 @@ class LDAPProvider(OutpostModel, BackchannelProvider):
         help_text=_("DN under which objects are accessible."),
     )
 
-    search_group = models.ForeignKey(
-        Group,
-        null=True,
-        default=None,
-        on_delete=models.SET_DEFAULT,
-        help_text=_(
-            "Users in this group can do search queries. "
-            "If not set, every user can execute search queries."
-        ),
-    )
-
     tls_server_name = models.TextField(
         default="",
         blank=True,
@@ -113,3 +102,6 @@ def get_required_objects(self) -> Iterable[models.Model | str]:
     class Meta:
         verbose_name = _("LDAP Provider")
         verbose_name_plural = _("LDAP Providers")
+        permissions = [
+            ("search_full_directory", _("Search full LDAP directory")),
+        ]
diff --git a/authentik/providers/radius/api/providers.py b/authentik/providers/radius/api/providers.py
@@ -154,6 +154,7 @@ def define_attribute(
         responses={
             200: RadiusCheckAccessSerializer(),
         },
+        operation_id="outposts_radius_access_check",
     )
     @action(detail=True)
     def check_access(self, request: Request, pk) -> Response:

diff --git a/...es/authenticator_duo/migrations/0006_duodevice_created_duodevice_last_updated_and_more.py b/...es/authenticator_duo/migrations/0006_duodevice_created_duodevice_last_updated_and_more.py
@@ -14,7 +14,9 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="duodevice",
             name="created",
-            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0)),
+            field=models.DateTimeField(
+                auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0, tzinfo=datetime.UTC)
+            ),
             preserve_default=False,
         ),
         migrations.AddField(

diff --git a/...es/authenticator_sms/migrations/0007_smsdevice_created_smsdevice_last_updated_and_more.py b/...es/authenticator_sms/migrations/0007_smsdevice_created_smsdevice_last_updated_and_more.py
@@ -14,7 +14,9 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="smsdevice",
             name="created",
-            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0)),
+            field=models.DateTimeField(
+                auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0, tzinfo=datetime.UTC)
+            ),
             preserve_default=False,
         ),
         migrations.AddField(

diff --git a/...ticator_static/migrations/0010_staticdevice_created_staticdevice_last_updated_and_more.py b/...ticator_static/migrations/0010_staticdevice_created_staticdevice_last_updated_and_more.py
@@ -14,7 +14,9 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="staticdevice",
             name="created",
-            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0)),
+            field=models.DateTimeField(
+                auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0, tzinfo=datetime.UTC)
+            ),
             preserve_default=False,
         ),
         migrations.AddField(

diff --git a/...authenticator_totp/migrations/0011_totpdevice_created_totpdevice_last_updated_and_more.py b/...authenticator_totp/migrations/0011_totpdevice_created_totpdevice_last_updated_and_more.py
@@ -14,7 +14,9 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="totpdevice",
             name="created",
-            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0)),
+            field=models.DateTimeField(
+                auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0, tzinfo=datetime.UTC)
+            ),
             preserve_default=False,
         ),
         migrations.AddField(

diff --git a/...r_webauthn/migrations/0012_webauthndevice_created_webauthndevice_last_updated_and_more.py b/...r_webauthn/migrations/0012_webauthndevice_created_webauthndevice_last_updated_and_more.py
@@ -14,7 +14,9 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name="webauthndevice",
             name="created",
-            field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0)),
+            field=models.DateTimeField(
+                auto_now_add=True, default=datetime.datetime(1, 1, 1, 0, 0, tzinfo=datetime.UTC)
+            ),
             preserve_default=False,
         ),
         migrations.AddField(

diff --git a/blueprints/schema.json b/blueprints/schema.json
@@ -5131,12 +5131,6 @@
                     "title": "Base dn",
                     "description": "DN under which objects are accessible."
                 },
-                "search_group": {
-                    "type": "string",
-                    "format": "uuid",
-                    "title": "Search group",
-                    "description": "Users in this group can do search queries. If not set, every user can execute search queries."
-                },
                 "certificate": {
                     "type": "string",
                     "format": "uuid",

diff --git a/internal/outpost/flow/executor.go b/internal/outpost/flow/executor.go
@@ -120,21 +120,6 @@ func (fe *FlowExecutor) DelegateClientIP(a string) {
 	fe.api.GetConfig().AddDefaultHeader(HeaderAuthentikRemoteIP, fe.cip)
 }
 
-func (fe *FlowExecutor) CheckApplicationAccess(appSlug string) (bool, error) {
-	acsp := sentry.StartSpan(fe.Context, "authentik.outposts.flow_executor.check_access")
-	defer acsp.Finish()
-	p, _, err := fe.api.CoreApi.CoreApplicationsCheckAccessRetrieve(acsp.Context(), appSlug).Execute()
-	if err != nil {
-		return false, fmt.Errorf("failed to check access: %w", err)
-	}
-	if !p.Passing {
-		fe.log.Info("Access denied for user")
-		return false, nil
-	}
-	fe.log.Debug("User has access")
-	return true, nil
-}
-
 func (fe *FlowExecutor) getAnswer(stage StageComponent) string {
 	if v, o := fe.Answers[stage]; o {
 		return v

diff --git a/internal/outpost/ldap/bind/direct/bind.go b/internal/outpost/ldap/bind/direct/bind.go
@@ -58,8 +58,10 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul
 		return ldap.LDAPResultInvalidCredentials, nil
 	}
 
-	access, err := fe.CheckApplicationAccess(db.si.GetAppSlug())
-	if !access {
+	access, _, err := fe.ApiClient().OutpostsApi.OutpostsLdapAccessCheck(
+		req.Context(), db.si.GetProviderID(),
+	).AppSlug(db.si.GetAppSlug()).Execute()
+	if !access.Access.Passing {
 		req.Log().Info("Access denied for user")
 		metrics.RequestsRejected.With(prometheus.Labels{
 			"outpost_name": db.si.GetOutpostName(),
@@ -93,12 +95,11 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul
 		req.Log().WithError(err).Warning("failed to get user info")
 		return ldap.LDAPResultOperationsError, nil
 	}
-	cs := db.SearchAccessCheck(userInfo.User)
 	flags.UserPk = userInfo.User.Pk
-	flags.CanSearch = cs != nil
+	flags.CanSearch = access.HasSearchPermission != nil
 	db.si.SetFlags(req.BindDN, &flags)
 	if flags.CanSearch {
-		req.Log().WithField("group", cs).Info("Allowed access to search")
+		req.Log().Debug("Allowed access to search")
 	}
 	uisp.Finish()
 	return ldap.LDAPResultSuccess, nil

diff --git a/internal/outpost/ldap/bind/direct/direct.go b/internal/outpost/ldap/bind/direct/direct.go
@@ -7,7 +7,6 @@ import (
 
 	goldap "github.com/go-ldap/ldap/v3"
 	log "github.com/sirupsen/logrus"
-	"goauthentik.io/api/v3"
 	"goauthentik.io/internal/outpost/flow"
 	"goauthentik.io/internal/outpost/ldap/server"
 	"goauthentik.io/internal/outpost/ldap/utils"
@@ -47,22 +46,6 @@ func (db *DirectBinder) GetUsername(dn string) (string, error) {
 	return "", errors.New("failed to find cn")
 }
 
-// SearchAccessCheck Check if the current user is allowed to search
-func (db *DirectBinder) SearchAccessCheck(user api.UserSelf) *string {
-	for _, group := range user.Groups {
-		for _, allowedGroup := range db.si.GetSearchAllowedGroups() {
-			if allowedGroup == nil {
-				continue
-			}
-			db.log.WithField("userGroup", group.Pk).WithField("allowedGroup", allowedGroup).Trace("Checking search access")
-			if group.Pk == allowedGroup.String() {
-				return &group.Name
-			}
-		}
-	}
-	return nil
-}
-
 func (db *DirectBinder) TimerFlowCacheExpiry(ctx context.Context) {
 	fe := flow.NewFlowExecutor(ctx, db.si.GetAuthenticationFlowSlug(), db.si.GetAPIClient().GetConfig(), log.Fields{})
 	fe.Params.Add("goauthentik.io/outpost/ldap", "true")

diff --git a/internal/outpost/ldap/instance.go b/internal/outpost/ldap/instance.go
@@ -5,7 +5,6 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/go-openapi/strfmt"
 	log "github.com/sirupsen/logrus"
 
 	"goauthentik.io/api/v3"
@@ -31,14 +30,13 @@ type ProviderInstance struct {
 	s                      *LDAPServer
 	log                    *log.Entry
 
-	tlsServerName       *string
-	cert                *tls.Certificate
-	certUUID            string
-	outpostName         string
-	outpostPk           int32
-	searchAllowedGroups []*strfmt.UUID
-	boundUsersMutex     *sync.RWMutex
-	boundUsers          map[string]*flags.UserFlags
+	tlsServerName   *string
+	cert            *tls.Certificate
+	certUUID        string
+	outpostName     string
+	providerPk      int32
+	boundUsersMutex *sync.RWMutex
+	boundUsers      map[string]*flags.UserFlags
 
 	uidStartNumber int32
 	gidStartNumber int32
@@ -105,8 +103,8 @@ func (pi *ProviderInstance) GetInvalidationFlowSlug() string {
 	return pi.invalidationFlowSlug
 }
 
-func (pi *ProviderInstance) GetSearchAllowedGroups() []*strfmt.UUID {
-	return pi.searchAllowedGroups
+func (pi *ProviderInstance) GetProviderID() int32 {
+	return pi.providerPk
 }
 
 func (pi *ProviderInstance) GetNeededObjects(scope int, baseDN string, filterOC string) (bool, bool) {