From 07e9584451da47d2ee16040667f22c90f483d58e Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Tue, 1 Aug 2023 17:07:44 +0500 Subject: [PATCH 1/4] Competition api security solved --- src/apps/api/views/competitions.py | 31 ++++++++++--- src/apps/competitions/views.py | 44 ++++++++++++++++++- src/static/js/ours/client.js | 10 ++++- .../riot/competitions/detail/detail.tag | 2 +- src/templates/competitions/detail.html | 2 +- src/templates/competitions/form.html | 2 +- 6 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/apps/api/views/competitions.py b/src/apps/api/views/competitions.py index 4dc0e4b30..d5cf28875 100644 --- a/src/apps/api/views/competitions.py +++ b/src/apps/api/views/competitions.py @@ -45,15 +45,23 @@ class CompetitionViewSet(ModelViewSet): permission_classes = (AllowAny,) def get_queryset(self): + qs = super().get_queryset() + # Filter for search bar search_query = self.request.query_params.get('search') + + # Competition Secret key check + secret_key = self.request.query_params.get('secret_key') + # If user is logged in if self.request.user.is_authenticated: # filter by competition_type first, 'competition' by default competition_type = self.request.query_params.get('type', Competition.COMPETITION) if competition_type != 'any' and self.detail is False: qs = qs.filter(competition_type=competition_type) + + # `mine` is true when this is called from "Benchmarks I'm Running" # Filter to only see competitions you own mine = self.request.query_params.get('mine', None) if mine: @@ -65,6 +73,7 @@ def get_queryset(self): (Q(collaborators__in=[self.request.user])) ).distinct() + # `participating_in` is true when this is called from "Benchmarks I'm in" participating_in = self.request.query_params.get('participating_in', None) if participating_in: qs = qs.filter(participants__user=self.request.user, participants__status="approved") @@ -73,11 +82,9 @@ def get_queryset(self): user=self.request.user ).values_list('status')[:1] qs = qs.annotate(participant_status=Subquery(participant_status_query)) - # `mine` is true when this is called from "Benchmarks I'm Running" - # `participating_in` is true when this is called from "Benchmarks I'm in" - # `search_query` is true when this is called from the search bar + + # if `search_query` is true, this is called form search bar if search_query: - # User is logged in then filter # competitions which this user owns # or # competitions in which this user is collaborator @@ -91,10 +98,20 @@ def get_queryset(self): (Q(published=True) & ~Q(created_by=self.request.user)) | (Q(participants__user=self.request.user) & Q(participants__status="approved")) ).distinct() + + # if `secret_key` is true, this is called for a secret competition + if secret_key: + qs = qs.filter(Q(secret_key=secret_key)) + else: - # if user is not authenticated only show public competitions in the search - if (search_query): - qs = qs.filter(Q(published=True)) + # if user is not authenticated only show + # public competitions + # or + # competition with valid secret key + qs = qs.filter( + (Q(published=True)) | + (Q(secret_key=secret_key)) + ) # On GETs lets optimize the query to reduce DB calls if self.request.method == 'GET': diff --git a/src/apps/competitions/views.py b/src/apps/competitions/views.py index 61d09e0f3..4651357ce 100644 --- a/src/apps/competitions/views.py +++ b/src/apps/competitions/views.py @@ -13,8 +13,39 @@ class CompetitionPublic(TemplateView): template_name = 'competitions/public.html' -class CompetitionForm(LoginRequiredMixin, TemplateView): +class CompetitionForm(LoginRequiredMixin, DetailView): template_name = 'competitions/form.html' + queryset = Competition.objects.all() + + def get_object(self, *args, **kwargs): + competition = super().get_object(*args, **kwargs) + + is_admin, is_creator, is_collaborator, is_participant = False, False, False, False + + # check if user is loggedin + if self.request.user.is_authenticated: + + # check if user is admin + is_admin = self.request.user.is_superuser + + # check if user is the creator of this competition + is_creator = self.request.user == competition.created_by + + # check if user is collaborator of this competition + is_collaborator = self.request.user in competition.collaborators.all() + + # check if user is a participant of this competition + # get participants from CompetitionParticipant where user=user and competition=competition + is_participant = CompetitionParticipant.objects.filter(user=self.request.user, competition=competition).count() > 0 + + if ( + is_admin or + is_creator or + is_collaborator or + is_participant + ): + return competition + raise Http404() class CompetitionUpload(LoginRequiredMixin, TemplateView): @@ -60,6 +91,17 @@ def get_object(self, *args, **kwargs): return competition raise Http404() + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + + # Retrieve the secret_key from the request.GET dictionary + secret_key = self.request.GET.get('secret_key') + + # Add the secret_key to the context dictionary + context['secret_key'] = secret_key + + return context + class CompetitionDetailedResults(LoginRequiredMixin, TemplateView): template_name = 'competitions/detailed_results.html' diff --git a/src/static/js/ours/client.js b/src/static/js/ours/client.js index f3e1a21dc..80f7cf3a0 100644 --- a/src/static/js/ours/client.js +++ b/src/static/js/ours/client.js @@ -22,8 +22,14 @@ CODALAB.api = { /*--------------------------------------------------------------------- Competitions ---------------------------------------------------------------------*/ - get_competition: function (pk) { - return CODALAB.api.request('GET', URLS.API + "competitions/" + pk + "/") + get_competition: function (pk, secret_key) { + + if(secret_key == undefined || secret_key == 'None'){ + return CODALAB.api.request('GET', URLS.API + "competitions/" + pk + "/") + }else{ + return CODALAB.api.request('GET', URLS.API + "competitions/" + pk + "/?secret_key="+secret_key) + } + }, get_competitions: function (query) { return CODALAB.api.request('GET', URLS.API + "competitions/", query) diff --git a/src/static/riot/competitions/detail/detail.tag b/src/static/riot/competitions/detail/detail.tag index 9c2c074b4..06da45698 100644 --- a/src/static/riot/competitions/detail/detail.tag +++ b/src/static/riot/competitions/detail/detail.tag @@ -17,7 +17,7 @@ }) self.update_competition_data = function () { - CODALAB.api.get_competition(self.opts.competition_pk) + CODALAB.api.get_competition(self.opts.competition_pk, self.opts.secret_key) .done(function (data) { self.competition = data CODALAB.events.trigger('competition_loaded', self.competition) diff --git a/src/templates/competitions/detail.html b/src/templates/competitions/detail.html index 143dbee2e..e89853323 100644 --- a/src/templates/competitions/detail.html +++ b/src/templates/competitions/detail.html @@ -1,4 +1,4 @@ {% extends "base.html" %} {% block content %} - + {% endblock %} \ No newline at end of file diff --git a/src/templates/competitions/form.html b/src/templates/competitions/form.html index 6f2196ce1..a3b9d9ccb 100644 --- a/src/templates/competitions/form.html +++ b/src/templates/competitions/form.html @@ -1,7 +1,7 @@ {% extends "base.html" %} {% block content %} - + {# NOTE: First added for phase task sorting, SemanticUI does not come with sorting..! #} From 0c3175c54494d1603cc70bde97e9cff7bcf4d8f0 Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Tue, 1 Aug 2023 17:09:34 +0500 Subject: [PATCH 2/4] 404 page on edit competition which does not belong to you --- src/apps/competitions/views.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/apps/competitions/views.py b/src/apps/competitions/views.py index 4651357ce..984305d33 100644 --- a/src/apps/competitions/views.py +++ b/src/apps/competitions/views.py @@ -20,7 +20,7 @@ class CompetitionForm(LoginRequiredMixin, DetailView): def get_object(self, *args, **kwargs): competition = super().get_object(*args, **kwargs) - is_admin, is_creator, is_collaborator, is_participant = False, False, False, False + is_admin, is_creator, is_collaborator = False, False, False # check if user is loggedin if self.request.user.is_authenticated: @@ -34,15 +34,10 @@ def get_object(self, *args, **kwargs): # check if user is collaborator of this competition is_collaborator = self.request.user in competition.collaborators.all() - # check if user is a participant of this competition - # get participants from CompetitionParticipant where user=user and competition=competition - is_participant = CompetitionParticipant.objects.filter(user=self.request.user, competition=competition).count() > 0 - if ( is_admin or is_creator or - is_collaborator or - is_participant + is_collaborator ): return competition raise Http404() From 734c02e6fb6332bd198bf47ec87d1d04f8e9169c Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Tue, 1 Aug 2023 19:13:50 +0500 Subject: [PATCH 3/4] competition create and update view changed --- src/apps/competitions/urls.py | 4 ++-- src/apps/competitions/views.py | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/apps/competitions/urls.py b/src/apps/competitions/urls.py index a3d4419a9..705c1d77c 100644 --- a/src/apps/competitions/urls.py +++ b/src/apps/competitions/urls.py @@ -8,8 +8,8 @@ # path('', views.CompetitionList.as_view(), name="list"), path('', views.CompetitionManagement.as_view(), name="management"), path('/', views.CompetitionDetail.as_view(), name="detail"), - path('create/', views.CompetitionForm.as_view(), name="create"), - path('edit//', views.CompetitionForm.as_view(), name="edit"), + path('create/', views.CompetitionCreateForm.as_view(), name="create"), + path('edit//', views.CompetitionUpdateForm.as_view(), name="edit"), path('upload/', views.CompetitionUpload.as_view(), name="upload"), path('public/', views.CompetitionPublic.as_view(), name="public"), path('/detailed_results//', views.CompetitionDetailedResults.as_view(), name="detailed_results"), diff --git a/src/apps/competitions/views.py b/src/apps/competitions/views.py index 984305d33..6d2aa3c3e 100644 --- a/src/apps/competitions/views.py +++ b/src/apps/competitions/views.py @@ -13,7 +13,11 @@ class CompetitionPublic(TemplateView): template_name = 'competitions/public.html' -class CompetitionForm(LoginRequiredMixin, DetailView): +class CompetitionCreateForm(LoginRequiredMixin, TemplateView): + template_name = 'competitions/form.html' + + +class CompetitionUpdateForm(LoginRequiredMixin, DetailView): template_name = 'competitions/form.html' queryset = Competition.objects.all() From e9e725074f8110d9860aed4b8175319dbd709204 Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Wed, 9 Aug 2023 18:46:19 +0500 Subject: [PATCH 4/4] default condition added to filter out private competitions from other users in api --- src/apps/api/views/competitions.py | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/apps/api/views/competitions.py b/src/apps/api/views/competitions.py index d5cf28875..a3df272f5 100644 --- a/src/apps/api/views/competitions.py +++ b/src/apps/api/views/competitions.py @@ -48,6 +48,11 @@ def get_queryset(self): qs = super().get_queryset() + # filter by competition_type first, 'competition' by default + competition_type = self.request.query_params.get('type', Competition.COMPETITION) + if competition_type != 'any' and self.detail is False: + qs = qs.filter(competition_type=competition_type) + # Filter for search bar search_query = self.request.query_params.get('search') @@ -56,10 +61,6 @@ def get_queryset(self): # If user is logged in if self.request.user.is_authenticated: - # filter by competition_type first, 'competition' by default - competition_type = self.request.query_params.get('type', Competition.COMPETITION) - if competition_type != 'any' and self.detail is False: - qs = qs.filter(competition_type=competition_type) # `mine` is true when this is called from "Benchmarks I'm Running" # Filter to only see competitions you own @@ -77,6 +78,7 @@ def get_queryset(self): participating_in = self.request.query_params.get('participating_in', None) if participating_in: qs = qs.filter(participants__user=self.request.user, participants__status="approved") + participant_status_query = CompetitionParticipant.objects.filter( competition=OuterRef('pk'), user=self.request.user @@ -101,8 +103,28 @@ def get_queryset(self): # if `secret_key` is true, this is called for a secret competition if secret_key: + print(secret_key) qs = qs.filter(Q(secret_key=secret_key)) + # Default condition + # not called from my competitions tab + # not called from i'm participating in tab + # not called from search bar + # not called with a valid secret key + # Return the following --- + # All competitions which belongs to you (private or public) + # And competitions where you are admin + # And public competitions + # And competitions where you are approved participant + # this filters out all private compettions from other users + if (not mine) and (not participating_in) and (not secret_key) and (not search_query): + qs = qs.filter( + (Q(created_by=self.request.user)) | + (Q(collaborators__in=[self.request.user])) | + (Q(published=True) & ~Q(created_by=self.request.user)) | + (Q(participants__user=self.request.user) & Q(participants__status="approved")) + ).distinct() + else: # if user is not authenticated only show # public competitions