This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/allura.git
commit 5a99493a5aecc40b9438b85c70a455648f3b4305 Author: Dave Brondsema <[email protected]> AuthorDate: Fri May 15 18:16:58 2026 -0400 [#8607] check before mailbox sub too --- Allura/allura/controllers/auth.py | 10 +++++---- Allura/allura/tests/functional/test_auth.py | 32 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py index 6ff0c7577..385cc6c08 100644 --- a/Allura/allura/controllers/auth.py +++ b/Allura/allura/controllers/auth.py @@ -1373,10 +1373,12 @@ def index(self, **kw): def update_subscriptions(self, subscriptions=None, email_format=None, **kw): for s in subscriptions: if s['subscribed']: - if s['tool_id'] and s['project_id']: - M.Mailbox.subscribe( - project_id=bson.ObjectId(s['project_id']), - app_config_id=bson.ObjectId(s['tool_id'])) + if s['tool_id']: + app_config = M.AppConfig.query.get(_id=bson.ObjectId(s['tool_id'])) + if app_config and has_access(app_config, 'read'): + M.Mailbox.subscribe( + project_id=app_config.project_id, + app_config_id=app_config._id) else: mbox = s['subscription_id'] if mbox is not None and mbox.user_id == c.user._id: diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py index 347ec996e..31cb3e413 100644 --- a/Allura/allura/tests/functional/test_auth.py +++ b/Allura/allura/tests/functional/test_auth.py @@ -984,6 +984,38 @@ def test_prefs_subscriptions_unsubscribe(self): s = M.Mailbox.query.get(_id=s_id) assert not s, "User still has subscription with Mailbox._id %s" % s_id + @td.with_user_project('test-admin') + @patch('allura.controllers.auth.has_access') + def test_update_subscriptions_subscribe_requires_read_access(self, has_access_mock): + # Subscribe must be rejected when the user lacks read access to the tool. + has_access_mock.side_effect = lambda obj, *a, **kw: not isinstance(obj, M.AppConfig) + project = M.Project.query.get(shortname='test') + app_config = project.app_configs[0] + attacker = M.User.by_username('test-admin') + M.Mailbox.query.remove(dict( + user_id=attacker._id, + app_config_id=app_config._id, + artifact_index_id=None)) + ThreadLocalODMSession.flush_all() + + self.app.get('/').follow() + self.app.post('/auth/subscriptions/update_subscriptions', + params={ + 'subscriptions-0.subscribed': 'on', + 'subscriptions-0.tool_id': str(app_config._id), + 'subscriptions-0.project_id': str(project._id), + 'subscriptions-0.subscription_id': '', + 'subscriptions-0.topic': '', + 'subscriptions-0.artifact_index_id': '', + '_csrf_token': self.app.cookies['_csrf_token'], + }) + + mbox = M.Mailbox.query.get( + user_id=attacker._id, + app_config_id=app_config._id, + artifact_index_id=None) + assert mbox is None + @td.with_user_project('test-admin') def test_update_subscriptions_rejects_foreign_mailbox(self): # Create a Mailbox owned by test-user-1 (the victim).
