Merge lp:~cprov/uci-engine/webui-user-info into lp:uci-engine

[Celso Providelo]

Implements and exposes a new resource in the TS API with information about the user logged in (/api/v1/userinfo/).

Implements and exposes a new resource in the TS API with information about the user logged in (/api/v1/userinfo/).

This resource will be consumed by the WebUI and will allow users to actually login (using openid on TS) or visualise they are logged in (username/name/email/etc).

Despite of the name, this MP only involves the TS-side of this feature.

The implementation to satisfy Tastypie is a little involving because it requires a resource wrapper to allow proper schema introspection (<resource>/schema). Other existing abstract resources (status & workflow_<statuses>) have some flaws and, at some point, should be refactored to use the just-introduced model.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:802
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1462/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1462/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:803
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1463/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1463/rebuild

review: Approve (continuous-integration)

[Evan (ev)]

Didn't go very deep on this, so I think it's worth getting others to review as well. Some minor nits/questions, otherwise looks very good.

review: Needs Information

[Celso Providelo (cprov)]

On Thu, Sep 25, 2014 at 12:55 PM, Evan Dandrea
<email address hidden> wrote:
> Review: Needs Information
>
> Didn't go very deep on this, so I think it's worth getting others to review as well. Some minor nits/questions, otherwise looks very good.

Thanks, I will get additional attention to this.

> It'd be helpful to note what the permissions actually map to. Ursula mentioned in IRC it's permission to do root things (what?) and permission to grant permission.
>

Right now, while we haven't yet implemented any authorization, the
fields preserved their original contrib.auth meaningness,
https://docs.djangoproject.com/en/1.5/ref/contrib/auth/#django.contrib.auth.models.User.is_staff

I.e. staff allows /admin access but requires specific permissions to
perform any action, and superuser gives full /admin access.

I think the none will be necessary for the ci-train transition, since
all records be public for reading and writes will require
authentication (in different forms).

I intend to use user_info.superuser to link to /admin, since ci-train
admins might need it for transitional tweaks. 'staff' is not
immediately useful, but we can use this flag during the transition to
control 'review/edit' powers in the UI. Everything is pretty much open
up to now.

>> + def test_anonymous_info(self):
>> + # Anonymous requests to /userinfo/ returns an object with
>> + # 'authenticated' set as False. The other fields are bogus/empty.
>
> Shouldn't this return 401 instead? I'm not really fussed, but it does seem strange that we return 200 with a response here.

The semantics are flexible at this point, I preferred to assume the
UserInfoResource is a public resource, intrinsically safe, which
happens to be empty when the app has no information about the user,
this way the code handling it relies on a constant schema for dealing
with authenticated and anonymous access. Making it available to
anonymous requests also gives us the ability to shelve public
information about the service (status/announce messages, profile and
display flags, etc) that are also relevant for unauthenticated
requests.

That kind of flexibility is lost if the resource become private, only
available to authenticated users, specially in an application that
offer a considerable amount of features to anonymous users.

Obviously the scenario might change as the application evolve it
shouldn't be any difficult to *privatise* this resource and deal with
Unauthorized errors in the client-side.

[]
--
Celso Providelo
<email address hidden>

[Joe Talbott (joetalbott)]

Looks good to me.

review: Approve

[Evan (ev)]

Thanks for the thorough response, Celso. That all makes sense to me. +1

review: Approve

[Francis Ginther (fginther)]

Your explanation helped a lot. As we finish up the other parts of the authentication work, we should be documenting how we're making use of staff and admin, etc.

review: Approve

[Ubuntu CI Bot (uci-bot)]

The attempt to merge lp:~cprov/uci-engine/webui-user-info into lp:uci-engine failed. Below is the output from the failed tests.

Running cm...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
uploading webui-content.tgz to swift
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
Checking juju status
Private PPAs: disabled
Preparing local branch upload...
Uploading local branch, fingerprint dcadc56fc813d78f863a31f4b376a3a6ecaf821b
Building charm: lander
Building charm: wsgi-app
Building charm: rabbitmq-worker
Building charm: webui
Building charm: key-secret-subordinate
Building charm: system-image-server
Building charm: chroot-builder
Installing keys from bzr+ssh://bazaar.launchpad.net/~ci-engineering-private/+junk/ci-airline-dev-keys/
Running juju-deployer -v -c /tmp/tmplHx3cq/deployer/branch-source-builder.yaml -c /tmp/tmplHx3cq/deployer/britney-proxy.yaml -c /tmp/tmplHx3cq/deployer/gatekeeper.yaml -c /tmp/tmplHx3cq/deployer/image-builder.yaml -c /tmp/tmplHx3cq/deployer/lander.yaml -c /tmp/tmplHx3cq/deployer/nf-stats-service.yaml -c /tmp/tmplHx3cq/deployer/ppa-creator.yaml -c /tmp/tmplHx3cq/deployer/publisher.yaml -c /tmp/tmplHx3cq/deployer/relations.yaml -c /tmp/tmplHx3cq/deployer/test-runner.yaml -c /tmp/tmplHx3cq/deployer/ticket-system.yaml -c /tmp/tmplHx3cq/deployer/validator.yaml -c /tmp/tmplHx3cq/deployer/webui.yaml ci-airline
Tests running...
ci-utils.ci_utils.tests.test_amqp.TestAMQP.testConnectFailed ... OK (0.002 secs)
ci-utils.ci_utils.tests.test_amqp.TestAMQP.testProcessQueue ... OK (0.002 secs)
ci-utils.ci_utils.tests.test_amqp.TestAMQP.testRunForever ... OK (0.102 secs)
ci-utils.ci_utils.tests.test_amqp.TestAMQP.testSent ... OK (0.002 secs)
ci-utils.ci_utils.tests.test_amqp.TestProgressTrigger.testProgress ... OK (0.003 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testCancel ... OK (0.104 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testNoQueue ... OK (0.003 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testNoTicket ... OK (0.004 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testOnMessageCalledProcessError ... OK (0.004 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testOnMessageFail ... OK (0.003 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testOnMessageKilled ... OK (0.003 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testOnMessageSimple ... OK (0.003 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testOnMessageUnexpected ... OK (0.004 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestAMQPWorker.testSaveLastRun ... OK (0.004 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestTimer.testCBRuns ... OK (0.021 secs)
ci-utils.ci_utils.tests.test_amqp_worker.TestTimer.testCanCancel ... OK (0.000 secs)
ci-utils.ci_utils.tests.test_data_store.TestDataStoreConfig.test_invalid_auth_config ... OK (0.000 secs)
ci-utils.ci_utils.tests.test_data_store.TestDataStoreConfig.test_valid_auth_config ... OK (0.000 secs)
ci-utils.ci_utils.tests.test_data_store.TestDataStoreFileName.test_get_file_name...

[lp:~cprov/uci-engine/webui-user-info]

804. By Celso Providelo on 2014-09-25

merge trunk

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:804
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1475/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1475/rebuild

review: Approve (continuous-integration)

[Evan (ev)]

On 25 September 2014 20:46, Ubuntu CI Bot <email address hidden> wrote:
> The attempt to merge lp:~cprov/uci-engine/webui-user-info into lp:uci-engine failed. Below is the output from the failed tests.

Do we know what caused this?

=== modified file 'ticket_system/ticket/api.py'
--- ticket_system/ticket/api.py	2014-09-24 12:32:33 +0000
+++ ticket_system/ticket/api.py	2014-09-25 23:06:03 +0000
@@ -26,7 +26,10 @@
     ALL,
     ALL_WITH_RELATIONS,
 )
-from tastypie.exceptions import BadRequest
+from tastypie.exceptions import (
+    BadRequest,
+    NotFound,
+)
 from tastypie.paginator import Paginator
 from tastypie.resources import (
     Bundle,
@@ -471,3 +474,88 @@
         ticket_uuid = resp['location'].split('/')[-2]
         Ticket.objects.get(uuid=ticket_uuid).delete()
         return True
+
+
+class ResourceWrapper(object):
+    """Minimal wrapper class for abstract (non-ORM) resources.
+
+    It optionally copies given 'fields' attributes (supports nested specs
+    and callables) from a given 'obj'.
+
+    It allows complete model/field instrospection (tastypie-style) and
+    isolatation between the exposed resource and the copied (mirrored)
+    model.
+    """
+
+    def __init__(self, **kwargs):
+        self.__dict__['_info'] = {}
+        # Dismiss if 'obj' or 'fields' are not given.
+        obj = kwargs.get('obj')
+        if obj is None:
+            return
+        fields = kwargs.get('fields', [])
+        # Build local attributes with values mirrored from the given
+        # object.
+        for name, field in fields.items():
+            if not field.attribute:
+                continue
+            try:
+                value = reduce(getattr, field.attribute.split('.'), obj)
+            except AttributeError:
+                value = ''
+            else:
+                if callable(value):
+                    value = value()
+            self.__dict__['_info'][name] = value
+
+    def __getattr__(self, name):
+        return self._info.get(name, None)
+
+    def to_dict(self):
+        return self._info
+
+
+class UserInfoResource(Resource):
+    """Expose users details for the current requestor."""
+
+    username = fields.CharField(
+        'username', readonly=True,
+        help_text='Username of the authenticated user')
+    email = fields.CharField(
+        'email', readonly=True,
+        help_text='Email address of the authenticated user.')
+    fullname = fields.CharField(
+        'get_full_name', readonly=True,
+        help_text='Full name (<first> <last>) of the authenticated user.')
+    superuser = fields.BooleanField(
+        'is_superuser', readonly=True,
+        help_text='Whether the authenticated user is a superuser or not.')
+    staff = fields.BooleanField(
+        'is_staff', readonly=True,
+        help_text='Whether the authenticated user is a staff member or not.')
+    authenticated = fields.BooleanField(
+        'is_authenticated', readonly=True,
+        help_text='Whether the user is authenticated or not.')
+
+    class Meta():
+        resource_name = 'userinfo'
+        object_class = ResourceWrapper
+        allowed_methods = ['get']
+
+    def get_list(self, request, **kwargs):
+        """Re-implement 'userinfo' resource list.
+
+        It actually returns a single userinfo resource corresponding to the
+        current requestor.
+        """
+        user_info = ResourceWrapper(obj=request.user, fields=self.fields)
+        return self.create_response(request, user_info.to_dict())
+
+    def get_object_list(self, request):
+        """Satisfy tastypie schema instrospection (/<resource>/schema)."""
+        user_info = ResourceWrapper(obj=request.user, fields=self.fields)
+        return [user_info]
+
+    def obj_get(self, bundle, **kwargs):
+        """Render a 404 for 'userinfo' details lookups."""
+        raise NotFound('UserInfo does not allow details lookups')

=== modified file 'ticket_system/ticket/tests/__init__.py'
--- ticket_system/ticket/tests/__init__.py	2014-04-16 18:36:03 +0000
+++ ticket_system/ticket/tests/__init__.py	2014-09-25 23:06:03 +0000
@@ -13,10 +13,11 @@
 # 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/>.
 
+from test_full_read_api import *
 from test_models import *
 from test_read_api import *
-from test_full_read_api import *
 from test_style import *
+from test_user_info import *
 from test_write_api import *
 
 

=== added file 'ticket_system/ticket/tests/test_user_info.py'
--- ticket_system/ticket/tests/test_user_info.py	1970-01-01 00:00:00 +0000
+++ ticket_system/ticket/tests/test_user_info.py	2014-09-25 23:06:03 +0000
@@ -0,0 +1,55 @@
+# Ubuntu Continuous Integration Engine
+# Copyright 2013, 2014 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/>.
+from __future__ import unicode_literals
+
+from ci_utils.tastypie.test import TastypieTestCase
+from django.contrib.auth.models import User
+
+
+class UserInfoTest(TastypieTestCase):
+
+    def setUp(self):
+        super(UserInfoTest, self).setUp('/api/v1')
+
+    def test_anonymous_info(self):
+        # Anonymous requests to /userinfo/ returns an object with
+        # 'authenticated' set as False. The other fields are bogus/empty.
+        user_info = self.getResource('userinfo/')
+        self.assertEqual({
+            'authenticated': False,
+            'username': '',
+            'email': '',
+            'fullname': '',
+            'staff': False,
+            'superuser': False,
+        }, user_info)
+
+    def test_authenticated_info(self):
+        # Authenticated requests to /userinfo/ returns the user information.
+        password = '1234'
+        user = User.objects.create_user(
+            username='test', first_name='First', last_name='Last',
+            password=password, email='[e-mail]')
+        self.client.client.login(username=user.username, password=password)
+
+        user_info = self.getResource('userinfo/')
+        self.assertEqual({
+            'authenticated': True,
+            'username': 'test',
+            'email': '[e-mail]',
+            'fullname': 'First Last',
+            'staff': False,
+            'superuser': False,
+        }, user_info)

=== modified file 'ticket_system/ticket_system/urls.py'
--- ticket_system/ticket_system/urls.py	2014-08-29 18:28:56 +0000
+++ ticket_system/ticket_system/urls.py	2014-09-25 23:06:03 +0000
@@ -48,6 +48,7 @@
 v1_api.register(ticket_api.SubTicketWorkflowStepStatusResource())
 v1_api.register(ticket_api.ReviewResource())
 v1_api.register(ticket_api.TSStatus())
+v1_api.register(ticket_api.UserInfoResource())
 
 
 urlpatterns = patterns(