Merge ~silverdrake11/canonical-identity-provider:ax-verified-emails into canonical-identity-provider:master

Proposed by Kevin Nasto
Status: Merged
Approved by: Daniel Manrique
Approved revision: 5c23068c8d3ef9cd7b670c025c577e62f6ba169a
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~silverdrake11/canonical-identity-provider:ax-verified-emails
Merge into: canonical-identity-provider:master
Diff against target: 183 lines (+93/-0)
5 files modified
README (+2/-0)
src/identityprovider/const.py (+3/-0)
src/identityprovider/forms.py (+14/-0)
src/identityprovider/tests/test_forms.py (+46/-0)
src/identityprovider/tests/test_views_server.py (+28/-0)
Reviewer Review Type Date Requested Status
John Paraskevopoulos Approve
Daniel Manrique (community) Approve
Review via email: mp+419248@code.launchpad.net

Commit message

This adds an ax attribute which can be requested during login via the ax uri (alias "extra_emails"). This attribute contains a comma separated list of additional verified email addresses not including the preferred one.

This is useful to get emails associated with an ubuntu login id. In Landscape we would want to prevent using notification emails not associated with the current ubuntu login. Also tie invitations to current ubuntu login, and verify emails in one interface.

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

Looks good, a minor suggestion to simplify the _get_extra_emails method but it's optional.

review: Approve
9f3b1f2... by Kevin Nasto

insert spaces for ui separating ax emails, unicode, list comprehension, linter errors

Revision history for this message
Kevin Nasto (silverdrake11) wrote :

> Looks good, a minor suggestion to simplify the _get_extra_emails method but
> it's optional.

Thanks for the heads up I fixed it

Revision history for this message
John Paraskevopoulos (quantifics) wrote :

From what I see this MP doesn't clash with the issues (and fixes) we faced in the Python 3 transition. Those issues were related to the way python and django were validating email addresses when sending emails. So since this is for extra form fields, there's no conflict.

LGTM as well

review: Approve
5e856c0... by Kevin Nasto

Added readme instructions if common issue occurs with LXC

5c23068... by Kevin Nasto

REAME typo version

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/README b/README
2index c7bf2ed..bf67ddb 100644
3--- a/README
4+++ b/README
5@@ -65,6 +65,8 @@ you can e.g. ``git clone`` from Launchpad without problems)::
6 From now on, instructions will assume you're inside the sso-xenial container,
7 unless otherwise noted.
8
9+Important note. If you have issues with accessing the network in the LXC container and you are running 21.10 or greater as your host system. For example ``Temporary failure resolving 'archive.ubuntu.com'``. Then you are impacted by this issue https://github.com/lxc/lxd/issues/9413 Follow the instructions there to update your grub config to fix it. Otherwise use Multipass.
10+
11 1. Get the code
12
13 .. note::
14diff --git a/src/identityprovider/const.py b/src/identityprovider/const.py
15index 44756d0..be9e055 100644
16--- a/src/identityprovider/const.py
17+++ b/src/identityprovider/const.py
18@@ -32,6 +32,7 @@ AX_URI_EMAIL = 'http://axschema.org/contact/email'
19 AX_URI_TIMEZONE = 'http://axschema.org/timezone'
20 AX_URI_LANGUAGE = 'http://axschema.org/language/pref'
21 AX_URI_ACCOUNT_VERIFIED = 'http://ns.login.ubuntu.com/2013/validation/account'
22+AX_URI_EXTRA_EMAILS = 'http://ns.login.ubuntu.com/2022/emails'
23
24 AX_DATA_FIELDS = NamespaceMap()
25 AX_DATA_FIELDS.addAlias(AX_URI_FULL_NAME, 'fullname')
26@@ -40,6 +41,7 @@ AX_DATA_FIELDS.addAlias(AX_URI_EMAIL, 'email')
27 AX_DATA_FIELDS.addAlias(AX_URI_TIMEZONE, 'timezone')
28 AX_DATA_FIELDS.addAlias(AX_URI_LANGUAGE, 'language')
29 AX_DATA_FIELDS.addAlias(AX_URI_ACCOUNT_VERIFIED, 'account_verified')
30+AX_DATA_FIELDS.addAlias(AX_URI_EXTRA_EMAILS, 'extra_emails')
31
32 AX_DATA_LABELS = {
33 'fullname': 'Full name',
34@@ -48,6 +50,7 @@ AX_DATA_LABELS = {
35 'timezone': 'Time zone',
36 'language': 'Preferred language',
37 'account_verified': 'Account verified',
38+ 'extra_emails': 'Additional emails',
39 }
40
41 MACAROON_NS = 'http://ns.login.ubuntu.com/2016/openid-macaroon'
42diff --git a/src/identityprovider/forms.py b/src/identityprovider/forms.py
43index 61ab346..cc8c21b 100644
44--- a/src/identityprovider/forms.py
45+++ b/src/identityprovider/forms.py
46@@ -550,6 +550,17 @@ class UserAttribsRequestForm(Form):
47 self.label_suffix = ''
48 self._init_fields(self.data)
49
50+ def _get_extra_emails(self):
51+ '''
52+ Get verified emails for ax field but filter out preferred one since
53+ we already have it, keeping the user facing UI clean
54+ '''
55+ user = self.request.user
56+ preferred = user.preferredemail.email
57+ verified_emails = user.verified_emails()
58+ emails = [e.email for e in verified_emails if e.email != preferred]
59+ return emails
60+
61 def _split_and_filter_requested_attributes(self):
62 # Merge the requested attributes from sreg_request and ax_request and
63 # filter out any that we don't recognise or that aren't allowed by our
64@@ -595,6 +606,9 @@ class UserAttribsRequestForm(Form):
65 else:
66 values['language'] = translation.get_language_from_request(
67 self.request)
68+ extra_emails = self._get_extra_emails()
69+ if extra_emails: # Comma and space delimited for UI readability
70+ values['extra_emails'] = u', '.join(extra_emails)
71 values['account_verified'] = (
72 'token_via_email' if user.is_verified else 'no')
73 logger.debug('user attrib values = %s', str(values))
74diff --git a/src/identityprovider/tests/test_forms.py b/src/identityprovider/tests/test_forms.py
75index 4bcf04c..2c03604 100644
76--- a/src/identityprovider/tests/test_forms.py
77+++ b/src/identityprovider/tests/test_forms.py
78@@ -23,6 +23,7 @@ from openid.extensions.sreg import SRegRequest
79 from identityprovider.const import (
80 AX_URI_ACCOUNT_VERIFIED,
81 AX_URI_EMAIL,
82+ AX_URI_EXTRA_EMAILS,
83 AX_URI_FULL_NAME,
84 AX_URI_LANGUAGE,
85 )
86@@ -988,6 +989,51 @@ class UserAttribsRequestFormTest(SSOBaseTestCase):
87 self.assertIn('fullname', form.data_approved_for_request)
88 self.assertNotIn('email', form.data_approved_for_request)
89
90+ def test_ax_extra_emails_field(self):
91+ '''
92+ Basic test of the additional emails field. Test that emails show up
93+ correctly in the form and preferred does not
94+ '''
95+ hello_email = u'helloƑ@example.com' # Test unicode char as well
96+ world_email = u'world@example.com'
97+ self.factory.make_email_for_account(email=hello_email,
98+ account=self.test_user)
99+ self.factory.make_email_for_account(email=world_email,
100+ account=self.test_user)
101+ self.rpconfig.allowed_user_attribs = 'email,extra_emails'
102+ ax_request = FetchRequest()
103+ ax_request.add(
104+ AttrInfo(AX_URI_EXTRA_EMAILS, alias='extra_emails', required=True))
105+ ax_request.add(
106+ AttrInfo(AX_URI_EMAIL, alias='email', required=True))
107+ form = UserAttribsRequestForm(
108+ request=self._get_request_with_post_args(), sreg_request=None,
109+ ax_request=ax_request, rpconfig=self.rpconfig)
110+ form_data = form.data_approved_for_request
111+ self.assertIn('extra_emails', form_data)
112+ self.assertIn('email', form_data)
113+ extra_emails = set(form_data['extra_emails'].split(', '))
114+ self.assertEqual(len(extra_emails), 2)
115+ self.assertIn(hello_email, extra_emails)
116+ self.assertIn(world_email, extra_emails)
117+
118+ def test_ax_extra_emails_field_empty(self):
119+ '''
120+ Test that field does not show up if there are no extra emails
121+ '''
122+ self.rpconfig.allowed_user_attribs = 'email,extra_emails'
123+ ax_request = FetchRequest()
124+ ax_request.add(
125+ AttrInfo(AX_URI_EXTRA_EMAILS, alias='extra_emails', required=True))
126+ ax_request.add(
127+ AttrInfo(AX_URI_EMAIL, alias='email', required=True))
128+ form = UserAttribsRequestForm(
129+ request=self._get_request_with_post_args(), sreg_request=None,
130+ ax_request=ax_request, rpconfig=self.rpconfig)
131+ form_data = form.data_approved_for_request
132+ self.assertNotIn('extra_emails', form_data)
133+ self.assertIn('email', form_data)
134+
135 def test_ax_required_fields_for_trusted_site(self):
136 """The server should always return values for required fields to
137 trusted sites, regardless of the state of the checkbox in the UI.
138diff --git a/src/identityprovider/tests/test_views_server.py b/src/identityprovider/tests/test_views_server.py
139index 29a8b5a..3b01834 100644
140--- a/src/identityprovider/tests/test_views_server.py
141+++ b/src/identityprovider/tests/test_views_server.py
142@@ -44,6 +44,7 @@ from identityprovider.const import (
143 AX_DATA_FIELDS,
144 AX_URI_ACCOUNT_VERIFIED,
145 AX_URI_EMAIL,
146+ AX_URI_EXTRA_EMAILS,
147 AX_URI_FULL_NAME,
148 AX_URI_LANGUAGE,
149 MACAROON_NS,
150@@ -1021,6 +1022,33 @@ class DecideTestCase(DecideBaseTestCase):
151 self.assertContains(response, "Email address")
152 self.assertContains(response, "Account verified")
153
154+ def test_list_of_details_extra_emails_present(self):
155+ '''
156+ Ensure that the extra emails field gets displayed correctly
157+ '''
158+ self.factory.make_email_for_account(email="hello@world.com",
159+ account=self.account)
160+
161+ # create a trusted rpconfig
162+ rpconfig = OpenIDRPConfig(
163+ trust_root='http://localhost/',
164+ allowed_user_attribs='email,extra_emails',
165+ description="Some description",
166+ )
167+ rpconfig.save()
168+ param_overrides = {
169+ 'openid.ns.ax': AXMessage.ns_uri,
170+ 'openid.ax.mode': FetchRequest.mode,
171+ 'openid.ax.type.extra_emails': AX_URI_EXTRA_EMAILS,
172+ 'openid.ax.type.email': AX_URI_EMAIL,
173+ 'openid.ax.required': 'email,extra_emails',
174+ }
175+ self._prepare_openid_token(param_overrides=param_overrides)
176+ response = self.client.get('/%s/+decide' % self.token)
177+ self.assertContains(response, "Additional emails")
178+ self.assertContains(response, "Email address")
179+ self.assertContains(response, "hello@world.com")
180+
181 def _test_state_of_checkboxes_and_data_formats(
182 self, dom, field, label=None, value=None, required=False,
183 disabled=False, checked=False):

Subscribers

People subscribed via source and target branches