Merge lp:~milo/linaro-patchmetrics/crowd-teams into lp:linaro-patchmetrics

[Milo Casagrande]

The following MP is the starting work to move away from Launchpad and use Crowd in patchwork/patchmetrics.
Here the Launchpad teams extraction has been reworked in order to interact with Crowd API. Launchpad integration has not been removed, and Crowd integration at the moment is commented out. When we roll out the final changes to patchwork/patchmetrics, it can be enabled.

What has been done:
- Introduced a Crowd class, that handles the connection to the Crowd API and that perform basic queries for the scope needed (users).
- Introduced a CrowdUser class that returns a handy Python object for a Crowd user as extracted from the Crowd API.

Two new variables in settings.py have been introduced: one is necessary to configure the Crowd integration, and it needs a user, password, url, and optional port to connect to the Crowd API. The file is a INI-style file, with a [crowd] section.

The second variable, optional, is to define a text file with whitelisted groups. Since when we retrieve a user information we get all the groups she is a member of, we might end up creating too many groups in patchwork/patchmetrics that we actually need. Also, we might expose groups that are not supposed to be public. With this text file we define a list of valid groups that we want to have in patchwork/patchmetrics. The format is just a plain text file, with a group name per line. If this file does not exists, all groups are considered valid.

There are two things still missing:
- Extracting or constructing a team display_name attribute: this is needed when creating new teams to give them a better visible name. Unfortunately Crowd does not expose that information, and we might need to use LDAP to achieve that.
- Modifying the 'sync_openid_urls' function: this function is called when syncing teams and users, and patchmetrics creates new users. The function patch the DB with a openid login URL from Launchpad. That needs to be modified to match the Linaro Login URL, but it should be done when we switch the authentication to Crowd too.

[lp:~milo/linaro-patchmetrics/crowd-teams]

379. By Milo Casagrande on 2013-09-17

Added Crowd settings.

380. By Milo Casagrande on 2013-09-18

Merged from trunk.

[Paul Sokolovsky (pfalcon)]

I have few concerns with crowd.py:

1. it doesn't follow DRY, for example has "/crowd/rest/usermanagement/1" twice (even rough example for Crowd API access addressed that providing layered functions for API calls).

2. I find "login_with" to be pretty confusing name for a factory method. It rather would be called something like "connect", though I don't understand why that work can't be done in old good constructor.

3.
264 + elif resp.status == 401 or resp.status == 403:
265 + raise CrowdException("Impossible to fulfill the request")

403 is rather important HTTP result code to shove it together with any other code, especially not mentioning "forbidden" explicitly in a result.

4. Crowd credentials storage scheme is not compatible with django-crowd-backend which would be used for user auth support.

I'd suggest to do something about 3. before merging. Otherwise, based on the discussion in the email (i.e. confirming it was tested), this can go in, so we'll definitely need to address 4 later.

review: Approve

[Paul Sokolovsky (pfalcon)]

"(even rough example for Crowd API access addressed that providing layered functions for API calls)." - I mean example I provided some time ago.

[Milo Casagrande (milo)]

On Sun, Sep 22, 2013 at 11:12 PM, Paul Sokolovsky
<email address hidden> wrote:
> Review: Approve
>
> I have few concerns with crowd.py:
>
> 1. it doesn't follow DRY, for example has "/crowd/rest/usermanagement/1" twice (even rough example for Crowd API access addressed that providing layered functions for API calls).

Addressed this, even if I think more refactoring might be needed.

> 2. I find "login_with" to be pretty confusing name for a factory method. It rather would be called something like "connect", though I don't understand why that work can't be done in old good constructor.
>
> 3.
> 264 + elif resp.status == 401 or resp.status == 403:
> 265 + raise CrowdException("Impossible to fulfill the request")
>
> 403 is rather important HTTP result code to shove it together with any other code, especially not mentioning "forbidden" explicitly in a result.

Treated 401 and 403 separately: CrowdUnauthorizedException and
CrowdForbiddenException.

> 4. Crowd credentials storage scheme is not compatible with django-crowd-backend which would be used for user auth support.

It is not, and I hope we can reuse it in some way. I didn't want to
jump into it right away, because I would have had to go through the
same studies and tests you have done, spending more time on it.

--
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

[lp:~milo/linaro-patchmetrics/crowd-teams]

381. By Milo Casagrande on 2013-09-23

Addressed comments from merge proposal.

=== modified file 'apps/patchmetrics/bin/get-linaro-membership.py'
--- apps/patchmetrics/bin/get-linaro-membership.py	2012-03-29 16:25:26 +0000
+++ apps/patchmetrics/bin/get-linaro-membership.py	2013-09-23 07:09:44 +0000
@@ -2,24 +2,27 @@
 
 import _pythonpath
 import sys
+
+from patchmetrics.crowd import Crowd
 from django.conf import settings
 from launchpadlib.launchpad import Launchpad
 from launchpadlib.uris import LPNET_SERVICE_ROOT
-from patchwork.models import Person
 from patchmetrics.utils import (
     sync_launchpad_memberships_for,
-    )
+    sync_user_memberships,
+)
+from patchwork.models import Person
 
 
 if __name__ == '__main__':
-    """Maps email addresses to Launchpad users to track Linaro's patches
+    """Maps email addresses to Linaro personnel to track Linaro's patches
 
     Pulls all email addresses out of the database, checks to see if each
-    email address is associated with a Launchpad user who is a member of
+    email address is associated with a Linaro Login user who is a member of
     a Linaro sub-team, and if they are, add a mapping between that Person
     object and a User object (many people (email addresses) can be mapped
-    onto a single User (Launchpad identity)). This allows us to map patches
-    by email address on to Launchpad users, and thus Linaro teams.
+    onto a single User (Linaro identity)). This allows us to map patches
+    by email address on to Linaro users, and thus Linaro teams.
 
     This script has been significantly re-written. Previously we pulled all
     users who were members of a linaro sub-team out of Launchpad and saved
@@ -28,7 +31,6 @@
     because someone has emailed a patch to [e-mail]. For this reason
     we don't automatically pick up new Linaro engineers until they have
     submitted a patch in this way, which is a change vs the old behaviour.
-
     """
     if settings.LPLIB_CREDENTIALS_FILE is None:
         print 'No LPLIB_CREDENTIALS_FILE defined in settings.py'
@@ -45,4 +47,20 @@
 
     linaro = lp.people['linaro']
 
+    # TODO: crowd stuff, enable when ready.
+    # if settings.CROWD_CREDENTIALS_FILE is None:
+    #     sys.stderr.write("CROWD_CREDENTIALS_FILE not defined in "
+    #                      "settings.py.\n")
+    #     sys.exit(1)
+
+    # whitelisted_groups = []
+    # if settings.CROWD_GROUPS_WHITELIST is None:
+    #     print "No CROWD_GROUPS_WHITELIST defined, all groups will be used."
+    # else:
+    #     with open(settings.CROWD_GROUPS_WHITELIST) as read_file:
+    #         whitelisted_groups = [x.strip() for x in read_file if x.strip()]
+
+    # crwd = Crowd.login_with(settings.CROWD_CREDENTIALS_FILE)
+    # sync_user_memberships(input_email_addresses, crwd, whitelisted_groups)
+
     sync_launchpad_memberships_for(input_email_addresses, linaro, lp)

=== added file 'apps/patchmetrics/crowd.py'
--- apps/patchmetrics/crowd.py	1970-01-01 00:00:00 +0000
+++ apps/patchmetrics/crowd.py	2013-09-23 07:09:44 +0000
@@ -0,0 +1,238 @@
+# Copyright (C) 2013 Linaro
+#
+# Author: Milo Casagrande <[e-mail]>
+# This file is part of the Patchmetrics package.
+#
+# Patchmetrics is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# Patchmetrics is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Patchwork; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+import base64
+import httplib
+import urllib
+import ConfigParser
+import json
+import types
+
+
+class CrowdException(Exception):
+    """Base class for Crowd exceptions."""
+
+
+class CrowdNotFoundException(CrowdException):
+    """An exception for 404 status."""
+
+
+class CrowdForbiddenException(CrowdException):
+    """An exception for 403 status."""
+
+
+class CrowdUnauthorizedException(CrowdException):
+    """ An exception for 401 status."""
+
+
+class CrowdUser(object):
+    """An object that depicts a user from the Crowd system.
+
+    It has the following properties:
+    name (str)
+    display_name (str)
+    teams (list)
+    emails (list)
+    """
+
+    def __init__(self):
+        self._display_name = None
+        self._emails = None
+        self._teams = None
+        self._name = None
+
+    @property
+    def emails(self):
+        return self._emails
+
+    @emails.setter
+    def emails(self, value):
+        if isinstance(value, types.StringTypes):
+            self._emails = [value]
+        else:
+            self._emails = list(value)
+
+    @property
+    def teams(self):
+        return self._teams
+
+    @teams.setter
+    def teams(self, value):
+        if isinstance(value, types.StringTypes):
+            self._teams = [value]
+        else:
+            self._teams = list(value)
+
+    @property
+    def display_name(self):
+        return self._display_name
+
+    @display_name.setter
+    def display_name(self, value):
+        self._display_name = value
+
+    @property
+    def name(self):
+        return self._name
+
+    @name.setter
+    def name(self, value):
+        self._name = value
+
+    @staticmethod
+    def _user_from_json(data):
+        user = CrowdUser()
+        user.display_name = data['display-name']
+        user.name = data['name']
+        user.emails = data['email']
+        return user
+
+    @staticmethod
+    def from_json(data):
+        """Creates a CrowdUser from JSON data.
+
+        The JSON data has to contain these fields:
+        display-name
+        name
+        email
+
+        :param data: The JSON file to load.
+        """
+        json_data = json.load(data)
+        return CrowdUser._user_from_json(json_data)
+
+    @staticmethod
+    def from_json_s(string):
+        """Creates a CrowdUser from a JSON string.
+
+        The JSON data has to contain these fields:
+        display-name
+        name
+        email
+
+        :param string: The JSON string.
+        :type str
+        """
+        json_data = json.loads(string)
+        return CrowdUser._user_from_json(json_data)
+
+
+class Crowd(object):
+    """A Crowd object used to perform query operations."""
+
+    def __init__(self, usr, pwd, url, port=8443):
+        self._usr = usr
+        self._pwd = pwd
+        self._url = url
+        self._port = port
+
+        self._auth = base64.encodestring('{0}:{1}'.format(self._usr,
+                                                          self._pwd))
+        self._headers = {
+            "Authorization": "Basic {0}".format(self._auth),
+            "Accept": "application/json"
+        }
+
+    def get_user(self, email):
+        params = {"username": email}
+
+        resource = "/user?{0}".format(urllib.urlencode(params))
+        return CrowdUser.from_json_s(self._get_rest_usermanagement(resource))
+
+    def get_user_with_groups(self, email):
+        """Gets all the groups a user is member of.
+
+        :param email: The user email.
+        :return A CrowdUser object.
+        """
+        # First get the user, if it does not exist, we skip all the operations
+        # here.
+        user = self.get_user(email)
+
+        params = {"username": email}
+
+        resource = "/user/group/nested?{0}".format(
+            urllib.urlencode(params))
+        data = json.loads(self._get_rest_usermanagement(resource))
+
+        teams = []
+        if data["groups"]:
+            teams = [x["name"] for x in data["groups"]]
+        user.teams = teams
+
+        return user
+
+    def _get_rest_usermanagement(self, resource):
+        api_url = "/crowd/rest/usermanagement/1{0}".format(resource)
+        return self._get(api_url)
+
+    def _get(self, api_url):
+        """Performs a GET operation on the API URL.
+
+        :param api_url: The URL of the API to use.
+        :return The response data.
+        :raise CrowdNotFoundException if the response status is 404,
+            CrowdForbiddenException if status is 403,
+            CrowdUnauthorizedException if status is 401, and CrowdException
+            in other cases.
+        """
+        connection = httplib.HTTPSConnection(self._url, self._port)
+        connection.request("GET", api_url, headers=self._headers)
+        resp = connection.getresponse()
+
+        if resp.status == 200:
+            return resp.read()
+        elif resp.status == 404:
+            raise CrowdNotFoundException("Resource not found")
+        elif resp.status == 401:
+            raise CrowdUnauthorizedException(
+                "Authorization not granted to fulfill the request")
+        elif resp.status == 403:
+            raise CrowdForbiddenException(
+                "Access forbidden to fulfill the request")
+        else:
+            raise CrowdException("Unknow Crowd status {0}".format(resp.status))
+
+    @staticmethod
+    def login_with(credentials_file):
+        """Create a Crowd object with the provided credentials file."""
+        config = ConfigParser.ConfigParser()
+        config.read(credentials_file)
+
+        crowd = None
+        if config:
+            try:
+                usr = config.get("crowd", "user")
+                pwd = config.get("crowd", "password")
+                url = config.get("crowd", "url")
+            except ConfigParser.NoOptionError:
+                raise CrowdException("Missing mandatory config parameter.")
+
+            try:
+                port = config.get("crowd", "port")
+                crowd = Crowd(usr, pwd, url, port=port)
+            except ConfigParser.NoOptionError:
+                pass
+
+            if crowd is None:
+                crowd = Crowd(usr, pwd, url)
+
+            return crowd
+        else:
+            raise CrowdException("Missing Crowd configuration values.")

=== modified file 'apps/patchmetrics/utils.py'
--- apps/patchmetrics/utils.py	2013-04-30 09:01:32 +0000
+++ apps/patchmetrics/utils.py	2013-09-23 07:09:44 +0000
@@ -1,15 +1,18 @@
 import logging
+import time
+import urllib2
 
+from patchmetrics.crowd import (
+    CrowdException,
+    CrowdNotFoundException,
+)
 from django.contrib.auth.models import User
 from django.db.models import Q
 from django_openid_auth.models import UserOpenID
+from lazr.restfulclient.errors import HTTPError
+from patchmetrics.models import Team
 from patchwork.models import Person
-from patchmetrics.models import Team
-import urllib2
-import time
 from xml.dom.minidom import parseString
-from lazr.restfulclient.errors import HTTPError
-
 
 logging.basicConfig()
 logger = logging.getLogger()
@@ -20,6 +23,61 @@
 MAX_RETRIES = 3
 
 
+def sync_crowd_memberships(memberships):
+    """Make sure the given memberships are represented in the database.
+
+    :param memberships: A dict mapping each team to a list of members. Each
+        member is in turn represented by a CrowdUser object.
+        For example: {Team: [CrowdUser1, CrowdUser2, ...]}
+    """
+    created_memberships = []
+    for team, members in memberships.iteritems():
+        try:
+            db_team = Team.objects.get(name=team)
+        except Team.DoesNotExist:
+            # TODO: need to extract the display_name for a team.
+            logger.info(
+                'Creating new team: {0} ({1})'.format(team, team.display_name))
+            db_team = Team(name=team, display_name=team.display_name)
+            db_team.save()
+
+        for member in members:
+            people = []
+            for email in member.emails:
+                try:
+                    person = Person.objects.get(email=email)
+                except Person.DoesNotExist:
+                    logger.info("Creating new person: {0} "
+                                "({1})".format(member.name, email))
+                    person = Person(name=member.display_name, email=email)
+                    person.save()
+                people.append(person)
+
+            # Get all Person entries that might represent other email
+            # addresses of this same user.
+            people.extend(Person.objects.filter(name=member.display_name))
+
+            user = get_user(member.name, member.emails)
+            if user is None:
+                user = User.objects.create_user(
+                    member.name, member.emails[0], password=None)
+
+            # Now link all the Person entries to the user account.
+            for person in people:
+                person.user = user
+                person.save()
+
+            # And finally, make sure the user is a member of the team.
+            if user not in db_team.members:
+                logger.info("Adding {0} as a member "
+                            "of {%1}".format(member.name, db_team.name))
+                membership = db_team.add_member(user)
+                membership.save()
+                created_memberships.append(membership)
+
+    return created_memberships
+
+
 def sync_memberships(memberships):
     """Make sure the given memberships are represented in our database.
 
@@ -133,12 +191,66 @@
     if users.count() == 1:
         return users[0]
     elif users.count() > 1:
-        print "Found more than one user for %s; using the first one" % emails
+        logger.info("Found more than one user for {0}; "
+                    "using the first one".format(emails))
         return users[0]
     else:
         return None
 
 
+def sync_user_memberships(email_addresses, crowd, whitelisted_groups=[]):
+    """If an input email matches one from Linaro Login, add them to the db.
+
+    If an email address matches a user in Linaro Login, takes the CrowdUser
+    object and passes it to sync_crowd_memberships, where CrowdUser are
+    matched with User objects (many CrowdUser can map to a single User).
+    The tail call to `sync_openid_urls` then adds OpenID URLs to each User
+    object.
+
+    :param: email_addresses: List of email addresses to look in Linaro Login.
+    :type list
+    :param crowd: The Crowd object instance to perform query to Linaro Login.
+    :type Crowd
+    :param whitelisted_groups: A list of valid groups/teams to create.
+    :type list
+    """
+    memberships = {}
+    for email in email_addresses:
+        email = email.lower()
+
+        user = None
+        try:
+            user = crowd.get_user_with_groups(email)
+        except CrowdNotFoundException:
+            # If there is not a user matching that email addess in Linaro,
+            # move on.
+            pass
+        except CrowdException:
+            # If something else went wrong, report it.
+            logger.warning("Error while searching email address "
+                           "'{0}'.".format(email))
+
+        # No user with that email, or user is not part of any team.
+        if user is None or not user.teams:
+            continue
+        else:
+            for team in user.teams:
+                # If we have a list of valid groups, obey it. Otherwise, all
+                # groups are valid.
+                if whitelisted_groups:
+                    if team in whitelisted_groups:
+                        if not team in memberships:
+                            memberships[team] = []
+                        memberships[team].append(user)
+                else:
+                    if not team in memberships:
+                        memberships[team] = []
+                    memberships[team].append(user)
+
+    sync_crowd_memberships(memberships)
+    sync_openid_urls()
+
+
 def sync_launchpad_memberships_for(input_email_addresses, team, lp):
     """If an input email matches a LP user, add them to the database.
 
@@ -161,7 +273,7 @@
             # https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290
             person = None  # Launchpad lookup failed.
             logger.warning("Launchpad rejected email lookup and returned "
-                            "HTTPError. Offending email address: %s" % email)
+                           "HTTPError. Offending email address: %s" % email)
 
         if not person:
             # No Launchpad entry for this person.
@@ -198,6 +310,9 @@
     When sync_memberships creates a User it doesn't do this, so this function
     just patches up the database with that extra information.
     """
+    # TODO
+    # Crowd openid urls are like this:
+    # https://login.linaro.org:8443/openidserver/users/[e-mail]
     for user in User.objects.all():
         if UserOpenID.objects.filter(user=user).count() == 0:
             try:

=== modified file 'apps/settings.py'
--- apps/settings.py	2012-10-18 09:55:07 +0000
+++ apps/settings.py	2013-09-23 07:09:44 +0000
@@ -100,7 +100,7 @@
     "django.core.context_processors.media",
     "patchwork.context_processors.login_url",
     "patchwork.context_processors.register_url",
-    )
+)
 
 AUTH_PROFILE_MODULE = "patchwork.userprofile"
 
@@ -127,6 +127,12 @@
 # means it will use the default, which is ~/.launchpadlib.
 LPLIB_DIR = None
 
+# The file containing Crowd credentials to access Linaro Login Crowd API.
+CROWD_CREDENTIALS_FILE = None
+# File with a list of groups that should be created in the DB. If other groups
+# are taken out from Crowd, and they are not here, they will be skipped.
+CROWD_GROUPS_WHITELIST = None
+
 # Where to store the git repos that we scan to identify which patches have
 # been committed.
 PATCHWORK_GIT_REPOS_DIR = '/tmp'
@@ -143,8 +149,7 @@
     from local_settings import *
 except ImportError, ex:
     import sys
-    sys.stderr.write(\
-            ("settings.py: error importing local settings file:\n" + \
-            "\t%s\n" + \
-            "Do you have a local_settings.py module?\n") % str(ex))
+    sys.stderr.write("settings.py: error importing local settings file:\n"
+                     "\t%s\n"
+                     "Do you have a local_settings.py module?\n" % str(ex))
     raise