You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2013/06/25 04:01:28 UTC
[3/4] git commit: [#6053] ticket:376 Check for disabled users in user
lookups
[#6053] ticket:376 Check for disabled users in user lookups
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/e7850dca
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/e7850dca
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/e7850dca
Branch: refs/heads/master
Commit: e7850dca52d302f701a15616e2037f016c71d743
Parents: 4fc80e1
Author: Yuriy Arhipov <yu...@yandex.ru>
Authored: Fri Jun 21 15:25:21 2013 +0400
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Tue Jun 25 02:00:44 2013 +0000
----------------------------------------------------------------------
Allura/allura/controllers/project.py | 1 +
Allura/allura/lib/plugin.py | 4 +-
Allura/allura/model/notification.py | 9 ++-
Allura/allura/model/project.py | 2 +-
Allura/allura/tasks/mail_tasks.py | 6 +-
Allura/allura/tests/functional/test_home.py | 8 +++
Allura/allura/tests/model/test_notification.py | 42 +++++++++++++
Allura/allura/tests/model/test_project.py | 12 ++++
Allura/allura/tests/test_tasks.py | 65 +++++++++++++++++++++
9 files changed, 141 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/controllers/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py
index e42bd2f..bff0399 100644
--- a/Allura/allura/controllers/project.py
+++ b/Allura/allura/controllers/project.py
@@ -418,6 +418,7 @@ class ProjectController(FeedController):
users = M.User.query.find({
'_id': {'$in': named_roles.userids_that_reach},
'display_name': re.compile(r'(?i)%s' % re.escape(term)),
+ 'disabled': False,
}).sort('username').limit(10).all()
return dict(
users=[
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 4bbf557..861fb4f 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -315,7 +315,7 @@ class LdapAuthenticationProvider(AuthenticationProvider):
def by_username(self, username):
from allura import model as M
- return M.User.query.get(username=username)
+ return M.User.query.get(username=username, disabled=False)
def set_password(self, user, old_password, new_password):
try:
@@ -329,7 +329,7 @@ class LdapAuthenticationProvider(AuthenticationProvider):
def _login(self):
from allura import model as M
- user = M.User.query.get(username=self.request.params['username'])
+ user = M.User.query.get(username=self.request.params['username'], disabled=False)
if user is None: raise exc.HTTPUnauthorized()
try:
dn = 'uid=%s,%s' % (user.username, config['auth.ldap.suffix'])
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/model/notification.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/notification.py b/Allura/allura/model/notification.py
index 5889e10..5370c92 100644
--- a/Allura/allura/model/notification.py
+++ b/Allura/allura/model/notification.py
@@ -234,9 +234,13 @@ class Notification(MappedClass):
text=(self.text or '') + self.footer(toaddr))
def send_direct(self, user_id):
- user = User.query.get(_id=ObjectId(user_id))
+ user = User.query.get(_id=ObjectId(user_id), disabled=False)
artifact = self.ref.artifact
log.debug('Sending direct notification %s to user %s', self._id, user_id)
+ # Don't send if user disabled
+ if not user:
+ log.debug("Skipping notification - User %s isn't active " % user_id)
+ return
# Don't send if user doesn't have read perms to the artifact
if user and artifact and \
not security.has_access(artifact, 'read', user)():
@@ -260,9 +264,10 @@ class Notification(MappedClass):
def send_digest(self, user_id, from_address, subject, notifications,
reply_to_address=None):
if not notifications: return
+ user = User.query.get(_id=ObjectId(user_id), disabled=False)
+ if not user: return
# Filter out notifications for which the user doesn't have read
# permissions to the artifact.
- user = User.query.get(_id=ObjectId(user_id))
artifact = self.ref.artifact
def perm_check(notification):
return not (user and artifact) or \
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/model/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/project.py b/Allura/allura/model/project.py
index 6988ffc..b889515 100644
--- a/Allura/allura/model/project.py
+++ b/Allura/allura/model/project.py
@@ -705,7 +705,7 @@ class Project(MappedClass, ActivityNode, ActivityObject):
g.credentials,
g.credentials.project_roles(project_id=self.root_project._id).named)
uids = [uid for uid in named_roles.userids_that_reach if uid is not None]
- return list(User.query.find({'_id': {'$in': uids}}))
+ return list(User.query.find({'_id': {'$in': uids}, 'disabled': False}))
def users_with_role(self, *role_names):
"""Return all users in this project that have at least one of the roles
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/tasks/mail_tasks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tasks/mail_tasks.py b/Allura/allura/tasks/mail_tasks.py
index 0be5a8c..6526987 100644
--- a/Allura/allura/tasks/mail_tasks.py
+++ b/Allura/allura/tasks/mail_tasks.py
@@ -84,7 +84,7 @@ def sendmail(fromaddr, destinations, text, reply_to, subject,
fromaddr = u'noreply@in.sf.net'
elif '@' not in fromaddr:
log.warning('Looking up user with fromaddr: %s', fromaddr)
- user = M.User.query.get(_id=ObjectId(fromaddr))
+ user = M.User.query.get(_id=ObjectId(fromaddr), disabled=False)
if not user:
log.warning('Cannot find user with ID: %s', fromaddr)
fromaddr = u'noreply@in.sf.net'
@@ -96,7 +96,7 @@ def sendmail(fromaddr, destinations, text, reply_to, subject,
addrs_plain.append(addr)
else:
try:
- user = M.User.query.get(_id=ObjectId(addr))
+ user = M.User.query.get(_id=ObjectId(addr), disabled=False)
if not user:
log.warning('Cannot find user with ID: %s', addr)
continue
@@ -146,7 +146,7 @@ def sendsimplemail(
fromaddr = u'noreply@in.sf.net'
elif '@' not in fromaddr:
log.warning('Looking up user with fromaddr: %s', fromaddr)
- user = M.User.query.get(_id=ObjectId(fromaddr))
+ user = M.User.query.get(_id=ObjectId(fromaddr), disabled=False)
if not user:
log.warning('Cannot find user with ID: %s', fromaddr)
fromaddr = u'noreply@in.sf.net'
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/tests/functional/test_home.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_home.py b/Allura/allura/tests/functional/test_home.py
index aa9a684..f57107f 100644
--- a/Allura/allura/tests/functional/test_home.py
+++ b/Allura/allura/tests/functional/test_home.py
@@ -90,6 +90,14 @@ class TestProjectHome(TestController):
j = json.loads(r.body)
assert j['users'][0]['id'].startswith('test')
+ def test_user_search_for_disabled_user(self):
+ user = M.User.by_username('test-admin')
+ user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ r = self.app.get('/p/test/user_search?term=test', status=200)
+ j = json.loads(r.body)
+ assert j == {'users': []}
+
def test_user_search_noparam(self):
r = self.app.get('/p/test/user_search', status=400)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/tests/model/test_notification.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_notification.py b/Allura/allura/tests/model/test_notification.py
index 469816d..4e9dd5b 100644
--- a/Allura/allura/tests/model/test_notification.py
+++ b/Allura/allura/tests/model/test_notification.py
@@ -310,6 +310,48 @@ class TestSubscriptionTypes(unittest.TestCase):
notifications[3].send_direct.assert_called_once_with(u0)
notifications[4].send_direct.assert_called_once_with(u0)
+ def test_send_direct_disabled_user(self):
+ user = M.User.by_username('test-admin')
+ thd = M.Thread.query.get(ref_id=self.pg.index_id())
+ notification = M.Notification()
+ notification.ref_id = thd.index_id()
+ user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ notification.send_direct(user._id)
+ count = M.MonQTask.query.find(dict(
+ task_name='allura.tasks.mail_tasks.sendmail',
+ state='ready')).count()
+ assert_equal(count, 0)
+ user.disabled = False
+ ThreadLocalORMSession.flush_all()
+ notification.send_direct(user._id)
+ count = M.MonQTask.query.find(dict(
+ task_name='allura.tasks.mail_tasks.sendmail',
+ state='ready')).count()
+ assert_equal(count, 1)
+
+ @mock.patch('allura.model.notification.Notification.ref')
+ def test_send_digest_disabled_user(self, ref):
+ thd = M.Thread.query.get(ref_id=self.pg.index_id())
+ notification = M.Notification()
+ notification.ref_id = thd.index_id()
+ ref.artifact = thd
+ user = M.User.by_username('test-admin')
+ user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ M.Notification.send_digest(user._id, 'test@mail.com', 'subject', [notification])
+ count = M.MonQTask.query.find(dict(
+ task_name='allura.tasks.mail_tasks.sendmail',
+ state='ready')).count()
+ assert_equal(count, 0)
+ user.disabled = False
+ ThreadLocalORMSession.flush_all()
+ M.Notification.send_digest(user._id, 'test@mail.com', 'subject', [notification])
+ count = M.MonQTask.query.find(dict(
+ task_name='allura.tasks.mail_tasks.sendmail',
+ state='ready')).count()
+ assert_equal(count, 1)
+
def _clear_subscriptions():
M.Mailbox.query.remove({})
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/tests/model/test_project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_project.py b/Allura/allura/tests/model/test_project.py
index d88fbdd..33791f4 100644
--- a/Allura/allura/tests/model/test_project.py
+++ b/Allura/allura/tests/model/test_project.py
@@ -124,3 +124,15 @@ def test_users_and_roles():
assert p.users_with_role('Admin') == [u]
assert p.users_with_role('Admin') == sub.users_with_role('Admin')
assert p.users_with_role('Admin') == p.admins()
+
+def test_project_disabled_users():
+ p = M.Project.query.get(shortname='test')
+ users = p.users()
+ assert users[0].username == 'test-admin'
+ user = M.User.by_username('test-admin')
+ user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ users = p.users()
+ assert users == []
+
+
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/e7850dca/Allura/allura/tests/test_tasks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py
index e1885c0..c85ce50 100644
--- a/Allura/allura/tests/test_tasks.py
+++ b/Allura/allura/tests/test_tasks.py
@@ -27,6 +27,7 @@ from pylons import tmpl_context as c, app_globals as g
from datadiff.tools import assert_equal
from nose.tools import assert_in
from ming.orm import FieldProperty, Mapper
+from ming.orm import ThreadLocalORMSession
from alluratest.controller import setup_basic_test, setup_global_objects
@@ -196,6 +197,70 @@ class TestMailTasks(unittest.TestCase):
assert_in('Content-Transfer-Encoding: base64', body)
assert_in(b64encode(u'Громады стройные теснятся'.encode('utf-8')), body)
+ def test_send_email_with_disabled_user(self):
+ c.user = M.User.by_username('test-admin')
+ c.user.disabled = True
+ destination_user = M.User.by_username('test-user-1')
+ destination_user.preferences['email_address'] = 'user1@mail.com'
+ ThreadLocalORMSession.flush_all()
+ with mock.patch.object(mail_tasks.smtp_client, '_client') as _client:
+ mail_tasks.sendmail(
+ fromaddr=str(c.user._id),
+ destinations=[ str(destination_user._id) ],
+ text=u'This is a test',
+ reply_to=u'noreply@sf.net',
+ subject=u'Test subject',
+ message_id=h.gen_message_id())
+ assert_equal(_client.sendmail.call_count, 1)
+ return_path, rcpts, body = _client.sendmail.call_args[0]
+ body = body.split('\n')
+ assert_in('From: noreply@in.sf.net', body)
+
+ def test_send_email_with_disabled_destination_user(self):
+ c.user = M.User.by_username('test-admin')
+ destination_user = M.User.by_username('test-user-1')
+ destination_user.preferences['email_address'] = 'user1@mail.com'
+ destination_user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ with mock.patch.object(mail_tasks.smtp_client, '_client') as _client:
+ mail_tasks.sendmail(
+ fromaddr=str(c.user._id),
+ destinations=[ str(destination_user._id) ],
+ text=u'This is a test',
+ reply_to=u'noreply@sf.net',
+ subject=u'Test subject',
+ message_id=h.gen_message_id())
+ assert_equal(_client.sendmail.call_count, 0)
+
+ def test_sendsimplemail_with_disabled_user(self):
+ c.user = M.User.by_username('test-admin')
+ with mock.patch.object(mail_tasks.smtp_client, '_client') as _client:
+ mail_tasks.sendsimplemail(
+ fromaddr=str(c.user._id),
+ toaddr='test@mail.com',
+ text=u'This is a test',
+ reply_to=u'noreply@sf.net',
+ subject=u'Test subject',
+ message_id=h.gen_message_id())
+ assert_equal(_client.sendmail.call_count, 1)
+ return_path, rcpts, body = _client.sendmail.call_args[0]
+ body = body.split('\n')
+ assert_in('From: "Test Admin" <te...@users.localhost>', body)
+
+ c.user.disabled = True
+ ThreadLocalORMSession.flush_all()
+ mail_tasks.sendsimplemail(
+ fromaddr=str(c.user._id),
+ toaddr='test@mail.com',
+ text=u'This is a test',
+ reply_to=u'noreply@sf.net',
+ subject=u'Test subject',
+ message_id=h.gen_message_id())
+ assert_equal(_client.sendmail.call_count, 2)
+ return_path, rcpts, body = _client.sendmail.call_args[0]
+ body = body.split('\n')
+ assert_in('From: noreply@in.sf.net', body)
+
@td.with_wiki
def test_receive_email_ok(self):
c.user = M.User.by_username('test-admin')