Merge branch '306-apidownloadview' into 'django'

Resolve "APIDownloadView doesn't seem to work"

Closes #119, #311 et #306

See merge request !178

django/allgo/api/v1/urls.py

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

django/allgo/api/v1/views.py

@@ -96,6 +96,5 @@ def jobs(request):
 class APIDownloadView(JobAuthMixin, View):
 
     def get(self, request, *args, **kwargs):
-        jobid = args[0]
-        filename = args[1]
-        return redirect("/datastore/%s/%s" % (jobid, filename))
+        log.error("datastore requests must be served by nginx (bad config!)")
+        return JsonResponse({'error': 'Internal Server Error'}, status=500)

django/allgo/main/helpers.py

@@ -179,9 +179,12 @@ def get_base_url(request):
 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
+    Depending on the request path, the authentication is attempted on:
+    - the token provided in the HTTP Authorization header for /api/ urls
+    - the session cookie for other urls
+
+    In case of /auth requests we assume that 'X-Original-URI' is the path of
+    the current request.
 
     Args:
         request
@@ -189,17 +192,25 @@ def get_request_user(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)
+    path = request.path
+    if path == "/auth":
+        path = request.META['HTTP_X_ORIGINAL_URI']
+    if path.startswith("/api/"):
+        # authenticated by token for API requests
+        #
+        # NOTE: we must NOT authenticate by cookie because the CORS
+        #       configuration in the nginx.conf allows all origins
+        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)
+    else:
+        # authenticated by cookie for other requests
+        if request.user.is_authenticated:
+            return request.user
 
 
 def query_webapps_for_user(user):

django/allgo/main/views.py

@@ -1506,7 +1506,7 @@ def auth(request):
         return HttpResponse(status=401)
 
     # find the relevant job
-    mo = re.match(r'/datastore/(\d+)/', request.META['HTTP_X_ORIGINAL_URI'])
+    mo = re.match(r'(?:/api/v1)?/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:

django/setup/files/etc/nginx/conf.d/allgo.conf.template

@@ -15,89 +15,82 @@ server
   client_body_in_file_only clean;
   client_body_buffer_size 32K;
 
-# Disabled until #227 is implemented
-#
-#  # registry endpoints
-#  # - forwarded to the registry
-#  # - except manifest push/pull -> forwarded through the django server (to
-#  #   guarantee that the db is transactionally updated)
-#  location /v2/
-#  {
-#	proxy_pass	{ALLGO_REGISTRY_PRIVATE_URL}/v2/;
-#	proxy_redirect  off;
-#	proxy_buffering	off;
-#
-#	location ~ ^/v2/.*/manifests/[^/]*$ {
-#		proxy_pass	http://aio;
-#	}
-#  }
+  # ----
+  # location are presented in their application/priority order
 
-  # allgo async endpoints
-  location /aio/
-  {
-	proxy_pass	http://aio/aio/;
-	proxy_redirect  off;
-	proxy_buffering	off;
-  }
+  location /api/
+  { # The CORS config allows any origin. These endpoints MUST NOT use
+    # authentication by cookie.
+    if ($request_method = 'OPTIONS')
+    {
+      add_header 'Access-Control-Allow-Origin' '*';
+      add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
 
+      add_header 'Access-Control-Max-Age' 1728000;
+      add_header 'Content-Type' 'text/plain; charset=utf-8';
+      add_header 'Content-Length' 0;
 
-  location ~ /datastore/([0-9]+)/(.*)$ {
-        autoindex on;
-        auth_request     /auth;
-        auth_request_set $auth_status $upstream_status;
-        root /vol/rw/;
-
-	
-	# This is a security measure (DO NOT REMOVE)
-	#
-	# By default nginx follows symbolic links, which would be a major
-	# vulnerability because jobs could create symbolic links to any file
-	# inside django container (like the secret key for signing tokens)
-	#
-	disable_symlinks on;
-    }
+      # Custom headers and headers various browsers *should* be OK with but aren't
+      add_header 'Access-Control-Allow-Headers' 'Content-Type,Authorization';
 
-    location = /auth {
-        internal;
-        proxy_pass              http://django/auth;
-        proxy_redirect off;
-        proxy_set_header        X-Original-URI $request_uri;
+      return 204;
     }
 
+    location /api/v1/
+    { # it's not illegal access, go through django
+      add_header Access-Control-Allow-Origin "*";
 
-  # allgo endpoints
-  # - static files served directly by nginx
-  # - other requests forwarded to the django server
-  location /
-  {
-	sendfile on;
-	send_timeout 300s;
+      proxy_pass http://django;
+      proxy_redirect off; # work without it, maybe it's bad to remove it
 
-	keepalive_timeout 5;
-	root /var/www/html;
-	try_files $uri/index.html $uri.html $uri @django;
-  }
+      # header set to distinguish between requests going directly from nginx and
+      # requests going through aio
+      #
+      # This is a security feature. Django trusts this value (like the
+      # X-Forwarded-* headers), do not remove it !
+      proxy_set_header	X-Origin	"nginx";
+    }
 
-  location /api/v1
-  {
-     if ($request_method = 'OPTIONS') {
-        add_header 'Access-Control-Allow-Origin' '*';
-        add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
+    location /api/v1/datastore/
+    { # it's not illegal access, access to static file
+      autoindex on;
+      auth_request     /auth;
+      auth_request_set $auth_status $upstream_status;
+      alias /vol/rw/datastore/;
+
+
+      # This is a security measure (DO NOT REMOVE)
+      #
+      # By default nginx follows symbolic links, which would be a major
+      # vulnerability because jobs could create symbolic links to any file
+      # inside django container (like the secret key for signing tokens)
+      #
+      disable_symlinks on;
+    }
+  } #location /api/
 
-        add_header 'Access-Control-Max-Age' 1728000;
-        add_header 'Content-Type' 'text/plain; charset=utf-8';
-        add_header 'Content-Length' 0;
 
-        # Custom headers and headers various browsers *should* be OK with but aren't
-        #
-        add_header 'Access-Control-Allow-Headers' 'Content-Type,Authorization';
+  location = /auth
+  { # call the auth view in django
+    # = grant that only known user can go through
+    internal;
+    proxy_pass              http://django/auth;
+    proxy_redirect off;
+    proxy_set_header        X-Original-URI $request_uri;
+  }
 
-        return 204;
-    }
 
-    add_header Access-Control-Allow-Origin "*";
+  location /aio/
+  { # allgo async endpoints
+	proxy_pass	http://aio/aio/;
+	proxy_redirect  off;
+	proxy_buffering	off;
+  }
+
 
-    # proxy_redirect off; # work without it, maybe it's bad to remove it
+  location @django
+  { # simple access to the web site
+    proxy_redirect off;
     proxy_pass http://django;
 
     # header set to distinguish between requests going directly from nginx and
@@ -108,18 +101,50 @@ server
     proxy_set_header	X-Origin	"nginx";
   }
 
-  location @django
-  {
-    proxy_redirect off;
-    proxy_pass http://django;
 
-    # header set to distinguish between requests going directly from nginx and
-    # requests going through aio
+# Disabled until #227 is implemented
+#
+#  # registry endpoints
+#  # - forwarded to the registry
+#  # - except manifest push/pull -> forwarded through the django server (to
+#  #   guarantee that the db is transactionally updated)
+#  location /v2/
+#  {
+#	proxy_pass	{ALLGO_REGISTRY_PRIVATE_URL}/v2/;
+#	proxy_redirect  off;
+#	proxy_buffering	off;
+#
+#	location ~ ^/v2/.*/manifests/[^/]*$ {
+#		proxy_pass	http://aio;
+#	}
+#  }
+
+
+  location /datastore/
+  { # access to static files
+    autoindex on;
+    auth_request     /auth;
+    auth_request_set $auth_status $upstream_status;
+    alias /vol/rw/datastore/;
+
+    # This is a security measure (DO NOT REMOVE)
     #
-    # This is a security feature. Django trusts this value (like the
-    # X-Forwarded-* headers), do not remove it !
-    proxy_set_header	X-Origin	"nginx";
+    # By default nginx follows symbolic links, which would be a major
+    # vulnerability because jobs could create symbolic links to any file
+    # inside django container (like the secret key for signing tokens)
+    #
+    disable_symlinks on;
   }
 
+  location /
+  { # allgo endpoints
+    # - static files served directly by nginx
+    # - other requests forwarded to the django server
+	sendfile on;
+	send_timeout 300s;
 
-}
+	keepalive_timeout 5;
+	root /var/www/html;
+	try_files $uri/index.html $uri.html $uri @django;
+  }
+} #server