This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch db/8607 in repository https://gitbox.apache.org/repos/asf/allura.git
commit a47c0bce603c41e3898b02e30a5b5d9399fab2da Author: Dave Brondsema <[email protected]> AuthorDate: Thu May 21 14:30:26 2026 -0400 [#8607] various has_access improvements --- Allura/allura/lib/security.py | 13 +++++++++- Allura/allura/tests/test_tasks.py | 30 ++++++++++++++++++++++ .../forgetracker/tests/functional/test_rest.py | 7 +++++ ForgeTracker/forgetracker/tests/test_app.py | 20 +++++++++++++++ ForgeTracker/forgetracker/tracker_main.py | 21 +++++++++------ 5 files changed, 82 insertions(+), 9 deletions(-) diff --git a/Allura/allura/lib/security.py b/Allura/allura/lib/security.py index 10805f093..1b2b1fd71 100644 --- a/Allura/allura/lib/security.py +++ b/Allura/allura/lib/security.py @@ -307,7 +307,9 @@ def debug_obj(obj) -> str: def has_access(obj, permission: str, user: M.User | None = None, project: M.Project | None = None, roles=None) -> bool: '''Return whether the given user has the permission name on the given object. - - First, all the roles for a user in the given project context are computed. + - For individual artifacts, check for project 'read' permission + + - Then all the roles for a user in the given project context are computed. - If the given object's ACL contains a DENY for this permission on this user's project role, return False and deny access. TODO: make ACL order @@ -367,6 +369,15 @@ def has_access(obj, permission: str, user: M.User | None = None, project: M.Proj else: project = getattr(obj, 'project', None) or c.project project = project.root_project + + # check for project 'read' so that calls outside normal HTTP /p/foo/ dispatch + # (e.g. REST, email, notifications etc) will get the same checks as ProjectController._check_security. + # (note AppConfig.parent_security_context() returns None so the ACL chaining stops at the app) + # Skip for Project/Neighborhood else we'd recurse on this very call. + if (not isinstance(obj, (M.Project, M.Neighborhood)) + and not has_access(project, 'read', user=user)): + return False + roles: RoleCache = cred.user_roles(user_id=user._id, project_id=project._id).reaching_roles # TODO: move deny logic into loop below; see ticket [#6715] diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py index 5dfc88822..953a44ac1 100644 --- a/Allura/allura/tests/test_tasks.py +++ b/Allura/allura/tests/test_tasks.py @@ -544,6 +544,36 @@ def test_email_posting_disabled(self): message) assert hm.call_count == 0 + @td.with_tool('test', 'Tickets', 'bugs') + def test_receive_email_denied_without_project_read(self): + # A non-member sending email to a private project's tracker must be rejected, + # even though the tracker's own ACL grants *authenticated 'post'. + import forgetracker + from forgetracker import model as TM + # Ensure ticket #1 exists so the per-ticket has_access check would otherwise pass. + with h.push_config(c, user=M.User.by_username('test-admin')): + TM.Ticket.new(form_fields=dict(summary='public-ish')) + ThreadLocalODMSession.flush_all() + + project = M.Project.query.get(shortname='test') + was_private = project.private + project.private = True + ThreadLocalODMSession.flush_all() + try: + sender = M.User.by_username('test-user-2') + assert sender, 'test-user-2 fixture missing' + with mock.patch.object(forgetracker.tracker_main.ForgeTrackerApp, 'handle_message') as hm: + mail_tasks.route_email( + '0.0.0.0', + sender.email_addresses[0] if sender.email_addresses else '[email protected]', + ['[email protected]'], + 'Hello, world!') + assert hm.call_count == 0, \ + 'route_email called handle_message despite sender lacking project read' + finally: + project.private = was_private + ThreadLocalODMSession.flush_all() + class TestUserNotificationTasks(TestController): def setup_method(self, method): diff --git a/ForgeTracker/forgetracker/tests/functional/test_rest.py b/ForgeTracker/forgetracker/tests/functional/test_rest.py index 8092e2fcb..5727fabc5 100644 --- a/ForgeTracker/forgetracker/tests/functional/test_rest.py +++ b/ForgeTracker/forgetracker/tests/functional/test_rest.py @@ -26,6 +26,7 @@ from alluratest.controller import TestRestApiBase from forgetracker import model as TM +from ming.odm import ThreadLocalODMSession class TestTrackerApiBase(TestRestApiBase): @@ -152,6 +153,12 @@ def test_ticket_index_noauth(self): assert (ticket_config.options.get('TicketMonitoringEmail') == 'test@localhost') + def test_ticket_index_private_project_denied(self): + project = M.Project.query.get(shortname='test') + project.private = True + ThreadLocalODMSession.flush_all() + self.api_get('/rest/p/test/bugs/', user='*anonymous', status=[401, 403]) + @td.with_tool('test', 'Tickets', 'dummy') def test_move_ticket_redirect(self): p = M.Project.query.get(shortname='test') diff --git a/ForgeTracker/forgetracker/tests/test_app.py b/ForgeTracker/forgetracker/tests/test_app.py index d94a5e6d2..2dd9ca49e 100644 --- a/ForgeTracker/forgetracker/tests/test_app.py +++ b/ForgeTracker/forgetracker/tests/test_app.py @@ -66,6 +66,26 @@ def test_inbound_email_no_match(self): post = M.Post.query.get(_id=message_id) assert post is None + @td.with_tracker + def test_has_access_checks_per_ticket_acl(self): + # private ticket gets DENY_ALL for everyone except Developer + reporter + ticket = TM.Ticket.new() + ticket.summary = 'private ticket' + ticket.private = True + ThreadLocalODMSession.flush_all() + + admin = M.User.by_username('test-admin') # Admin/Developer + non_dev = M.User.by_username('test-user') # *authenticated, not Developer + + assert c.app.has_access(admin, str(ticket.ticket_num)) + assert not c.app.has_access(non_dev, str(ticket.ticket_num)) + + @td.with_tracker + def test_has_access_rejects_unknown_ticket(self): + non_dev = M.User.by_username('test-user') + assert not c.app.has_access(non_dev, '99999') + assert not c.app.has_access(non_dev, 'not-a-number') + @td.with_tracker def test_uninstall(self): t = TM.Ticket.new() diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py index 7cc17780c..b7ab8a64a 100644 --- a/ForgeTracker/forgetracker/tracker_main.py +++ b/ForgeTracker/forgetracker/tracker_main.py @@ -272,20 +272,25 @@ def __init__(self, project, config): def globals(self): return TM.Globals.query.get(app_config_id=self.config._id) + def _ticket_for_email_topic(self, topic): + try: + return TM.Ticket.query.get( + app_config_id=self.config._id, + ticket_num=int(topic)) + except (ValueError, TypeError): + return None + def has_access(self, user, topic): - return has_access(c.app, 'post', user) + ticket = self._ticket_for_email_topic(topic) + if ticket is None: + return False + return has_access(ticket, 'post', user) def handle_message(self, topic, message): log.info('Message from %s (%s)', topic, self.config.options.mount_point) log.info('Headers are: %s', message['headers']) - try: - ticket = TM.Ticket.query.get( - app_config_id=self.config._id, - ticket_num=int(topic)) - except Exception: - log.exception('Error getting ticket %s', topic) - return + ticket = self._ticket_for_email_topic(topic) if not ticket: log.info('No such ticket num: %s', topic) elif ticket.discussion_disabled:
