Feature: Add sponser viewset #14
Conversation
sponsor/viewsets.py
Outdated
|
|
||
| class SponsorViewSet(ReadOnlyModelViewSet): | ||
| serializer_class = SponsorSerializer | ||
| permission_classes = [IsAuthenticatedOrReadOnly] # 로그인된 사용자에게만 허용 |
There was a problem hiding this comment.
IsAuthenticatedOrReadOnly 에 대하여 '로그인된 사용자에게만 허용' 이란 주석이 설명이 부족한 것 같습니다. (해깔릴 수 있을것같습니다)
제가 알기로는 해당 권한은 인증되지 않은 사용자에 대해서는 GET 등의 요청에 대한 접근을 허용 하는 것으로 알고 있습니다.
자료: https://www.django-rest-framework.org/api-guide/permissions/#isauthenticatedorreadonly
There was a problem hiding this comment.
@hanlee55 님이 코멘트 주신 것처럼 주석 내용 수정이 필요할 것 같아요~!
해당 permission은 로그인한 사용자에게는 모든 작업을 그렇지 않으면 조회를 허용하는 케이스입니다.
There was a problem hiding this comment.
@oleveloper
퍼미션에 대한 수정도 필요할 것 같아요~!
ModelViewSet을 상속받는다는 가정 하에, 지금의 퍼미션에서는 아래의 이슈가 예상됩니다.
- CFS 기능은 로그인 적용 이전에 배포될 예정이어서, 아무도 POST를 할 수 없습니다.
- 제출된 후원사 내용을 누구나 조회할 수 있습니다.
- 일단 로그인만 되면, DB 저장된 레코드를 추정해, 다른 사용자의 데이터도 수정/삭제할 수 있습니다.
지금 DRF에서 본인의 데이터만 조회할 수 있음 퍼미션은 없어서,
- 로직에 해당 내용을 포함시키거나,
- 커스텀 퍼미션을 만들거나
하는 방법이 있을 것 같아요.
이건 같이 작업해보실까요 ㅎㅎ
There was a problem hiding this comment.
꼼꼼한 리뷰 감사드립니다. 생각지 못한 부분이네요.
@hanlee55 @golony6449 두 분께서 말씀하신 바와 같이, 인증되지 않은 사용자에 대해 Read only 권한만 부여합니다.
이 부분은 API 이름을 통해 명확하게 기능을 판단할 수 있으므로 혼동을 야기시킬 수 있는 본 주석을 제거하도록 하겠습니다.
코드 확인을 해보니 커스텀 퍼미션이 적용되며 해당 부분이 이미 수정 되었네요 🙂
|
아하 Viewset과 라우터도 적용을 해주셨군요 :-) 저도 자세히는 몰라서.. |
| urlpatterns = [ | ||
| path("api-auth/", include("rest_framework.urls")), | ||
| path("summernote/", include("django_summernote.urls")), | ||
| path("admin/", admin.site.urls), | ||
| path("sponsors/", include("sponsor.urls")), | ||
| path("sponsors/", include(sponsor.routers.get_router().urls)), |
| class SponsorListSerializer(ModelSerializer): | ||
| class Meta: | ||
| model = Sponsor | ||
| fields = [ |
| if request.method in permissions.SAFE_METHODS: | ||
| return True | ||
|
|
||
| return obj.manager_id == request.user or obj.creator == request.user |
There was a problem hiding this comment.
값을 바꾸지 않는 methods 일 때, 또는 매니저 또는 작성자가 접근할 때에만 Permission이 부여되는 커스텀 퍼미션이군요. 확인했습니다.
|
|
||
| class OwnerOnly(permissions.BasePermission): | ||
| def has_object_permission(self, request, view, obj: Sponsor): | ||
| return obj.manager_id == request.user or obj.creator == request.user |
There was a problem hiding this comment.
관리자가 creator와 manager 둘 다 있었네요 👍
| from sponsor.models import Sponsor | ||
| from sponsor.permissions import IsOwnerOrReadOnly, OwnerOnly | ||
| from sponsor.serializers import SponsorListSerializer, SponsorSerializer |
| serializer = SponsorSerializer(sponsor_data) | ||
| # 본인 소유인 경우는 모든 필드 | ||
| # 그렇지 않은 경우는 공개 가능한 필드만 응답 | ||
| serializer = ( |
|
코드 잘 읽었습니다 :) |
| # 그렇지 않은 경우는 공개 가능한 필드만 응답 | ||
| serializer = ( | ||
| SponsorSerializer(sponsor_data) | ||
| if self.check_owner_permission(request, sponsor_data) |
There was a problem hiding this comment.
단순한 호기심이 있다면 🤔
해당 코드에서 not을 사용하지 않고 else로 처리한건 관용적으로 사용하는 부분이 있는지 궁금하네요 :)
목표
작업내용
유의사항
PyConKR-2023을 지정 해 주세요.