Skip to content

Commit

Permalink
Merge pull request #954 from syucream/fix/check-role-permissions
Browse files Browse the repository at this point in the history
Check role editability before update/destroy in the new UI
  • Loading branch information
userlocalhost authored Sep 25, 2023
2 parents 4150591 + 0c5b62d commit b817397
Show file tree
Hide file tree
Showing 14 changed files with 315 additions and 23 deletions.
2 changes: 1 addition & 1 deletion apiclient/typescript-fetch/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@dmm-com/airone-apiclient-typescript-fetch",
"version": "0.0.5",
"version": "0.0.6",
"description": "AirOne APIv2 client in TypeScript",
"main": "src/autogenerated/index.ts",
"scripts": {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/role/RoleList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ test("should render a component with essential props", function () {
groups: [],
adminUsers: [],
adminGroups: [],
isEditable: true,
},
];

Expand Down
12 changes: 10 additions & 2 deletions frontend/src/components/role/RoleList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const RoleList: FC = ({}) => {
}
};

console.log(roles.value);
return (
<Box>
{roles.loading ? (
Expand Down Expand Up @@ -132,7 +133,10 @@ export const RoleList: FC = ({}) => {
<TableCell>
<Confirmable
componentGenerator={(handleOpen) => (
<StyledIconButton onClick={handleOpen}>
<StyledIconButton
disabled={!role.isEditable}
onClick={handleOpen}
>
<DeleteOutlineIcon />
</StyledIconButton>
)}
Expand All @@ -141,7 +145,11 @@ export const RoleList: FC = ({}) => {
/>
</TableCell>
<TableCell>
<StyledIconButton component={Link} to={rolePath(role.id)}>
<StyledIconButton
disabled={!role.isEditable}
component={Link}
to={rolePath(role.id)}
>
<EditOutlinedIcon />
</StyledIconButton>
</TableCell>
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/role/roleForm/RoleFormSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { z } from "zod";

import { schemaForType } from "../../../services/ZodSchemaUtil";

export const schema = schemaForType<Role>()(
type RoleForSchema = Omit<Role, "isEditable">;

export const schema = schemaForType<RoleForSchema>()(
z
.object({
id: z.number().default(0),
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/EditRolePage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ test("should match snapshot", async () => {
groups: [],
adminUsers: [],
adminGroups: [],
isEditable: true,
};

/* eslint-disable */
Expand Down
7 changes: 6 additions & 1 deletion frontend/src/pages/EditRolePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ export const EditRolePage: FC = () => {
>
<SubmitButton
name="保存"
disabled={!isValid || isSubmitting || isSubmitSuccessful}
disabled={
!isValid ||
isSubmitting ||
isSubmitSuccessful ||
role.value?.isEditable === false
}
handleSubmit={handleSubmit(handleSubmitOnValid)}
handleCancel={handleCancel}
/>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/RolePage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ test("should match snapshot", async () => {
groups: [],
adminUsers: [],
adminGroups: [],
isEditable: true,
},
];

Expand Down
13 changes: 12 additions & 1 deletion frontend/src/services/AironeAPIErrorUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ type AironeApiFieldsError<T> = {
};

type AironeApiError<T> = AironeApiFieldsError<T> & {
// root-level
code?: string;
message?: string;

non_field_errors?: Array<ErrorDetail>;
};

// https://github.com/dmm-com/airone/wiki/(Blueprint)-AirOne-API-Error-code-mapping
const aironeAPIErrors: Record<string, string> = {
"AE-220000": "入力データが既存のデータと重複しています",
"AE-122000": "入力データが大きすぎます",
"AE-210000": "操作に必要な権限が不足しています",
"AE-220000": "入力データが既存のデータと重複しています",
};

const extractErrorDetail = (errorDetail: ErrorDetail): string =>
Expand All @@ -38,6 +43,12 @@ export const extractAPIException = async <T>(
const json = await errpr.response.json();
const typed = json as AironeApiError<T>;

// root-level error will drop field-level errors
if (typed.code != null && typed.message != null) {
nonFieldReporter(extractErrorDetail(typed as ErrorDetail));
return;
}

if (typed.non_field_errors != null) {
const fullMessage = typed.non_field_errors
.map((e) => extractErrorDetail(e))
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"zod": "^3.20.6"
},
"dependencies": {
"@dmm-com/airone-apiclient-typescript-fetch": "^0.0.5",
"@dmm-com/airone-apiclient-typescript-fetch": "^0.0.6",
"@emotion/react": "^11.10.5",
"@emotion/styled": "^11.10.5",
"@jest/globals": "^27.0.6",
Expand Down
6 changes: 6 additions & 0 deletions role/api_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class RoleSerializer(serializers.ModelSerializer):
groups = RoleGroupSerializer(many=True)
admin_users = RoleUserSerializer(many=True)
admin_groups = RoleGroupSerializer(many=True)
is_editable = serializers.SerializerMethodField(method_name="get_is_editable", read_only=True)

class Meta:
model = Role
Expand All @@ -44,8 +45,13 @@ class Meta:
"groups",
"admin_users",
"admin_groups",
"is_editable",
]

def get_is_editable(self, obj: Role) -> bool:
current_user: User = self.context["request"].user
return Role.editable(current_user, obj.admin_users.all(), obj.admin_groups.all())


class RoleCreateUpdateSerializer(serializers.ModelSerializer):
class Meta:
Expand Down
19 changes: 16 additions & 3 deletions role/api_v2/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from rest_framework import generics, serializers, status, viewsets
from rest_framework.permissions import IsAuthenticated
from rest_framework.permissions import BasePermission, IsAuthenticated
from rest_framework.response import Response

from acl.models import ACLBase
Expand All @@ -15,9 +15,22 @@
from user.models import User


class RolePermission(BasePermission):
def has_object_permission(self, request, view, obj: Role):
current_user: User = request.user
is_editable = Role.editable(current_user, obj.admin_users.all(), obj.admin_groups.all())
permission = {
"retrieve": True,
"create": True,
"destroy": is_editable,
"update": is_editable,
}
return permission.get(view.action)


class RoleAPI(viewsets.ModelViewSet):
queryset = Role.objects.filter(is_active=True)
permission_classes = [IsAuthenticated]
queryset = Role.objects.filter(is_active=True).prefetch_related("admin_users", "admin_groups")
permission_classes = [IsAuthenticated & RolePermission]

def get_serializer_class(self):
serializer = {
Expand Down
5 changes: 3 additions & 2 deletions role/models.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import importlib
import sys
from datetime import datetime
from typing import List

from django.contrib.auth.models import Permission
from django.contrib.auth.models import Group, Permission
from django.db import models
from django.db.models import Q
from simple_history.models import HistoricalRecords
Expand All @@ -22,7 +23,7 @@ class Role(models.Model):
admin_groups = models.ManyToManyField("group.Group", related_name="admin_role", blank=True)

@classmethod
def editable(kls, user, admin_users, admin_groups):
def editable(kls, user, admin_users, admin_groups: List[Group]):
# This checks whether spcified user is belonged to the specified
# admin_users and admin_groups.
if user.is_superuser:
Expand Down
Loading

0 comments on commit b817397

Please sign in to comment.