How to check with decorator if user has access to some objects in Django-CodePudding

I'm working on a project, with structure as below. And I was trying to check if user is a member of the requested team and is allowed to see it's details. So I was trying to achieve this by decorators, but i'm still getting http 500 error.

I'm not sure if it's right solution and just bad implementation or it's better to make it in different way.

I'll be gratefull for Your help.

Team/Models.py

roles = (('admin', 'admin'), ('member', 'member'), ('guest', 'guest'))

class Team(models.Model):
    name = models.CharField(max_length=150)
    description = models.TextField(blank=True)
    members = models.ManyToManyField(User, through='Membership')
    cppTables = models.JSONField(default=list, blank=True)
    
    def __str__(self):
        return self.name

    def get_absolute_url(self):
        return f'teams/{self.pk}/'

    def has_user(self, user_id):
        return self.members.get(pk=user_id).exists()

class Membership(models.Model):
    member = models.ForeignKey(User, on_delete=models.CASCADE)
    team = models.ForeignKey(Team, on_delete=models.CASCADE)
    role = models.CharField(max_length=15, choices=roles, default='member')

    def __str__(self):
        return str(self.member)

    class Meta:
        unique_together = [['member', 'team']]

Team/Decorators.py

def required_membership(view_func):
    def wrapper(request, *args, **kwargs):
        team_id = kwargs['team_id']
        team = Team.objects.get(pk=team_id)
        if team.has_user(request.user.id):
            return view_func(request, *args, **kwargs)
        else:
            raise HttpResponseForbidden
    return wrapper

Team/Views.py

@method_decorator(required_membership, name="dispatch")
class TeamDetail(APIView):
    def get_object(self, pk):
        try:
            return Team.objects.get(pk=int(pk))
        except Team.DoesNotExist:
            raise Http404

    def get(self, request, pk, format=None):
        team = self.get_object(pk)
        serializer = TeamSerializer(team, many=False)
        return Response(serializer.data)

CodePudding user response：

in you decorator put wrapper inside inner function. your code will look something like this and plus no need to put it inside method decorator simple use like this @required_membership()

from django.core.exceptions import PermissionDenied
def required_membership():
    def inner(view_func):
        def wrapper(request, *args, **kwargs):
            team_id = kwargs['team_id']
            team = Team.objects.get(pk=team_id)
            if team.has_user(request.user.id):
                return view_func(request, *args, **kwargs)
            else:
                raise PermissionDenied
        return wrapper


    return inner

UPDATE

put decorator above get or post method instead of class

CodePudding user response：

The simplest way is probably to just filter in the view:

from django.shortcuts import get_object_or_404

class TeamDetail(APIView):
    
    def get_object(self, pk):
        return get_object_or_404(Team, pk=pk, members=self.request.user)

    def get(self, request, pk, format=None):
        team = self.get_object(pk)
        serializer = TeamSerializer(team, many=False)
        return Response(serializer.data)

This will return a 404 in case the user is not a member of a team, which gives less information to exploit when the user aims to visit a team for which he is not a member.

You can also raise a HttpResponseForbidden instead by checking membership with:

from django.shortcuts import get_object_or_404

class TeamDetail(APIView):
    
    def get_object(self, pk):
        team = get_object_or_404(Team, pk=pk)
        if not team.members.filter(pk=self.request.user.pk).exists():
            raise HttpResponseForbidden
        return team

    def get(self, request, pk, format=None):
        team = self.get_object(pk)
        serializer = TeamSerializer(team, many=False)
        return Response(serializer.data)

Your has_user(…) method has a bug: it should use .filter(…) since a model object has no .exists() method, and .get(…) will raise an error in case it can not find the item:

class Team(models.Model):
    # …

    def has_user(self, user_id):
        return self.members.filter(pk=user_id).exists()