Merge branch '250-implement-access-control-in-job-views' into 'django'

Resolve "implement access control in job views"

Closes #250

See merge request !116

django/allgo/api/v1/mixins.py

+from django.http import HttpResponse
+from django.utils.decorators import method_decorator
+from django.views.decorators.csrf import csrf_exempt
+
+from main.helpers import get_request_user
+from main.models import Job
+
+

django/allgo/api/v1/urls.py

@@ -6,7 +6,6 @@ app_name = 'api'
 
 urlpatterns = [
     url(r'^jobs$', views.jobs, name='jobs'),
-    url(r'^jobs/(\d+)', views.job, name='job'),
-    url(r'^datastore/(\d+)/(.*)/(.*)', views.download, name='download'),
-
+    url(r'^jobs/(?P<pk>\d+)', views.APIJobView.as_view(), name='job'),
+    url(r'^datastore/(?P<pk>\d+)/(.*)/(.*)', views.APIDownloadView, name='download'),
 ]

django/allgo/api/v1/views.py

@@ -3,12 +3,16 @@ import logging
 import os
 
 import config.env
+
 from django.core.validators import ValidationError
 from django.http import HttpResponse, JsonResponse, FileResponse
 from django.shortcuts import redirect
 from django.views.decorators.csrf import csrf_exempt
-from main.models import Job, AllgoUser, Webapp, JobQueue
-from main.helpers import upload_data, get_base_url, lookup_job_file
+from django.views.generic import View
+
+from main.models import Job, Webapp, JobQueue
+from main.helpers import upload_data, get_base_url, lookup_job_file, get_request_user
+from main.mixins import JobAuthMixin
 
 log = logging.getLogger('allgo')
 
@@ -16,30 +20,9 @@ DATASTORE = config.env.ALLGO_DATASTORE
 BUF_SIZE = 65536
 
 
-def get_user(request):
-    auth_header = request.META.get('HTTP_AUTHORIZATION', '')
-    if not auth_header:
-        return None
-    token_type, credentials = auth_header.split(' ')
-    token = credentials.split('=')[-1]
-    log.info('API token checked for token %s', token)
-    return AllgoUser.objects.get(token=token)
-
-
-def sha1file(filepath):
-    sha1 = hashlib.sha1()
-    with open(filepath, 'rb') as f:
-        while True:
-            data = f.read(BUF_SIZE)
-            if not data:
-                break
-            sha1.update(data)
-    return sha1.hexdigest()
-
-
 def get_link(jobid, dir, filename, request):
     filepath = os.path.join(dir, filename)
-    return '/'.join((get_base_url(request), "api/v1/datastore", str(jobid), sha1file(filepath), filename))
+    return '/'.join((get_base_url(request), "datastore", str(jobid), filename))
 
 
 def job_response(job, request):
@@ -58,26 +41,16 @@ def job_response(job, request):
                           }
             }
 
+class APIJobView(JobAuthMixin, View):
 
-@csrf_exempt
-def job(request, jobid):
-    user = get_user(request)
-    if not user:
-        log.info("API request without http authorisation %s %s %s", request.META['HTTP_USER_AGENT'],
-                 request.META['REMOTE_ADDR'], request.META['QUERY_STRING'])
-        return HttpResponse(status=401)
-
-    job = Job.objects.get(id=int(jobid), user=user.user)
-    if not job:
-        log.error("Job %s does not exist", jobid)
-        return HttpResponse(status=404)
-    else:
+    def get(self, request, pk):
+        job = Job.objects.get(id=pk)
         return JsonResponse(job_response(job, request))
 
 
 @csrf_exempt
 def jobs(request):
-    user = get_user(request)
+    user = get_request_user(request)
     if not user:
         log.info("API request without http authorisation %s %s %s", request.META['HTTP_USER_AGENT'],
                  request.META['REMOTE_ADDR'], request.META['QUERY_STRING'])
@@ -120,7 +93,9 @@ def jobs(request):
     return JsonResponse(job_response(job, request))
 
 
+class APIDownloadView(JobAuthMixin, View):
 
-@csrf_exempt
-def download(request, jobid, digest, filename):
-    return redirect("/datastore/%s/%s" % (jobid, filename))
+    def get(self, request, *args, **kwargs):
+        jobid = args[0]
+        filename = args[1]
+        return redirect("/datastore/%s/%s" % (jobid, filename))

django/allgo/main/helpers.py

 import base64
 import hashlib
+import logging
 import os
+import re
 
 import redis
 import IPy
 from django.conf import settings
 
 import config
-from .models import Job, Webapp
+from .models import Job, Webapp, AllgoUser
 
 
+log = logging.getLogger('allgo')
 DEFAULT_ENTROPY = 32 # number of bytes to return by default
 
 
@@ -171,3 +174,30 @@ def get_base_url(request):
         host = request.get_host()
     return "%s://%s" % (scheme, host)
 
+
+def get_request_user(request):
+    """Return the authenticated user from the provided request
+
+    The authentication is attempted:
+    - first with the session cookie
+    - then with the token provided in the HTTP Authorization header
+
+    Args:
+        request
+
+    Returns:
+        a User or None
+    """
+    
+    if request.user.is_authenticated:
+        return request.user
+
+    mo = re.match("Token token=(\S+)",
+            request.META.get('HTTP_AUTHORIZATION', ''))
+    if mo:
+        return getattr(
+                # FIXME: user token should have a unicity constraint
+                AllgoUser.objects.filter(token=mo.group(1)).first(),
+                "user", None)
+
+

django/allgo/main/mixins.py

 from django.conf import settings
+from django.contrib.auth.mixins import UserPassesTestMixin
 from django.core.exceptions import PermissionDenied
+from django.http import HttpResponse
+
+from .models import Job
+from .helpers import get_request_user
 
 
 class IsProviderMixin(object):
@@ -12,3 +17,20 @@ class IsProviderMixin(object):
             return super().dispatch(request, *args, **kwargs)
         else:
             raise PermissionDenied
+
+
+class JobAuthMixin(UserPassesTestMixin):
+    """Check authorization to access a given job"""
+
+    def test_func(self):
+        """Check if user has access to a job
+
+        - redirects to the login page if unauthenticated
+        - allow access if user is the job owner or if user is a superuser
+        """
+        user = get_request_user(self.request)
+        if user is None:
+            return False
+        self.raise_exception = True # to return a 403
+        job = Job.objects.filter(pk=self.kwargs['pk']).first()
+        return user.is_superuser or user == getattr(job, "user", ())

django/allgo/main/views.py

@@ -13,6 +13,7 @@ import glob
 import io
 import json
 import logging
+import re
 import os
 import re
 import shutil
@@ -67,8 +68,8 @@ from .forms import (
 )
 # Local imports
 import config
-from .helpers import get_base_url, get_ssh_data, upload_data, notify_controller, lookup_job_file
-from .mixins import IsProviderMixin
+from .helpers import get_base_url, get_ssh_data, upload_data, notify_controller, lookup_job_file, get_request_user
+from .mixins import IsProviderMixin, JobAuthMixin
 from .models import (
     AllgoUser,
     DockerOs,
@@ -899,7 +900,7 @@ class JobList(LoginRequiredMixin, ListView):
         return queryset
 
 
-class JobDetail(LoginRequiredMixin, DetailView):
+class JobDetail(JobAuthMixin, DetailView):
     """Get a job detail for a specific user
 
     Attributes:
@@ -961,6 +962,7 @@ class JobDetail(LoginRequiredMixin, DetailView):
         else:
             return super().render_to_response(context, **kwargs)
 
+
 class JobCreate(SuccessMessageMixin, CreateView):
     """ Display the data related a specific web and create a job instance
         into the database
@@ -1075,7 +1077,7 @@ class JobCreate(SuccessMessageMixin, CreateView):
         kwargs['webapp'] = queryset
         return kwargs
 
-class JobAbort(LoginRequiredMixin, View):
+class JobAbort(JobAuthMixin, View):
     def post(self, request, *, pk):
         job_id = int(pk)
         # switch state to ABORTING if the job is running (this is done
@@ -1091,7 +1093,7 @@ class JobAbort(LoginRequiredMixin, View):
         
 
 
-class JobDelete(LoginRequiredMixin, DeleteView):
+class JobDelete(JobAuthMixin,  DeleteView):
     """Delete a job from the database
 
     Attributes:
@@ -1151,7 +1153,7 @@ class JobDelete(LoginRequiredMixin, DeleteView):
 
 
 
-class JobFileDownload(LoginRequiredMixin, View):
+class JobFileDownload(JobAuthMixin, View):
     """Download a given file"""
 
     def get(self, request, *args, **kwargs):
@@ -1163,7 +1165,7 @@ class JobFileDownload(LoginRequiredMixin, View):
         return redirect("/datastore/%s/%s" % (job_id, filename))
 
 
-class JobFileDownloadAll(LoginRequiredMixin, SingleObjectMixin, View):
+class JobFileDownloadAll(JobAuthMixin, View):
     """Archive and download all files of a given job
     """
     model = Job
@@ -1349,29 +1351,15 @@ def auth(request):
     log.debug("Auth request for %r", request.META.get('HTTP_X_ORIGINAL_URI'))
 
     # authenticate the user
-    user = None
-    if request.user and request.user.is_authenticated(): # django authentification
-        user = request.user
-    elif request.META.get('HTTP_AUTHORIZATION', ''):  # token authentification
-        _, credentials = request.META.get('HTTP_AUTHORIZATION', '').split(' ')
-        _, token = credentials.split('=')
-        try:
-            user = AllgoUser.objects.get(token=token)
-        except AllgoUser.DoesNotExist:
-            pass
+    user = get_request_user(request)
     if user is None:
         return HttpResponse(status=401)
 
     # find the relevant job
-    params = request.META['HTTP_X_ORIGINAL_URI'].split('/')
-    try:
-        job = Job.objects.get(id=int(params[2]))
-    except ObjectDoesNotExist:
-        # technically this should be a 404, but nginx auth_request only
-        # understands 401 & 403
-        return HttpResponse(status=403)
-
-    if user.id == job.user.id:
-        return HttpResponse(status=200)
-    else:
-        return HttpResponse(status=403)
+    mo = re.match(r'/datastore/(\d+)/', request.META['HTTP_X_ORIGINAL_URI'])
+    if mo:
+        job = Job.objects.filter(id=int(mo.group(1))).first()
+        if job is not None and job.user == user:
+            return HttpResponse(status=200)
+
+    return HttpResponse(status=403)