Merge lp:~canonical-isd-hackers/canonical-identity-provider/sreg_121533 into lp:canonical-identity-provider/release

[Danny Tamez]

Enhancements to the decide page as per bug 121533

This branch adds enhancements to the decide page. It provides checkboxes that let the user decide whether or not to send individual items requested by the consumer on to the consumer. It remembers the state of those choices for the next time the user is presented with those options. If also employs javascript to condense multiple teams that are requested with a single line and one checkbox for convenience. The user can choose to expand the list and deselect or select individual teams as desired. One part of the bug is not implemented because of another SSO bug and that is the section dealing with additional info requested from the consumer changing the behavior of auto login.

To test:
fab bootstrap
fab setup_postgresql_server
. .env/bin/activate
./django_project/manage.py loaddata tests.json
fab run

Go to http://localhost:8000/consumer
select whatever sreg data you wish to request.
for the teams you can enter the following: ubuntu-team,kubuntu-team,isd-team

[Ricardo Kirkner (ricardokirkner)]

A few changes:

1. No need to add those extra files to the global .bzrignore file as they are not relevant to the project.
2. l. 363: don't like embedding html just for style issues: we should mark the field as required or some other class using css. you can do this by specific the css class in the widget in l. 367
3. l. 376 you're hiding an attribute: instead of changing the meaning of the 'value' attribute, use a different name. a way to do this would be

for key, v in self.data.items():
    if value == v:
        break
else:
    key = None

4. l. 578: why using JSONDecoder, JSONEncoder, instead of just simplejson.loads, simplejson.dumps ? That way you can be sure the returned data is a dictionary, for example.
5. l. 600: this breaks installing the function during the normal server setup
6. l. 680: since you're testing for number of items, if it's one, why do you loop over them? you could just use team_form.fields.0
7. l. 681,684: punctuation should be included in the string to be translated
8. l. 885: not sure that's a valid request. If the method is GET, shouldn't the args be somewhere else? The test is testing a GET request, not an invalid request.
9. l. 1836: why not do 'approved_data = {}'?
10. l. 1837: this can be replaced by "if 'teams' in approved_data" if the previous change was applied
11: l. 1914,1918: use virtualenv('python...') instead of local with the explicit path
12: l. 1917: maybe you can integrate this into the 'test' target directly, instead of having an extra target?

Small issues:

l. 90: fix proper indentation
l. 102: I'd rather have all arguments go into the next line than having a dangling one
l. 277,283,284: break multiple imports into separate lines
l. 359: can just be 'for key in data', or since you're using the value later 'for key, value in data.items()'
l. 374: since you're getting the value anyway, you could just do 'for k, v in self.data.items()' and use v
l. 381,463,1607: use parenthesis for line continuations instead of \
l. 632: \ not needed as parenthesis are already used
l. 1191,1199,1591,1595: use multiple lines for different imports
l. 1477: this can be replaced by a much nicer idiom. something like:

self.patch = patch('settings', 'SSO_RESTRICT_RP', False)
self.patch.start()

and in tearDown

self.patch.stop()

l. 1764: I think you can just use 'if form_data.approved_by_user'

Questions:

l. 318: is this condition intended only for the default case, or should it be reevaluated everytime it's False?
l. 447: does this *have* to be a dict? as you're returning the same value as for the keys.
l. 651: are we using yui somewhere else? if so we may want to just import yui from a template we then include where we need to, so as to make sure we always require the same version of it, and not try to load different versions on separate pages.

Comments:
l. 877: the tearDown for this should not be needed, but I'm fine with it

review: Needs Fixing

[Danny Tamez (zematynnad)]

Made all but a few changes::
#6 below. Fields is a dictionary not a list. Iterating over it returns each key. Trying to do something like teams_form.fields.keys.0 didn't produce the same result - perhaps because of the order in which django applies the . operator.
#8 The point of the test was to show that if the method was GET (even if all the other conditions were the same) the approved fields wouldn't be shown.
#12 This is not using nose so it didn't make sense to merge it with the test() target.

question re: l. 318: I didn't write that portion but as best as I can tell it was only meant to be evaluated with the form's creation so I moved the logic to __init__(). (here and for teams as well)

question re: l. 651, We are using yui in other places and the page does "work" when using <script type="text/javascript" src="/assets/identityprovider/lazr-js/yui/yui-min.js"></script> but the page always first shows the teams expanded and then collapses them. It doesn't do that with the yui-min from the cdn.

[ISD Branch Mangler (isd-branches-mangler)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file '.bzrignore'
--- .bzrignore	2011-06-17 15:34:24 +0000
+++ .bzrignore	2011-07-05 15:46:33 +0000
@@ -11,3 +11,5 @@
 .coverage*
 coverage
 *.xml
+tags
+.backup

=== modified file 'django_project/config_dev/urls.py'
--- django_project/config_dev/urls.py	2011-06-17 12:59:08 +0000
+++ django_project/config_dev/urls.py	2011-07-05 15:46:33 +0000
@@ -5,8 +5,6 @@
 import preflight
 import preflight.urls
 
-from identityprovider.decorators import check_readonly
-
 admin.autodiscover()
 preflight.autodiscover()
 
@@ -23,11 +21,10 @@
     (r'^preflight/', include(preflight.urls)),
     (r'^', include('identityprovider.urls')),
 )
-
 if settings.SERVE_STATIC_MEDIA:
     urlpatterns += patterns('',
         (r'assets/identityprovider/(.*)', 'django.views.static.serve',
             {'document_root': settings.SSO_MEDIA_ROOT}),
         (r'media/(.*)', 'django.views.static.serve',
             {'document_root': settings.MEDIA_ROOT}),
-    )
+)

=== modified file 'identityprovider/api10/handlers.py'
--- identityprovider/api10/handlers.py	2011-06-15 22:24:43 +0000
+++ identityprovider/api10/handlers.py	2011-07-05 15:46:33 +0000
@@ -37,7 +37,7 @@
     NewCaptchaError,
     VerifyCaptchaError,
 )
-from identityprovider.views.server import get_team_memberships
+from identityprovider.models.team import TeamMembership
 from identityprovider.signals import (
     account_created,
     account_email_validated,
@@ -78,6 +78,7 @@
         return seria
 Emitter.register('lazr.restful', LazrRestfulEmitter, 'application/json')
 
+
 class LazrRestfulHandler(BaseHandler):
     allowed_methods = ('GET', 'POST')
 
@@ -140,6 +141,7 @@
         else:
             return self.response
 
+
 class CaptchaHandler(LazrRestfulHandler):
     response = {
         "total_size": 0,
@@ -280,7 +282,7 @@
             'status': 'ok',
             'message': "Password changed"
         }
-    
+
 
 def _serialize_account(user):
     emails = EmailAddress.objects.filter(account=user,
@@ -354,7 +356,6 @@
         application_token_invalidated.send(
             sender=self, openid_identifier=data['consumer_key'])
 
-
     @api_user_required
     @named_operation
     def team_memberships(self, request):
@@ -365,8 +366,9 @@
 
         if len(accounts) == 1:
             account = accounts[0]
-            memberships = get_team_memberships(data['team_names'],
-                account, False)
+            memberships = (
+                TeamMembership.get_team_memberships_for_user(
+                    data['team_names'], account, False))
             return memberships
         else:
             return []
@@ -402,7 +404,10 @@
     @named_operation
     def team_memberships(self, request):
         team_names = request.data['team_names']
-        memberships = get_team_memberships(team_names, request.user, True)
+
+        memberships = (
+            TeamMembership.get_team_memberships(
+                team_names, request.user, True))
         return memberships
 
     @named_operation

=== modified file 'identityprovider/fixtures/test.json'
--- identityprovider/fixtures/test.json	2011-03-08 14:31:20 +0000
+++ identityprovider/fixtures/test.json	2011-07-05 15:46:33 +0000
@@ -173,6 +173,28 @@
         }
     },
     {
+        "pk": 46,
+        "model": "identityprovider.emailaddress",
+        "fields": {
+            "status": 4,
+            "lp_person": 18,
+            "account": null,
+            "email": "[e-mail]",
+            "date_created": "2006-10-16 18:31:43.621341"
+        }
+    },
+    {
+        "pk": 47,
+        "model": "identityprovider.emailaddress",
+        "fields": {
+            "status": 4,
+            "lp_person": 19,
+            "account": null,
+            "email": "[e-mail]",
+            "date_created": "2006-10-16 18:31:43.621341"
+        }
+    },
+    {
         "pk": 61,
         "model": "identityprovider.emailaddress",
         "fields": {
@@ -367,6 +389,88 @@
         }
     },
     {
+        "pk": 18,
+        "model": "identityprovider.person",
+        "fields": {
+            "displayname": "Kubuntu Team",
+            "teamowner": 1,
+            "teamdescription": "This Team is responsible for the Kubuntu Distribution",
+            "name": "kubuntu-team",
+            "language": null,
+            "fti": "'team':3A,5A 'kubuntu':2A,4A 'kubuntu-team':1A",
+            "defaultmembershipperiod": null,
+            "defaultrenewalperiod": null,
+            "subscriptionpolicy": 1,
+            "merged": null,
+            "datecreated": "2005-06-06 08:59:51.60576",
+            "addressline1": null,
+            "addressline2": null,
+            "organization": null,
+            "city": null,
+            "province": null,
+            "country": null,
+            "postcode": null,
+            "phone": null,
+            "homepage_content": null,
+            "icon": null,
+            "mugshot": null,
+            "hide_email_addresses": false,
+            "creation_rationale": null,
+            "creation_comment": null,
+            "registrant": null,
+            "logo": null,
+            "renewal_policy": 10,
+            "personal_standing": 0,
+            "personal_standing_reason": null,
+            "mail_resumption_date": null,
+            "mailing_list_auto_subscribe_policy": 1,
+            "mailing_list_receive_duplicates": true,
+            "visibility": 1,
+            "verbose_bugnotifications": false
+        }
+    },
+    {
+        "pk": 19,
+        "model": "identityprovider.person",
+        "fields": {
+            "displayname": "ISD Team",
+            "teamowner": 1,
+            "teamdescription": "This Team is responsible for ISD",
+            "name": "isd-team",
+            "language": null,
+            "fti": "'team':3A,5A 'isd':2A,4A 'isd-team':1A",
+            "defaultmembershipperiod": null,
+            "defaultrenewalperiod": null,
+            "subscriptionpolicy": 1,
+            "merged": null,
+            "datecreated": "2005-06-06 08:59:51.60576",
+            "addressline1": null,
+            "addressline2": null,
+            "organization": null,
+            "city": null,
+            "province": null,
+            "country": null,
+            "postcode": null,
+            "phone": null,
+            "homepage_content": null,
+            "icon": null,
+            "mugshot": null,
+            "hide_email_addresses": false,
+            "creation_rationale": null,
+            "creation_comment": null,
+            "registrant": null,
+            "logo": null,
+            "renewal_policy": 10,
+            "personal_standing": 0,
+            "personal_standing_reason": null,
+            "mail_resumption_date": null,
+            "mailing_list_auto_subscribe_policy": 1,
+            "mailing_list_receive_duplicates": true,
+            "visibility": 1,
+            "verbose_bugnotifications": false
+        }
+    },
+    {
         "pk": 243610,
         "model": "identityprovider.person",
         "fields": {
@@ -501,6 +605,22 @@
         }
     },
     {
+        "pk": 2,
+        "model": "identityprovider.teamparticipation",
+        "fields": {
+            "team": 18,
+            "person": 1
+        }
+    },
+    {
+        "pk": 3,
+        "model": "identityprovider.teamparticipation",
+        "fields": {
+            "team": 19,
+            "person": 1
+        }
+    },
+    {
         "pk": 158,
         "model": "identityprovider.teamparticipation",
         "fields": {
@@ -524,7 +644,14 @@
             "person": 243610
         }
     },
-
+    {
+        "pk": 164,
+        "model": "identityprovider.teamparticipation",
+        "fields": {
+            "team": 18,
+            "person": 243610
+        }
+    },
     {
         "pk": 5,
         "model": "identityprovider.openidrpsummary",

=== modified file 'identityprovider/forms.py'
--- identityprovider/forms.py	2010-09-21 22:04:10 +0000
+++ identityprovider/forms.py	2011-07-05 15:46:33 +0000
@@ -5,17 +5,33 @@
 
 from django import forms
 from django.contrib import auth
-from django.forms import fields, widgets
+from django.forms import Form, fields, widgets
+from django.utils import translation
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext as _
 
-from identityprovider.models import Account, EmailAddress, verify_token_string
+from identityprovider.const import (
+    SREG_DATA_FIELDS_ORDER,
+    SREG_LABELS,
+)
+from identityprovider.models import (
+    Account,
+    EmailAddress,
+    verify_token_string,
+    TeamMembership,
+)
 from identityprovider.models.const import EmailStatus
 from identityprovider.utils import (
-    get_person_and_account_by_email, CannotResetPasswordException,
-    PersonAndAccountNotFoundException, encrypt_launchpad_password,
-    password_policy_compliant)
-from identityprovider.widgets import ROAwareTextInput, ROAwareSelect
+    get_person_and_account_by_email,
+    CannotResetPasswordException,
+    PersonAndAccountNotFoundException,
+    encrypt_launchpad_password,
+    password_policy_compliant,
+)
+from identityprovider.widgets import (
+    ROAwareTextInput,
+    ROAwareSelect,
+)
 
 logger = logging.getLogger('sso')
 
@@ -289,3 +305,159 @@
 class PreAuthorizeForm(forms.Form):
     trust_root = forms.CharField(error_messages=default_errors)
     callback = forms.CharField(error_messages=default_errors)
+
+
+class SRegRequestForm(Form):
+    """A form object for user control over OpenID sreg data.
+    """
+    fields = {}
+
+    @property
+    def data_approved_by_user(self):
+        """Get the list of sreg data actually approved by the user for the
+        request.
+        """
+        if self.request_method == 'POST':
+            return dict([(f, self.data[f]) for f in self.data
+                if self.field_approved(f)])
+        else:
+            return {}
+
+    def __init__(self, request, sreg_request, rpconfig, approved_data=None):
+        self.request = request
+        self.request_method = request.META.get('REQUEST_METHOD')
+        self.sreg_request = sreg_request
+        self.rpconfig = rpconfig
+        self.approved_data = approved_data
+        sreg_fields = [f for f in SREG_DATA_FIELDS_ORDER if f in set(
+            self.sreg_request.required + self.sreg_request.optional)]
+        self.data = self._get_sreg_data_for_user(request.user, sreg_fields)
+
+        super(SRegRequestForm, self).__init__(self.data)
+        self._init_fields(self.data)
+        self.should_display = len(self.data) > 0
+
+    def _get_sreg_data_for_user(self, user, sreg_fields):
+        """Get the sreg data to ask about in the form based on the user's
+        account record.
+        """
+        values = {}
+        values['fullname'] = user.displayname
+        if user.preferredemail is not None:
+            values['email'] = user.preferredemail.email
+        if user.person is not None:
+            values['nickname'] = user.person.name
+            if user.person.time_zone is not None:
+                values['timezone'] = user.person.time_zone
+        if user.preferredlanguage is not None:
+            values['language'] = user.preferredlanguage
+        else:
+            values['language'] = translation.get_language_from_request(
+                self.request)
+        logger.debug("values (sreg_fields) = " + str(values))
+
+        return dict([(f, values[f]) for f in sreg_fields if f in values])
+
+    def _init_fields(self, data):
+        """Initialises form fields for the user's sreg data.
+        """
+        for key, val in data.items():
+            label = "%s: %s" % (SREG_LABELS.get(key, key), val)
+            attrs = {}
+            if key in self.sreg_request.required:
+                attrs['class'] = 'required'
+                if self.rpconfig is not None:
+                    attrs['disabled'] = 'disabled'
+            self.fields[key] = fields.BooleanField(
+                label=label, widget=forms.CheckboxInput(attrs=attrs,
+                    check_test=self.check_test))
+
+    def check_test(self, value):
+        """Determines if a checkbox should be pre-checked based on previously
+        approved user data, openid request and relying party type.
+        """
+        for k, v in self.data.items():
+            if value == v:
+                value = k
+                break
+
+        if self.rpconfig and value in self.sreg_request.required:
+            return True
+        elif (self.approved_data and
+                value in self.approved_data.get('requested', [])):
+            return value in self.approved_data.get('approved', [])
+        elif self.rpconfig:
+            return True
+        else:
+            return value in self.sreg_request.required
+
+    def field_approved(self, field):
+        """Check if the field should be returned in the response based on user
+        preferences and overridden for trusted relying parties.
+        """
+        if self.rpconfig is not None and field in self.sreg_request.required:
+            return True
+        elif field in self.request.POST:
+            return True
+        else:
+            return False
+
+
+class TeamsRequestForm(Form):
+    """A form object for user control over OpenID teams data.
+    """
+
+    fields = {}
+
+    @property
+    def teams_approved_by_user(self):
+        """Get the list of teams actually approved by the user for the request.
+        """
+        if self.request_method == 'POST':
+            return [t for t in self.data if t in self.request.POST]
+        else:
+            return []
+
+    def __init__(self, request, teams_request, rpconfig, approved_data=None):
+        self.request = request
+        self.request_method = request.META.get('REQUEST_METHOD')
+        self.teams_request = teams_request
+        self.rpconfig = rpconfig
+        self.approved_data = approved_data
+        self.data = self._get_teams_for_user(request.user,
+            rpconfig and rpconfig.can_query_any_team)
+
+        super(TeamsRequestForm, self).__init__(self.data)
+        self._init_fields(self.data)
+        all_teams = self.teams_request.allRequestedTeams()
+        membership = TeamMembership.get_team_memberships_for_user(
+            all_teams, self.request.user,
+            self.rpconfig and self.rpconfig.can_query_any_team)
+        self.should_display = len(membership) > 0
+
+    def _get_teams_for_user(self, user, include_private=False):
+        """Get the list of teams to ask about in the form based on the user's
+        team membership.
+        """
+        all_teams = self.teams_request.allRequestedTeams()
+        return dict([(t, t) for t in
+            TeamMembership.get_team_memberships_for_user(all_teams, user,
+                include_private)])
+
+    def _init_fields(self, form_data):
+        """Initialises form fields for the user's team memberships.
+        """
+        for team in form_data:
+            self.fields[team] = fields.BooleanField(
+                label=team, widget=forms.CheckboxInput(
+                    check_test=self.check_test))
+
+    def check_test(self, value):
+        """Determines if a checkbox should be pre-checked based on previously
+        approved user data and relying party type.
+        """
+        if (self.approved_data and
+            value in self.approved_data.get('requested', [])):
+            return value in self.approved_data.get('approved', [])
+        else:
+            return self.rpconfig != None

=== modified file 'identityprovider/media/ubuntu/narrow.css'
--- identityprovider/media/ubuntu/narrow.css	2010-09-18 05:38:59 +0000
+++ identityprovider/media/ubuntu/narrow.css	2011-07-05 15:46:33 +0000
@@ -36,7 +36,12 @@
     margin: 0px;
 }
 
-#mainbar form input[type="text"], #mainbar form input[type="password"], form textarea, #content input {
+#mainbar form input[type="text"],
+#mainbar form input[type="password"],
+form textarea,
+#content input[type="text"],
+#content input[type="password"]
+{
     width: 96%;
     padding: 2px;
 }

=== modified file 'identityprovider/media/ubuntu/styles.css'
--- identityprovider/media/ubuntu/styles.css	2010-11-16 15:02:08 +0000
+++ identityprovider/media/ubuntu/styles.css	2011-07-05 15:46:33 +0000
@@ -249,7 +249,14 @@
 ul li:last-child {
     margin-bottom: 0;
 }
-
+ul.teams-list {
+    list-style: none;
+    margin-left: 20px;
+    margin-top: 5px;
+}
+.required + label {
+    font-weight: bold;
+}
 p.call-to-action {
     color: #333;
 }
@@ -711,7 +718,12 @@
         margin: 0px;
     }
 
-    #mainbar form input[type="text"], #mainbar form input[type="password"], form textarea, #content input {
+    #mainbar form input[type="text"],
+    #mainbar form input[type="password"],
+    form textarea,
+    #content input[type="text"],
+    #content input[type="password"]
+    {
         width: 96%;
         padding: 2%;
     }

=== modified file 'identityprovider/models/account.py'
--- identityprovider/models/account.py	2011-05-12 13:44:51 +0000
+++ identityprovider/models/account.py	2011-07-05 15:46:33 +0000
@@ -271,7 +271,6 @@
         except Consumer.DoesNotExist:
             return []
 
-
     def save(self, force_insert=False, force_update=False, **kwargs):
         if settings.READ_ONLY_MODE:
             return
@@ -321,4 +320,4 @@
         verbose_name_plural = _("LP OpenID Identifiers")
 
     def __unicode__(self):
-        return _("LP OpenID Idetifier for %s") % unicode(self.account)
+        return _("LP OpenID Identifier for %s") % unicode(self.lp_account)

=== modified file 'identityprovider/models/openidmodels.py'
--- identityprovider/models/openidmodels.py	2010-11-16 14:11:00 +0000
+++ identityprovider/models/openidmodels.py	2011-07-05 15:46:33 +0000
@@ -12,6 +12,7 @@
 
 from django.conf import settings
 from django.db import models
+from django.utils import simplejson as json
 from django.utils.translation import ugettext_lazy as _
 
 from identityprovider.const import NEVER_EXPIRES, SREG_LABELS
@@ -139,7 +140,7 @@
             select={'len': 'length(trust_root)'},
             where=[condition],
             params=(url, url),
-            order_by=['-len'] # Select the longest match
+            order_by=['-len']  # Select the longest match
             )
         return q[0] if q else None
 
@@ -177,7 +178,8 @@
 
 class OpenIDRPSummaryManager(models.Manager):
 
-    def record(self, account, trust_root, openid_identifier=None):
+    def record(self, account, trust_root, openid_identifier=None,
+               approved_data=None):
         if settings.READ_ONLY_MODE:
             return None
         if openid_identifier is None:
@@ -193,6 +195,8 @@
                 account=account,
                 trust_root=trust_root,
                 openid_identifier=openid_identifier)
+        if approved_data:
+            summary.set_approved_data(approved_data)
         return summary
 
 
@@ -205,6 +209,7 @@
     date_last_used = models.DateTimeField(default=datetime.datetime.utcnow,
         blank=True, editable=False)
     total_logins = models.IntegerField(default=1)
+    approved_data = models.TextField(blank=True, null=True, default='')
 
     objects = OpenIDRPSummaryManager()
 
@@ -218,6 +223,18 @@
         self.date_last_used = datetime.datetime.utcnow()
         self.save()
 
+    def get_approved_data(self):
+        try:
+            data = json.loads(self.approved_data)
+            assert(type(data) is dict)
+            return data
+        except (AssertionError, TypeError, ValueError):
+            return None
+
+    def set_approved_data(self, approved_data):
+        self.approved_data = json.dumps(approved_data)
+        self.save()
+
 
 class DjangoOpenIDStore(OpenIDStore):
     """

=== modified file 'identityprovider/models/team.py'
--- identityprovider/models/team.py	2010-04-21 15:29:24 +0000
+++ identityprovider/models/team.py	2011-07-05 15:46:33 +0000
@@ -3,6 +3,7 @@
 
 from django.db import models
 
+from identityprovider.const import PERSON_VISIBILITY_PUBLIC
 from identityprovider.models import Person
 
 __all__ = (
@@ -47,6 +48,24 @@
         db_table = u'teammembership'
         unique_together = ('person', 'team')
 
+    @staticmethod
+    def get_team_memberships_for_user(team_names, user, include_private=False):
+        memberships = []
+        for team_name in team_names:
+            try:
+                team = Person.objects.get(name=team_name)
+            except Person.DoesNotExist:
+                team = None
+            if team is None or not team.is_team():
+                continue
+            # Control access to private teams
+            if (team.visibility != PERSON_VISIBILITY_PUBLIC and
+                    not include_private):
+                continue
+            if user.person_in_team(team):
+                memberships.append(team_name)
+        return memberships
+
 
 class TeamParticipation(models.Model):
     team = models.ForeignKey(Person, db_column='team',

=== modified file 'identityprovider/teams.py'
--- identityprovider/teams.py	2010-04-21 15:29:24 +0000
+++ identityprovider/teams.py	2011-07-05 15:46:33 +0000
@@ -39,8 +39,10 @@
 @since: 2.1.1
 """
 
-from openid.message import registerNamespaceAlias, \
-     NamespaceAliasRegistrationError
+from openid.message import (
+    registerNamespaceAlias,
+    NamespaceAliasRegistrationError,
+)
 from openid.extension import Extension
 from openid import oidutil
 

=== modified file 'identityprovider/templates/decide.html'
--- identityprovider/templates/decide.html	2011-05-11 13:02:21 +0000
+++ identityprovider/templates/decide.html	2011-07-05 15:46:33 +0000
@@ -6,6 +6,11 @@
 GNU Affero General Public License version 3 (see the file  LICENSE).
 {% endcomment %}
 
+{% block extra_header %}
+-    <script src="http://yui.yahooapis.com/3.3.0/build/yui/yui-min.js"></script>
+{% endblock %}
+
+
 {% block title %}
     {% blocktrans %}Authenticate to {{ trust_root }}{% endblocktrans %}
 {% endblock %}
@@ -27,25 +32,35 @@
             {% blocktrans %}If you proceed, the following information will be available to the website:{% endblocktrans %}
         {% endif %}
 
+    <div class="actions">
+        <form action="{{ action }}" method="POST" name="decideform">
+            {% csrf_token %}
         <div class="info-items">
-            {% if sreg_data or teams_data %}
+            {% if sreg_form.should_display or teams_form.should_display %}
             <ul class="list">
-            {% for key, value in sreg_data %}
-                <li>{{ key }}: <b>{{ value }}</b></li>
-            {% endfor %}
-            {% if teams_data %}
-                <li>{% trans "Team membership"%}: <b>{{ teams_data|join:", " }}</b></li>
-            {% endif %}
+                {% for field in sreg_form %}
+                <li>{{ field|safe }} {{ field.label_tag }}</li>
+                {% endfor %}
+                {% if teams_form.should_display %}
+                    {% ifequal teams_form.fields|length 1 %}
+                        {% for field in teams_form %}
+                <li>{{ field|safe }} {% trans "Team membership:"%} {{ field.label_tag }}</li>
+                        {% endfor %}
+                    {% else %}
+                <li id="teamslist_item">{% trans "Team membership:"%}
+                    <ul class="teams-list" id="teamslist">
+                        {% for field in teams_form %}
+                        <li>{{ field|safe }} {{ field.label_tag }}</li>
+                        {% endfor %}
+                    </ul>
+                </li>
+                    {% endifequal %}
+                {% endif %}
             </ul>
-            {% else %}
-                <p>{% trans "Only your identity URL" %}</p>
             {% endif %}
         </div>
     </div>
 
-    <div class="actions">
-        <form action="{{ action }}" method="POST" name="decideform">
-			{% csrf_token %}
             <p>
                 <button type="submit" class="btn" name="yes"><span><span>{% trans "Yes, sign me in" %}</span></span></button>
                 {% trans "or" %}
@@ -54,6 +69,78 @@
         </form>
         <script type="text/javascript">
         document.decideform.yes.focus();
+
+        YUI().use('anim-base', function(Y) {
+            var anim = new Y.Anim({
+                node: '#teamslist',
+                duration: 0.15,
+                to: { height: 0 },
+            });
+
+            var onClick = function(e) {
+                e.preventDefault();
+                Y.one('#reveal_wrapper').remove();
+                node = Y.one('#teamslist');
+                node.setStyle('height', '0');
+                node.setStyle('display', 'block');
+                anim.setAttrs({ 'to': { height: teamslist_height } });
+                anim.run();
+            };
+
+            var setTeamStates = function(e) {
+                state = e.currentTarget.get('checked');
+                nl = Y.all('#teamslist li input').set('checked', state);
+            }
+
+            var refreshTeamStates = function(e) {
+                nl = Y.all('#teamslist li input');
+                checked = 0;
+                total = nl.size();
+                for (i = 0; i < nl.size(); i++) {
+                    if (nl.item(i).get('checked')) {
+                        checked ++;
+                    }
+                }
+                if (checked == 0) {
+                    Y.one('#checkme').set('checked', false);
+                } else {
+                    Y.one('#checkme').set('checked', true);
+                }
+            }
+
+            Y.on("domready", function() {
+                // don't error out because no teams matched
+                if (Y.one('#teamslist_item') == null) {
+                    return;
+                }
+                Y.one('#teamslist_item').prepend('<input type="checkbox" id="checkme" /> ');
+                Y.one('#checkme').on('change', setTeamStates);
+                Y.all('#teamslist li').on('change', refreshTeamStates);
+                refreshTeamStates();
+
+                nl = Y.all('#teamslist li input');
+                checked = 0;
+                total = nl.size();
+                for (i = 0; i < nl.size(); i++) {
+                    if (nl.item(i).get('checked')) {
+                        checked ++;
+                    }
+                }
+                if (checked == total || checked == 0) {
+                    node = Y.one('#teamslist');
+                    teamslist_height = parseInt(node.getComputedStyle('height'));
+                    node.setStyle('display', 'none');
+                    buf = new Array();
+                    c = node.get('children');
+                    for (i = 0; i < c.size(); i++) {
+                        buf.push(c.item(i).get('text'));
+                    }
+                    node.insert('<span id="reveal_wrapper">' + buf.join(', ') + ' <small style="font-size: 80%;">(<a href="#" id="reveal">details</a>)</small></span>', "before");
+                    Y.one('#reveal').on('click', onClick);
+                }
+            });
+        });
+
         </script>
     </div>
     <br style="clear: both" />

=== modified file 'identityprovider/tests/test_admin.py'
--- identityprovider/tests/test_admin.py	2011-01-03 21:58:15 +0000
+++ identityprovider/tests/test_admin.py	2011-07-05 15:46:33 +0000
@@ -19,6 +19,7 @@
 class AdminTestCase(BasicAccountTestCase):
 
     fixtures = ['admin.json', 'test.json']
+    pgsql_functions = ['generate_openid_identifier']
 
     def setUp(self):
         self.disable_csrf()
@@ -41,8 +42,10 @@
                 'allowed_sreg': 'fullname',
                 'creation_rationale': '13',
                 }
-        response = self.client.get('/admin/identityprovider/openidrpconfig/add/')
-        response = self.client.post('/admin/identityprovider/openidrpconfig/add/',
+        response = self.client.get(
+            '/admin/identityprovider/openidrpconfig/add/')
+        response = self.client.post(
+            '/admin/identityprovider/openidrpconfig/add/',
             data)
         self.assertEquals(302, response.status_code)
         # We don't get the ID back, so continue on to the next test to

=== modified file 'identityprovider/tests/test_forms.py'
--- identityprovider/tests/test_forms.py	2011-02-20 23:49:21 +0000
+++ identityprovider/tests/test_forms.py	2011-07-05 15:46:33 +0000
@@ -4,10 +4,20 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from django.conf import settings
+from django.http import HttpRequest
+from django_openid_auth.teams import TeamsRequest
+from openid.extensions.sreg import SRegRequest
+
 from identityprovider.models.account import Account
-from identityprovider.forms import EditAccountForm, ResetPasswordForm
-from identityprovider.api10 import forms
+from identityprovider.forms import (
+    EditAccountForm,
+    ResetPasswordForm,
+    SRegRequestForm,
+    TeamsRequestForm,
+)
 from identityprovider.api10.forms import WebserviceCreateAccountForm
+from identityprovider.models.openidmodels import OpenIDRPConfig
+
 from utils import BasicAccountTestCase, SQLCachedTestCase
 
 
@@ -51,7 +61,7 @@
         account = Account.objects.get(openid_identifier='name12_oid')
         form = EditAccountForm(account=account,
                                data={'password': 'tooweak',
-                                     'passwordconfirm' : 'other'})
+                                     'passwordconfirm': 'other'})
         self.assertFalse(form.is_valid())
         self.assertEqual(1, len(form.errors['password']))
         self.assertFalse('passwordconfirm' in form.errors)
@@ -61,7 +71,7 @@
         account = Account.objects.get(openid_identifier='name12_oid')
         form = EditAccountForm(account=account,
                                data={'password': 'tooweak',
-                                     'passwordconfirm' : 'tooweak'})
+                                     'passwordconfirm': 'tooweak'})
         self.assertFalse(form.is_valid())
         self.assertEqual(1, len(form.errors['password']))
         self.assertFalse('passwordconfirm' in form.errors)
@@ -94,3 +104,249 @@
         self.assertFalse(form.is_valid())
         self.assertEqual(form.errors['password'][0],
                          'Invalid characters in password')
+
+
+class SRegRequestFormTest(SQLCachedTestCase):
+    pgsql_functions = ['generate_openid_identifier']
+
+    def _get_request_with_post_args(self, args={}):
+        request = HttpRequest()
+        request.user = self.test_user
+        request.POST = args
+        request.META = {'REQUEST_METHOD': 'POST'}
+        return request
+
+    def setUp(self):
+        self.test_user = Account.objects.create_account('My name',
+            '[e-mail]', 'password')
+        self.rpconfig = OpenIDRPConfig.objects.create(
+            trust_root='http://localhost/', description="Some description")
+
+    def test_no_approved_fields_without_post_request(self):
+        """The server should not generate a list of approved fields when the
+        request is not a POST request.
+        """
+        request = self._get_request_with_post_args()
+        request.META['REQUEST_METHOD'] = 'GET'
+        form = SRegRequestForm(
+            request,
+            SRegRequest(required=['fullname', 'email']),
+            self.rpconfig)
+        self.assertEqual(len(form.data_approved_by_user), 0)
+
+    def test_required_fields_for_trusted_site(self):
+        """The server should always return values for required fields to trusted
+        sites, regardless of the state of the checkbox in the UI.  Optional
+        fields should not be returned if the user has unchecked them.
+        """
+        form = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            self.rpconfig)
+        self.assertTrue('fullname' in form.data_approved_by_user)
+        self.assertFalse('email' in form.data_approved_by_user)
+
+    def test_optional_fields_for_trusted_site(self):
+        """The server should return values for optional fields to trusted
+        sites only when the user checks the checkbox in the UI.
+        """
+        post_args = {'email': 'email'}
+        form = SRegRequestForm(
+            self._get_request_with_post_args(post_args),
+            SRegRequest(optional=['fullname', 'email']),
+            self.rpconfig)
+        self.assertFalse('fullname' in form.data_approved_by_user)
+        self.assertTrue('email' in form.data_approved_by_user)
+
+    def test_required_fields_for_untrusted_site(self):
+        """The server should return values for required fields to untrusted
+        sites only when the user checks the checkbox in the UI.
+        """
+        post_args = {'email': 'email'}
+        form = SRegRequestForm(
+            self._get_request_with_post_args(post_args),
+            SRegRequest(required=['fullname', 'email']),
+            None)
+        self.assertFalse('fullname' in form.data_approved_by_user)
+        self.assertTrue('email' in form.data_approved_by_user)
+
+    def test_optional_fields_for_untrusted_site(self):
+        """The server should return values for optional fields to untrusted
+        sites only when the user checks the checkbox in the UI.
+        """
+        post_args = {'fullname': 'fullname'}
+        form = SRegRequestForm(
+            self._get_request_with_post_args(post_args),
+            SRegRequest(optional=['fullname', 'email']),
+            None)
+        self.assertTrue('fullname' in form.data_approved_by_user)
+        self.assertFalse('email' in form.data_approved_by_user)
+
+    def test_checkbox_status_for_trusted_site(self):
+        """Checkboxes are always checked if the site is trusted
+        """
+        form = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            self.rpconfig)
+        self.assertTrue(form.check_test('fullname'))
+        self.assertTrue(form.check_test('email'))
+
+    def test_checkbox_status_for_trusted_site_with_approved_data(self):
+        """If the user has previously approved sending data to a trusted site
+        the same checkbox settings should be returned on the next request unless
+        those conflict with the required fields.
+        """
+        approved_data = {
+            'requested': ['fullname', 'email'],
+            'approved': ['email']}
+        form1 = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            self.rpconfig, approved_data=approved_data)
+        self.assertTrue(form1.check_test('fullname'))
+        self.assertTrue(form1.check_test('email'))
+
+        approved_data['approved'] = []
+        form2 = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            self.rpconfig, approved_data=approved_data)
+        self.assertFalse(form2.check_test('email'))
+
+    def test_checkbox_status_for_untrusted_site(self):
+        """Checkboxes are only checked on untrusted site requests if the field
+        is required
+        """
+        form = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            None)
+        self.assertTrue(form.check_test('fullname'))
+        self.assertFalse(form.check_test('email'))
+
+    def test_checkbox_status_for_untrusted_site_with_approved_data(self):
+        """If the user has previously approved sending data to an untrusted site
+        the same checkbox settings should be returned on the next request.
+        """
+        approved_data = {
+            'requested': ['fullname', 'email'],
+            'approved': ['email']}
+        form = SRegRequestForm(
+            self._get_request_with_post_args(),
+            SRegRequest(required=['fullname'], optional=['email']),
+            None, approved_data=approved_data)
+        self.assertFalse(form.check_test('fullname'))
+        self.assertTrue(form.check_test('email'))
+
+
+class TeamsRequestFormTest(BasicAccountTestCase):
+
+    fixtures = ["test"]
+
+    def _get_request_with_post_args(self, args={}):
+        request = HttpRequest()
+        request.user = self.account
+        request.POST = args
+        request.META = {'REQUEST_METHOD': 'POST'}
+        return request
+
+    def setUp(self):
+        self.account = Account.objects.get(pk=1)
+        self.rpconfig = OpenIDRPConfig.objects.create(
+            trust_root='http://localhost/', description="Some description",
+            can_query_any_team=True)
+
+    def test_selected_teams_for_trusted_sites(self):
+        """If a user checks a requested team in the form for a trusted consumer,
+        it should be in the list of teams approved by the user.
+        """
+        post_args = {'ubuntu-team': 'ubuntu-team'}
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(post_args),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            self.rpconfig)
+        self.assertTrue('ubuntu-team' in form.teams_approved_by_user)
+
+    def test_unselected_teams_for_trusted_sites(self):
+        """If a user unchecks a requested team in the form for a trusted
+        consumer, it should not be in the list of teams approved by the user.
+        """
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            self.rpconfig)
+        self.assertFalse('ubuntu-team' in form.teams_approved_by_user)
+
+    def test_selected_teams_for_untrusted_sites(self):
+        """If a user checks a requested team in the form for an untrusted
+        consumer, it should be in the list of teams approved by the user.
+        """
+        post_args = {'ubuntu-team': 'ubuntu-team'}
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(post_args),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            None)
+        self.assertTrue('ubuntu-team' in form.teams_approved_by_user)
+
+    def test_unselected_teams_for_untrusted_sites(self):
+        """If a user unchecks a requested team in the form for an untrusted
+        consumer, it should not be in the list of teams approved by the user.
+        """
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            None)
+        self.assertFalse('ubuntu-team' in form.teams_approved_by_user)
+
+    def test_checkbox_status_for_trusted_site(self):
+        """Checkboxes should always be checked by default for trusted sites.
+        """
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            self.rpconfig)
+        self.assertTrue(form.check_test('ubuntu-team'))
+
+    def test_checkbox_status_for_trusted_site_with_approved_data(self):
+        """Checkboxes should respect user preferences on trusted sites where
+        available.
+        """
+        approved_data = {
+            'requested': ['ubuntu-team', 'myteam'],
+            'approved': ['myteam']}
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team', 'myteam']),
+            self.rpconfig, approved_data=approved_data)
+        self.assertFalse(form.check_test('ubuntu-team'))
+        self.assertTrue(form.check_test('myteam'))
+
+    def test_checkbox_status_for_untrusted_site(self):
+        """Checkboxes should always be checked by default for trusted sites.
+        """
+        form = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team']),
+            None)
+        self.assertFalse(form.check_test('ubuntu-team'))
+
+    def test_checkbox_status_for_untrusted_site_with_approved_data(self):
+        """Checkboxes should respect user preferences on untrusted sited where
+        available.
+        """
+        approved_data = {
+            'requested': ['ubuntu-team'],
+            'approved': ['ubuntu-team']}
+        form1 = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team', 'myteam']),
+            None, approved_data=approved_data)
+        self.assertTrue(form1.check_test('ubuntu-team'))
+
+        approved_data['approved'] = []
+        form2 = TeamsRequestForm(
+            self._get_request_with_post_args(),
+            TeamsRequest(query_membership=['ubuntu-team', 'myteam']),
+            None, approved_data=approved_data)
+        self.assertFalse(form2.check_test('ubuntu-team'))

=== modified file 'identityprovider/tests/test_models_openidmodels.py'
--- identityprovider/tests/test_models_openidmodels.py	2010-07-09 13:44:39 +0000
+++ identityprovider/tests/test_models_openidmodels.py	2011-07-05 15:46:33 +0000
@@ -158,7 +158,7 @@
         self.expires = datetime.now() + timedelta(1)
 
     def create_auth(self):
-        return OpenIDAuthorization.objects.create( account=self.account,
+        return OpenIDAuthorization.objects.create(account=self.account,
             client_id=None, date_expires=self.expires,
             trust_root=self.trust_root)
 
@@ -219,13 +219,19 @@
 
 
 class OpenIDRPSummaryTestCase(BasicAccountTestCase):
+    approved_data = {
+        'test1': {'one': [1, 2], 'two': [2, 3]},
+        'test2': {'three': [3, 4], 'four': [4, 5]}}
+
     def setUp(self):
         self.account = Account.objects.get_by_email('[e-mail]')
         self.trust_root = 'http://openid.launchpad.dev'
 
     def create_summary(self):
-        return OpenIDRPSummary.objects.create(account=self.account,
+        summary = OpenIDRPSummary.objects.create(account=self.account,
             trust_root=self.trust_root, openid_identifier='oid')
+        summary.set_approved_data(self.approved_data)
+        return summary
 
     def test_record_when_readonly(self):
         rm = ReadOnlyManager()
@@ -244,6 +250,7 @@
     def test_record_with_existing_and_no_openid_identifier(self):
         summary1 = self.create_summary()
         summary2 = OpenIDRPSummary.objects.record(self.account, self.trust_root)
+        summary2.set_approved_data(self.approved_data)
         self.assertEqual(summary1.total_logins, 1)
         self.assertEqual(summary2.total_logins, 1)
         self.assertNotEqual(summary1, summary2)
@@ -258,7 +265,8 @@
     def test_record_existing_different_oid(self):
         summary1 = self.create_summary()
         summary2 = OpenIDRPSummary.objects.record(self.account,
-            self.trust_root, openid_identifier='oid1')
+            self.trust_root, openid_identifier='oid1',
+            approved_data=self.approved_data)
         self.assertEqual(summary1.total_logins, 1)
         self.assertEqual(summary2.total_logins, 1)
         self.assertNotEqual(summary1, summary2)
@@ -267,7 +275,6 @@
         summary1 = OpenIDRPSummary.objects.record(self.account, self.trust_root,
             openid_identifier='oid')
         self.assertEqual(summary1.total_logins, 1)
-
         summary2 = OpenIDRPSummary.objects.get(account=self.account,
             trust_root=self.trust_root, openid_identifier='oid')
         self.assertEqual(summary1, summary2)
@@ -278,3 +285,10 @@
 
         summary.increment()
         self.assertEqual(summary.total_logins, 2)
+
+    def test_approved_data(self):
+        summary = self.create_summary()
+        summary.set_approved_data(self.approved_data)
+        summary2 = OpenIDRPSummary.objects.get(account=self.account,
+            trust_root=self.trust_root, openid_identifier='oid')
+        self.assertEqual(summary2.get_approved_data(), self.approved_data)

=== modified file 'identityprovider/tests/test_models_team.py'
--- identityprovider/tests/test_models_team.py	2010-05-20 21:27:05 +0000
+++ identityprovider/tests/test_models_team.py	2011-07-05 15:46:33 +0000
@@ -1,9 +1,20 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-from identityprovider.models.person import Person
-from identityprovider.models.team import TeamParticipation
-from identityprovider.tests.utils import SQLCachedTestCase
+from identityprovider.const import (
+    PERSON_VISIBILITY_PUBLIC,
+    PERSON_VISIBILITY_PRIVATE_MEMBERSHIP,
+)
+from identityprovider.models import (
+    Account,
+    Person,
+    TeamMembership,
+    TeamParticipation,
+)
+from identityprovider.tests.utils import (
+    BasicAccountTestCase,
+    SQLCachedTestCase,
+)
 
 
 class TeamParticipationTestCase(SQLCachedTestCase):
@@ -12,3 +23,46 @@
         person = Person.objects.create(displayname='Person', name='person')
         tp = TeamParticipation.objects.create(team=team, person=person)
         self.assertEqual(unicode(tp), u'Person in Team')
+
+
+class TeamMembershipTest(BasicAccountTestCase):
+
+    fixtures = ["test"]
+
+    def _set_team_to_private_membership(self, team_name):
+        self._set_team_privacy(team_name, PERSON_VISIBILITY_PRIVATE_MEMBERSHIP)
+
+    def _set_team_to_public_membership(self, team_name):
+        self._set_team_privacy(team_name, PERSON_VISIBILITY_PUBLIC)
+
+    def _set_team_privacy(self, team_name, privacy):
+        team = Person.objects.get(name=team_name)
+        team.visibility = privacy
+        team.save()
+
+    def setUp(self):
+        self.account = Account.objects.get(pk=1)
+
+    def test_get_team_memberships_on_personless_account(self):
+        account = Account.objects.create_account('test', '[e-mail]',
+            'password')
+        memberships = TeamMembership.get_team_memberships_for_user(
+            ['ubuntu-team'], account, False)
+        self.assertEquals(memberships, [])
+
+    def test_get_team_memberships_when_team_is_visible(self):
+        memberships = TeamMembership.get_team_memberships_for_user(
+            ['ubuntu-team'], self.account, False)
+        self.assertEquals(memberships, ['ubuntu-team'])
+
+    def test_get_team_membership_when_team_is_not_visible(self):
+        self._set_team_to_private_membership('ubuntu-team')
+        memberships = TeamMembership.get_team_memberships_for_user(
+            ['ubuntu-team', 'myteam'], self.account, False)
+        self.assertEquals(memberships, [])
+
+    def test_team_is_private_but_you_can_see_them(self):
+        self._set_team_to_private_membership('ubuntu-team')
+        memberships = TeamMembership.get_team_memberships_for_user(
+            ['ubuntu-team', 'myteam'], self.account, True)
+        self.assertEquals(memberships, ['ubuntu-team'])

=== modified file 'identityprovider/tests/test_views_server.py'
--- identityprovider/tests/test_views_server.py	2011-01-17 15:54:37 +0000
+++ identityprovider/tests/test_views_server.py	2011-07-05 15:46:33 +0000
@@ -3,14 +3,17 @@
 
 import datetime
 import urlparse
-
 from random import randint
+
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
+from django.http import HttpRequest
 from django.test import TestCase
 from openid.extensions import pape
 from openid.extensions.sreg import SRegRequest
 
+from mock import patch
+
 from openid.message import (Message, IDENTIFIER_SELECT, OPENID1_URL_LIMIT,
     OPENID2_NS)
 from openid.yadis.constants import YADIS_HEADER_NAME
@@ -20,16 +23,16 @@
 from identityprovider.models import (Account, OpenIDAuthorization,
     OpenIDRPConfig, Person)
 from identityprovider.models.account import LPOpenIdIdentifier
-from identityprovider.models.const import (AccountStatus, EmailStatus,
-    AccountCreationRationale)
+from identityprovider.models.const import (
+    AccountStatus,
+    AccountCreationRationale,
+)
 from identityprovider.models.person import PersonLocation
 from identityprovider.models.authtoken import create_token
 
 from identityprovider.views import server
 from identityprovider.tests.utils import (SQLCachedTestCase,
     BasicAccountTestCase, AuthenticatedTestCase, OpenIDProviderTestCase)
-from identityprovider.const import PERSON_VISIBILITY_PRIVATE_MEMBERSHIP
-
 
 
 class DummyORequest(object):
@@ -41,13 +44,15 @@
     def idSelect(self):
         return False
 
+
 class DummySession(dict):
 
     @property
     def session_key(self):
         return 'abc'
 
-    def flush(self): pass
+    def flush(self):
+        pass
 
 
 class DummyRequest(object):
@@ -55,7 +60,7 @@
     def __init__(self):
         self.session = DummySession()
         self.COOKIES = {}
-        self.META ={}
+        self.META = {}
 
 
 class HandleOpenIDErrorTestCase(OpenIDProviderTestCase):
@@ -175,6 +180,7 @@
         self.assertTrue(r['Location'].endswith('+decide'))
         self.assertEqual(openid_referer.value, META['HTTP_REFERER'])
 
+
 class MultiLangOpenIDTestCase(TestCase):
     def setUp(self):
         self.orig_language_code = settings.LANGUAGE_CODE
@@ -218,6 +224,7 @@
         response = self.client.get('/+openid')
         self.assertEqual('de', self.client.session['django_language'])
 
+
 class ValidOpenIDTestCase(OpenIDProviderTestCase):
 
     def test_is_valid_openid_idselect(self):
@@ -280,6 +287,7 @@
 
 class DecideTestCase(AuthenticatedTestCase, OpenIDProviderTestCase):
     fixtures = ['test']
+
     def _prepare_openid_token(self, param_overrides=None):
         request = {'openid.mode': 'checkid_setup',
                    'openid.trust_root': 'http://localhost/',
@@ -363,14 +371,12 @@
         self.assertEqual(r.status_code, 200)
         self.assertTemplateUsed(r, 'registration/login.html')
 
-
     def test_list_of_details_is_complete(self):
         self.client.login(username='[e-mail]',
                           password='test')
 
         # create a trusted rpconfig
         rpconfig = OpenIDRPConfig(trust_root='http://localhost/',
-            allowed_sreg="nickname,email,fullname,timezone,language",
             can_query_any_team=True,
             description="Some description",
         )
@@ -386,6 +392,95 @@
         self.assertContains(response, "Email address")
         self.assertContains(response, "Preferred language")
 
+    def test_state_of_checkboxes_and_data_formats_trusted(self):
+        self.client.login(username='[e-mail]',
+                          password='test')
+
+        # create a trusted rpconfig
+        rpconfig = OpenIDRPConfig(trust_root='http://localhost/',
+            can_query_any_team=True,
+            description="Some description",
+        )
+        rpconfig.save()
+        param_overrides = {
+            'openid.sreg.required': 'nickname,email,fullname',
+            'openid.sreg.optional': 'language',
+            'openid.lp.query_membership': 'ubuntu-team,launchpad-team,isd-team',
+        }
+        self._prepare_openid_token(param_overrides=param_overrides)
+        response = self.client.get('/%s/+decide' % self.token)
+        # checkbox checked and disabled for required fields and label is bold
+        username_html = ('<li><input checked="checked" name="nickname" '
+                         'value="mark" class="required" disabled="disabled" '
+                         'type="checkbox" id="id_nickname" /> <label '
+                         'for="id_nickname">Username: mark</label></li>')
+        email_html = ('<li><input checked="checked" name="email" '
+                      'value="[e-mail]" class="required" '
+                      'disabled="disabled" type="checkbox" id="id_email" /> '
+                      '<label for="id_email">Email address: '
+                      '[e-mail]</label></li>')
+        # checkbox checked and enabled for optional fields and label is plain
+        language_html = ('<li><input checked="checked" type="checkbox" '
+                         'name="language" value="en" id="id_language" /> '
+                         '<label for="id_language">Preferred language: en'
+                         '</label></li>')
+        # team data is enabled and checked, the label is plain
+        team_html_1 = ('<li><input checked="checked" type="checkbox" '
+                       'name="ubuntu-team" value="ubuntu-team" id="id_'
+                       'ubuntu-team" /> <label for="id_ubuntu-team">'
+                       'ubuntu-team</label></li>')
+        team_html_2 = ('<li><input checked="checked" type="checkbox" '
+                       'name="isd-team" value="isd-team" id="id_isd-team" /> '
+                       '<label for="id_isd-team">isd-team</label></li>')
+
+        self.assertContains(response, username_html)
+        self.assertContains(response, email_html)
+        self.assertContains(response, language_html)
+        self.assertContains(response, team_html_1)
+        self.assertContains(response, team_html_2)
+
+    def test_state_of_checkboxes_and_data_formats_untrusted(self):
+        self.client.login(username='[e-mail]',
+                          password='test')
+
+        # create a trusted rpconfig
+        rpconfig = OpenIDRPConfig(trust_root='http://untrusted/',
+            can_query_any_team=True,
+            description="Some description",
+        )
+        rpconfig.save()
+        param_overrides = {
+            'openid.sreg.required': 'nickname,email,fullname',
+            'openid.sreg.optional': 'language',
+            'openid.lp.query_membership': 'ubuntu-team',
+        }
+        self._prepare_openid_token(param_overrides=param_overrides)
+        response = self.client.get('/%s/+decide' % self.token)
+        # checkbox checked and *enabled* for required fields and label is bold
+        username_html = ('<li><input checked="checked" name="nickname" '
+                         'value="mark" class="required" type="checkbox" '
+                         'id="id_nickname" /> <label for="id_nickname">'
+                         'Username: mark</label></li>')
+        email_html = ('<li><input checked="checked" name="email" '
+                      'value="[e-mail]" class="required" type='
+                      '"checkbox" id="id_email" /> <label for="id_email">'
+                      'Email address: [e-mail]</label></li>')
+        # checkbox *not checked* and enabled for optional fields & label's plain
+        language_html = ('<li><input type="checkbox" name="language" value="'
+                         'en" id="id_language" /> <label for="id_language">'
+                         'Preferred language: en</label></li>')
+        # team data is enabled and *not checked*, the label is plain
+        team_html = ('<li><input type="checkbox" name="ubuntu-team" '
+                     'value="ubuntu-team" id="id_ubuntu-team" /> Team '
+                     'membership: <label for="id_ubuntu-team">ubuntu-team'
+                     '</label></li>')
+
+        self.assertContains(response, username_html)
+        self.assertContains(response, email_html)
+        self.assertContains(response, language_html)
+        self.assertContains(response, team_html)
+
+
 class PreAuthorizeTestCase(AuthenticatedTestCase):
     def setUp(self):
         super(PreAuthorizeTestCase, self).setUp(disableCSRF=True)
@@ -666,7 +761,7 @@
         old_fromOpenIDRequest = pape.Request.fromOpenIDRequest
         pape.Request.fromOpenIDRequest = mock_fromOpenIDRequest
 
-        r  = server._openid_is_authorized(self.request, self.orequest)
+        r = server._openid_is_authorized(self.request, self.orequest)
         self.assertFalse(r)
 
         pape.Request.fromOpenIDRequest = old_fromOpenIDRequest
@@ -832,66 +927,6 @@
         self.request = DummyRequest()
         self.request.user = self.account
 
-    def test_sreg_fields_no_preferredemail(self):
-        for email in self.account.emailaddress_set.all():
-            email.status = EmailStatus.NEW
-            email.save()
-
-        expected = []
-        result = server._sreg_fields(self.request, self.sreg_request,
-                                     self.rpconfig)
-        self.assertEqual(expected, result)
-
-    def test_sreg_fields_timezone(self):
-        self.sreg_request.optional = ['timezone']
-        expected = [('timezone', 'UTC')]
-        result = server._sreg_fields(self.request, self.sreg_request,
-                                     self.rpconfig)
-        self.assertEqual(result, expected)
-
-    def test_sreg_fields_language(self):
-        self.sreg_request.optional = ['language']
-        expected = [('language', 'en')]
-        result = server._sreg_fields(self.request, self.sreg_request,
-                                     self.rpconfig)
-        self.assertEqual(result, expected)
-
-class TestGetTeamMemberships(BasicAccountTestCase):
-
-    fixtures = ["test"]
-
-    def setUp(self):
-        self.account = Account.objects.get(pk=1)
-
-    def test_get_team_memberships_on_personless_account(self):
-        account = Account.objects.create_account('test', '[e-mail]', 'password')
-        memberships = server.get_team_memberships(
-            ["ubuntu-team"], account, True)
-        self.assertEquals(memberships, [])
-
-    def test_get_team_memberships_when_team_is_visible(self):
-        memberships = server.get_team_memberships(
-            ["ubuntu-team"], self.account, True)
-        self.assertEquals(memberships, ["ubuntu-team"])
-
-    def test_get_team_membership_when_team_is_not_visible(self):
-        self.set_team_to_private_membership('ubuntu-team')
-
-        memberships = server.get_team_memberships(
-            ['ubuntu-team', "myteam"], self.account, True)
-        self.assertEquals(memberships, [])
-
-    def test_team_is_private_but_you_can_see_them(self):
-        self.set_team_to_private_membership('ubuntu-team')
-        memberships = server.get_team_memberships(
-            ['ubuntu-team', "myteam"], self.account, False)
-        self.assertEquals(memberships, ['ubuntu-team'])
-
-    def set_team_to_private_membership(self, team_name):
-        team = Person.objects.get(name=team_name)
-        team.visibility = PERSON_VISIBILITY_PRIVATE_MEMBERSHIP
-        team.save()
-
 
 class MarkupTestCase(SQLCachedTestCase):
 
@@ -908,3 +943,69 @@
 
         r = self.client.get('/%s/+decide' % token)
         self.assertContains(r, '<em>not</em>')
+
+
+class ApprovedDataTest(OpenIDProviderTestCase):
+    fixtures = ['test']
+
+    def _get_openid_request(self, with_sreg=True, with_teams=True):
+        request = {
+            'openid.mode': 'checkid_setup',
+            'openid.trust_root': 'http://localhost/',
+            'openid.return_to': 'http://localhost/',
+            'openid.identity': IDENTIFIER_SELECT}
+        if with_sreg:
+            request['openid.sreg.required'] = 'email,fullname'
+        if with_teams:
+            request['openid.lp.query_membership'] = 'ubuntu-team'
+        openid_server = server._get_openid_server()
+        return openid_server.decodeRequest(request)
+
+    def _get_request_with_post_args(self, args={}):
+        request = HttpRequest()
+        request.user = Account.objects.get(pk=1)
+        request.POST = args
+        request.META = {'REQUEST_METHOD': 'POST'}
+        return request
+
+    def setUp(self):
+        # Ensure that we're restricting RPs for these tests
+        self.old_restrict = getattr(settings, 'SSO_RESTRICT_RP', True)
+        settings.SSO_RESTRICT_RP = False
+
+    def tearDown(self):
+        settings.SSO_RESTRICT_RP = self.old_restrict
+
+    def test_approved_data_returns_none_for_no_request(self):
+        result = server._get_approved_data(HttpRequest(), None)
+        self.assertEqual(result, None)
+
+    def test_approved_data_for_sreg_only(self):
+        post_args = {'email': 'email', 'ubuntu-team': 'ubuntu-team'}
+        result = server._get_approved_data(
+            self._get_request_with_post_args(post_args),
+            self._get_openid_request(True, False))
+        self.assertEqual(sorted(result['sreg']['requested']),
+            ['email', 'fullname'])
+        self.assertEqual(result['sreg']['approved'], ['email'])
+        self.assertFalse(result.has_key('teams'))
+
+    def test_approved_data_for_teams_only(self):
+        post_args = {'ubuntu-team': 'ubuntu-team'}
+        result = server._get_approved_data(
+            self._get_request_with_post_args(post_args),
+            self._get_openid_request(False, True))
+        self.assertEqual(result['teams']['requested'], ['ubuntu-team'])
+        self.assertEqual(result['teams']['approved'], ['ubuntu-team'])
+        self.assertFalse(result.has_key('sreg'))
+
+    def test_approved_data_for_sreg_and_teams(self):
+        post_args = {'email': 'email', 'ubuntu-team': 'ubuntu-team'}
+        result = server._get_approved_data(
+            self._get_request_with_post_args(post_args),
+            self._get_openid_request())
+        self.assertEqual(sorted(result['sreg']['requested']),
+            ['email', 'fullname'])
+        self.assertEqual(result['sreg']['approved'], ['email'])
+        self.assertEqual(result['teams']['requested'], ['ubuntu-team'])
+        self.assertEqual(result['teams']['approved'], ['ubuntu-team'])

=== modified file 'identityprovider/tests/utils.py'
--- identityprovider/tests/utils.py	2011-06-17 14:22:43 +0000
+++ identityprovider/tests/utils.py	2011-07-05 15:46:33 +0000
@@ -302,3 +302,54 @@
         resp.code = 200
         resp.msg = 'OK'
         return resp
+
+
+def add_user_to_team(account, teamname):
+    """ Note: will not work on the staging or production databases as those are
+        replicated from LP and read-only.
+    """
+    from datetime import date
+    from identityprovider.models.person import Person
+    from identityprovider.models.account import (Account, LPOpenIdIdentifier)
+    from identityprovider.models.team import (TeamMembership, TeamParticipation)
+
+    memberships = TeamMembership.get_team_memberships_for_user([teamname],
+                                                               account, True)
+    if len(memberships) > 0:
+        return
+
+    p = account.person
+    if p is None:
+        p = Person.objects.create(lp_account=account.id,
+                                  displayname=account.displayname,
+                                  name=get_unique_username())
+
+    try:
+        oi = LPOpenIdIdentifier.objects.get(lp_account=account.id)
+    except LPOpenIdIdentifier.DoesNotExist:
+        oi = LPOpenIdIdentifier.objects.create(
+                identifier=account.openid_identifier, lp_account=account.id)
+
+    try:
+        t = Person.objects.get(name=teamname)
+    except Person.DoesNotExist:
+        t = Person.objects.create(displayname="Team %s" % teamname, teamowner=p,
+                                  name=teamname)
+
+    now = date.today()
+    exp = date(day=now.day, month=now.month, year=now.year+1)
+    TeamMembership.objects.create(person=p, date_expires=exp, team=t, status=1)
+    TeamParticipation.objects.create(team=t, person=p)
+
+def get_unique_username():
+    from identityprovider.models.person import Person
+    i = 0
+    while True:
+        try:
+            username = "user%d" % i
+            i += 1
+            Person.objects.get(name=username)
+        except Person.DoesNotExist:
+            break
+    return username
+

=== modified file 'identityprovider/views/server.py'
--- identityprovider/views/server.py	2011-05-31 20:29:08 +0000
+++ identityprovider/views/server.py	2011-07-05 15:46:33 +0000
@@ -6,13 +6,26 @@
 import urllib
 import urlparse
 
-from datetime import datetime, timedelta
+from datetime import (
+    datetime,
+    timedelta,
+)
 
 from openid.extensions import pape
-from openid.extensions.sreg import SRegRequest, SRegResponse
-from openid.message import IDENTIFIER_SELECT, registerNamespaceAlias
-from openid.server.server import (CheckIDRequest, ENCODE_URL, ProtocolError,
-    Server)
+from openid.extensions.sreg import (
+    SRegRequest,
+    SRegResponse,
+)
+from openid.message import (
+    IDENTIFIER_SELECT,
+    registerNamespaceAlias,
+)
+from openid.server.server import (
+    CheckIDRequest,
+    ENCODE_URL,
+    ProtocolError,
+    Server,
+)
 from openid.server.trustroot import TrustRoot
 from openid.urinorm import urinorm
 from openid.yadis.constants import YADIS_HEADER_NAME
@@ -21,30 +34,47 @@
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.http import (Http404, HttpResponse, HttpResponseBadRequest,
-    HttpResponseRedirect)
-from django.template import RequestContext, loader
-from django.shortcuts import get_object_or_404, render_to_response
+from django.http import (
+    Http404,
+    HttpResponse,
+    HttpResponseBadRequest,
+    HttpResponseRedirect,
+)
+from django.template import (
+    RequestContext,
+    loader,
+)
+from django.shortcuts import (
+    get_object_or_404,
+    render_to_response,
+)
 from django.utils.decorators import decorator_from_middleware
 from django.utils import translation
 
 import identityprovider.signed as signed
 
-from identityprovider.const import (
-    LAUNCHPAD_TEAMS_NS,
-    PERSON_VISIBILITY_PUBLIC,
-    SREG_DATA_FIELDS_ORDER,
-    SREG_LABELS,
+from identityprovider.const import LAUNCHPAD_TEAMS_NS
+from identityprovider.forms import (
+    PreAuthorizeForm,
+    SRegRequestForm,
+    TeamsRequestForm,
 )
-from identityprovider.forms import PreAuthorizeForm
+
 from identityprovider.middleware.xrds import XRDSMiddleware
-from identityprovider.models import (Account, DjangoOpenIDStore,
-                        OpenIDAuthorization, OpenIDRPSummary,
-                        Person)
+from identityprovider.models import (
+    Account,
+    DjangoOpenIDStore,
+    OpenIDAuthorization,
+    OpenIDRPSummary,
+    TeamMembership,
+)
 from identityprovider.models.authtoken import create_token
 from identityprovider.teams import TeamsRequest
 from identityprovider.utils import is_django_13
-from identityprovider.views import ui, utils
+from identityprovider.views import (
+    ui,
+    utils,
+)
 from identityprovider.views.i18n import set_language_info
 
 if not is_django_13():
@@ -56,13 +86,14 @@
 registerNamespaceAlias(LAUNCHPAD_TEAMS_NS, 'lp')
 logger = logging.getLogger('sso')
 
+
 @csrf_exempt
 @accept_xrds
 def openid_provider(request, lang='en'):
     if lang not in settings.SUPPORTED_LANGUAGES:
         # Next we check for a primary language.
         if lang is not None:
-            lang = lang.split('_')[0] # e.g. de_CH becomes de.
+            lang = lang.split('_')[0]  # e.g. de_CH becomes de.
         if lang not in settings.SUPPORTED_LANGUAGES:
             lang = translation.get_language_from_request(request)
     translation.activate(lang)
@@ -77,6 +108,7 @@
     set_language_info(request, response, lang)
     return response
 
+
 def _process_openid_request(request, orequest, openid_server):
     if not orequest:
         context = RequestContext(request)
@@ -111,14 +143,15 @@
     if orequest.immediate:
         rp_config = utils.get_rpconfig(orequest.trust_root)
         auto_authorized = _is_auto_authorized_rp(rp_config)
-        if auto_authorized and request.user.is_authenticated() and _is_identity_owner(request.user, orequest):
+        if (auto_authorized and request.user.is_authenticated() and
+            _is_identity_owner(request.user, orequest)):
             if orequest.idSelect():
                 oresponse = orequest.answer(True,
                     identity=request.user.openid_identity_url)
             else:
                 oresponse = orequest.answer(True)
             _add_sreg(request, orequest, oresponse)
-            _check_team_membership(request.user, orequest, oresponse)
+            _check_team_membership(request, orequest, oresponse)
             response = _django_response(request, oresponse, True)
         else:
             oresponse = orequest.answer(False)
@@ -138,7 +171,7 @@
         else:
             oresponse = orequest.answer(True)
         _add_sreg(request, orequest, oresponse)
-        _check_team_membership(request.user, orequest, oresponse)
+        _check_team_membership(request, orequest, oresponse)
         response = _django_response(request, oresponse, True)
     elif (request.user.is_authenticated() and not
           _is_identity_owner(request.user, orequest)):
@@ -196,15 +229,21 @@
             return _process_decide(request, orequest, True)
         else:
             sreg_request = SRegRequest.fromOpenIDRequest(orequest)
-            sreg_data = _sreg_fields(request, sreg_request, rpconfig)
-            sreg_data = _humanize_sreg_labels(sreg_data)
-            teams_data = _teams_data(request.user, orequest, rpconfig)
+            teams_request = TeamsRequest.fromOpenIDRequest(orequest)
+            try:
+                summary = OpenIDRPSummary.objects.get(
+                    account=request.user, trust_root=orequest.trust_root)
+                approved_data = summary.get_approved_data()
+            except OpenIDRPSummary.DoesNotExist:
+                approved_data = {}
             context = RequestContext(request, {
                 'account': request.user,
                 'trust_root': orequest.trust_root,
                 'rpconfig': rpconfig,
-                'sreg_data': sreg_data,
-                'teams_data': teams_data,
+                'sreg_form': SRegRequestForm(request, sreg_request, rpconfig,
+                    approved_data=approved_data.get('sreg')),
+                'teams_form': TeamsRequestForm(request, teams_request, rpconfig,
+                    approved_data=approved_data.get('teams')),
                 'token': token,
                 'message': request.session.get('message', None),
                 'message_style': 'informational',
@@ -422,7 +461,7 @@
     return user.last_login <= cutoff
 
 
-def _django_response(request, oresponse, auth_success=False):
+def _django_response(request, oresponse, auth_success=False, orequest=None):
     """ Convert an OpenID response into a Django HttpResponse """
     webresponse = _get_openid_server().encodeResponse(oresponse)
     response = HttpResponse(webresponse.body, mimetype="text/plain")
@@ -433,54 +472,44 @@
     logger.debug("response_body = " + webresponse.body)
     if auth_success and isinstance(oresponse.request, CheckIDRequest):
         logger.debug("oresponse.fields = " + str(oresponse.fields))
-        OpenIDRPSummary.objects.record(request.user,
-                                       oresponse.request.trust_root)
+        approved_data = _get_approved_data(request, orequest)
+        OpenIDRPSummary.objects.record(
+            request.user,
+            oresponse.request.trust_root,
+            None,
+            approved_data)
     return response
 
 
-def _sreg_fields(request, sreg_request, rpconfig):
-    account = request.user
-    field_names = set(sreg_request.required + sreg_request.optional)
-    if rpconfig is None:
-        # The nickname field is permitted by default.
-        field_names.intersection_update(['nickname'])
-    elif rpconfig.allowed_sreg is not None:
-        field_names.intersection_update(rpconfig.allowed_sreg.split(','))
-    sreg_field_names = [name for name in SREG_DATA_FIELDS_ORDER
-                        if name in field_names]
-    # Collect registration values
-    values = {}
-    values['fullname'] = account.displayname
-    if account.preferredemail is not None:
-        values['email'] = account.preferredemail.email
-    if account.person is not None:
-        values['nickname'] = account.person.name
-        if account.person.time_zone is not None:
-            values['timezone'] = account.person.time_zone
-    if account.preferredlanguage is not None:
-        values['language'] = account.preferredlanguage
-    else:
-        values['language'] = translation.get_language_from_request(request)
-    logger.debug("values (sreg_fields) = " + str(values))
-    return [(field, values[field])
-            for field in sreg_field_names if field in values]
-
-
-def _humanize_sreg_labels(sreg_data):
-    new_data = []
-    for k, v in sreg_data:
-        k = SREG_LABELS.get(k, k)
-        new_data.append((k, v))
-    return new_data
-
-
-def _teams_data(account, orequest, rpconfig):
-    teams_request = TeamsRequest.fromOpenIDRequest(orequest)
-    restrict_teams = True
-    if rpconfig is not None:
-        restrict_teams = not rpconfig.can_query_any_team
-    return get_team_memberships(teams_request.allRequestedTeams(),
-        account, restrict_teams)
+def _get_approved_data(request, orequest):
+    """Given an HTTP request and an OpenID request, return a nested dict of
+    values requested in the request and approved by the user.
+    """
+    if not orequest:
+        return None
+
+    approved_data = {}
+
+    sreg_request = SRegRequest.fromOpenIDRequest(orequest)
+    rpconfig = utils.get_rpconfig(orequest.trust_root)
+    sreg_form = SRegRequestForm(request, sreg_request, rpconfig)
+    if len(sreg_form.data.keys()) > 0:
+        approved_data['sreg'] = {
+            'requested': sreg_form.data.keys(),
+            'approved': sreg_form.data_approved_by_user.keys()}
+
+    args = orequest.message.getArgs(LAUNCHPAD_TEAMS_NS)
+    team_names = args.get('query_membership')
+    if team_names:
+        team_names = team_names.split(',')
+        teams_form = TeamsRequestForm(request,
+            TeamsRequest.fromOpenIDRequest(orequest),
+            utils.get_rpconfig(orequest.trust_root))
+        approved_data['teams'] = {
+            'requested': team_names,
+            'approved': teams_form.teams_approved_by_user}
+
+    return approved_data
 
 
 def _is_identity_owner(user, openid_request):
@@ -495,11 +524,11 @@
 def _add_sreg(request, openid_request, openid_response):
     # Add sreg result data
     sreg_request = SRegRequest.fromOpenIDRequest(openid_request)
-    sreg_fields = _sreg_fields(request, sreg_request,
-        utils.get_rpconfig(openid_request.trust_root))
-    if sreg_fields:
+    rpconfig = utils.get_rpconfig(openid_request.trust_root)
+    form = SRegRequestForm(request, sreg_request, rpconfig)
+    if form.data_approved_by_user:
         sreg_response = SRegResponse.extractResponse(
-            sreg_request, dict(sreg_fields))
+            sreg_request, dict(form.data_approved_by_user))
         openid_response.addExtension(sreg_response)
 
 
@@ -522,8 +551,8 @@
             datetime.now(),
             request.session.session_key)
         _add_sreg(request, orequest, oresponse)
-        _check_team_membership(request.user, orequest, oresponse)
-    r = _django_response(request, oresponse, decision)
+        _check_team_membership(request, orequest, oresponse, False)
+    r = _django_response(request, oresponse, decision, orequest)
     if r.content:
         # Only user-visible content is generated from this view.  Wrap
         # it up and set the Content-Type.
@@ -543,47 +572,43 @@
     return openid_server
 
 
-def _check_team_membership(account, openid_request, openid_response):
+def _check_team_membership(request, orequest, oresponse, immediate=True):
     """Perform team membership checks.
 
     If any team membership checks have been requested as part of
     the OpenID request, annotate the response with the list of
     teams the user is actually a member of.
     """
-    assert account is not None, (
+    assert request.user is not None, (
         'Must be logged in to calculate team membership')
-    if account.person is None:
+    if request.user.person is None:
         return
-    args = openid_request.message.getArgs(LAUNCHPAD_TEAMS_NS)
+    args = orequest.message.getArgs(LAUNCHPAD_TEAMS_NS)
     team_names = args.get('query_membership')
     if not team_names:
         return
     team_names = team_names.split(',')
-    only_public_teams = (
-        utils.get_rpconfig(openid_request.trust_root) is None or not
-        utils.get_rpconfig(openid_request.trust_root).can_query_any_team)
-    memberships = get_team_memberships(
-        team_names, account, only_public_teams)
-    openid_response.fields.namespaces.addAlias(LAUNCHPAD_TEAMS_NS, 'lp')
-    openid_response.fields.setArg(
-        LAUNCHPAD_TEAMS_NS, 'is_member', ','.join(memberships))
-
-
-def get_team_memberships(team_names, account, only_public_teams):
-    memberships = []
-    for team_name in team_names:
+    if immediate:
         try:
-            team = Person.objects.get(name=team_name)
-        except Person.DoesNotExist:
-            team = None
-        if team is None or not team.is_team():
-            continue
-        # Control access to private teams
-        if (team.visibility != PERSON_VISIBILITY_PUBLIC and only_public_teams):
-            continue
-        if account.person_in_team(team):
-            memberships.append(team_name)
-    return memberships
+            summary = OpenIDRPSummary.objects.get(
+                account=request.user, trust_root=orequest.trust_root)
+            approved_data = summary.get_approved_data()
+        except OpenIDRPSummary.DoesNotExist:
+            approved_data = {}
+        if 'teams' in approved_data:
+            teams = ','.join(approved_data['teams'].get('approved', []))
+        else:
+            rpconfig = utils.get_rpconfig(orequest.trust_root)
+            teams = ','.join(TeamMembership.get_team_memberships_for_user(
+                team_names, request.user,
+                rpconfig and rpconfig.can_query_any_team))
+    else:
+        form = TeamsRequestForm(request,
+            TeamsRequest.fromOpenIDRequest(orequest),
+            utils.get_rpconfig(orequest.trust_root))
+        teams = ','.join(form.teams_approved_by_user)
+    oresponse.fields.namespaces.addAlias(LAUNCHPAD_TEAMS_NS, 'lp')
+    oresponse.fields.setArg(LAUNCHPAD_TEAMS_NS, 'is_member', teams)
 
 
 def untrusted(request, token):

=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py	2011-05-31 15:44:44 +0000
+++ identityprovider/views/ui.py	2011-07-05 15:46:33 +0000
@@ -206,6 +206,7 @@
         })
     return render_to_response('enter_token.html', context)
 
+
 def _redirect_to_enter_token(confirmation_code=None, email=None):
     params = {}
     if confirmation_code is not None:
@@ -214,6 +215,7 @@
         params['email'] = email
     return HttpResponseRedirect('/+enter_token?%s' % urlencode(params))
 
+
 def claim_token(request, authtoken):
     email = request.GET.get('email') or request.session.get('token_email')
     if email is None:
@@ -221,6 +223,7 @@
     token_type = get_type_of_token(authtoken)
     return _handle_confirmation(token_type, authtoken, email, request)
 
+
 # As soon as _finish_account_creation doesn't need `request`, neither
 # does this.
 def _handle_confirmation(confirmation_type, confirmation_code, email, request,
@@ -258,7 +261,8 @@
             return HttpResponseRedirect(redirection_url)
 
         view = old_confirm_account
-        if 'token' in args: del args['token']
+        if 'token' in args:
+            del args['token']
 
     elif confirmation_type == LoginTokenType.VALIDATEEMAIL:
         view = confirm_email
@@ -572,9 +576,11 @@
     })
     return render_to_response('registration/bad_token.html', context)
 
+
 def logout_to_confirm(request):
     return render_to_response('registration/logout_to_confirm.html')
 
+
 def deactivated(request):
     context = RequestContext(request, {
         'message': request.session.pop('message', None),

=== modified file 'payload/__init__.py'
--- payload/__init__.py	2011-06-23 19:51:28 +0000
+++ payload/__init__.py	2011-07-05 15:46:33 +0000
@@ -210,4 +210,7 @@
             print 'Nothing to do; translations are up to date.'
 
 
-
+def manage(command, *args):
+    """Run manage.py command"""
+    virtualenv("python django_project/manage.py {0} {1}".format(
+        command, " ".join(args)), capture=False,)