Merge lp:~gary/charms/precise/juju-gui/authtoken3 into lp:~juju-gui/charms/precise/juju-gui/trunk

[Gary Poster]

Fully integrate AuthenticationTokenHandler

This might be the last of the charm branches for the authtoken feature. It hooks up the handler and tests the integration.

https://codereview.appspot.com/31290043/

[Gary Poster (gary)]

Reviewers: mp+196414_code.launchpad.net,

Message:
Please take a look.

Description:
Fully integrate AuthenticationTokenHandler

This might be the last of the charm branches for the authtoken feature.
It hooks up the handler and tests the integration. We might need more
work once we actually have a way to QA this! It looks good so far
though.

https://code.launchpad.net/~gary/charms/precise/juju-gui/authtoken3/+merge/196414

Requires:
https://code.launchpad.net/~gary/charms/precise/juju-gui/authtoken2/+merge/196351

(do not edit description out of merge proposal)

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

Affected files (+240, -25 lines):
   A [revision details]
   M server/guiserver/auth.py
   M server/guiserver/handlers.py
   M server/guiserver/tests/helpers.py
   M server/guiserver/tests/test_auth.py
   M server/guiserver/tests/test_handlers.py

[lp:~gary/charms/precise/juju-gui/authtoken3]

145. By Gary Poster on 2013-11-23

tweak encoding logic

146. By Gary Poster on 2013-11-23

tweak encoding

[Gary Poster (gary)]

Please take a look.

https://codereview.appspot.com/31290043/

[lp:~gary/charms/precise/juju-gui/authtoken3]

147. By Gary Poster on 2013-11-23

get more paranoid about unicode handling

[Gary Poster (gary)]

Please take a look.

https://codereview.appspot.com/31290043/

[Francesco Banconi (frankban)]

Very nice branch Gary, thank you!
LGTM (with some comments below), and QA ok!
To QA this branch I deployed the charm in a local environment using
"make deploy" and then I successfully run this script:
http://pastebin.ubuntu.com/6473403/

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py
File server/guiserver/auth.py (right):

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode27
server/guiserver/auth.py:27: - login_succeeded(data) -> bool.
- make_request(request_id, username, password) -> dict

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode29
server/guiserver/auth.py:29: requests' data based on the API
implementation currently in use. Backends
They can also be used to create authentication requests... or similar?

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode36
server/guiserver/auth.py:36: if the authentication succeeds.
Since this docstring summarizes the purposes of objects in this module,
it would be nice to provide some info about the
AuthenticationTokenHandler here.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode198
server/guiserver/auth.py:198: def make_request(self, request_id,
username, password):
"""Create and return an authentication request."""? <shrug>

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode349
server/guiserver/auth.py:349: def expire_token():
I know this is pre-existing code, but what do you think about logging
the token expiration here?

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode385
server/guiserver/auth.py:385: if credentials is not None:
... and maybe here we could log the fact that someone is actually using
the token?

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/handlers.py
File server/guiserver/handlers.py (right):

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/handlers.py#newcode157
server/guiserver/handlers.py:157: elif
self.tokens.token_requested(data):
A token request is only handled by the guiserver if the user is
authenticated. This means that a GUIToken Create request from an
"anonymous" client is propagated to juju-core, and the response will be
something like this:
{
  'RequestId': 0,
  'Error': 'unknown object type "GUIToken"',
  'Response': {},
}
This can be ok but seems confusing from the client perspective; what we
might want to send back is probably something like "unauthorized access:
no user logged in".
The quick fix is to always check if a token is requested (s/elif/if,
similar to what already done with deployer requests at line 145) and
ensure the user is authenticated in the process_token_request method
itself, returning a response or an error accordingly. Of course this can
be done (if you agree with the proposed change) in another branch.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/tests/test_auth.py
File server/guiserver/tests/test_auth.py (right):

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/tests/test_auth.py#newcode151
server/guiserver/tests/test_auth.py...

[lp:~gary/charms/precise/juju-gui/authtoken3]

148. By Gary Poster on 2013-11-25

respond to review

[Gary Poster (gary)]

Please take a look.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py
File server/guiserver/auth.py (right):

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode27
server/guiserver/auth.py:27: - login_succeeded(data) -> bool.
On 2013/11/25 11:59:34, frankban wrote:
> - make_request(request_id, username, password) -> dict

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode29
server/guiserver/auth.py:29: requests' data based on the API
implementation currently in use. Backends
On 2013/11/25 11:59:34, frankban wrote:
> They can also be used to create authentication requests... or similar?

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode36
server/guiserver/auth.py:36: if the authentication succeeds.
On 2013/11/25 11:59:34, frankban wrote:
> Since this docstring summarizes the purposes of objects in this
module, it would
> be nice to provide some info about the AuthenticationTokenHandler
here.

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode198
server/guiserver/auth.py:198: def make_request(self, request_id,
username, password):
On 2013/11/25 11:59:34, frankban wrote:
> """Create and return an authentication request."""? <shrug>

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode349
server/guiserver/auth.py:349: def expire_token():
On 2013/11/25 11:59:34, frankban wrote:
> I know this is pre-existing code, but what do you think about logging
the token
> expiration here?

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/auth.py#newcode385
server/guiserver/auth.py:385: if credentials is not None:
On 2013/11/25 11:59:34, frankban wrote:
> ... and maybe here we could log the fact that someone is actually
using the
> token?

Done.

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/handlers.py
File server/guiserver/handlers.py (right):

https://codereview.appspot.com/31290043/diff/30001/server/guiserver/handlers.py#newcode157
server/guiserver/handlers.py:157: elif
self.tokens.token_requested(data):
On 2013/11/25 11:59:34, frankban wrote:
> A token request is only handled by the guiserver if the user is
authenticated.
> This means that a GUIToken Create request from an "anonymous" client
is
> propagated to juju-core, and the response will be something like this:
> {
> 'RequestId': 0,
> 'Error': 'unknown object type "GUIToken"',
> 'Response': {},
> }
> This can be ok but seems confusing from the client perspective; what
we might
> want to send back is probably something like "unauthorized access: no
user
> logged in".
> The quick fix is to always check if a token is requested (s/elif/if,
similar to
> what already done with deployer requests at line 145) and ensure the
user is
> authenticated in the process_token_request method itself, returning a
response
> or an error accordingly. Of course this can be done (if you agree with
the
> proposed change) in another branch.

Good idea, thank you. I went ahead and did it.

https://codereview.appspot.com/31290043/

[Francesco Banconi (frankban)]

Changes LGTM, thank you!

https://codereview.appspot.com/31290043/

[Gary Poster (gary)]

*** Submitted:

Fully integrate AuthenticationTokenHandler

This might be the last of the charm branches for the authtoken feature.
It hooks up the handler and tests the integration.

R=frankban
CC=
https://codereview.appspot.com/31290043

https://codereview.appspot.com/31290043/

[Gary Poster (gary)]

Thanks for the fantastic review(s), and for going above and beyond with
the QA, Francesco.

https://codereview.appspot.com/31290043/

=== modified file 'server/guiserver/auth.py'
--- server/guiserver/auth.py	2013-11-23 02:14:15 +0000
+++ server/guiserver/auth.py	2013-11-25 14:02:21 +0000
@@ -16,24 +16,32 @@
 
 """Juju GUI server authentication management.
 
-This module includes the pieces required to process user authentication:
+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:
+    - User: this is a simple data structure representing a logged in or
+      anonymous user.
+    - Authentication backends (GoBackend and PythonBackend): the primary
+      purpose of auth backends is to provide the logic to parse requests' data
+      based on the API implementation currently in use. They can also be used
+      to create authentication requests.  They must implement the following
+      interface:
         - get_request_id(data) -> id or None;
         - request_is_login(data) -> bool;
         - get_credentials(data) -> (str, str);
-        - login_succeeded(data) -> bool.
-      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.
+        - login_succeeded(data) -> bool; and
+        - make_request(request_id, username, password) -> dict.
+      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: this middleware processes authentication requests and
+      responses, using the backend to parse the WebSocket messages, logging in
+      the current user if the authentication succeeds.
+    - AuthenticationTokenHandler: This handles authentication token creation
+      and usage requests.  It is used both by the AuthMiddleware and by
+      handlers.WebSocketHandler in the ``on_message`` and ``on_juju_message``
+      methods.
 """
 
 import datetime
@@ -76,12 +84,12 @@
         self._backend = backend
         self._tokens = tokens
         self._write_message = write_message
-        self._request_id = None
+        self._request_ids = {}
 
     def in_progress(self):
-        """Return True if the authentication is in progress, False otherwise.
+        """Return True if authentication is in progress, False otherwise.
         """
-        return self._request_id is not None
+        return bool(self._request_ids)
 
     def process_request(self, data):
         """Parse the WebSocket data arriving from the client.
@@ -90,11 +98,33 @@
         performed by the GUI user.
         """
         backend = self._backend
+        tokens = self._tokens
         request_id = backend.get_request_id(data)
-        if request_id is not None and backend.request_is_login(data):
-            self._request_id = request_id
-            credentials = backend.get_credentials(data)
-            self._user.username, self._user.password = credentials
+        if request_id is not None:
+            credentials = None
+            is_token = False
+            if backend.request_is_login(data):
+                credentials = backend.get_credentials(data)
+            elif tokens.authentication_requested(data):
+                is_token = True
+                credentials = tokens.process_authentication_request(
+                    data, self._write_message)
+                if credentials is None:
+                    # This means that the tokens object handled the request.
+                    return None
+                else:
+                    # We need a "real" authentication request.
+                    data = backend.make_request(request_id, *credentials)
+            if credentials is not None:
+                # Stashing credentials is a security risk.  We currently deem
+                # this risk to be acceptably small.  Even keeping an
+                # authenticated websocket in memory seems to be of a similar
+                # risk profile, and we cannot operate without that.
+                self._request_ids[request_id] = dict(
+                    is_token=is_token,
+                    username=credentials[0],
+                    password=credentials[1])
+        return data
 
     def process_response(self, data):
         """Parse the WebSocket data arriving from the Juju API server.
@@ -104,14 +134,23 @@
         authentication succeeded.
         """
         request_id = self._backend.get_request_id(data)
-        if request_id == self._request_id:
+        if request_id in self._request_ids:
+            info = self._request_ids.pop(request_id)
+            user = self._user
             logged_in = self._backend.login_succeeded(data)
             if logged_in:
-                logging.info('auth: user {} logged in'.format(self._user))
-                self._user.is_authenticated = True
-            else:
-                self._user.username = self._user.password = ''
-            self._request_id = None
+                # Stashing credentials is a security risk.  We currently deem
+                # this risk to be acceptably small.  Even keeping an
+                # authenticated websocket in memory seems to be of a similar
+                # risk profile, and we cannot operate without that.
+                user.username = info['username']
+                user.password = info['password']
+                logging.info('auth: user {} logged in'.format(user))
+                user.is_authenticated = True
+                if info['is_token']:
+                    data = self._tokens.process_authentication_response(
+                        data, user)
+        return data
 
 
 class GoBackend(object):
@@ -164,6 +203,14 @@
         """
         return 'Error' not in data
 
+    def make_request(self, request_id, username, password):
+        """Create and return an authentication request."""
+        return dict(
+            RequestId=request_id,
+            Type='Admin',
+            Request='Login',
+            Params=dict(AuthTag=username, Password=password))
+
 
 class PythonBackend(object):
     """Authentication backend for the Juju Python implementation.
@@ -216,6 +263,14 @@
         """
         return data.get('result') and not data.get('err')
 
+    def make_request(self, request_id, username, password):
+        """Create and return an authentication request."""
+        return dict(
+            request_id=request_id,
+            op='login',
+            user=username,
+            password=password)
+
 
 def get_backend(apiversion):
     """Return the auth backend instance to use for the given API version."""
@@ -235,7 +290,7 @@
             'Params': {},
         }
 
-    Here is an example of a token creation response.
+    Here is an example of a successful token creation response.
 
         {
             'RequestId': 42,
@@ -246,6 +301,15 @@
             }
         }
 
+    If the user is not authenticated, the failure response will look like this.
+
+        {
+            'RequestId': 42,
+            'Error': 'tokens can only be created by authenticated users.',
+            'ErrorCode': 'unauthorized access',
+            'Response': {},
+        }
+
     A token authentication request looks like the following:
 
         {
@@ -299,10 +363,18 @@
 
     def process_token_request(self, data, user, write_message):
         """Create a single-use, time-expired token and send it back."""
+        if not user.is_authenticated:
+            write_message(dict(
+                RequestId=data['RequestId'],
+                Error='tokens can only be created by authenticated users.',
+                ErrorCode='unauthorized access',
+                Response={}))
+            return
         token = uuid.uuid4().hex
 
         def expire_token():
             self._data.pop(token, None)
+            logging.info('auth: expired token {}'.format(token))
         handle = self._io_loop.add_timeout(self._max_life, expire_token)
         now = datetime.datetime.utcnow()
         # Stashing these is a security risk.  We currently deem this risk to
@@ -336,8 +408,10 @@
 
     def process_authentication_request(self, data, write_message):
         """Get the credentials for the token, or send an error."""
-        credentials = self._data.pop(data['Params']['Token'], None)
+        token = data['Params']['Token']
+        credentials = self._data.pop(token, None)
         if credentials is not None:
+            logging.info('auth: using token {}'.format(token))
             self._io_loop.remove_timeout(credentials['handle'])
             return credentials['username'], credentials['password']
         else:

=== modified file 'server/guiserver/handlers.py'
--- server/guiserver/handlers.py	2013-11-22 15:53:02 +0000
+++ server/guiserver/handlers.py	2013-11-25 14:02:21 +0000
@@ -22,6 +22,7 @@
 import time
 
 from tornado import (
+    escape,
     gen,
     web,
     websocket,
@@ -138,15 +139,27 @@
         established are queued for later delivery.
         """
         data = json_decode_dict(message)
+        encoded = None
         if data is not None:
             # Handle deployment requests.
             if self.deployment.requested(data):
                 return self.deployment.process_request(data)
             # Handle authentication requests.
             if not self.user.is_authenticated:
-                self.auth.process_request(data)
+                new_data = self.auth.process_request(data)
+                if new_data is None:
+                    # The None marker indicates that a response was sent.
+                    return
+                elif new_data != data:
+                    encoded = escape.json_encode(new_data)
+                    message = encoded.decode('utf8')
+            # Handle authentication token requests.
+            if self.tokens.token_requested(data):
+                return self.tokens.process_token_request(
+                    data, self.user, wrap_write_message(self))
         # Propagate messages to the Juju API server.
-        encoded = message.encode('utf-8')
+        if encoded is None:
+            encoded = message.encode('utf-8')
         if self.juju_connected:
             logging.debug(self._summary + 'client -> juju: {}'.format(encoded))
             return self.juju_connection.write_message(message)
@@ -163,8 +176,11 @@
             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)
-        encoded = message.encode('utf-8')
+            encoded = escape.json_encode(
+                self.auth.process_response(data))
+            message = encoded.decode('utf8')
+        else:
+            encoded = message.encode('utf-8')
         logging.debug(self._summary + 'juju -> client: {}'.format(encoded))
         self.write_message(message)
 

=== modified file 'server/guiserver/tests/helpers.py'
--- server/guiserver/tests/helpers.py	2013-11-07 11:17:54 +0000
+++ server/guiserver/tests/helpers.py	2013-11-25 14:02:21 +0000
@@ -96,6 +96,16 @@
             data['Error'] = 'invalid entity name or password'
         return json.dumps(data) if encoded else data
 
+    def make_token_login_request(self, tokens=None, request_id=42,
+                                 token='DEFACED', username=None,
+                                 password=None):
+        if username is not None and password is not None:
+            tokens._data[token] = dict(
+                username=username, password=password, handle="handle")
+        return dict(
+            RequestId=request_id, Type='GUIToken', Request='Login',
+            Params={'Token': token})
+
 
 class PythonAPITestMixin(object):
     """Add helper methods for testing the Python API implementation."""

=== modified file 'server/guiserver/tests/test_auth.py'
--- server/guiserver/tests/test_auth.py	2013-11-23 02:14:15 +0000
+++ server/guiserver/tests/test_auth.py	2013-11-25 14:02:21 +0000
@@ -78,17 +78,20 @@
 
     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)
+        response = self.auth.process_request(request)
+        self.assertEqual(request, response)
         self.assertTrue(self.auth.in_progress())
-        self.assert_user('user', 'passwd', False)
+        self.assert_user('', '', 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)
+        result = self.auth.process_response(response)
+        self.assertEqual(response, result)
         self.assertFalse(self.auth.in_progress())
         self.assert_user('user', 'passwd', True)
 
@@ -97,7 +100,8 @@
         request = self.make_login_request()
         self.auth.process_request(request)
         response = self.make_login_response(successful=False)
-        self.auth.process_response(response)
+        result = self.auth.process_response(response)
+        self.assertEqual(response, result)
         self.assertFalse(self.auth.in_progress())
         self.assert_user('', '', False)
 
@@ -110,10 +114,10 @@
         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)
+        self.assert_user('', '', False)
 
     def test_multiple_auth_requests(self):
-        # Only the last authentication request is taken into consideration.
+        # The last authentication request is honored.
         request1 = self.make_login_request(request_id=1)
         request2 = self.make_login_request(
             request_id=2, username='user2', password='passwd2')
@@ -122,8 +126,8 @@
         # 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)
+        # The user is authenticated but the auth is still in progress.
+        self.assertTrue(self.user.is_authenticated)
         self.assertTrue(self.auth.in_progress())
         # The second response arrives.
         response = self.make_login_response(request_id=2)
@@ -143,7 +147,61 @@
 class TestGoAuthMiddleware(
         helpers.GoAPITestMixin, AuthMiddlewareTestMixin,
         LogTrapTestCase, unittest.TestCase):
-    pass
+
+    def test_token_login_request(self):
+        # The authentication process starts with a token login request also.
+        request = self.make_token_login_request(
+            self.tokens, username='user', password='passwd')
+        response = self.auth.process_request(request)
+        # The response now looks as if it were made without a token.
+        self.assertEqual(
+            self.make_login_request(username='user', password='passwd'),
+            response)
+        self.assertTrue(self.auth.in_progress())
+        self.assert_user('', '', False)
+        self.assertFalse(self.write_message.called)
+
+    def test_token_login_success(self):
+        # The user is logged in if the authentication process completes.
+        request = self.make_token_login_request(
+            self.tokens, username='user', password='passwd')
+        self.auth.process_request(request)
+        response = self.make_login_response()
+        result = self.auth.process_response(response)
+        self.assertEqual(
+            dict(RequestId=42,
+                 Response=dict(AuthTag='user', Password='passwd')),
+            result)
+        self.assertFalse(self.auth.in_progress())
+        self.assert_user('user', 'passwd', True)
+        self.assertFalse(self.write_message.called)
+
+    def test_token_login_failure(self):
+        # The user is not logged in if the authentication process fails.
+        request = self.make_token_login_request(
+            self.tokens, username='user', password='passwd')
+        self.auth.process_request(request)
+        response = self.make_login_response(successful=False)
+        result = self.auth.process_response(response)
+        self.assertEqual(response, result)
+        self.assertFalse(self.auth.in_progress())
+        self.assert_user('', '', False)
+        self.assertFalse(self.write_message.called)
+
+    def test_token_login_missing(self):
+        # The user is not logged in if the authentication process fails.
+        request = self.make_token_login_request()
+        response = self.auth.process_request(request)
+        # None is a marker indicating that the request has been handled and
+        # should not be continued on through to Juju.
+        self.assertIsNone(response)
+        self.write_message.assert_called_once_with(dict(
+            RequestId=42,
+            Error='unknown, fulfilled, or expired token',
+            ErrorCode='unauthorized access',
+            Response={}))
+        self.assertFalse(self.auth.in_progress())
+        self.assert_user('', '', False)
 
 
 class TestPythonAuthMiddleware(
@@ -192,6 +250,12 @@
         response = self.make_login_response(successful=False)
         self.assertFalse(self.backend.login_succeeded(response))
 
+    def test_make_request(self):
+        expected = self.make_login_request(
+            request_id=42, username='user', password='passwd')
+        self.assertEqual(
+            expected, self.backend.make_request(42, 'user', 'passwd'))
+
 
 class TestGoBackend(
         helpers.GoAPITestMixin, BackendTestMixin, unittest.TestCase):
@@ -304,7 +368,7 @@
                        datetime.datetime(2013, 11, 21, 21)}))
     def test_process_token_request(self):
         # It correctly responds to token requests.
-        user = auth.User('user-admin', 'ADMINSECRET')
+        user = auth.User('user-admin', 'ADMINSECRET', True)
         write_message = mock.Mock()
         data = dict(RequestId=42, Type='GUIToken', Request='Create')
         self.tokens.process_token_request(data, user, write_message)
@@ -331,6 +395,21 @@
         expire_token()
         self.assertFalse('DEFACED' in self.tokens._data)
 
+    def test_unauthenticated_process_token_request(self):
+        # Unauthenticated token requests get an informative error.
+        user = auth.User(is_authenticated=False)
+        write_message = mock.Mock()
+        data = dict(RequestId=42, Type='GUIToken', Request='Create')
+        self.tokens.process_token_request(data, user, write_message)
+        write_message.assert_called_once_with(dict(
+            RequestId=42,
+            Error='tokens can only be created by authenticated users.',
+            ErrorCode='unauthorized access',
+            Response={}
+        ))
+        self.assertEqual({}, self.tokens._data)
+        self.assertFalse(self.io_loop.add_timeout.called)
+
     def test_authentication_requested(self):
         # It recognizes an authentication request.
         request = dict(
@@ -393,7 +472,7 @@
     def test_token_request_and_authentication_collaborate(self):
         # process_token_request and process_authentication_request collaborate.
         # This is a small integration test of the two functions' interaction.
-        user = auth.User('user-admin', 'ADMINSECRET')
+        user = auth.User('user-admin', 'ADMINSECRET', True)
         write_message = mock.Mock()
         request = dict(RequestId=42, Type='GUIToken', Request='Create')
         self.tokens.process_token_request(request, user, write_message)
@@ -406,7 +485,7 @@
 
     def test_process_authentication_response(self):
         # It translates a normal authentication success.
-        user = auth.User('user-admin', 'ADMINSECRET')
+        user = auth.User('user-admin', 'ADMINSECRET', True)
         response = {'RequestId': 42, 'Response': {}}
         self.assertEqual(
             dict(RequestId=42,

=== modified file 'server/guiserver/tests/test_handlers.py'
--- server/guiserver/tests/test_handlers.py	2013-11-22 15:53:02 +0000
+++ server/guiserver/tests/test_handlers.py	2013-11-25 14:02:21 +0000
@@ -16,6 +16,7 @@
 
 """Tests for the Juju GUI server handlers."""
 
+import datetime
 import json
 import os
 import shutil
@@ -330,6 +331,96 @@
         self.assertFalse(self.handler.user.is_authenticated)
         self.assertFalse(self.handler.auth.in_progress())
 
+    @mock.patch('uuid.uuid4', mock.Mock(return_value=mock.Mock(hex='DEFACED')))
+    @mock.patch('datetime.datetime',
+                mock.Mock(
+                    **{'utcnow.return_value':
+                       datetime.datetime(2013, 11, 21, 21)}))
+    def test_token_request(self):
+        # It supports requesting a token when authenticated.
+        self.handler.user.username = 'user'
+        self.handler.user.password = 'passwd'
+        self.handler.user.is_authenticated = True
+        request = json.dumps(
+            dict(RequestId=42, Type='GUIToken', Request='Create'))
+        self.handler.on_message(request)
+        message = self.handler.ws_connection.write_message.call_args[0][0]
+        self.assertEqual(
+            dict(
+                RequestId=42,
+                Response=dict(
+                    Token='DEFACED',
+                    Created='2013-11-21T21:00:00Z',
+                    Expires='2013-11-21T21:02:00Z'
+                )
+            ),
+            json.loads(message))
+        self.assertFalse(self.handler.juju_connected)
+        self.assertEqual(0, len(self.handler._juju_message_queue))
+
+    def test_unauthenticated_token_request(self):
+        # When not authenticated, the request is passed on to Juju for error.
+        self.assertFalse(self.handler.user.is_authenticated)
+        request = json.dumps(
+            dict(RequestId=42, Type='GUIToken', Request='Create'))
+        self.handler.on_message(request)
+        message = self.handler.ws_connection.write_message.call_args[0][0]
+        self.assertEqual(
+            dict(
+                RequestId=42,
+                Error='tokens can only be created by authenticated users.',
+                ErrorCode='unauthorized access',
+                Response={},
+            ),
+            json.loads(message))
+        self.assertFalse(self.handler.juju_connected)
+        self.assertEqual(0, len(self.handler._juju_message_queue))
+
+    def test_token_authentication_success(self):
+        # It supports authenticating with a token.
+        request = self.make_token_login_request(
+            self.tokens, username='user', password='passwd')
+        with mock.patch.object(self.io_loop,
+                               'remove_timeout') as mock_remove_timeout:
+            self.handler.on_message(json.dumps(request))
+            mock_remove_timeout.assert_called_once_with('handle')
+        self.assertEqual(
+            self.make_login_request(
+                request_id=42, username='user', password='passwd'),
+            json.loads(self.handler._juju_message_queue[0]))
+        self.assertTrue(self.handler.auth.in_progress())
+        self.send_login_response(True)
+        self.assertEqual(
+            dict(RequestId=42,
+                 Response={'AuthTag': 'user', 'Password': 'passwd'}),
+            json.loads(
+                self.handler.ws_connection.write_message.call_args[0][0]))
+
+    def test_token_authentication_failure(self):
+        # It correctly handles a token that will not authenticate.
+        request = self.make_token_login_request(
+            self.tokens, username='user', password='passwd')
+        with mock.patch.object(self.io_loop,
+                               'remove_timeout') as mock_remove_timeout:
+            self.handler.on_message(json.dumps(request))
+            mock_remove_timeout.assert_called_once_with('handle')
+        self.send_login_response(False)
+        message = self.handler.ws_connection.write_message.call_args[0][0]
+        self.assertEqual(
+            'invalid entity name or password',
+            json.loads(message)['Error'])
+
+    def test_unknown_authentication_token(self):
+        # It correctly handles an unknown token.
+        request = self.make_token_login_request()
+        self.handler.on_message(json.dumps(request))
+        message = self.handler.ws_connection.write_message.call_args[0][0]
+        self.assertEqual(
+            'unknown, fulfilled, or expired token',
+            json.loads(message)['Error'])
+        self.assertFalse(self.handler.juju_connected)
+        self.assertEqual(0, len(self.handler._juju_message_queue))
+
 
 class TestWebSocketHandlerBundles(
         WebSocketHandlerTestMixin, helpers.WSSTestMixin,