Skip to content

Commit

Permalink
core: optionally don't return groups' users and users' groups by defa…
Browse files Browse the repository at this point in the history
…ult (#9179)

* core: don't return groups' users and users' groups by default

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* explicitly fetch users and groups in LDAP

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* add indicies

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
  • Loading branch information
BeryJu authored Apr 15, 2024
1 parent bc9984f commit 85fedec
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 30 deletions.
34 changes: 29 additions & 5 deletions authentik/core/api/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
from django.http import Http404
from django_filters.filters import CharFilter, ModelMultipleChoiceFilter
from django_filters.filterset import FilterSet
from drf_spectacular.utils import OpenApiResponse, extend_schema
from drf_spectacular.utils import (
OpenApiParameter,
OpenApiResponse,
extend_schema,
extend_schema_field,
)
from guardian.shortcuts import get_objects_for_user
from rest_framework.decorators import action
from rest_framework.fields import CharField, IntegerField
from rest_framework.fields import CharField, IntegerField, SerializerMethodField
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.serializers import ListSerializer, ModelSerializer, ValidationError
Expand Down Expand Up @@ -45,9 +50,7 @@ class GroupSerializer(ModelSerializer):
"""Group Serializer"""

attributes = JSONDictField(required=False)
users_obj = ListSerializer(
child=GroupMemberSerializer(), read_only=True, source="users", required=False
)
users_obj = SerializerMethodField()
roles_obj = ListSerializer(
child=RoleSerializer(),
read_only=True,
Expand All @@ -58,6 +61,19 @@ class GroupSerializer(ModelSerializer):

num_pk = IntegerField(read_only=True)

@property
def _should_include_users(self) -> bool:
request: Request = self.context.get("request", None)
if not request:
return True
return str(request.query_params.get("include_users", "true")).lower() == "true"

@extend_schema_field(GroupMemberSerializer(many=True))
def get_users_obj(self, instance: Group) -> list[GroupMemberSerializer] | None:
if not self._should_include_users:
return None
return GroupMemberSerializer(instance.users, many=True).data

def validate_parent(self, parent: Group | None):
"""Validate group parent (if set), ensuring the parent isn't itself"""
if not self.instance or not parent:
Expand Down Expand Up @@ -145,6 +161,14 @@ class GroupViewSet(UsedByMixin, ModelViewSet):
filterset_class = GroupFilter
ordering = ["name"]

@extend_schema(
parameters=[
OpenApiParameter("include_users", bool, default=True),
]
)
def list(self, request, *args, **kwargs):
return super().list(request, *args, **kwargs)

@permission_required(None, ["authentik_core.add_user"])
@extend_schema(
request=UserAccountSerializer,
Expand Down
23 changes: 22 additions & 1 deletion authentik/core/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,26 @@ class UserSerializer(ModelSerializer):
queryset=Group.objects.all().order_by("name"),
default=list,
)
groups_obj = ListSerializer(child=UserGroupSerializer(), read_only=True, source="ak_groups")
groups_obj = SerializerMethodField()
uid = CharField(read_only=True)
username = CharField(
max_length=150,
validators=[UniqueValidator(queryset=User.objects.all().order_by("username"))],
)

@property
def _should_include_groups(self) -> bool:
request: Request = self.context.get("request", None)
if not request:
return True
return str(request.query_params.get("include_groups", "true")).lower() == "true"

@extend_schema_field(UserGroupSerializer(many=True))
def get_groups_obj(self, instance: User) -> list[UserGroupSerializer] | None:
if not self._should_include_groups:
return None
return UserGroupSerializer(instance.ak_groups, many=True).data

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if SERIALIZER_CONTEXT_BLUEPRINT in self.context:
Expand Down Expand Up @@ -397,6 +410,14 @@ class UserViewSet(UsedByMixin, ModelViewSet):
def get_queryset(self): # pragma: no cover
return User.objects.all().exclude_anonymous().prefetch_related("ak_groups")

@extend_schema(
parameters=[
OpenApiParameter("include_groups", bool, default=True),
]
)
def list(self, request, *args, **kwargs):
return super().list(request, *args, **kwargs)

def _create_recovery_link(self) -> tuple[str, Token]:
"""Create a recovery link (when the current brand has a recovery flow set),
that can either be shown to an admin or sent to the user directly"""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 5.0.4 on 2024-04-10 19:05

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("auth", "0012_alter_user_first_name_max_length"),
("authentik_core", "0033_alter_user_options"),
("authentik_rbac", "0003_alter_systempermission_options"),
]

operations = [
migrations.AddIndex(
model_name="group",
index=models.Index(fields=["name"], name="authentik_c_name_9ba8e4_idx"),
),
migrations.AddIndex(
model_name="user",
index=models.Index(fields=["last_login"], name="authentik_c_last_lo_f0179a_idx"),
),
migrations.AddIndex(
model_name="user",
index=models.Index(
fields=["password_change_date"], name="authentik_c_passwor_eec915_idx"
),
),
migrations.AddIndex(
model_name="user",
index=models.Index(fields=["uuid"], name="authentik_c_uuid_3dae2f_idx"),
),
migrations.AddIndex(
model_name="user",
index=models.Index(fields=["path"], name="authentik_c_path_b1f502_idx"),
),
migrations.AddIndex(
model_name="user",
index=models.Index(fields=["type"], name="authentik_c_type_ecf60d_idx"),
),
]
8 changes: 8 additions & 0 deletions authentik/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ class Meta:
"parent",
),
)
indexes = [models.Index(fields=["name"])]
verbose_name = _("Group")
verbose_name_plural = _("Groups")

Expand Down Expand Up @@ -323,6 +324,13 @@ class Meta:
("preview_user", _("Can preview user data sent to providers")),
("view_user_applications", _("View applications the user has access to")),
]
indexes = [
models.Index(fields=["last_login"]),
models.Index(fields=["password_change_date"]),
models.Index(fields=["uuid"]),
models.Index(fields=["path"]),
models.Index(fields=["type"]),
]
authentik_signals_ignored_fields = [
# Logged by the events `password_set`
# the `password_set` action/signal doesn't currently convey which user
Expand Down
4 changes: 2 additions & 2 deletions internal/outpost/ldap/search/direct/direct.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult,
errs.Go(func() error {
if flags.CanSearch {
uapisp := sentry.StartSpan(errCtx, "authentik.providers.ldap.search.api_user")
searchReq, skip := utils.ParseFilterForUser(c.CoreApi.CoreUsersList(uapisp.Context()), parsedFilter, false)
searchReq, skip := utils.ParseFilterForUser(c.CoreApi.CoreUsersList(uapisp.Context()).IncludeGroups(true), parsedFilter, false)

if skip {
req.Log().Trace("Skip backend request")
Expand Down Expand Up @@ -150,7 +150,7 @@ func (ds *DirectSearcher) Search(req *search.Request) (ldap.ServerSearchResult,
if needGroups {
errs.Go(func() error {
gapisp := sentry.StartSpan(errCtx, "authentik.providers.ldap.search.api_group")
searchReq, skip := utils.ParseFilterForGroup(c.CoreApi.CoreGroupsList(gapisp.Context()), parsedFilter, false)
searchReq, skip := utils.ParseFilterForGroup(c.CoreApi.CoreGroupsList(gapisp.Context()).IncludeUsers(true), parsedFilter, false)
if skip {
req.Log().Trace("Skip backend request")
return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/outpost/ldap/search/memory/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func NewMemorySearcher(si server.LDAPServerInstance) *MemorySearcher {
ds: direct.NewDirectSearcher(si),
}
ms.log.Debug("initialised memory searcher")
ms.users = paginator.FetchUsers(ms.si.GetAPIClient().CoreApi.CoreUsersList(context.TODO()))
ms.groups = paginator.FetchGroups(ms.si.GetAPIClient().CoreApi.CoreGroupsList(context.TODO()))
ms.users = paginator.FetchUsers(ms.si.GetAPIClient().CoreApi.CoreUsersList(context.TODO()).IncludeGroups(true))
ms.groups = paginator.FetchGroups(ms.si.GetAPIClient().CoreApi.CoreGroupsList(context.TODO()).IncludeUsers(true))
return ms
}

Expand Down
30 changes: 10 additions & 20 deletions schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3611,6 +3611,11 @@ paths:
schema:
type: string
description: Attributes
- in: query
name: include_users
schema:
type: boolean
default: true
- in: query
name: is_superuser
schema:
Expand Down Expand Up @@ -4558,6 +4563,11 @@ paths:
format: uuid
explode: true
style: form
- in: query
name: include_groups
schema:
type: boolean
default: true
- in: query
name: is_active
schema:
Expand Down Expand Up @@ -43623,26 +43633,6 @@ components:
- num_pk
- parent_name
- pk
UserGroupRequest:
type: object
description: Simplified Group Serializer for user's groups
properties:
name:
type: string
minLength: 1
maxLength: 80
is_superuser:
type: boolean
description: Users added to this group will be superusers.
parent:
type: string
format: uuid
nullable: true
attributes:
type: object
additionalProperties: {}
required:
- name
UserLoginChallenge:
type: object
description: Empty challenge
Expand Down
1 change: 1 addition & 0 deletions web/src/admin/groups/GroupListPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class GroupListPage extends TablePage<Group> {
page: page,
pageSize: (await uiConfig()).pagination.perPage,
search: this.search || "",
includeUsers: false,
});
}

Expand Down
1 change: 1 addition & 0 deletions web/src/admin/groups/MemberSelectModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class MemberSelectTable extends TableModal<User> {
page: page,
pageSize: (await uiConfig()).pagination.perPage,
search: this.search || "",
includeGroups: false,
});
}

Expand Down
1 change: 1 addition & 0 deletions web/src/admin/groups/RelatedGroupList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class RelatedGroupList extends Table<Group> {
pageSize: (await uiConfig()).pagination.perPage,
search: this.search || "",
membersByPk: this.targetUser ? [this.targetUser.pk] : [],
includeUsers: false,
});
}

Expand Down
1 change: 1 addition & 0 deletions web/src/admin/groups/RelatedUserList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export class RelatedUserList extends WithBrandConfig(WithCapabilitiesConfig(Tabl
type: this.hideServiceAccounts
? [CoreUsersListTypeEnum.External, CoreUsersListTypeEnum.Internal]
: undefined,
includeGroups: false,
});
this.me = await me();
return users;
Expand Down
1 change: 1 addition & 0 deletions web/src/admin/users/GroupSelectModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export class GroupSelectModal extends TableModal<Group> {
page: page,
pageSize: (await uiConfig()).pagination.perPage,
search: this.search || "",
includeUsers: false,
});
}

Expand Down
1 change: 1 addition & 0 deletions web/src/admin/users/UserListPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export class UserListPage extends WithBrandConfig(WithCapabilitiesConfig(TablePa
search: this.search || "",
pathStartswith: getURLParam("path", ""),
isActive: this.hideDeactivated ? true : undefined,
includeGroups: false,
});
this.userPaths = await new CoreApi(DEFAULT_CONFIG).coreUsersPathsRetrieve({
search: this.search,
Expand Down

0 comments on commit 85fedec

Please sign in to comment.