Permalink
Browse files

Bug 794634 - Add Persona unverified email support

Updating with review comments
  • Loading branch information...
1 parent c345f16 commit b65fad4f6d0f9f7d76fb2c89ec7d7dfef5070963 jrconlin committed with cvan Nov 15, 2012
View
@@ -133,6 +133,7 @@ class UserProfile(amo.models.OnChangeMixin, amo.models.ModelBase):
source = models.PositiveIntegerField(default=amo.LOGIN_SOURCE_UNKNOWN,
editable=False, db_index=True)
user = models.ForeignKey(DjangoUser, null=True, editable=False, blank=True)
+ is_verified = models.BooleanField(default=True)
class Meta:
db_table = 'users'
@@ -354,7 +355,7 @@ def log_login_attempt(self, successful):
self.save()
- def create_django_user(self):
+ def create_django_user(self, **kw):
"""Make a django.contrib.auth.User for this UserProfile."""
# Reusing the id will make our life easier, because we can use the
# OneToOneField as pk for Profile linked back to the auth.user
@@ -367,6 +368,9 @@ def create_django_user(self):
self.user.password = self.password
self.user.date_joined = self.created
+ for k, v in kw.iteritems():
+ setattr(self.user, k, v)
+
if self.groups.filter(rules='*:*').count():
self.user.is_superuser = self.user.is_staff = True
@@ -4,7 +4,6 @@
from django.conf import settings
from django.core import mail
-from django import http
from django.core.cache import cache
from django.contrib.auth.models import User
from django.contrib.auth.tokens import default_token_generator
@@ -14,9 +13,9 @@
from mock import Mock, patch
from nose.tools import eq_
from nose import SkipTest
-import waffle
# Unused, but needed so that we can patch jingo.
from waffle import helpers
+import waffle
import amo
import amo.tests
@@ -365,6 +364,7 @@ def test_admin_no_password(self):
FakeResponse = collections.namedtuple("FakeResponse", "status_code content")
+
class TestPasswordAdmin(UserViewBase):
fixtures = ['base/users']
@@ -579,6 +579,15 @@ def test_login_fails_increment(self):
self.client.post(self.url, data={'username': self.data['username']})
eq_(user.get().failed_login_attempts, 4)
+
+class TestPersonaLogin(UserViewBase):
+ fixtures = ('users/test_backends',)
+
+ def setUp(self):
+ super(TestPersonaLogin, self).setUp()
+ self.url = reverse('users.browserid_login')
+ self.data = {'username': 'jbalogh@mozilla.com', 'password': 'foo'}
+
@patch.object(waffle, 'switch_is_active', lambda x: True)
@patch('requests.post')
def test_browserid_login_success(self, http_request):
@@ -597,13 +606,37 @@ def test_browserid_login_success(self, http_request):
eq_(self.client.post(url).status_code, 200)
@patch.object(waffle, 'switch_is_active', lambda x: True)
+ @patch('requests.post')
+ def test_browserid_unverified_login_success(self, http_request):
+ """A success response from BrowserID results in a successful login."""
+
+ # Preverified addresses should not be able to log in as unverified.
+ http_request.return_value = FakeResponse(200, json.dumps(
+ {'status': 'okay', 'unverified-email': 'jbalogh@mozilla.com'}))
+ res = self.client.post(self.url, {'assertion': 'fake-assertion',
+ 'audience': 'fakeamo.org'})
+ eq_(res.status_code, 401)
+ eq_(self.user_profile.reload().is_verified, True)
+
+ # A completely unverified address should be able to log in.
+ self.user_profile.update(is_verified=False)
+ http_request.return_value = FakeResponse(200, json.dumps(
+ {'status': 'okay', 'unverified-email': 'unverified@example.org'}))
+ res = self.client.post(self.url, {'assertion': 'fake-assertion',
+ 'audience': 'fakeamo.org'})
+ eq_(res.status_code, 200)
+ eq_(self.user_profile.reload().is_verified, False)
+
+ # If the user is already logged in, then we return fast.
+ eq_(self.client.post(self.url).status_code, 200)
+
+ @patch.object(waffle, 'switch_is_active', lambda x: True)
@patch('users.models.UserProfile.log_login_attempt')
@patch('requests.post')
def test_browserid_login_logged(self, http_request, log_login_attempt):
url = reverse('users.browserid_login')
http_request.return_value = FakeResponse(200, json.dumps(
- {'status': 'okay',
- 'email': 'jbalogh@mozilla.com'}))
+ {'status': 'okay', 'email': 'jbalogh@mozilla.com'}))
self.client.post(url, data=dict(assertion='fake-assertion',
audience='fakeamo.org'))
log_login_attempt.assert_called_once_with(True)
@@ -674,7 +707,7 @@ def test_browserid_login_failure(self, http_request):
data=dict(assertion='fake-assertion',
audience='fakeamo.org'))
eq_(res.status_code, 401)
- assert 'BrowserID authentication failure' in res.content
+ assert 'Persona authentication failure' in res.content
@patch.object(waffle, 'switch_is_active', lambda x: True)
@patch('requests.post')
View
@@ -18,7 +18,7 @@
from django.utils.encoding import smart_str
from django.utils.http import base36_to_int
-from django_browserid.auth import BrowserIDBackend
+from django_browserid import get_audience, verify
from waffle.decorators import waffle_flag, waffle_switch
import commonware.log
@@ -309,33 +309,59 @@ def _clean_next_url(request):
def browserid_authenticate(request, assertion):
"""
Verify a BrowserID login attempt. If the BrowserID assertion is
- good, but no account exists on AMO, create one.
+ good, but no account exists, create one.
- Request is only needed for logging. :(
"""
- backend = BrowserIDBackend()
- result = backend.verify(assertion, settings.SITE_URL)
+ issuer = None
+ # If this call is for the B2G U-A, then set the appropriate issuer
+ # for potentially unverified emails.
+ if request.POST.get('native'):
+ issuer = getattr(settings, 'UNVERIFIED_ISSUER', None)
+ extra_params = {'issuer': issuer, 'allowUnverified': issuer is not None}
+ log.debug('Verifying Persona assertion: %s, audience: %s, issuer: %s, '
+ 'extra_params: %s')
+ result = verify(assertion, get_audience(request),
+ extra_params=extra_params)
if not result:
- return (None, "BrowserID authentication failure.")
- email = result['email']
- users = UserProfile.objects.filter(email=email)
- if len(users) == 1:
- users[0].user.backend = 'django_browserid.auth.BrowserIDBackend'
- return (users[0], None)
+ return None, _('Persona authentication failure.')
+
+ if 'unverified-email' in result:
+ email = result['unverified-email']
+ verified = False
+ else:
+ email = result['email']
+ verified = True
+
+ try:
+ profile = UserProfile.objects.filter(email=email)[0]
+ except IndexError:
+ profile = None
+
+ if profile:
+ if profile.is_verified and not verified:
+ # An attempt to log in to a verified address with an unverified
+ # assertion is a very bad thing. Don't let that happen.
+ log.debug('Verified user %s attempted to log in with an '
+ 'unverified assertion!' % profile)
+ return None, _('Please use the verified email for this account.')
@kumar303

kumar303 Jan 7, 2013

Member

@cvan are you sure this is right? It looks like this is returned as a string and not jsonified like other messages. @krupa saw:

[JavaScript Error: "Error: Syntax error, unrecognized expression: Please use the verified email for this ." {file: "https://marketplace-dev-cdn.allizom.org/media/js/mkt/consumer-min.js?build=a918a1b" line: 12}]
+ profile.is_verified = verified
+ profile.save()
+ profile.user.backend = 'django_browserid.auth.BrowserIDBackend'
+ profile.user.save()
+ return profile, None
+
username = autocreate_username(email.partition('@')[0])
source = (amo.LOGIN_SOURCE_MMO_BROWSERID if settings.MARKETPLACE else
amo.LOGIN_SOURCE_AMO_BROWSERID)
profile = UserProfile.objects.create(username=username, email=email,
- source=source, display_name=username)
-
- profile.create_django_user()
- profile.user.backend = 'django_browserid.auth.BrowserIDBackend'
- profile.user.save()
- profile.save()
+ source=source, display_name=username,
+ is_verified=verified)
+ profile.create_django_user(
+ backend='django_browserid.auth.BrowserIDBackend')
log_cef('New Account', 5, request, username=username,
signature='AUTHNOTICE',
- msg='User created a new account (from BrowserID)')
- return (profile, None)
+ msg='User created a new account (from Persona)')
+ return profile, None
@csrf_exempt
View
@@ -9,6 +9,8 @@ define('login', ['notification'], function(notification) {
$this.addClass('loading-submit');
requestedLogin = true;
navigator.id.request({
+ issuer: z.body.data('persona-unverified-issuer'),
+ allowUnverified: true,
termsOfService: '/terms-of-use',
privacyPolicy: '/privacy-policy',
oncancel: function() {
@@ -26,21 +28,25 @@ define('login', ['notification'], function(notification) {
});
function gotVerifiedEmail(assertion) {
if (assertion) {
+ var data = {assertion: assertion};
+ // TODO(Kumar): Change to z.capabilities.b2g.
+ if (z.capabilities.gaia) {
+ data.native = '1';
+ }
$.ajax({
url: $('body').data('login-url'),
type: 'POST',
- data: {'assertion': assertion},
+ data: data,
success: finishLogin,
error: function(jqXHR, textStatus, error) {
- // ask for additional credential info.
var err = {msg: jqXHR.responseText};
if (!err.msg) {
err.msg = gettext("BrowserID login failed. Maybe you don't have an account under that email address?") + " " + textStatus + " " + error;
}
z.page.trigger('notify', {msg: $(err.msg).text()});
$.Deferred().reject(err);
}
- })
+ });
} else {
$('.loading-submit').removeClass('loading-submit');
}
@@ -0,0 +1,3 @@
+-- Add verified email flag for unverified email addresses (bug 794634)
+
+ALTER TABLE users ADD COLUMN is_verified tinyint(1) unsigned DEFAULT 1;
View
@@ -391,3 +391,6 @@ def APPCACHE_MEDIA_DEBUG():
# Allow /developers/?refresh to refresh all MDN content for Developer Hub.
MDN_LAZY_REFRESH = False
+
+# The issuer for unverified Persona email addresses.
+UNVERIFIED_ISSUER = 'fxos.login.persona.org'
@@ -56,6 +56,7 @@
data-media-url="{{ MEDIA_URL }}"
data-login-url="{{ url('users.browserid_login') }}"
data-persona-url="{{ settings.BROWSERID_JS_URL }}"
+ data-persona-unverified-issuer="{{ settings.UNVERIFIED_ISSUER }}"
data-collect-timings="{{ url('mkt.timing.record') }}:{{ collect_timings_percent }}"
data-simulate-nav-pay="{{ settings.SIMULATE_NAV_PAY|json }}"
data-page-type="{{ pagetype or 'leaf' }}"
View
@@ -78,7 +78,8 @@ tower==0.3.4
-e git://github.com/mattbasta/addon-packager.git@576517360b6a9aadbfe77752ca0dbda5b0187474#egg=addon-packager
-e git://github.com/mozilla/amo-validator.git@7f1195f7d13a9fd64395e52bb8145700601a9030#egg=amo-validator
-e git://github.com/mozilla/app-validator.git@d791218125062f08538722fb99fe50967ace99a3#egg=app-validator
--e git://github.com/mozilla/django-browserid.git@1c23655faad0cc5ad2c18aebc0472a0ea078cfa8#egg=django-browserid
+# When available, use a version of django-browserid greater than 0.7.1
+-e git://github.com/mozilla/django-browserid.git@20fbd861dbf9f0000f0e7763dc4ce4b48c661744#egg=django-browserid
-e git://github.com/jsocol/commonware.git@27646ecaca40a89024cc581c3ecf5eb0fa87ee11#egg=commonware
-e git://github.com/jbalogh/django-queryset-transform@a1ba6ae41bd86f5bb9ff66fb56614e0fafe6e022#egg=django-queryset-transform
-e git://github.com/miracle2k/django-tables.git@546f339308103880c823b2056830fcdc9220edd0#egg=django-tables

0 comments on commit b65fad4

Please sign in to comment.