Merge pull request #234 from rolweber/websocket_auth

Websocket auth with token as query param

Author: parente

docs/source/devinstall.md

@@ -69,8 +69,8 @@ make docs
 After modifying any of the APIs in `jupyter-websocket` mode, you must update the project's Swagger API specification.
 
 1. Load the current
-[swagger.yaml](https://github.com/jupyter/kernel_gateway/blob/master/kernel_gateway/services/api/swagger.yaml) file into the [Swagger editor](http://editor.swagger.io/#/).
+[swagger.yaml](https://github.com/jupyter/kernel_gateway/blob/master/kernel_gateway/jupyter_websocket/swagger.yaml) file into the [Swagger editor](http://editor.swagger.io/#/).
 2. Make your changes.
 3. Export both the `swagger.json` and `swagger.yaml` files.
-4. Place the files in `kernel_gateway/services/api`.
+4. Place the files in `kernel_gateway/jupyter_websocket`.
 5. Add, commit, and PR the changes.

kernel_gateway/jupyter_websocket/swagger.json

@@ -34,11 +34,17 @@
         }
     },
     "securityDefinitions": {
-        "token": {
+        "tokenHeader": {
             "type": "apiKey",
             "name": "Authorization",
             "in": "header",
-            "description": "The authorization token to verify authorization. This is only needed when `KernelGatewayApp.auth_token` is set. This should take the form of `token {value}` where `{value}` is the value of the token."
+            "description": "The authorization token to verify authorization. This is only needed when `KernelGatewayApp.auth_token` is set. This should take the form of `token {value}` where `{value}` is the value of the token. Alternatively, the token can be passed as a query parameter."
+        },
+        "tokenParam": {
+            "type": "apiKey",
+            "name": "token",
+            "in": "query",
+            "description": "The authorization token to verify authorization. This is only needed when `KernelGatewayApp.auth_token` is set. This should take the form of `token={value}` where `{value}` is the value of the token. Alternatively, the token can be passed as a header."
         }
     },
     "paths": {
@@ -91,7 +97,10 @@
             "get": {
                 "security": [
                     {
-                        "token": []
+                        "tokenHeader": []
+                    },
+                    {
+                        "tokenParam": []
                     }
                 ],
                 "summary": "Get kernel specs",

kernel_gateway/jupyter_websocket/swagger.yaml

@@ -29,13 +29,20 @@ parameters:
     format: uuid
 
 securityDefinitions:
-  token:
+  tokenHeader:
     type: apiKey
     name: Authorization
     in: header
     description: The authorization token to verify authorization. This is only
       needed when `KernelGatewayApp.auth_token` is set. This should take the 
-      form of `token {value}` where `{value}` is the value of the token.
+      form of `token {value}` where `{value}` is the value of the token. Alternatively, the token can be passed as a query parameter.
+  tokenParam:
+    type: apiKey
+    name: token
+    in: query
+    description: The authorization token to verify authorization. This is only
+      needed when `KernelGatewayApp.auth_token` is set. This should take the 
+      form of `token={value}` where `{value}` is the value of the token. Alternatively, the token can be passed as a header.
 
 paths:
   /api:
@@ -69,7 +76,8 @@ paths:
   /api/kernelspecs:
     get:
       security: 
-        - token: []
+        - tokenHeader: []
+        - tokenParam: []
       summary: Get kernel specs
       tags:
         - kernelspecs

kernel_gateway/mixins.py

@@ -39,11 +39,15 @@ class TokenAuthorizationMixin(object):
     """Mixes token auth into tornado.web.RequestHandlers and
     tornado.websocket.WebsocketHandlers.
     """
+    header_prefix = "token "
+    header_prefix_len = len(header_prefix)
+
     def prepare(self):
-        """Ensures the correct `Authorization: token <value>` is present in
-        the request's header if an auth token is configured.
+        """Ensures the correct auth token is present, either as a parameter
+        `token=<value>` or as a header `Authorization: token <value>`.
+        Does nothing unless an auth token is configured in kg_auth_token.
 
-        If kg_auth_token is set and the token is not in the header, responds
+        If kg_auth_token is set and the token is not present, responds
         with 401 Unauthorized.
 
         Notes
@@ -54,8 +58,14 @@ def prepare(self):
         """
         server_token = self.settings.get('kg_auth_token')
         if server_token:
-            client_token = self.request.headers.get('Authorization')
-            if client_token != 'token %s' % server_token:
+            client_token = self.get_argument('token', None)
+            if client_token is None:
+                client_token = self.request.headers.get('Authorization')
+                if client_token and client_token.startswith(self.header_prefix):
+                    client_token = client_token[self.header_prefix_len:]
+                else:
+                    client_token = None
+            if client_token != server_token:
                 return self.send_error(401)
         return super(TokenAuthorizationMixin, self).prepare()
 

kernel_gateway/tests/test_mixins.py

@@ -4,10 +4,125 @@
 
 import json
 import unittest
+
+try:
+    from unittest.mock import Mock, MagicMock
+except ImportError:
+    # Python 2.7: use backport
+    from mock import Mock, MagicMock
+
 from tornado import web
-from kernel_gateway.mixins import JSONErrorsMixin
+from kernel_gateway.mixins import TokenAuthorizationMixin, JSONErrorsMixin
+
+class SuperTokenAuthHandler(object):
+    """Super class for the handler using TokenAuthorizationMixin."""
+    is_prepared = False
+
+    def prepare(self):
+        # called by the mixin when authentication succeeds
+        self.is_prepared = True
+
+class TestableTokenAuthHandler(TokenAuthorizationMixin, SuperTokenAuthHandler):
+    """Implementation that uses the TokenAuthorizationMixin for testing."""
+    def __init__(self, token=''):
+        self.settings = { 'kg_auth_token': token }
+        self.arguments = {}
+        self.response = None
+        self.status_code = None
+
+    def send_error(self, status_code):
+        self.status_code = status_code
+
+    def get_argument(self, name, default=''):
+        return self.arguments.get(name, default)
+
+
+class TestTokenAuthMixin(unittest.TestCase):
+    """Unit tests the Token authorization mixin."""
+    def setUp(self):
+        """Creates a handler that uses the mixin."""
+        self.mixin = TestableTokenAuthHandler('YouKnowMe')
+
+    def test_no_token_required(self):
+        """Status should be None."""
+        self.mixin.settings['kg_auth_token'] = ''
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, True)
+        self.assertEqual(self.mixin.status_code, None)
+
+    def test_missing_token(self):
+        """Status should be 'unauthorized'."""
+        attrs = { 'headers' : {
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, False)
+        self.assertEqual(self.mixin.status_code, 401)
+
+    def test_valid_header_token(self):
+        """Status should be None."""
+        attrs = { 'headers' : {
+            'Authorization' : 'token YouKnowMe'
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, True)
+        self.assertEqual(self.mixin.status_code, None)
+
+    def test_wrong_header_token(self):
+        """Status should be 'unauthorized'."""
+        attrs = { 'headers' : {
+            'Authorization' : 'token NeverHeardOf'
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, False)
+        self.assertEqual(self.mixin.status_code, 401)
+
+    def test_valid_url_token(self):
+        """Status should be None."""
+        self.mixin.arguments['token'] = 'YouKnowMe'
+        attrs = { 'headers' : {
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, True)
+        self.assertEqual(self.mixin.status_code, None)
+
+    def test_wrong_url_token(self):
+        """Status should be 'unauthorized'."""
+        self.mixin.arguments['token'] = 'NeverHeardOf'
+        attrs = { 'headers' : {
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, False)
+        self.assertEqual(self.mixin.status_code, 401)
+
+    def test_differing_tokens_valid_url(self):
+        """Status should be None, URL token takes precedence"""
+        self.mixin.arguments['token'] = 'YouKnowMe'
+        attrs = { 'headers' : {
+            'Authorization' : 'token NeverHeardOf'
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, True)
+        self.assertEqual(self.mixin.status_code, None)
+
+    def test_differing_tokens_wrong_url(self):
+        """Status should be 'unauthorized', URL token takes precedence"""
+        attrs = { 'headers' : {
+            'Authorization' : 'token YouKnowMe'
+        } }
+        self.mixin.request = Mock(**attrs)
+        self.mixin.arguments['token'] = 'NeverHeardOf'
+        self.mixin.prepare()
+        self.assertEqual(self.mixin.is_prepared, False)
+        self.assertEqual(self.mixin.status_code, 401)
+
 
-class TestableHandler(JSONErrorsMixin):
+class TestableJSONErrorsHandler(JSONErrorsMixin):
     """Implementation that uses the JSONErrorsMixin for testing."""
     def __init__(self):
         self.headers = {}
@@ -27,7 +142,7 @@ class TestJSONErrorsMixin(unittest.TestCase):
     """Unit tests the JSON errors mixin."""
     def setUp(self):
         """Creates a handler that uses the mixin."""
-        self.mixin = TestableHandler()
+        self.mixin = TestableJSONErrorsHandler()
 
     def test_status(self):
         """Status should be set on the response."""