diff --git a/web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py b/web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py index 90227ae3d11..5eed7974176 100644 --- a/web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py +++ b/web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py @@ -80,3 +80,64 @@ def tearDown(self): if sg: db.session.delete(sg) db.session.commit() + + +class AdminCannotSeeOtherUserGroupTestCase(BaseTestGenerator): + """Regression test for issue #9933. + + An Administrator must NOT auto-see another user's private server + group. Admin role grants management of pgAdmin itself (users, + preferences), not inheritance of other users' database + connections. Cross-user access requires explicit sharing + (Server.shared=True). + """ + + scenarios = [ + ('Admin cannot fetch a non-admin user\'s private server group', + dict(is_positive_test=False)), + ] + + @create_user_wise_test_client(test_user_details) + def setUp(self): + self.sg_id = None + if not config.SERVER_MODE: + self.skipTest( + 'Data isolation tests only apply to server mode.' + ) + + # Create a server group as the non-admin user + url = '/browser/server_group/obj/' + response = self.tester.post( + url, + data=json.dumps({'name': 'non_admin_private_group'}), + content_type='html/json' + ) + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.data.decode('utf-8')) + self.assertIn('node', response_data) + self.sg_id = response_data['node']['_id'] + + def runTest(self): + """Admin user must NOT access another user's private + server group properties.""" + if not self.sg_id: + raise Exception("Server group not created") + + # self.tester is the admin client by default + url = '/browser/server_group/obj/{0}'.format(self.sg_id) + response = self.tester.get(url, content_type='html/json') + self.assertEqual( + response.status_code, 410, + 'Admin user should not access another user\'s private ' + 'server group. Got status {0}'.format( + response.status_code) + ) + + def tearDown(self): + if self.sg_id is None: + return + with self.app.app_context(): + sg = ServerGroup.query.filter_by(id=self.sg_id).first() + if sg: + db.session.delete(sg) + db.session.commit() diff --git a/web/pgadmin/utils/server_access.py b/web/pgadmin/utils/server_access.py index 1e0c8fe6ad8..efceca83c19 100644 --- a/web/pgadmin/utils/server_access.py +++ b/web/pgadmin/utils/server_access.py @@ -21,11 +21,6 @@ import config -def _is_admin(): - """Check if current user has Administrator role.""" - return current_user.has_role('Administrator') - - def get_server(sid, only_owned=False): """Fetch a server by ID, verifying the current user has access. @@ -39,10 +34,16 @@ def get_server(sid, only_owned=False): Returns the server if: - Desktop mode (single user, no isolation needed), OR - The user owns it, OR - - The server is shared AND only_owned is False, OR - - The user has the Administrator role. + - The server is shared AND only_owned is False. + + Returns None otherwise (caller should return HTTP 410 Gone, in + line with the rest of the server views — see e.g. + web/pgadmin/browser/server_groups/servers/__init__.py). - Returns None otherwise (caller should return 404). + The Administrator role does not grant access to other users' + private servers — admins are subject to the same data isolation + as regular users. To make a server admin-visible, share it + (Server.shared=True). Note: In pgAdmin, Server.shared=True means the server is visible to all authenticated users. SharedServer records are created @@ -55,8 +56,7 @@ def get_server(sid, only_owned=False): return Server.query.filter_by( id=sid, user_id=current_user.id).first() - # Single query: owned OR shared - server = Server.query.filter( + return Server.query.filter( Server.id == sid, or_( Server.user_id == current_user.id, @@ -64,15 +64,6 @@ def get_server(sid, only_owned=False): ) ).first() - if server is not None: - return server - - # Administrators can access all servers - if _is_admin(): - return Server.query.filter_by(id=sid).first() - - return None - def get_server_group(gid): """Fetch a server group by ID, verifying user access. @@ -80,15 +71,15 @@ def get_server_group(gid): Returns the group if: - Desktop mode, OR - The user owns it, OR - - It contains shared servers (Server.shared=True), OR - - The user has the Administrator role. + - It contains shared servers (Server.shared=True). - Returns None otherwise. + Returns None otherwise. The Administrator role does not grant + access to other users' private groups. """ if not config.SERVER_MODE: return ServerGroup.query.filter_by(id=gid).first() - sg = ServerGroup.query.filter( + return ServerGroup.query.filter( ServerGroup.id == gid, or_( ServerGroup.user_id == current_user.id, @@ -100,30 +91,22 @@ def get_server_group(gid): ) ).first() - if sg is not None: - return sg - - if _is_admin(): - return ServerGroup.query.filter_by(id=gid).first() - - return None - def get_server_groups_for_user(): """Return server groups visible to the current user. Includes groups owned by the user plus groups containing shared servers (Server.shared=True, visible to all authenticated users). - Administrators see all groups. + + The Administrator role does not grant visibility into other + users' private groups — admins see the same set as a regular + user with the same ownership and sharing configuration. """ if not config.SERVER_MODE: return ServerGroup.query.filter_by( user_id=current_user.id ).all() - if _is_admin(): - return ServerGroup.query.all() - return ServerGroup.query.filter( or_( ServerGroup.user_id == current_user.id, @@ -140,14 +123,13 @@ def get_user_server_query(): """Return a base query for servers accessible to the current user. Includes owned servers + shared servers (visible to all users). - Administrators see all servers. + + The Administrator role does not grant visibility into other + users' private servers. """ if not config.SERVER_MODE: return Server.query - if _is_admin(): - return Server.query - return Server.query.filter( or_( Server.user_id == current_user.id,