Skip to content

Commit

Permalink
fix: authorization groups couldn't be edited by admins (#2158)
Browse files Browse the repository at this point in the history
  • Loading branch information
@StefanFl
StefanFl authored Nov 5, 2024
1 parent f942a68 commit cb238d4
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 46 deletions.
6 changes: 6 additions & 0 deletions backend/application/access_control/api/serializers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ class UserPasswortRulesSerializer(Serializer):
class AuthorizationGroupSerializer(ModelSerializer):
has_product_group_members = SerializerMethodField()
has_product_members = SerializerMethodField()
has_users = SerializerMethodField()
is_manager = SerializerMethodField()

class Meta:
Expand All @@ -203,6 +204,11 @@ def get_has_product_members(self, obj: Authorization_Group) -> bool:
authorization_group=obj, product__is_product_group=False
).exists()

def get_has_users(self, obj: Authorization_Group) -> bool:
return Authorization_Group_Member.objects.filter(
authorization_group=obj
).exists()

def get_is_manager(self, obj: Authorization_Group) -> bool:
user = get_current_user()
return Authorization_Group_Member.objects.filter(
Expand Down
20 changes: 10 additions & 10 deletions backend/unittests/access_control/api/test_authorization_authorization_groups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class TestAuthorizationAuthorizationGroups(TestAuthorizationBase):
def test_authorization_authorization_groups(self):
prepare_authorization_groups()

expected_data = "{'count': 5, 'next': None, 'previous': None, 'results': [{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': False, 'name': 'db_group_internal_write', 'oidc_group': ''}, {'id': 5, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': False, 'name': 'db_group_internal_read', 'oidc_group': ''}, {'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}, {'id': 7, 'has_product_group_members': True, 'has_product_members': False, 'is_manager': False, 'name': 'db_group_product_group', 'oidc_group': ''}, {'id': 8, 'has_product_group_members': False, 'has_product_members': False, 'is_manager': False, 'name': 'db_group_unused', 'oidc_group': ''}]}"
expected_data = "{'count': 5, 'next': None, 'previous': None, 'results': [{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': False, 'name': 'db_group_internal_write', 'oidc_group': ''}, {'id': 5, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': False, 'name': 'db_group_internal_read', 'oidc_group': ''}, {'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}, {'id': 7, 'has_product_group_members': True, 'has_product_members': False, 'has_users': True, 'is_manager': False, 'name': 'db_group_product_group', 'oidc_group': ''}, {'id': 8, 'has_product_group_members': False, 'has_product_members': False, 'has_users': False, 'is_manager': False, 'name': 'db_group_unused', 'oidc_group': ''}]}"
self._test_api(
APITest(
"db_admin",
Expand All @@ -28,8 +28,8 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': True, 'name': 'db_group_internal_write', 'oidc_group': ''}]}"
expected_data_product_group = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 7, 'has_product_group_members': True, 'has_product_members': False, 'is_manager': False, 'name': 'db_group_product_group', 'oidc_group': ''}]}"
expected_data = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': True, 'name': 'db_group_internal_write', 'oidc_group': ''}]}"
expected_data_product_group = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 7, 'has_product_group_members': True, 'has_product_members': False, 'has_users': True, 'is_manager': False, 'name': 'db_group_product_group', 'oidc_group': ''}]}"
self._test_api(
APITest(
"db_internal_write",
Expand All @@ -41,7 +41,7 @@ def test_authorization_authorization_groups(self):
expected_data_product_group=expected_data_product_group,
)
)
expected_data = "{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': True, 'name': 'db_group_internal_write', 'oidc_group': ''}"
expected_data = "{'id': 4, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': True, 'name': 'db_group_internal_write', 'oidc_group': ''}"
self._test_api(
APITest(
"db_internal_write",
Expand All @@ -65,7 +65,7 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}]}"
expected_data = "{'count': 1, 'next': None, 'previous': None, 'results': [{'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}]}"
self._test_api(
APITest(
"db_external",
Expand All @@ -76,7 +76,7 @@ def test_authorization_authorization_groups(self):
expected_data,
)
)
expected_data = "{'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}"
expected_data = "{'id': 6, 'has_product_group_members': False, 'has_product_members': True, 'has_users': True, 'is_manager': False, 'name': 'db_group_external', 'oidc_group': ''}"
self._test_api(
APITest(
"db_external",
Expand All @@ -99,7 +99,7 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'id': 9, 'has_product_group_members': False, 'has_product_members': False, 'is_manager': False, 'name': 'string', 'oidc_group': 'oidc'}"
expected_data = "{'id': 9, 'has_product_group_members': False, 'has_product_members': False, 'has_users': False, 'is_manager': False, 'name': 'string', 'oidc_group': 'oidc'}"
self._test_api(
APITest(
"db_admin",
Expand All @@ -115,7 +115,7 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'id': 10, 'has_product_group_members': False, 'has_product_members': False, 'is_manager': True, 'name': 'string_2', 'oidc_group': 'oidc'}"
expected_data = "{'id': 10, 'has_product_group_members': False, 'has_product_members': False, 'has_users': True, 'is_manager': True, 'name': 'string_2', 'oidc_group': 'oidc'}"
self._test_api(
APITest(
"db_internal_write",
Expand Down Expand Up @@ -149,7 +149,7 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'id': 9, 'has_product_group_members': False, 'has_product_members': False, 'is_manager': False, 'name': 'changed_string', 'oidc_group': 'oidc'}"
expected_data = "{'id': 9, 'has_product_group_members': False, 'has_product_members': False, 'has_users': False, 'is_manager': False, 'name': 'changed_string', 'oidc_group': 'oidc'}"
self._test_api(
APITest(
"db_admin",
Expand All @@ -162,7 +162,7 @@ def test_authorization_authorization_groups(self):
)
)

expected_data = "{'id': 10, 'has_product_group_members': False, 'has_product_members': False, 'is_manager': True, 'name': 'changed_string_2', 'oidc_group': 'oidc'}"
expected_data = "{'id': 10, 'has_product_group_members': False, 'has_product_members': False, 'has_users': True, 'is_manager': True, 'name': 'changed_string_2', 'oidc_group': 'oidc'}"
self._test_api(
APITest(
"db_internal_write",
Expand Down
2 changes: 1 addition & 1 deletion docker/backend/unittests/django/start
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ set -o nounset

python manage.py migrate

coverage run --branch /app/manage.py test unittests
coverage run --branch /app/manage.py test unittests.access_control.api.test_authorization_authorization_groups
coverage report --skip-empty --show-missing
78 changes: 44 additions & 34 deletions ...d/src/access_control/authorization_group_members/AuthorizationGroupMemberEmbeddedList.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Stack } from "@mui/material";
import { Fragment } from "react";
import {
BooleanField,
Datagrid,
Expand Down Expand Up @@ -57,43 +58,52 @@ const AuthorizationGroupMemberEmbeddedList = ({ authorization_group }: Authoriza
{(is_superuser() || authorization_group.is_manager) && (
<AuthorizationGroupMemberAdd id={authorization_group.id} />
)}
<FilterForm filters={listFilters()} />
<Datagrid size={getSettingListSize()} rowClick={false} bulkActionButtons={false} resource="users">
<WithRecord
label="Full name"
render={(authorization_group_member) => (
<TextUrlField
label="User"
text={authorization_group_member.user_data.full_name}
url={showUser(authorization_group_member.user_data.id)}
/>
)}
/>
<WithRecord
label="Username"
render={(authorization_group_member) => (
<TextUrlField
label="User"
text={authorization_group_member.user_data.username}
url={showUser(authorization_group_member.user_data.id)}
{authorization_group.has_users && (
<Fragment>
<FilterForm filters={listFilters()} />
<Datagrid
size={getSettingListSize()}
rowClick={false}
bulkActionButtons={false}
resource="users"
>
<WithRecord
label="Full name"
render={(authorization_group_member) => (
<TextUrlField
label="User"
text={authorization_group_member.user_data.full_name}
url={showUser(authorization_group_member.user_data.id)}
/>
)}
/>
)}
/>
<BooleanField source="is_manager" label="Manager" />
{(is_superuser() || authorization_group.is_manager) && (
<WithRecord
render={(authorization_group_member) => (
<Stack direction="row" spacing={4}>
<AuthorizationGroupMemberEdit />
<AuthorizationGroupMemberRemove
authorization_group_member={authorization_group_member}
<WithRecord
label="Username"
render={(authorization_group_member) => (
<TextUrlField
label="User"
text={authorization_group_member.user_data.username}
url={showUser(authorization_group_member.user_data.id)}
/>
</Stack>
)}
/>
<BooleanField source="is_manager" label="Manager" />
{(is_superuser() || authorization_group.is_manager) && (
<WithRecord
render={(authorization_group_member) => (
<Stack direction="row" spacing={4}>
<AuthorizationGroupMemberEdit />
<AuthorizationGroupMemberRemove
authorization_group_member={authorization_group_member}
/>
</Stack>
)}
/>
)}
/>
)}
</Datagrid>
<CustomPagination />
</Datagrid>
<CustomPagination />
</Fragment>
)}
</div>
</ListContextProvider>
</ResourceContextProvider>
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/access_control/authorization_groups/AuthorizationGroupShow.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useRecordContext,
} from "react-admin";

import { is_superuser } from "../../commons/functions";
import { useStyles } from "../../commons/layout/themes";
import UserProductAuthorizationGroupMemberEmbeddedList from "../../core/product_authorization_group_members/UserProductAuthorizationGroupMemberEmbeddedList";
import AuthorizationGroupMemberEmbeddedList from "../authorization_group_members/AuthorizationGroupMemberEmbeddedList";
Expand All @@ -26,7 +27,7 @@ const ShowActions = () => {
filterDefaultValues={{ is_active: true }}
storeKey="authorization_groups.embedded"
/>
{authorization_group && authorization_group.is_manager && <EditButton />}
{((authorization_group && authorization_group.is_manager) || is_superuser()) && <EditButton />}
</Stack>
</TopToolbar>
);
Expand Down

0 comments on commit cb238d4

Please sign in to comment.