You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by je...@apache.org on 2014/09/12 12:29:21 UTC
[07/28] git commit: [#7527] Allow different users to claim the same
email address + tests
[#7527] Allow different users to claim the same email address + tests
Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/3e6fdda3
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/3e6fdda3
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/3e6fdda3
Branch: refs/heads/je/42cc_4905
Commit: 3e6fdda33a95a69a0fe68655e802db2afbf52c06
Parents: 7c30568
Author: Alexander Luberg <al...@slashdotmedia.com>
Authored: Tue Jul 22 18:15:06 2014 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Thu Aug 28 20:26:59 2014 +0000
----------------------------------------------------------------------
Allura/allura/controllers/auth.py | 24 +++++++--
Allura/allura/lib/mail_util.py | 4 +-
Allura/allura/model/auth.py | 25 ++++-----
Allura/allura/tests/functional/test_auth.py | 65 +++++++++++++++++++++---
Allura/allura/tests/model/test_auth.py | 12 +++--
Allura/allura/tests/test_mail_util.py | 8 +--
6 files changed, 104 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/controllers/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 247a94b..dc3a177 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -179,7 +179,7 @@ class AuthController(BaseController):
if not email:
redirect('/')
- email_record = M.EmailAddress.query.find({'_id': email}).first()
+ email_record = M.EmailAddress.query.find({'email': email}).first()
user_record = M.User.by_email_address(email)
if user_record and email_record.confirmed:
@@ -225,7 +225,7 @@ class AuthController(BaseController):
@expose()
def send_verification_link(self, a):
- addr = M.EmailAddress.query.get(_id=a)
+ addr = M.EmailAddress.query.get(email=a, claimed_by_user_id=c.user._id)
if addr:
addr.send_verification_link()
flash('Verification link sent')
@@ -236,8 +236,25 @@ class AuthController(BaseController):
@expose()
def verify_addr(self, a):
addr = M.EmailAddress.query.get(nonce=a)
+ # import pydevd
+ # pydevd.settrace('localhost', port=14000, stdoutToServer=True, stderrToServer=True)
+
if addr:
addr.confirmed = True
+ # Remove other non-confirmed emails claimed by other users
+ claimed_by_others = M.EmailAddress.query.find({
+ 'email': addr.email,
+ 'claimed_by_user_id': {"$ne": addr.claimed_by_user_id}
+ }).all()
+
+ users = [email.claimed_by_user() for email in claimed_by_others]
+ for user in users:
+ user.email_addresses.remove(addr.email)
+
+ M.EmailAddress.query.remove({
+ 'email': addr.email,
+ 'claimed_by_user_id': {'$ne': addr.claimed_by_user_id}
+ })
flash('Email address confirmed')
M.AuditLog.log_user('Email address verified: %s', addr._id)
else:
@@ -442,7 +459,8 @@ class PreferencesController(BaseController):
if not kw.get('password') or not provider.validate_password(c.user, kw.get('password')):
flash('You must provide your current password to claim new email', 'error')
redirect('.')
- if M.EmailAddress.query.get(_id=new_addr['addr']):
+ if M.EmailAddress.query.get(email=new_addr['addr'], confirmed=True) \
+ or M.EmailAddress.query.get(email=new_addr['addr'], claimed_by_user_id=c.user._id):
flash('Email address already claimed', 'error')
elif mail_util.isvalid(new_addr['addr']):
c.user.email_addresses.append(new_addr['addr'])
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/lib/mail_util.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/mail_util.py b/Allura/allura/lib/mail_util.py
index 4da1819..a932f13 100644
--- a/Allura/allura/lib/mail_util.py
+++ b/Allura/allura/lib/mail_util.py
@@ -159,13 +159,13 @@ def identify_sender(peer, email_address, headers, msg):
from allura import model as M
# Dumb ID -- just look for email address claimed by a particular user
addr = M.EmailAddress.query.get(
- _id=M.EmailAddress.canonical(email_address))
+ email=M.EmailAddress.canonical(email_address))
if addr and addr.claimed_by_user_id:
return addr.claimed_by_user()
from_address = headers.get('From', '').strip()
if not from_address:
return M.User.anonymous()
- addr = M.EmailAddress.query.get(_id=M.EmailAddress.canonical(from_address))
+ addr = M.EmailAddress.query.get(email=M.EmailAddress.canonical(from_address))
if addr and addr.claimed_by_user_id:
return addr.claimed_by_user()
return M.User.anonymous()
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/model/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 5d7900b..1c753d5 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -115,12 +115,12 @@ class EmailAddress(MappedClass):
class __mongometa__:
name = 'email_address'
session = main_orm_session
- indexes = [
- 'claimed_by_user_id']
+ unique_indexes = [('email', 'claimed_by_user_id'),]
- _id = FieldProperty(str)
+ _id = FieldProperty(S.ObjectId)
+ email = FieldProperty(str)
claimed_by_user_id = FieldProperty(S.ObjectId, if_missing=None)
- confirmed = FieldProperty(bool)
+ confirmed = FieldProperty(bool, if_missing=False)
nonce = FieldProperty(str)
def claimed_by_user(self):
@@ -129,10 +129,7 @@ class EmailAddress(MappedClass):
@classmethod
def upsert(cls, addr):
addr = cls.canonical(addr)
- result = cls.query.get(_id=addr)
- if not result:
- result = cls(_id=addr)
- return result
+ return cls(email=addr)
@classmethod
def canonical(cls, addr):
@@ -147,13 +144,13 @@ class EmailAddress(MappedClass):
def send_verification_link(self):
self.nonce = sha256(os.urandom(10)).hexdigest()
- log.info('Sending verification link to %s', self._id)
+ log.info('Sending verification link to %s', self.email)
text = '''
To verify the email address %s belongs to the user %s,
please visit the following URL:
%s
-''' % (self._id, self.claimed_by_user().username, g.url('/auth/verify_addr', a=self.nonce))
+''' % (self.email, self.claimed_by_user().username, g.url('/auth/verify_addr', a=self.nonce))
log.info('Verification email:\n%s', text)
allura.tasks.mail_tasks.sendsimplemail.post(
fromaddr=g.noreply,
@@ -550,7 +547,7 @@ class User(MappedClass, ActivityNode, ActivityObject):
@classmethod
def by_email_address(cls, addr):
- ea = EmailAddress.query.get(_id=addr)
+ ea = EmailAddress.query.get(email=addr)
if ea is None:
return None
return ea.claimed_by_user()
@@ -573,7 +570,7 @@ class User(MappedClass, ActivityNode, ActivityObject):
state(self).soil()
def address_object(self, addr):
- return EmailAddress.query.get(_id=addr, claimed_by_user_id=self._id)
+ return EmailAddress.query.get(email=addr, claimed_by_user_id=self._id)
def claim_address(self, email_address):
addr = EmailAddress.canonical(email_address)
@@ -592,10 +589,10 @@ class User(MappedClass, ActivityNode, ActivityObject):
addresses = set(self.email_addresses)
for addr in EmailAddress.query.find(
dict(claimed_by_user_id=self._id)):
- if addr._id in addresses:
+ if addr.email in addresses:
if not addr.confirmed:
addr.confirmed = True
- addresses.remove(addr._id)
+ addresses.remove(addr.email)
else:
addr.delete()
for a in addresses:
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/tests/functional/test_auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index 8af23ca..65d3a50 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -57,8 +57,7 @@ class TestAuth(TestController):
def test_login(self):
self.app.get('/auth/')
- r = self.app.post('/auth/send_verification_link',
- params=dict(a='test@example.com'))
+ r = self.app.post('/auth/send_verification_link', params=dict(a='test@example.com'))
email = M.User.query.get(username='test-admin').email_addresses[0]
r = self.app.post('/auth/send_verification_link', params=dict(a=email))
ThreadLocalORMSession.flush_all()
@@ -115,6 +114,60 @@ class TestAuth(TestController):
if header == 'Set-cookie':
assert_in('expires', contents)
+ def test_claimed_duplicate_emails_by_different_users(self):
+ email_address = 'test-email@domain.com'
+
+ user1 = M.User.query.get(username='test-user-0')
+ user2 = M.User.query.get(username='test-user-1')
+
+ user1.claim_address(email_address)
+ user2.claim_address(email_address)
+
+ ThreadLocalORMSession.flush_all()
+ r = self.app.post('/auth/send_verification_link',
+ params=dict(a=email_address),
+ extra_environ={'username': 'test-user-0'})
+ email1 = M.EmailAddress.query.find(dict(email=email_address, claimed_by_user_id=user1._id)).first()
+ email2 = M.EmailAddress.query.find(dict(email=email_address, claimed_by_user_id=user2._id)).first()
+
+ # User1 verifies claimed address. All other duplicates should be removed once it is confirmed
+ r = self.app.get('/auth/verify_addr', params=dict(a=email1.nonce))
+
+ assert email1.confirmed == True
+ assert M.EmailAddress.query.find(dict(email=email_address)).count() == 1
+ assert email_address in M.User.query.get(username='test-user-0').email_addresses
+ assert email_address not in M.User.query.get(username='test-user-1').email_addresses
+
+ @td.with_user_project('test-admin')
+ def test_user_can_not_claim_duplicate_emails(self):
+ email_address = 'test_abcd_123@domain.net'
+ user = M.User.query.get(username='test-admin')
+ addresses_number = len(user.email_addresses)
+ self.app.post('/auth/preferences/update',
+ params={
+ 'preferences.display_name': 'Test Admin',
+ 'new_addr.addr': email_address,
+ 'new_addr.claim': 'Claim Address',
+ 'primary_addr': 'test-admin@users.localhost',
+ 'preferences.email_format': 'plain'
+ },
+ extra_environ=dict(username='test-admin'))
+
+ assert M.EmailAddress.query.find(dict(email=email_address, claimed_by_user_id=user._id)).count() == 1
+ r = self.app.post('/auth/preferences/update',
+ params={
+ 'preferences.display_name': 'Test Admin',
+ 'new_addr.addr': email_address,
+ 'new_addr.claim': 'Claim Address',
+ 'primary_addr': 'test-admin@users.localhost',
+ 'preferences.email_format': 'plain'
+ },
+ extra_environ=dict(username='test-admin'))
+
+ assert json.loads(self.webflash(r))['status'] == 'error', self.webflash(r)
+ assert M.EmailAddress.query.find(dict(email=email_address, claimed_by_user_id=user._id)).count() == 1
+ assert len(M.User.query.get(username='test-admin').email_addresses) == addresses_number + 1
+
@td.with_user_project('test-admin')
def test_prefs(self):
r = self.app.get('/auth/preferences/',
@@ -792,7 +845,7 @@ class TestPasswordReset(TestController):
{'claimed_by_user_id': user._id}).first()
email.confirmed = False
ThreadLocalORMSession.flush_all()
- self.app.post('/auth/password_recovery_hash', {'email': email._id})
+ self.app.post('/auth/password_recovery_hash', {'email': email.email})
hash = user.get_tool_data('AuthPasswordReset', 'hash')
assert hash is None
@@ -804,7 +857,7 @@ class TestPasswordReset(TestController):
{'claimed_by_user_id': user._id}).first()
user.disabled = True
ThreadLocalORMSession.flush_all()
- self.app.post('/auth/password_recovery_hash', {'email': email._id})
+ self.app.post('/auth/password_recovery_hash', {'email': email.email})
hash = user.get_tool_data('AuthPasswordReset', 'hash')
assert hash is None
@@ -818,7 +871,7 @@ class TestPasswordReset(TestController):
ThreadLocalORMSession.flush_all()
old_pw_hash = user.password
with td.audits('Password recovery link sent to: test-admin@users.localhost'):
- r = self.app.post('/auth/password_recovery_hash', {'email': email._id})
+ r = self.app.post('/auth/password_recovery_hash', {'email': email.email})
hash = user.get_tool_data('AuthPasswordReset', 'hash')
hash_expiry = user.get_tool_data('AuthPasswordReset', 'hash_expiry')
assert hash is not None
@@ -865,7 +918,7 @@ To reset your password on %s, please visit the following URL:
{'claimed_by_user_id': user._id}).first()
email.confirmed = True
ThreadLocalORMSession.flush_all()
- r = self.app.post('/auth/password_recovery_hash', {'email': email._id})
+ r = self.app.post('/auth/password_recovery_hash', {'email': email.email})
user = M.User.by_username('test-admin')
hash = user.get_tool_data('AuthPasswordReset', 'hash')
user.set_tool_data('AuthPasswordReset',
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/tests/model/test_auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_auth.py b/Allura/allura/tests/model/test_auth.py
index f3869ee..165e331 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -51,24 +51,26 @@ def setUp():
@with_setup(setUp)
def test_email_address():
- addr = M.EmailAddress(_id='test_admin@domain.net',
+ addr = M.EmailAddress(email='test_admin@domain.net',
claimed_by_user_id=c.user._id)
ThreadLocalORMSession.flush_all()
assert addr.claimed_by_user() == c.user
addr2 = M.EmailAddress.upsert('test@domain.net')
addr3 = M.EmailAddress.upsert('test_admin@domain.net')
- assert addr3 is addr
+
+ # Duplicate emails are allowed, until the email is confirmed
+ assert addr3 is not addr
+
assert addr2 is not addr
assert addr2
addr4 = M.EmailAddress.upsert('test@DOMAIN.NET')
- assert addr4 is addr2
+ assert addr4 is not addr2
with patch('allura.lib.app_globals.request', Request.blank('/')):
addr.send_verification_link()
assert addr is c.user.address_object('test_admin@domain.net')
c.user.claim_address('test@DOMAIN.NET')
assert 'test@domain.net' in c.user.email_addresses
-
@td.with_user_project('test-admin')
@with_setup(setUp)
def test_user():
@@ -173,7 +175,7 @@ def test_default_project_roles():
@with_setup(setUp)
def test_email_address_claimed_by_user():
- addr = M.EmailAddress(_id='test_admin@domain.net',
+ addr = M.EmailAddress(email='test_admin@domain.net',
claimed_by_user_id=c.user._id)
c.user.disabled = True
ThreadLocalORMSession.flush_all()
http://git-wip-us.apache.org/repos/asf/allura/blob/3e6fdda3/Allura/allura/tests/test_mail_util.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_mail_util.py b/Allura/allura/tests/test_mail_util.py
index 64a1c92..c2fe363 100644
--- a/Allura/allura/tests/test_mail_util.py
+++ b/Allura/allura/tests/test_mail_util.py
@@ -193,7 +193,7 @@ class TestIdentifySender(object):
EA.query.get.side_effect = [
mock.Mock(claimed_by_user_id=True, claimed_by_user=lambda:'user')]
assert_equal(identify_sender(None, 'arg', None, None), 'user')
- EA.query.get.assert_called_once_with(_id='arg')
+ EA.query.get.assert_called_once_with(email='arg')
@mock.patch('allura.model.EmailAddress')
def test_header(self, EA):
@@ -203,7 +203,7 @@ class TestIdentifySender(object):
assert_equal(
identify_sender(None, 'arg', {'From': 'from'}, None), 'user')
assert_equal(EA.query.get.call_args_list,
- [mock.call(_id='arg'), mock.call(_id='from')])
+ [mock.call(email='arg'), mock.call(email='from')])
@mock.patch('allura.model.User')
@mock.patch('allura.model.EmailAddress')
@@ -213,7 +213,7 @@ class TestIdentifySender(object):
EA.query.get.side_effect = [
None, mock.Mock(claimed_by_user_id=True, claimed_by_user=lambda:'user')]
assert_equal(identify_sender(None, 'arg', {}, None), anon)
- assert_equal(EA.query.get.call_args_list, [mock.call(_id='arg')])
+ assert_equal(EA.query.get.call_args_list, [mock.call(email='arg')])
@mock.patch('allura.model.User')
@mock.patch('allura.model.EmailAddress')
@@ -224,7 +224,7 @@ class TestIdentifySender(object):
assert_equal(
identify_sender(None, 'arg', {'From': 'from'}, None), anon)
assert_equal(EA.query.get.call_args_list,
- [mock.call(_id='arg'), mock.call(_id='from')])
+ [mock.call(email='arg'), mock.call(email='from')])
def test_parse_message_id():