Hello Paul!

Thanks for pulling this together!
Following are my findings!

On Mon, Jun 3, 2013 at 1:57 PM, Paul Sokolovsky
<email address hidden> wrote:
>
> === modified file 'README'
> --- README      2013-02-15 09:45:08 +0000
> +++ README      2013-06-03 11:56:29 +0000
> @@ -16,7 +16,9 @@
> - * openid restrictions (by Launchpad teams)
> + * group-based access authorization restrictions (using group information
> +   from external services, currently OpenID with team extensions is
> +   (as used by Launchpad.net) and Atlassian Crowd are supported).

I guess the "is" on the second changed line should be removed.

> + * Auth-Groups: (optional)
> +    Names of groups, members of which are allowed to access protected files.

"Name of groups" should be fine, but better have a
real-English-speaking person review for this one :-)

> -Format-Version: 0.1
> +Format-Version: 0.5

>      def _set(self, key, value):
> +        "key: file pattern, value: list of dicts of field/val pairs"

Can you please split this on two lines?

> === added file 'license_protected_downloads/group_auth_crowd.py'
> --- license_protected_downloads/group_auth_crowd.py     1970-01-01 00:00:00 +0000
> +++ license_protected_downloads/group_auth_crowd.py     2013-06-03 11:56:29 +0000
> @@ -0,0 +1,52 @@
> +import logging
> +
> +from django.conf import settings
> +from django.shortcuts import redirect
> +
> +import requests
> +
> +
> +log = logging.getLogger(__file__)
> +
> +
> +def upgrade_requests():
> +    """Ubuntu 12.04 comes with pretty old requests version. Add convenience
> +    methods of newer versions straight to it, to avoid client-side
> +    workarounds."""
> +    if "json" not in dir(requests.models.Response):
> +        def patchy_json(self):
> +            import json
> +            return json.loads(self.content)
> +        requests.models.Response.json = patchy_json
> +
> +# We monkey-patch requests module on first load
> +upgrade_requests()
> +
> +
> +class GroupAuthError(Exception):
> +    pass
> +
> +
> +def process_group_auth(request, required_groups):
> +    if not required_groups:
> +        return True
> +    if not request.user.is_authenticated():
> +        # Force OpenID login
> +        return redirect(settings.LOGIN_URL + "?next=" + request.path)
> +
> +    log.warn("Authenticating using Crowd API: %s",
> +             request.user.username)
> +
> +    auth = (settings.ATLASSIAN_CROWD_API_USERNAME,
> +            settings.ATLASSIAN_CROWD_API_PASSWORD)
> +    params = {"username": request.user.username}
> +    r = requests.get(settings.ATLASSIAN_CROWD_API_URL
> +                     + "/user/group/nested.json", params=params, auth=auth)
> +    if r.status_code != 200:
> +        raise GroupAuthError(r.status_code)
> +    data = r.json()
> +    user_groups = set([x["name"] for x in data["groups"]])
> +    for g in required_groups:
> +        if g in user_groups:
> +            return True
> +    return False

I know there is nothing wrong with the for loop, but just a suggestion
to avoid it:

return user_groups.isdisjoint(set(required_groups))

Since you already have a set there in place.

> === added directory 'license_protected_downloads/tests/testserver_root/build-info/subdir2'
> === added file 'license_protected_downloads/tests/testserver_root/build-info/subdir2/BUILD-INFO.txt'
> --- license_protected_downloads/tests/testserver_root/build-info/subdir2/BUILD-INFO.txt 1970-01-01 00:00:00 +0000
> +++ license_protected_downloads/tests/testserver_root/build-info/subdir2/BUILD-INFO.txt 2013-06-03 11:56:29 +0000
> @@ -0,0 +1,7 @@
> +Format-Version: 0.1

Hmmm... shouldn't this be bumped to 0.5?

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