Code review comment for lp://staging/~hazmat/pyjuju/rapi-login

Reviewers: mp+140268_code.launchpad.net,

Message:
Please take a look.

Description:
Add a login api

Login support using admin credentials/secret

https://code.launchpad.net/~hazmat/juju/rapi-login/+merge/140268

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/agents/api.py
   M juju/rapi/context.py
   M juju/rapi/tests/test_context.py
   M juju/state/initialize.py
   M juju/state/tests/test_initialize.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: juju/agents/api.py
=== modified file 'juju/agents/api.py'
--- juju/agents/api.py 2012-09-10 03:43:29 +0000
+++ juju/agents/api.py 2012-12-12 19:30:20 +0000
@@ -11,6 +11,7 @@
  from juju.errors import JujuError
  from juju.lib.websockets import WebSocketsResource
  from juju.rapi.transport.ws import WebSocketAPIFactory
+from juju.state.auth import make_ace

  from twisted.web.static import Data
  from twisted.web.server import Site
@@ -32,7 +33,7 @@
          log.debug("Received environment data.")
          root = Data("Juju API Server\n", "text/plain")
          self.ws_factory = WebSocketAPIFactory(
- self.config['zookeeper_servers'], self.provider)
+ self.config['zookeeper_servers'], self.provider)
          yield self.ws_factory.startFactory()

          root.putChild('ws', WebSocketsResource(self.ws_factory))
@@ -79,10 +80,30 @@
                  environment = yield watch_d
              returnValue(environment)

+ # Lazy initialize login nodes if not present.
+ yield self._initialize_login()
+
          config = EnvironmentsConfig()
          config.parse(environment_data)
          returnValue(config.get_default())

+ @inlineCallbacks
+ def _initialize_login(self):
+ children = yield self.client.get_children("/")
+ if "login" in children:
+ return
+
+ acls, stat = yield self.client.get_acl("/initialized")
+ admin_acl = None
+
+ for a in acls:
+ if a['id'].startswith('admin'):
+ admin_acl = a
+ break
+
+ yield self.client.create(
+ "/login", acls=[make_ace(admin_acl['id'], all=True)])
+
      def configure(self, options):
          super(APIEndpointAgent, self).configure(options)
          if not options.get("port"):

Index: juju/rapi/context.py
=== modified file 'juju/rapi/context.py'
--- juju/rapi/context.py 2012-10-12 20:49:45 +0000
+++ juju/rapi/context.py 2012-12-12 19:30:20 +0000
@@ -1,5 +1,6 @@
  from StringIO import StringIO
-from twisted.internet.defer import maybeDeferred, succeed
+from twisted.internet.defer import (
+ maybeDeferred, succeed, returnValue, inlineCallbacks)

  from juju.rapi.cmd import add_relation
  from juju.rapi.cmd import add_unit
@@ -22,7 +23,10 @@

  from juju.rapi import rest

+from juju.state.security import Principal
+
  import logging
+import zookeeper

  log = logging.getLogger('juju.rapi.context')

@@ -78,7 +82,7 @@
          d = maybeDeferred(func, *args, **kw)
          return d.addCallback(
              lambda r: {'result': r, 'log': self.log.reset()}
- ).addErrback(self._on_error)
+ ).addErrback(self._on_error)

      def _on_error(self, failure):
          io = StringIO()
@@ -134,8 +138,7 @@
          return self._invoke(
              constraints_get.constraints_get,
              self,
- named_entities
- )
+ named_entities)

      def get_config(self, service_name):
          """Get configuration of a service.
@@ -163,8 +166,7 @@
              constraints,
              config,
              config_raw,
- num_units
- )
+ num_units)

      def debug_hooks(self, unit_name, hook_names=()):
          """Enable hook debug on a unit.
@@ -206,6 +208,25 @@
              self,
              data)

+ def login(self, user, password):
+ return self._invoke(
+ self._login,
+ user,
+ password)
+
+ @inlineCallbacks
+ def _login(self, user, password):
+ principal = Principal(user, password)
+ yield principal.attach(self.client)
+ try:
+ yield self.client.get("/login")
+ except (zookeeper.NoAuthException, zookeeper.AuthFailedException):
+ self.log.error("Invalid credentials")
+ raise
+ else:
+ self.log.info("Login success")
+ returnValue(True)
+
      def remove_relation(self, endpoint_a, endpoint_b):
          """Remove a relation from a service(s).
          """

Index: juju/state/initialize.py
=== modified file 'juju/state/initialize.py'
--- juju/state/initialize.py 2012-07-18 01:41:26 +0000
+++ juju/state/initialize.py 2012-12-12 19:30:20 +0000
@@ -71,6 +71,10 @@
          settings = GlobalSettingsStateManager(self.client)
          yield settings.set_provider_type(self.provider_type)

+ # Create a node that only the admin can read.
+ yield self.client.create("/login", acls=[
+ make_ace(self.admin_identity, all=True)])
+
          # This must come last, since clients will wait on it.
          yield self.client.create("/initialized", acls=acls)

Index: juju/rapi/tests/test_context.py
=== modified file 'juju/rapi/tests/test_context.py'
--- juju/rapi/tests/test_context.py 2012-11-07 12:49:11 +0000
+++ juju/rapi/tests/test_context.py 2012-12-12 19:30:20 +0000
@@ -1,6 +1,9 @@
  from twisted.internet.defer import inlineCallbacks
  from juju.rapi.tests.common import ContextTestBase

+from juju.state.security import Principal
+from juju.state.auth import make_ace
+

  class APIContextTest(ContextTestBase):

@@ -159,6 +162,20 @@
          self.assertEqual(response['result'], cs)

      @inlineCallbacks
+ def test_login(self):
+ u, p = "admin", "cekret"
+ principal = Principal(u, p)
+ self.client.create(
+ "/login", acls=[make_ace(principal.get_token(), all=True)])
+ result = yield self.context.login(u, "hello")
+ self.assertEqual(result['log'], [('error', 'Invalid credentials')])
+ self.assertEqual(result['err'], True)
+ result = yield self.context.login(u, p)
+ self.assertEqual(result['result'], True)
+ self.assertEqual(result['log'], [('info', 'Login success')])
+ yield self.client.delete("/login")
+
+ @inlineCallbacks
      def test_expose_and_unexpose(self):
          yield self.mock_store_charms(["precise/wordpress"])
          self.mocker.replay()

Index: juju/state/tests/test_initialize.py
=== modified file 'juju/state/tests/test_initialize.py'
--- juju/state/tests/test_initialize.py 2012-04-06 14:42:06 +0000
+++ juju/state/tests/test_initialize.py 2012-12-12 19:30:20 +0000
@@ -4,11 +4,12 @@
  from txzookeeper.tests.utils import deleteTree

  from juju.environment.tests.test_config import EnvironmentsConfigTestBase
-from juju.state.auth import make_identity
+
  from juju.state.environment import (
      GlobalSettingsStateManager, EnvironmentStateManager)
  from juju.state.initialize import StateHierarchy
  from juju.state.machine import MachineStateManager
+from juju.state.security import Principal

  class LayoutTest(EnvironmentsConfigTestBase):
@@ -19,7 +20,9 @@
          self.log = self.capture_logging("juju.state.init")
          zookeeper.set_debug_level(0)
          self.client = self.get_zookeeper_client()
- self.identity = make_identity("admin:genie")
+ self.principal = Principal("admin", "genie")
+ self.identity = self.principal.get_token()
+
          constraints_data = {
              "arch": "arm",
              "cpu": None,
@@ -28,6 +31,7 @@
          self.layout = StateHierarchy(
              self.client, self.identity, "i-abcdef",
constraints_data, "dummy")
          yield self.client.connect()
+ yield self.principal.attach(self.client)

      def tearDown(self):
          deleteTree(handle=self.client.handle)
@@ -57,6 +61,7 @@
          yield self.assert_existence_and_acl("/machines")
          yield self.assert_existence_and_acl("/relations")
          yield self.assert_existence_and_acl("/initialized")
+ yield self.assert_existence_and_acl("/login")

          # To check that the constraints landed correctly, we need the
          # environment config to have been sent, or we won't be able to