Merge lp:~frankban/charms/precise/juju-gui/server-auth into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

GUI server authentication.

Added authentication management to the Tornado server.
The WebSocket handler includes a way to know if the
connected client is logged in to the API server.
Both Go and Python API implementations are supported.

Also implemented WebSocket messages validation.

I am sorry: this diff is quite long.
In my defence, the code is full of tests and
documentation... Well, sorry again.

Tests: `make unittest`

https://codereview.appspot.com/11725044/

[Francesco Banconi (frankban)]

Reviewers: mp+176738_code.launchpad.net,

Message:
Please take a look.

Description:
GUI server authentication.

Added authentication management to the Tornado server.
The WebSocket handler includes a way to know if the
connected client is logged in to the API server.
Both Go and Python API implementations are supported.

Also implemented WebSocket messages validation.

I am sorry: this diff is quite long.
In my defence, the code is full of tests and
documentation... Well, sorry again.

Tests: `make unittest`

https://code.launchpad.net/~frankban/charms/precise/juju-gui/server-auth/+merge/176738

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/11725044/

Affected files:
   A [revision details]
   M revision
   M server/guiserver/apps.py
   A server/guiserver/auth.py
   M server/guiserver/handlers.py
   M server/guiserver/manage.py
   M server/guiserver/tests/helpers.py
   A server/guiserver/tests/test_auth.py
   M server/guiserver/tests/test_handlers.py
   M server/guiserver/tests/test_manage.py
   M server/guiserver/tests/test_utils.py
   M server/guiserver/utils.py
   M server/runserver.py

[Benjamin Saller (bcsaller)]

My prelim look over the code was nice. make unittests fails with various
modules apparently missing. What is missing seems to vary from run to
run. I've missed shelltoolbox (from charmtools helpers import) but on
other runs apt, yaml. sometimes they load in.

Have you seen this? I didn't spend too long investigating but when I
hear back from you I can try it again.

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_handlers.py
File server/guiserver/tests/test_handlers.py (left):

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_handlers.py#oldcode82
server/guiserver/tests/test_handlers.py:82:
Abstract.

https://codereview.appspot.com/11725044/

[Francesco Banconi (frankban)]

On 2013/07/24 21:33:11, benjamin.saller wrote:
> My prelim look over the code was nice. make unittests fails with
various modules
> apparently missing. What is missing seems to vary from run to run.
I've missed
> shelltoolbox (from charmtools helpers import) but on other runs apt,
yaml.
> sometimes they load in.

> Have you seen this? I didn't spend too long investigating but when I
hear back
> from you I can try it again.

No I don't. It seems the virtualenv for tests is not being created
correctly.
Could you please try the following, from the branch root?
make clean # delete the venv
make # network errors?
make unittest

If the problem is still there, could you please paste the output of
`./tests/.venv/bin/pip freeze`? Thanks.

https://codereview.appspot.com/11725044/

[lp:~frankban/charms/precise/juju-gui/server-auth]

86. By Francesco Banconi on 2013-07-25

Changes as per review.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_manage.py
File server/guiserver/tests/test_manage.py (right):

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_manage.py#newcode116
server/guiserver/tests/test_manage.py:116: with
mock.patch('guiserver.manage.options', {'arg1': None}):
On 2013/07/25 13:20:59, benji wrote:
> I think this is the only test method that you didn't include a comment
for. :)

Done.

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_utils.py
File server/guiserver/tests/test_utils.py (right):

https://codereview.appspot.com/11725044/diff/1/server/guiserver/tests/test_utils.py#newcode60
server/guiserver/tests/test_utils.py:60: # if the resulting object is
not a dict-like object, a warning is
On 2013/07/25 13:20:59, benji wrote:
> There is an extra space at the beginning of the comment and "if"
should be
> capitalized.

Done.

https://codereview.appspot.com/11725044/

[Nicola Larosa (teknico)]

LGTM, impressive work indeed. One trivial below. Also, consider only
saying "log in" and "logged in" when used as a verb, but "login" when a
noun, i.e. "login request".

https://codereview.appspot.com/11725044/diff/9001/server/guiserver/manage.py
File server/guiserver/manage.py (right):

https://codereview.appspot.com/11725044/diff/9001/server/guiserver/manage.py#newcode86
server/guiserver/manage.py:86: 'of the bootstrap/state node as returned
by juju status.')
Double quotes around "juju status", maybe?

https://codereview.appspot.com/11725044/

[lp:~frankban/charms/precise/juju-gui/server-auth]

87. By Francesco Banconi on 2013-07-25

Changes as per review.

[Francesco Banconi (frankban)]

Thank you both for the reviews!

> LGTM, impressive work indeed. One trivial below. Also, consider only
saying "log
> in" and "logged in" when used as a verb, but "login" when a noun, i.e.
"login
> request".

Done.

https://codereview.appspot.com/11725044/diff/9001/server/guiserver/manage.py#newcode86
> server/guiserver/manage.py:86: 'of the bootstrap/state node as
returned by juju
> status.')
> Double quotes around "juju status", maybe?

Done.

https://codereview.appspot.com/11725044/

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/11725044/

[Francesco Banconi (frankban)]

*** Submitted:

GUI server authentication.

Added authentication management to the Tornado server.
The WebSocket handler includes a way to know if the
connected client is logged in to the API server.
Both Go and Python API implementations are supported.

Also implemented WebSocket messages validation.

I am sorry: this diff is quite long.
In my defence, the code is full of tests and
documentation... Well, sorry again.

Tests: `make unittest`

R=benjamin.saller, benji, teknico
CC=
https://codereview.appspot.com/11725044

https://codereview.appspot.com/11725044/

=== modified file 'revision'
--- revision	2013-07-23 08:21:47 +0000
+++ revision	2013-07-25 15:44:30 +0000
@@ -1,1 +1,1 @@
-60
+61

=== modified file 'server/guiserver/apps.py'
--- server/guiserver/apps.py	2013-07-19 10:25:55 +0000
+++ server/guiserver/apps.py	2013-07-25 15:44:30 +0000
@@ -21,7 +21,10 @@
 from tornado import web
 from tornado.options import options
 
-from guiserver import handlers
+from guiserver import (
+    auth,
+    handlers,
+)
 
 
 def server():
@@ -30,17 +33,20 @@
     The server app is responsible for serving the WebSocket connection, the
     Juju GUI static files and the main index file for dynamic URLs.
     """
+    # Set up static paths.
     guiroot = options.guiroot
     static_path = os.path.join(guiroot, 'juju-ui')
+    # Set up the authentication backend.
+    auth_backend = auth.get_backend(options.apiversion)
     return web.Application([
         # Handle WebSocket connections.
-        (r'^/ws$', handlers.WebSocketHandler, {'jujuapi': options.jujuapi}),
+        (r'^/ws$', handlers.WebSocketHandler, {'apiurl': options.apiurl}),
         # Handle static files.
         (r'^/juju-ui/(.*)', web.StaticFileHandler, {'path': static_path}),
         (r'^/(favicon\.ico)$', web.StaticFileHandler, {'path': guiroot}),
         # Any other path is served by index.html.
         (r'^/(.*)', handlers.IndexHandler, {'path': guiroot}),
-    ], debug=options.debug)
+    ], debug=options.debug, auth_backend=auth_backend)
 
 
 def redirector():

=== added file 'server/guiserver/auth.py'
--- server/guiserver/auth.py	1970-01-01 00:00:00 +0000
+++ server/guiserver/auth.py	2013-07-25 15:44:30 +0000
@@ -0,0 +1,215 @@
+# This file is part of the Juju GUI, which lets users view and manage Juju
+# environments within a graphical interface (https://launchpad.net/juju-gui).
+# Copyright (C) 2013 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Juju GUI server authentication management.
+
+This module includes the pieces required to process user authentication:
+
+    - User: a simple data structure representing a logged in or anonymous user;
+    - authentication backends (GoBackend and PythonBackend): any object
+      implementing the following interface:
+        - get_request_id(data);
+        - request_is_login(data);
+        - get_credentials(data);
+        - login_succeeded(data).
+      The only purpose of auth backends is to provide the logic to parse
+      requests' data based on the API implementation currently in use. Backends
+      don't know anything about the authentication process or the current user,
+      and are not intended to store state: one backend (the one suitable for
+      the current API implementation) is instantiated once when the application
+      is bootstrapped and used as a singleton by all WebSocket requests;
+    - AuthMiddleware: process authentication requests and responses, using
+      the backend to parse the WebSocket messages, logging in the current user
+      if the authentication succeeds.
+"""
+
+
+class User(object):
+    """The current WebSocket user."""
+
+    def __init__(self, username='', password='', is_authenticated=False):
+        self.is_authenticated = is_authenticated
+        # XXX (frankban) YAGNI: the username/password attributes are not
+        # required for now, but they will help handling the HA story, i.e. in
+        # the process of re-authenticating to the API after switching from one
+        # Juju state/API server to another.
+        self.username = username
+        self.password = password
+
+    def __repr__(self):
+        if self.is_authenticated:
+            status = 'authenticated'
+        else:
+            status = 'not authenticated'
+        username = self.username or 'anonymous'
+        return '<User: {} ({})>'.format(username, status)
+
+
+class AuthMiddleware(object):
+    """Handle user authentication.
+
+    This class handles the process of authenticating the provided user using
+    the given auth backend. Note that, since the GUI just disconnects when the
+    user logs out, there is no need to handle the log out process.
+    """
+
+    def __init__(self, user, backend):
+        self._user = user
+        self._backend = backend
+        self._request_id = None
+
+    def in_progress(self):
+        """Return True if the authentication is in progress, False otherwise.
+        """
+        return self._request_id is not None
+
+    def process_request(self, data):
+        """Parse the WebSocket data arriving from the client.
+
+        Start the authentication process if data represents a login request
+        performed by the GUI user.
+        """
+        backend = self._backend
+        request_id = backend.get_request_id(data)
+        if request_id and backend.request_is_login(data):
+            self._request_id = request_id
+            credentials = backend.get_credentials(data)
+            self._user.username, self._user.password = credentials
+
+    def process_response(self, data):
+        """Parse the WebSocket data arriving from the Juju API server.
+
+        Complete the authentication process if data represents the response
+        to a login request previously initiated. Authenticate the user if the
+        authentication succeeded.
+        """
+        request_id = self._backend.get_request_id(data)
+        if request_id == self._request_id:
+            logged_in = self._backend.login_succeeded(data)
+            if logged_in:
+                self._user.is_authenticated = True
+            else:
+                self._user.username = self._user.password = ''
+            self._request_id = None
+
+
+class GoBackend(object):
+    """Authentication backend for the Juju Go API implementation.
+
+    A login request looks like the following:
+
+        {
+            'RequestId': 42,
+            'Type': 'Admin',
+            'Request': 'Login',
+            'Params': {'AuthTag': 'user-admin', 'Password': 'ADMIN-SECRET'},
+        }
+
+    Here is an example of a successful login response:
+
+        {'RequestId': 42, 'Response': {}}
+
+    A login failure response is like the following:
+
+        {
+            'RequestId': 42,
+            'Error': 'invalid entity name or password',
+            'ErrorCode': 'unauthorized access',
+            'Response': {},
+        }
+    """
+
+    def get_request_id(self, data):
+        """Return the request identifier associated with the provided data."""
+        return data.get('RequestId')
+
+    def request_is_login(self, data):
+        """Return True if data represents a login request, False otherwise."""
+        params = data.get('Params', {})
+        return (
+            data.get('Type') == 'Admin' and
+            data.get('Request') == 'Login' and
+            'AuthTag' in params and
+            'Password' in params
+        )
+
+    def get_credentials(self, data):
+        """Parse the provided login data and return username and password."""
+        params = data['Params']
+        return params['AuthTag'], params['Password']
+
+    def login_succeeded(self, data):
+        """Return True if data represents a successful login, False otherwise.
+        """
+        return 'Error' not in data
+
+
+class PythonBackend(object):
+    """Authentication backend for the Juju Python implementation.
+
+    A login request looks like the following:
+
+        {
+            'request_id': 42,
+            'op': 'login',
+            'user': 'admin',
+            'password': 'ADMIN-SECRET',
+        }
+
+    A successful login response includes these fields:
+
+        {
+            'request_id': 42,
+            'op': 'login',
+            'user': 'admin',
+            'password': 'ADMIN-SECRET',
+            'result': True,
+        }
+
+    A login failure response is like the following:
+
+        {
+            'request_id': 42,
+            'op': 'login',
+            'user': 'admin',
+            'password': 'ADMIN-SECRET',
+            'err': True,
+        }
+    """
+
+    def get_request_id(self, data):
+        """Return the request identifier associated with the provided data."""
+        return data.get('request_id')
+
+    def request_is_login(self, data):
+        """Return True if data represents a login request, False otherwise."""
+        op = data.get('op')
+        return (op == 'login') and ('user' in data) and ('password' in data)
+
+    def get_credentials(self, data):
+        """Parse the provided login data and return username and password."""
+        return data['user'], data['password']
+
+    def login_succeeded(self, data):
+        """Return True if data represents a successful login, False otherwise.
+        """
+        return data.get('result') and not data.get('err')
+
+
+def get_backend(apiversion):
+    """Return the auth backend instance to use for the given API version."""
+    backend_class = {'go': GoBackend, 'python': PythonBackend}[apiversion]
+    return backend_class()

=== modified file 'server/guiserver/handlers.py'
--- server/guiserver/handlers.py	2013-07-23 08:21:12 +0000
+++ server/guiserver/handlers.py	2013-07-25 15:44:30 +0000
@@ -27,9 +27,14 @@
 )
 from tornado.ioloop import IOLoop
 
+from guiserver.auth import (
+    AuthMiddleware,
+    User,
+)
 from guiserver.clients import websocket_connect
 from guiserver.utils import (
     get_headers,
+    json_decode_dict,
     request_summary,
 )
 
@@ -56,11 +61,10 @@
     Methods:
       - write_message(message): send a message to the browser;
       - close(): terminate the browser connection.
-
     """
 
     @gen.coroutine
-    def initialize(self, jujuapi, io_loop=None):
+    def initialize(self, apiurl, io_loop=None):
         """Create a new WebSocket client and connect it to the Juju API."""
         if io_loop is None:
             io_loop = IOLoop.current()
@@ -70,13 +74,17 @@
         self.connected = True
         self.juju_connected = False
         self._juju_message_queue = queue = deque()
+        # Set up the authentication infrastructure.
+        self.user = User()
+        auth_backend = self.application.settings['auth_backend']
+        self.auth = AuthMiddleware(self.user, auth_backend)
         # Juju requires the Origin header to be included in the WebSocket
         # client handshake request. Propagate the client origin if present;
         # use the Juju API server as origin otherwise.
-        headers = get_headers(self.request, jujuapi)
+        headers = get_headers(self.request, apiurl)
         # Connect the WebSocket client to the Juju API server.
         self._juju_connected_future = websocket_connect(
-            io_loop, jujuapi, self.on_juju_message, headers=headers)
+            io_loop, apiurl, self.on_juju_message, headers=headers)
         try:
             self.juju_connection = yield self._juju_connected_future
         except Exception as err:
@@ -101,6 +109,9 @@
         Messages sent before the client connection to the Juju API server is
         established are queued for later delivery.
         """
+        data = json_decode_dict(message)
+        if (data is not None) and (not self.user.is_authenticated):
+            self.auth.process_request(data)
         if self.juju_connected:
             logging.debug(self._summary + 'client -> juju: {}'.format(message))
             return self.juju_connection.write_message(message)
@@ -115,6 +126,9 @@
         if message is None:
             # The Juju API closed the connection.
             return self.on_juju_close()
+        data = json_decode_dict(message)
+        if (data is not None) and self.auth.in_progress():
+            self.auth.process_response(data)
         logging.debug(self._summary + 'juju -> client: {}'.format(message))
         self.write_message(message)
 

=== modified file 'server/guiserver/manage.py'
--- server/guiserver/manage.py	2013-07-17 15:21:26 +0000
+++ server/guiserver/manage.py	2013-07-25 15:44:30 +0000
@@ -33,6 +33,7 @@
 )
 
 
+DEFAULT_API_VERSION = 'go'
 SSL_OPTIONS = {
     'certfile': '/etc/ssl/juju-gui/juju.crt',
     'keyfile': '/etc/ssl/juju-gui/juju.key',
@@ -62,13 +63,36 @@
             sys.exit('error: the {} argument is required'.format(name))
 
 
+def _validate_choices(option_name, choices):
+    """Ensure the value passed for the given option is included in the choices.
+
+    Exit with an error if the value is not in the accepted ones.
+    """
+    value = options[option_name]
+    if value not in choices:
+        sys.exit('error: accepted values for the {} argument are: {}'.format(
+            option_name, ', '.join(choices)))
+
+
 def setup():
     """Set up options and logger."""
-    define('guiroot', type=str, help='the Juju GUI static files path')
-    define('jujuapi', type=str, help='the Juju WebSocket server address')
+    define(
+        'guiroot', type=str,
+        help='The Juju GUI static files path, e.g.: '
+             '/var/lib/juju/agents/unit-juju-gui-0/charm/juju-gui/build-prod')
+    define(
+        'apiurl', type=str,
+        help='The Juju WebSocket server address. This is usually the address '
+             'of the bootstrap/state node as returned by "juju status".')
+    # Optional parameters.
+    define(
+        'apiversion', type=str, default=DEFAULT_API_VERSION,
+        help='the Juju API version/implementation. Currently the possible '
+             'values are "go" (default) or "python".')
     # In Tornado, parsing the options also sets up the default logger.
     parse_command_line()
-    _validate_required('guiroot', 'jujuapi')
+    _validate_required('guiroot', 'apiurl')
+    _validate_choices('apiversion', ('go', 'python'))
     _add_debug(logging.getLogger())
 
 

=== modified file 'server/guiserver/tests/helpers.py'
--- server/guiserver/tests/helpers.py	2013-07-23 08:21:12 +0000
+++ server/guiserver/tests/helpers.py	2013-07-25 15:44:30 +0000
@@ -16,8 +16,12 @@
 
 """Juju GUI server test utilities."""
 
+import json
+
 from tornado import websocket
 
+from guiserver import auth
+
 
 class EchoWebSocketHandler(websocket.WebSocketHandler):
     """A WebSocket server echoing back messages."""
@@ -50,6 +54,80 @@
             self._closed_future.set_result(None)
 
 
+class GoAPITestMixin(object):
+    """Add helper methods for testing the Go API implementation."""
+
+    def get_auth_backend(self):
+        """Return an authentication backend suitable for the Go API."""
+        return auth.get_backend('go')
+
+    def make_login_request(
+            self, request_id=42, username='user', password='passwd',
+            encoded=False):
+        """Create and return a login request message.
+
+        If encoded is set to True, the returned message will be JSON encoded.
+        """
+        data = {
+            'RequestId': request_id,
+            'Type': 'Admin',
+            'Request': 'Login',
+            'Params': {'AuthTag': username, 'Password': password},
+        }
+        return json.dumps(data) if encoded else data
+
+    def make_login_response(
+            self, request_id=42, successful=True, encoded=False):
+        """Create and return a login response message.
+
+        If encoded is set to True, the returned message will be JSON encoded.
+        By default, a successful response is returned. Set successful to False
+        to return an authentication failure.
+        """
+        data = {'RequestId': request_id, 'Response': {}}
+        if not successful:
+            data['Error'] = 'invalid entity name or password'
+        return json.dumps(data) if encoded else data
+
+
+class PythonAPITestMixin(object):
+    """Add helper methods for testing the Python API implementation."""
+
+    def get_auth_backend(self):
+        """Return an authentication backend suitable for the Python API."""
+        return auth.get_backend('python')
+
+    def make_login_request(
+            self, request_id=42, username='user', password='passwd',
+            encoded=False):
+        """Create and return a login request message.
+
+        If encoded is set to True, the returned message will be JSON encoded.
+        """
+        data = {
+            'request_id': request_id,
+            'op': 'login',
+            'user': username,
+            'password': password,
+        }
+        return json.dumps(data) if encoded else data
+
+    def make_login_response(
+            self, request_id=42, successful=True, encoded=False):
+        """Create and return a login response message.
+
+        If encoded is set to True, the returned message will be JSON encoded.
+        By default, a successful response is returned. Set successful to False
+        to return an authentication failure.
+        """
+        data = {'request_id': request_id, 'op': 'login'}
+        if successful:
+            data['result'] = True
+        else:
+            data['err'] = True
+        return json.dumps(data) if encoded else data
+
+
 class WSSTestMixin(object):
     """Add some helper methods for testing secure WebSocket handlers."""
 

=== added file 'server/guiserver/tests/test_auth.py'
--- server/guiserver/tests/test_auth.py	1970-01-01 00:00:00 +0000
+++ server/guiserver/tests/test_auth.py	2013-07-25 15:44:30 +0000
@@ -0,0 +1,231 @@
+# This file is part of the Juju GUI, which lets users view and manage Juju
+# environments within a graphical interface (https://launchpad.net/juju-gui).
+# Copyright (C) 2013 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for the Juju GUI server authentication management."""
+
+import unittest
+
+from guiserver import auth
+from guiserver.tests import helpers
+
+
+class TestUser(unittest.TestCase):
+
+    def test_authenticated_repr(self):
+        # An authenticated user is correctly represented.
+        user = auth.User(
+            username='the-doctor', password='bad-wolf', is_authenticated=True)
+        expected = '<User: the-doctor (authenticated)>'
+        self.assertEqual(expected, repr(user))
+
+    def test_not_authenticated_repr(self):
+        # A not authenticated user is correctly represented.
+        user = auth.User(
+            username='the-doctor', password='bad-wolf', is_authenticated=False)
+        expected = '<User: the-doctor (not authenticated)>'
+        self.assertEqual(expected, repr(user))
+
+    def test_anonymous_repr(self):
+        # An anonymous user is correctly represented.
+        user = auth.User()
+        expected = '<User: anonymous (not authenticated)>'
+        self.assertEqual(expected, repr(user))
+
+
+class AuthMiddlewareTestMixin(object):
+    """Include tests for the AuthMiddleware.
+
+    Subclasses must subclass one of the API test mixins in helpers.
+    """
+
+    def setUp(self):
+        self.user = auth.User()
+        self.auth = auth.AuthMiddleware(self.user, self.get_auth_backend())
+
+    def assert_user(self, username, password, is_authenticated):
+        """Ensure the current user reflects the given values."""
+        user = self.user
+        self.assertEqual(username, user.username)
+        self.assertEqual(password, user.password)
+        self.assertEqual(is_authenticated, user.is_authenticated)
+
+    def test_login_request(self):
+        # The authentication process starts if a login request is processed.
+        request = self.make_login_request(username='user', password='passwd')
+        self.auth.process_request(request)
+        self.assertTrue(self.auth.in_progress())
+        self.assert_user('user', 'passwd', False)
+
+    def test_login_success(self):
+        # The user is logged in if the authentication process completes.
+        request = self.make_login_request(username='user', password='passwd')
+        self.auth.process_request(request)
+        response = self.make_login_response()
+        self.auth.process_response(response)
+        self.assertFalse(self.auth.in_progress())
+        self.assert_user('user', 'passwd', True)
+
+    def test_login_failure(self):
+        # The user is not logged in if the authentication process fails.
+        request = self.make_login_request()
+        self.auth.process_request(request)
+        response = self.make_login_response(successful=False)
+        self.auth.process_response(response)
+        self.assertFalse(self.auth.in_progress())
+        self.assert_user('', '', False)
+
+    def test_request_mismatch(self):
+        # The authentication fails if the request and response identifiers
+        # don't match.
+        request = self.make_login_request(
+            request_id=42, username='user', password='passwd')
+        self.auth.process_request(request)
+        response = self.make_login_response(request_id=47)
+        self.auth.process_response(response)
+        self.assertTrue(self.auth.in_progress())
+        self.assert_user('user', 'passwd', False)
+
+    def test_multiple_auth_requests(self):
+        # Only the last authentication request is taken into consideration.
+        request1 = self.make_login_request(request_id=1)
+        request2 = self.make_login_request(
+            request_id=2, username='user2', password='passwd2')
+        self.auth.process_request(request1)
+        self.auth.process_request(request2)
+        # The first response arrives.
+        response = self.make_login_response(request_id=1)
+        self.auth.process_response(response)
+        # The user is still not autheticated and the auth is in progress.
+        self.assertFalse(self.user.is_authenticated)
+        self.assertTrue(self.auth.in_progress())
+        # The second response arrives.
+        response = self.make_login_response(request_id=2)
+        self.auth.process_response(response)
+        # The user logged in and the auth process completed.
+        self.assert_user('user2', 'passwd2', True)
+        self.assertFalse(self.auth.in_progress())
+
+
+class TestGoAuthMiddleware(
+        helpers.GoAPITestMixin, AuthMiddlewareTestMixin, unittest.TestCase):
+    pass
+
+
+class TestPythonAuthMiddleware(
+        helpers.PythonAPITestMixin, AuthMiddlewareTestMixin,
+        unittest.TestCase):
+    pass
+
+
+class BackendTestMixin(object):
+    """Include tests for the authentication backends.
+
+    Subclasses must subclass one of the API test mixins in helpers.
+    """
+
+    def setUp(self):
+        self.backend = self.get_auth_backend()
+
+    def test_get_request_id(self):
+        # The request id is correctly returned.
+        request = self.make_login_request(request_id=42)
+        self.assertEqual(42, self.backend.get_request_id(request))
+
+    def test_get_request_id_failure(self):
+        # If the request id cannot be found, None is returned.
+        self.assertIsNone(self.backend.get_request_id({}))
+
+    def test_request_is_login(self):
+        # True is returned if a login request is passed.
+        request = self.make_login_request()
+        self.assertTrue(self.backend.request_is_login(request))
+
+    def test_get_credentials(self):
+        # The user name and password are returned parsing the login request.
+        request = self.make_login_request(username='user', password='passwd')
+        username, password = self.backend.get_credentials(request)
+        self.assertEqual('user', username)
+        self.assertEqual('passwd', password)
+
+    def test_login_succeeded(self):
+        # True is returned if the login attempt succeeded.
+        response = self.make_login_response()
+        self.assertTrue(self.backend.login_succeeded(response))
+
+    def test_login_failed(self):
+        # False is returned if the login attempt failed.
+        response = self.make_login_response(successful=False)
+        self.assertFalse(self.backend.login_succeeded(response))
+
+
+class TestGoBackend(
+        helpers.GoAPITestMixin, BackendTestMixin, unittest.TestCase):
+
+    def test_request_is_not_login(self):
+        # False is returned if the passed data is not a login request.
+        requests = (
+            {},
+            {
+                'RequestId': 1,
+                'Type': 'INVALID',
+                'Request': 'Login',
+                'Params': {'AuthTag': 'user', 'Password': 'passwd'},
+            },
+            {
+                'RequestId': 2,
+                'Type': 'Admin',
+                'Request': 'INVALID',
+                'Params': {'AuthTag': 'user', 'Password': 'passwd'},
+            },
+            {
+                'RequestId': 3,
+                'Type': 'Admin',
+                'Request': 'Login',
+                'Params': {'Password': 'passwd'},
+            },
+        )
+        for request in requests:
+            is_login = self.backend.request_is_login(request)
+            self.assertFalse(is_login, request)
+
+
+class TestPythonBackend(
+        helpers.PythonAPITestMixin, BackendTestMixin, unittest.TestCase):
+
+    def test_request_is_not_login(self):
+        # False is returned if the passed data is not a login request.
+        requests = (
+            {},
+            {
+                'request_id': 42,
+                'op': 'INVALID',
+                'user': 'user',
+                'password': 'passwd',
+            },
+            {
+                'request_id': 42,
+                'op': 'login',
+                'password': 'passwd',
+            },
+            {
+                'request_id': 42,
+                'op': 'login',
+                'user': 'user',
+            },
+        )
+        for request in requests:
+            is_login = self.backend.request_is_login(request)
+            self.assertFalse(is_login, request)

=== modified file 'server/guiserver/tests/test_handlers.py'
--- server/guiserver/tests/test_handlers.py	2013-07-23 08:21:12 +0000
+++ server/guiserver/tests/test_handlers.py	2013-07-25 15:44:30 +0000
@@ -16,6 +16,7 @@
 
 """Tests for the Juju GUI server handlers."""
 
+import json
 import os
 import shutil
 import tempfile
@@ -34,20 +35,25 @@
 )
 
 from guiserver import (
+    auth,
     clients,
     handlers,
+    manage,
 )
 from guiserver.tests import helpers
 
 
-class TestWebSocketHandler(
-        AsyncHTTPSTestCase, LogTrapTestCase, helpers.WSSTestMixin):
+class WebSocketHandlerTestMixin(object):
+    """Base set up for all the WebSocketHandler test cases."""
+
+    hello_message = json.dumps({'hello': 'world'})
 
     def get_app(self):
-        # In this test case a WebSocket server is created. The server creates a
-        # new client on each request. This client should forward messages to a
-        # WebSocket echo server. In order to test the communication, some of
-        # the tests create another client that connects to the server, e.g.:
+        # In test cases including this mixin a WebSocket server is created.
+        # The server creates a new client on each request. This client should
+        # forward messages to a WebSocket echo server. In order to test the
+        # communication, some of the tests create another client that connects
+        # to the server, e.g.:
         #   ws-client -> ws-server -> ws-forwarding-client -> ws-echo-server
         # Messages arriving to the echo server are returned back to the client:
         #   ws-echo-server -> ws-forwarding-client -> ws-server -> ws-client
@@ -58,13 +64,13 @@
             'io_loop': self.io_loop,
         }
         ws_options = {
-            'jujuapi': self.echo_server_address,
+            'apiurl': self.echo_server_address,
             'io_loop': self.io_loop,
         }
         return web.Application([
             (r'/echo', helpers.EchoWebSocketHandler, echo_options),
             (r'/ws', handlers.WebSocketHandler, ws_options),
-        ])
+        ], auth_backend=auth.get_backend(manage.DEFAULT_API_VERSION))
 
     def make_client(self):
         """Return a WebSocket client ready to be connected to the server."""
@@ -73,12 +79,29 @@
         callback = lambda message: None
         return clients.websocket_connect(self.io_loop, url, callback)
 
-    def make_handler(self, headers=None):
+    def make_handler(self, headers=None, mock_protocol=False):
         """Create and return a WebSocketHandler instance."""
         if headers is None:
             headers = {}
         request = mock.Mock(headers=headers)
-        return handlers.WebSocketHandler(self.get_app(), request)
+        handler = handlers.WebSocketHandler(self.get_app(), request)
+        if mock_protocol:
+            # Mock the underlying connection protocol.
+            handler.ws_connection = mock.Mock()
+        return handler
+
+
+class TestWebSocketHandlerConnection(
+        WebSocketHandlerTestMixin, helpers.WSSTestMixin, LogTrapTestCase,
+        AsyncHTTPSTestCase):
+
+    def mock_websocket_connect(self):
+        """Mock the guiserver.clients.websocket_connect function."""
+        future = concurrent.Future()
+        future.set_result(mock.Mock())
+        mock_websocket_connect = mock.Mock(return_value=future)
+        return mock.patch(
+            'guiserver.handlers.websocket_connect', mock_websocket_connect)
 
     @gen_test
     def test_initialization(self):
@@ -124,30 +147,55 @@
         self.assertIn('Origin', headers)
         self.assertEqual(self.get_url('/echo'), headers['Origin'])
 
-    @mock.patch('guiserver.handlers.websocket_connect')
-    def test_client_callback(self, mock_websocket_connect):
+    def test_client_callback(self):
         # The WebSocket client is created passing the proper callback.
         handler = self.make_handler()
-        handler.initialize(self.echo_server_address, self.io_loop)
+        with self.mock_websocket_connect() as mock_websocket_connect:
+            handler.initialize(self.echo_server_address, self.io_loop)
         self.assertEqual(1, mock_websocket_connect.call_count)
         self.assertIn(
             handler.on_juju_message, mock_websocket_connect.call_args[0])
 
+    @gen_test
+    def test_connection_closed_by_client(self):
+        # The proxy connection is terminated when the client disconnects.
+        client = yield self.make_client()
+        yield client.close()
+        yield self.echo_server_closed_future
+
+    @gen_test
+    def test_connection_closed_by_server(self):
+        # The proxy connection is terminated when the server disconnects.
+        client = yield self.make_client()
+        # A server disconnection is logged as an error.
+        expected_log = '.*Juju API unexpectedly disconnected'
+        with ExpectLog('', expected_log, required=True):
+            # Fire the Future in order to force an echo server disconnection.
+            self.echo_server_closed_future.set_result(None)
+            message = yield client.read_message()
+        self.assertIsNone(message)
+
+
+class TestWebSocketHandlerProxy(
+        WebSocketHandlerTestMixin, helpers.WSSTestMixin, LogTrapTestCase,
+        AsyncHTTPSTestCase):
+
     @mock.patch('guiserver.clients.WebSocketClientConnection')
     def test_from_browser_to_juju(self, mock_juju_connection):
         # A message from the browser is forwarded to the remote server.
         handler = self.make_handler()
         yield handler.initialize(self.echo_server_address, self.io_loop)
-        handler.on_message('hello')
-        mock_juju_connection.write_message.assert_called_once_with('hello')
+        handler.on_message(self.hello_message)
+        mock_juju_connection.write_message.assert_called_once_with(
+            self.hello_message)
 
     def test_from_juju_to_browser(self):
         # A message from the remote server is returned to the browser.
         handler = self.make_handler()
         handler.initialize(self.echo_server_address, self.io_loop)
         with mock.patch('guiserver.handlers.WebSocketHandler.write_message'):
-            handler.on_juju_message('hello')
-            handler.write_message.assert_called_once_with('hello')
+            handler.on_juju_message(self.hello_message)
+            handler.write_message.assert_called_once_with(self.hello_message)
 
     @gen_test
     def test_queued_messages(self):
@@ -158,38 +206,90 @@
         with mock.patch(mock_path) as mock_write_message:
             initialization = handler.initialize(
                 self.echo_server_address, self.io_loop)
-            handler.on_message('hello')
+            handler.on_message(self.hello_message)
             self.assertFalse(mock_write_message.called)
             yield initialization
-        mock_write_message.assert_called_once_with('hello')
+        mock_write_message.assert_called_once_with(self.hello_message)
 
     @gen_test
     def test_end_to_end_proxy(self):
         # Messages are correctly forwarded from the client to the echo server
         # and back to the client.
         client = yield self.make_client()
-        client.write_message('boomerang')
+        client.write_message(self.hello_message)
         message = yield client.read_message()
-        self.assertEqual('boomerang', message)
-
-    @gen_test
-    def test_connection_closed_by_client(self):
-        # The proxy connection is terminated when the client disconnects.
-        client = yield self.make_client()
-        yield client.close()
-        yield self.echo_server_closed_future
-
-    @gen_test
-    def test_connection_closed_by_server(self):
-        # The proxy connection is terminated when the server disconnects.
-        client = yield self.make_client()
-        # A server disconnection is logged as an error.
-        expected_log = '.*Juju API unexpectedly disconnected'
-        with ExpectLog('', expected_log, required=True):
-            # Fire the Future in order to force an echo server disconnection.
-            self.echo_server_closed_future.set_result(None)
-            message = yield client.read_message()
-        self.assertIsNone(message)
+        self.assertEqual(self.hello_message, message)
+
+    @gen_test
+    def test_invalid_json(self):
+        # A warning is logged if the message is not valid JSON.
+        client = yield self.make_client()
+        expected_log = 'JSON decoder: message is not valid JSON: not-json'
+        with ExpectLog('', expected_log, required=True):
+            client.write_message('not-json')
+            yield client.read_message()
+
+    @gen_test
+    def test_not_a_dict(self):
+        # A warning is logged if the decoded message is not a dict.
+        client = yield self.make_client()
+        expected_log = 'JSON decoder: message is not a dict: "not-a-dict"'
+        with ExpectLog('', expected_log, required=True):
+            client.write_message('"not-a-dict"')
+            yield client.read_message()
+
+
+class TestWebSocketHandlerAuthentication(
+        WebSocketHandlerTestMixin, helpers.WSSTestMixin,
+        helpers.GoAPITestMixin, LogTrapTestCase, AsyncHTTPSTestCase):
+
+    def setUp(self):
+        super(TestWebSocketHandlerAuthentication, self).setUp()
+        self.handler = self.make_handler(mock_protocol=True)
+        self.handler.initialize(self.echo_server_address, self.io_loop)
+
+    def send_login_request(self):
+        """Create a login request and send it to the handler."""
+        request = self.make_login_request(encoded=True)
+        self.handler.on_message(request)
+
+    def send_login_response(self, successful):
+        """Create a login response and send it to the handler."""
+        response = self.make_login_response(
+            successful=successful, encoded=True)
+        self.handler.on_juju_message(response)
+
+    def test_authentication_success(self):
+        # The authentication process completes and the user is logged in.
+        self.assertFalse(self.handler.user.is_authenticated)
+        self.send_login_request()
+        self.assertFalse(self.handler.user.is_authenticated)
+        self.assertTrue(self.handler.auth.in_progress())
+        self.send_login_response(True)
+        self.assertTrue(self.handler.user.is_authenticated)
+        self.assertFalse(self.handler.auth.in_progress())
+
+    def test_authentication_failure(self):
+        # The user is not logged in if the authentication fails.
+        self.send_login_request()
+        self.send_login_response(False)
+        self.assertFalse(self.handler.user.is_authenticated)
+        self.assertFalse(self.handler.auth.in_progress())
+
+    def test_already_logged_in(self):
+        # Authentication is no longer attempted if the user already logged in.
+        self.send_login_request()
+        self.send_login_response(True)
+        self.send_login_request()
+        self.assertTrue(self.handler.user.is_authenticated)
+        self.assertFalse(self.handler.auth.in_progress())
+
+    def test_not_in_progress(self):
+        # Authentication responses are not processed if the authentication is
+        # not in progress.
+        self.send_login_response(True)
+        self.assertFalse(self.handler.user.is_authenticated)
+        self.assertFalse(self.handler.auth.in_progress())
 
 
 class TestIndexHandler(AsyncHTTPTestCase, LogTrapTestCase):

=== modified file 'server/guiserver/tests/test_manage.py'
--- server/guiserver/tests/test_manage.py	2013-07-18 15:56:09 +0000
+++ server/guiserver/tests/test_manage.py	2013-07-25 15:44:30 +0000
@@ -41,18 +41,23 @@
         mock_options.define.assert_called_once_with('debug', default=False)
 
 
-class TestValidateRequired(unittest.TestCase):
+class ValidatorTestMixin(object):
+    """Add methods for testing functions producing a system exit."""
 
     @contextmanager
-    def assert_sysexit(self, option):
+    def assert_sysexit(self, error):
         """Ensure the code in the context manager block produces a system exit.
 
-        Also check that the given error refers to the given option.
+        Also check that the given error is returned.
         """
-        expected_error = 'error: the {} argument is required'.format(option)
         with mock.patch('sys.exit') as mock_exit:
             yield
-            mock_exit.assert_called_once_with(expected_error)
+            mock_exit.assert_called_once_with(error)
+
+
+class TestValidateRequired(ValidatorTestMixin, unittest.TestCase):
+
+    error = 'error: the {} argument is required'
 
     def test_success(self):
         # The validation passes if the args are correctly found.
@@ -66,26 +71,49 @@
 
     def test_failure(self):
         with mock.patch('guiserver.manage.options', {'arg1': ''}):
-            with self.assert_sysexit('arg1'):
+            with self.assert_sysexit(self.error.format('arg1')):
                 manage._validate_required('arg1')
 
     def test_failure_multiple_args(self):
         options = {'arg1': 'value1', 'arg2': ''}
         with mock.patch('guiserver.manage.options', options):
-            with self.assert_sysexit('arg2'):
+            with self.assert_sysexit(self.error.format('arg2')):
                 manage._validate_required(*options.keys())
 
     def test_failure_missing(self):
         with mock.patch('guiserver.manage.options', {'arg1': None}):
-            with self.assert_sysexit('arg1'):
+            with self.assert_sysexit(self.error.format('arg1')):
                 manage._validate_required('arg1')
 
     def test_failure_empty(self):
         with mock.patch('guiserver.manage.options', {'arg1': ' '}):
-            with self.assert_sysexit('arg1'):
+            with self.assert_sysexit(self.error.format('arg1')):
                 manage._validate_required('arg1')
 
     def test_failure_invalid_type(self):
         with mock.patch('guiserver.manage.options', {'arg1': 42}):
-            with self.assert_sysexit('arg1'):
+            with self.assert_sysexit(self.error.format('arg1')):
                 manage._validate_required('arg1')
+
+
+class TestValidateChoices(ValidatorTestMixin, unittest.TestCase):
+
+    choices = ('choice1', 'choice2')
+    error = 'error: accepted values for the {} argument are: choice1, choice2'
+
+    def test_success(self):
+        # The validation passes if the value is included in the choices.
+        with mock.patch('guiserver.manage.options', {'arg1': 'choice1'}):
+            manage._validate_choices('arg1', self.choices)
+
+    def test_failure_invalid_choice(self):
+        # The validation fails if the value is not in choices.
+        with mock.patch('guiserver.manage.options', {'arg1': 'not-a-choice'}):
+            with self.assert_sysexit(self.error.format('arg1')):
+                manage._validate_choices('arg1', self.choices)
+
+    def test_failure_missing(self):
+        # The validation fails if the value is missing.
+        with mock.patch('guiserver.manage.options', {'arg1': None}):
+            with self.assert_sysexit(self.error.format('arg1')):
+                manage._validate_choices('arg1', self.choices)

=== modified file 'server/guiserver/tests/test_utils.py'
--- server/guiserver/tests/test_utils.py	2013-07-22 14:16:47 +0000
+++ server/guiserver/tests/test_utils.py	2013-07-25 15:44:30 +0000
@@ -16,9 +16,11 @@
 
 """Tests for the Juju GUI server utilities."""
 
+import json
 import unittest
 
 import mock
+from tornado.testing import ExpectLog
 
 from guiserver import utils
 
@@ -39,6 +41,29 @@
         self.assertEqual({'Origin': 'https://server.example.com'}, headers)
 
 
+class TestJsonDecodeDict(unittest.TestCase):
+
+    def test_valid(self):
+        # A valid JSON is decoded without errors.
+        data = {'key1': 'value1', 'key2': 'value2'}
+        message = json.dumps(data)
+        self.assertEqual(data, utils.json_decode_dict(message))
+
+    def test_invalid_json(self):
+        # If the message is not a valid JSON string, a warning is logged and
+        # None is returned.
+        expected_log = 'JSON decoder: message is not valid JSON: not-json'
+        with ExpectLog('', expected_log, required=True):
+            self.assertIsNone(utils.json_decode_dict('not-json'))
+
+    def test_invalid_type(self):
+        # If the resulting object is not a dict-like object, a warning is
+        # logged and None is returned.
+        expected_log = 'JSON decoder: message is not a dict: "not-a-dict"'
+        with ExpectLog('', expected_log, required=True):
+            self.assertIsNone(utils.json_decode_dict('"not-a-dict"'))
+
+
 class TestRequestSummary(unittest.TestCase):
 
     def test_summary(self):

=== modified file 'server/guiserver/utils.py'
--- server/guiserver/utils.py	2013-07-22 14:01:58 +0000
+++ server/guiserver/utils.py	2013-07-25 15:44:30 +0000
@@ -16,8 +16,12 @@
 
 """Juju GUI server utility functions and classes."""
 
+import collections
+import logging
 import urlparse
 
+from tornado import escape
+
 
 def get_headers(request, websocket_url):
     """Return additional headers to be included in the client connection.
@@ -32,6 +36,25 @@
     return {'Origin': origin}
 
 
+def json_decode_dict(message):
+    """Decode the given JSON message, returning a Python dict.
+
+    If the message is not a valid JSON string, or if the resulting object is
+    not a dict-like object, log a warning and return None.
+    """
+    try:
+        data = escape.json_decode(message)
+    except ValueError:
+        msg = 'JSON decoder: message is not valid JSON: {}'.format(message)
+        logging.warning(msg)
+        return None
+    if not isinstance(data, collections.Mapping):
+        msg = 'JSON decoder: message is not a dict: {}'.format(message)
+        logging.warning(msg)
+        return None
+    return data
+
+
 def request_summary(request):
     """Return a string representing a summary for the given request."""
     return '{} {} ({})'.format(request.method, request.uri, request.remote_ip)

=== modified file 'server/runserver.py'
--- server/runserver.py	2013-07-18 13:59:20 +0000
+++ server/runserver.py	2013-07-25 15:44:30 +0000
@@ -18,7 +18,8 @@
 
 Arguments example:
     --guiroot="/var/lib/juju/agents/unit-juju-gui-0/charm/juju-gui/build-prod"
-    --jujuapi="wss://ec2-75-101-177-185.compute-1.example.com:17070"
+    --apiurl="wss://ec2-75-101-177-185.compute-1.example.com:17070"
+    --apiversion="go"
 """
 
 from guiserver import manage