Code review comment for lp://staging/~gary/charms/precise/juju-gui/authtoken2

Reviewers: mp+196351_code.launchpad.net,

Message:
Please take a look.

Description:
No-op integration of AuthenticationTokenHandler

This branch simply hooks up the AuthenticationTokenHandler everywhere it
needs to be. It isn't actually utilized yet. That will be the next
branch (which has been sketched but will not arrive within the next 30
minutes :-) ). No QA.

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

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

(do not edit description out of merge proposal)

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

Affected files (+42, -9 lines):
   A [revision details]
   M server/guiserver/apps.py
   M server/guiserver/auth.py
   M server/guiserver/handlers.py
   M server/guiserver/tests/test_apps.py
   M server/guiserver/tests/test_auth.py
   M server/guiserver/tests/test_handlers.py
   M server/guiserver/tests/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: server/guiserver/apps.py
=== modified file 'server/guiserver/apps.py'
--- server/guiserver/apps.py 2013-11-14 14:08:23 +0000
+++ server/guiserver/apps.py 2013-11-22 15:53:02 +0000
@@ -44,6 +44,7 @@
      # Set up handlers.
      server_handlers = []
      if not options.sandbox:
+ tokens = auth.AuthenticationTokenHandler()
          websocket_handler_options = {
              # The Juju API backend url.
              'apiurl': options.apiurl,
@@ -51,6 +52,8 @@
              'auth_backend': auth.get_backend(options.apiversion),
              # The Juju deployer to use for importing bundles.
              'deployer': deployer,
+ # The tokens collection for authentication token requests.
+ 'tokens': tokens,
          }
          server_handlers.append(
              # Handle WebSocket connections.

Index: server/guiserver/auth.py
=== modified file 'server/guiserver/auth.py'
--- server/guiserver/auth.py 2013-11-22 18:06:17 +0000
+++ server/guiserver/auth.py 2013-11-22 18:10:50 +0000
@@ -71,9 +71,11 @@
      user logs out, there is no need to handle the log out process.
      """

- def __init__(self, user, backend):
+ def __init__(self, user, backend, tokens, write_message):
          self._user = user
          self._backend = backend
+ self._tokens = tokens
+ self._write_message = write_message
          self._request_id = None

      def in_progress(self):

Index: server/guiserver/handlers.py
=== modified file 'server/guiserver/handlers.py'
--- server/guiserver/handlers.py 2013-11-07 18:04:32 +0000
+++ server/guiserver/handlers.py 2013-11-22 15:53:02 +0000
@@ -69,7 +69,7 @@
      """

      @gen.coroutine
- def initialize(self, apiurl, auth_backend, deployer, io_loop=None):
+ def initialize(self, apiurl, auth_backend, deployer, tokens,
io_loop=None):
          """Initialize the WebSocket server.

          Create a new WebSocket client and connect it to the Juju API.
@@ -85,11 +85,13 @@
          self.juju_connected = False
          self._juju_message_queue = queue = deque()
          # Set up the authentication infrastructure.
+ self.tokens = tokens
+ write_message = wrap_write_message(self)
          self.user = User()
- self.auth = AuthMiddleware(self.user, auth_backend)
+ self.auth = AuthMiddleware(
+ self.user, auth_backend, tokens, write_message)
          # Set up the bundle deployment infrastructure.
- self.deployment = DeployMiddleware(
- self.user, deployer, wrap_write_message(self))
+ self.deployment = DeployMiddleware(self.user, deployer,
write_message)
          # 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.

Index: server/guiserver/tests/test_apps.py
=== modified file 'server/guiserver/tests/test_apps.py'
--- server/guiserver/tests/test_apps.py 2013-08-28 17:07:25 +0000
+++ server/guiserver/tests/test_apps.py 2013-11-22 15:53:02 +0000
@@ -102,6 +102,13 @@
          deployer = self.assert_in_spec(spec, 'deployer')
          self.assertIsInstance(deployer, base.Deployer)

+ def test_tokens(self):
+ # The tokens instance is correctly passed to the WebSocket handler.
+ app = self.get_app()
+ spec = self.get_url_spec(app, r'^/ws$')
+ tokens = self.assert_in_spec(spec, 'tokens')
+ self.assertIsInstance(tokens, auth.AuthenticationTokenHandler)
+
      def test_sandbox(self):
          # The WebSocket handler is excluded if sandbox mode is enabled.
          app = self.get_app(sandbox=True)

Index: server/guiserver/tests/test_auth.py
=== modified file 'server/guiserver/tests/test_auth.py'
--- server/guiserver/tests/test_auth.py 2013-11-22 18:06:17 +0000
+++ server/guiserver/tests/test_auth.py 2013-11-22 18:11:53 +0000
@@ -62,7 +62,12 @@

      def setUp(self):
          self.user = auth.User()
- self.auth = auth.AuthMiddleware(self.user, self.get_auth_backend())
+ self.io_loop = mock.Mock()
+ self.write_message = mock.Mock()
+ self.tokens = auth.AuthenticationTokenHandler(io_loop=self.io_loop)
+ self.auth = auth.AuthMiddleware(
+ self.user, self.get_auth_backend(), self.tokens,
+ self.write_message)

      def assert_user(self, username, password, is_authenticated):
          """Ensure the current user reflects the given values."""

Index: server/guiserver/tests/test_handlers.py
=== modified file 'server/guiserver/tests/test_handlers.py'
--- server/guiserver/tests/test_handlers.py 2013-11-07 18:01:12 +0000
+++ server/guiserver/tests/test_handlers.py 2013-11-22 15:53:02 +0000
@@ -66,6 +66,7 @@
          self.api_close_future = concurrent.Future()
          self.deployer = base.Deployer(
              self.apiurl, manage.DEFAULT_API_VERSION, io_loop=self.io_loop)
+ self.tokens = auth.AuthenticationTokenHandler(io_loop=self.io_loop)
          echo_options = {
              'close_future': self.api_close_future,
              'io_loop': self.io_loop,
@@ -75,6 +76,7 @@
              'auth_backend': self.auth_backend,
              'deployer': self.deployer,
              'io_loop': self.io_loop,
+ 'tokens': self.tokens,
          }
          return web.Application([
              (r'/echo', helpers.EchoWebSocketHandler, echo_options),
@@ -108,7 +110,8 @@
          handler = self.make_handler(
              headers=headers, mock_protocol=mock_protocol)
          yield handler.initialize(
- apiurl, self.auth_backend, self.deployer, self.io_loop)
+ apiurl, self.auth_backend, self.deployer, self.tokens,
+ self.io_loop)
          raise gen.Return(handler)

@@ -228,7 +231,8 @@
          mock_path
= 'guiserver.clients.WebSocketClientConnection.write_message'
          with mock.patch(mock_path) as mock_write_message:
              initialization = handler.initialize(
- self.apiurl, self.auth_backend, self.io_loop)
+ self.apiurl, self.auth_backend, self.deployer, self.tokens,
+ io_loop=self.io_loop)
              handler.on_message(self.hello_message)
              self.assertFalse(mock_write_message.called)
              yield initialization
@@ -279,7 +283,9 @@
      def setUp(self):
          super(TestWebSocketHandlerAuthentication, self).setUp()
          self.handler = self.make_handler(mock_protocol=True)
- self.handler.initialize(self.apiurl, self.auth_backend,
self.io_loop)
+ self.handler.initialize(
+ self.apiurl, self.auth_backend, self.deployer, self.tokens,
+ io_loop=self.io_loop)

      def send_login_request(self):
          """Create a login request and send it to the handler."""

Index: server/guiserver/tests/test_utils.py
=== modified file 'server/guiserver/tests/test_utils.py'
--- server/guiserver/tests/test_utils.py 2013-10-17 12:47:15 +0000
+++ server/guiserver/tests/test_utils.py 2013-11-22 17:44:39 +0000
@@ -158,6 +158,12 @@
              self.wrapped('hello')
          self.assertEqual([], self.messages)

+ def test_unicode(self):
+ # It handles unicode properly.
+ snowman = u'{"Here is a snowman\u00a1": "\u2603"}'
+ self.wrapped(snowman)
+ self.assertEqual(snowman, json.loads(self.messages[0]))
+

  class TestWsToHttp(unittest.TestCase):