Skip to content

Commit 022ff77

Browse files
asheshvasheshv
committed
test: add multi-user data isolation tests
- ServerGroupIsolationTestCase: verify non-admin cannot fetch another user's server group properties - ServerDataIsolationGetTestCase: verify non-admin cannot access another user's private server - SharedServerAccessTestCase: verify shared servers remain accessible cross-user (positive regression test) Tests use create_user_wise_test_client pattern and only run in SERVER_MODE with pgAdmin4_test_non_admin_credentials configured.
1 parent 0a6d4a0 commit 022ff77

18 files changed

Lines changed: 300 additions & 87 deletions

File tree

‎web/migrations/versions/add_user_id_to_debugger_func_args_.py‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,29 @@ def upgrade():
9696
)
9797

9898
# --- Indexes for data isolation query performance ---
99-
# CREATE INDEX IF NOT EXISTS is supported by both SQLite and PostgreSQL.
100-
# Some tables (e.g. sharedserver) may not exist in older schemas that
101-
# haven't run all prior migrations, so wrap each in try/except.
99+
# Only create indexes on tables that exist (sharedserver may be
100+
# absent in older schemas that haven't run all prior migrations).
101+
inspector = sa.inspect(conn)
102102
index_stmts = [
103-
'CREATE INDEX IF NOT EXISTS ix_server_user_id '
104-
'ON server (user_id)',
105-
'CREATE INDEX IF NOT EXISTS ix_server_servergroup_id '
106-
'ON server (servergroup_id)',
107-
'CREATE INDEX IF NOT EXISTS ix_sharedserver_user_id '
108-
'ON sharedserver (user_id)',
109-
'CREATE INDEX IF NOT EXISTS ix_sharedserver_osid '
110-
'ON sharedserver (osid)',
111-
'CREATE INDEX IF NOT EXISTS ix_servergroup_user_id '
112-
'ON servergroup (user_id)',
103+
('server',
104+
'CREATE INDEX IF NOT EXISTS ix_server_user_id '
105+
'ON server (user_id)'),
106+
('server',
107+
'CREATE INDEX IF NOT EXISTS ix_server_servergroup_id '
108+
'ON server (servergroup_id)'),
109+
('sharedserver',
110+
'CREATE INDEX IF NOT EXISTS ix_sharedserver_user_id '
111+
'ON sharedserver (user_id)'),
112+
('sharedserver',
113+
'CREATE INDEX IF NOT EXISTS ix_sharedserver_osid '
114+
'ON sharedserver (osid)'),
115+
('servergroup',
116+
'CREATE INDEX IF NOT EXISTS ix_servergroup_user_id '
117+
'ON servergroup (user_id)'),
113118
]
114-
for stmt in index_stmts:
115-
try:
119+
for table_name, stmt in index_stmts:
120+
if inspector.has_table(table_name):
116121
op.execute(stmt)
117-
except Exception:
118-
pass
119122

120123

121124
def downgrade():

‎web/pgadmin/browser/server_groups/__init__.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from pgadmin.model import db, ServerGroup, Server
2626
import config
2727
from pgadmin.utils.preferences import Preferences
28-
from pgadmin.utils.server_access import get_accessible_server_group, \
28+
from pgadmin.utils.server_access import get_server_group, \
2929
get_server_groups_for_user
3030

3131

@@ -288,7 +288,7 @@ def update(self, gid):
288288
def properties(self, gid):
289289
"""Update the server-group properties"""
290290

291-
sg = get_accessible_server_group(gid)
291+
sg = get_server_group(gid)
292292

293293
if sg is None:
294294
return make_json_response(
@@ -426,7 +426,7 @@ def nodes(self, gid=None):
426426
)
427427
)
428428
else:
429-
group = get_accessible_server_group(gid)
429+
group = get_server_group(gid)
430430

431431
if not group:
432432
return gone(

‎web/pgadmin/browser/server_groups/servers/__init__.py‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@
4545
from .... import socketio as sio
4646
from pgadmin.utils import get_complete_file_path
4747
from pgadmin.settings.utils import with_object_filters
48-
from pgadmin.utils.server_access import get_accessible_server, \
49-
get_user_server_query, get_accessible_server_group
48+
from pgadmin.utils.server_access import get_server, \
49+
get_user_server_query, get_server_group
5050

5151

5252
def has_any(data, keys):
@@ -626,7 +626,7 @@ def nodes(self, gid):
626626
@pga_login_required
627627
def node(self, gid, sid):
628628
"""Return a JSON document listing the server groups for the user"""
629-
server = get_accessible_server(sid)
629+
server = get_server(sid)
630630

631631
if server is None:
632632
return make_json_response(
@@ -751,7 +751,7 @@ def delete(self, gid, sid):
751751
@pga_login_required
752752
def update(self, gid, sid):
753753
"""Update the server settings"""
754-
server = get_accessible_server(sid)
754+
server = get_server(sid)
755755
sharedserver = None
756756

757757
if server is None:
@@ -956,7 +956,7 @@ def list(self, gid, object_filters):
956956
servers = get_user_server_query().filter(
957957
Server.servergroup_id == gid,
958958
Server.is_adhoc == 0).order_by(Server.name)
959-
sg = get_accessible_server_group(gid)
959+
sg = get_server_group(gid)
960960
res = []
961961

962962
driver = get_driver(PG_DEFAULT_DRIVER)
@@ -996,7 +996,7 @@ def list(self, gid, object_filters):
996996
def properties(self, gid, sid):
997997
"""Return list of attributes of a server"""
998998

999-
server = get_accessible_server(sid)
999+
server = get_server(sid)
10001000

10011001
if server is None:
10021002
return make_json_response(
@@ -1388,7 +1388,7 @@ def supported_servers(self, **kwargs):
13881388

13891389
def connect_status(self, gid, sid):
13901390
"""Check and return the connection status."""
1391-
server = get_accessible_server(sid)
1391+
server = get_server(sid)
13921392
if server is None:
13931393
return make_json_response(
13941394
status=410, success=0,
@@ -1462,7 +1462,7 @@ def connect(self, gid, sid, is_qt=False, server=None):
14621462
# function in that case no need to fetch the server detail based on
14631463
# sid.
14641464
if server is None:
1465-
server = get_accessible_server(sid)
1465+
server = get_server(sid)
14661466

14671467
if server is None:
14681468
return bad_request(self.not_found_error_msg())
@@ -1692,7 +1692,7 @@ def connect(self, gid, sid, is_qt=False, server=None):
16921692
def disconnect(self, gid, sid):
16931693
"""Disconnect the Server."""
16941694

1695-
server = get_accessible_server(sid)
1695+
server = get_server(sid)
16961696
if server is None:
16971697
return bad_request(self.not_found_error_msg())
16981698

@@ -1817,7 +1817,9 @@ def change_password(self, gid, sid):
18171817
raise CryptKeyMissing
18181818

18191819
# Fetch Server Details
1820-
server = get_accessible_server(sid)
1820+
# Owner-only: password changes must not modify another
1821+
# user's server record via shared access.
1822+
server = get_server(sid, only_owned=True)
18211823
if server is None:
18221824
return bad_request(self.not_found_error_msg())
18231825

@@ -2107,7 +2109,9 @@ def clear_saved_password(self, gid, sid):
21072109
:return:
21082110
"""
21092111
try:
2110-
server = get_accessible_server(sid)
2112+
# Owner-only: clearing saved password must not modify
2113+
# another user's server record via shared access.
2114+
server = get_server(sid, only_owned=True)
21112115
shared_server = None
21122116
if server is None:
21132117
return make_json_response(
@@ -2164,7 +2168,9 @@ def clear_sshtunnel_password(self, gid, sid):
21642168
:return:
21652169
"""
21662170
try:
2167-
server = get_accessible_server(sid)
2171+
# Owner-only: tunnel password changes must not modify
2172+
# another user's server record via shared access.
2173+
server = get_server(sid, only_owned=True)
21682174
if server is None:
21692175
return make_json_response(
21702176
success=0,

‎web/pgadmin/browser/server_groups/servers/databases/__init__.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
from pgadmin.tools.schema_diff.node_registry import SchemaDiffRegistry
3636
from pgadmin.model import db, Server, Database
37-
from pgadmin.utils.server_access import get_accessible_server
37+
from pgadmin.utils.server_access import get_server
3838
from pgadmin.browser.utils import underscore_escape
3939
from pgadmin.utils.constants import TWO_PARAM_STRING
4040

@@ -581,7 +581,7 @@ def connect(self, gid, sid, did):
581581
'connected': True,
582582
'info_prefix': TWO_PARAM_STRING.
583583
format(getattr(
584-
get_accessible_server(sid), 'name', ''), conn.db)
584+
get_server(sid), 'name', ''), conn.db)
585585
}
586586
)
587587

@@ -605,7 +605,7 @@ def disconnect(self, gid, sid, did):
605605
'connected': False,
606606
'info_prefix': TWO_PARAM_STRING.
607607
format(getattr(
608-
get_accessible_server(sid), 'name', ''), conn.db)
608+
get_server(sid), 'name', ''), conn.db)
609609
}
610610
)
611611

‎web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
from .schema_diff_view_utils import SchemaDiffViewCompare
3232
from pgadmin.utils import does_utility_exist, get_server
3333
from pgadmin.model import Server
34-
from pgadmin.utils.server_access import get_accessible_server
34+
from pgadmin.utils.server_access import get_server
3535
from pgadmin.misc.bgprocess.processes import BatchProcess, IProcessDesc
3636
from pgadmin.utils.constants import SERVER_NOT_FOUND
3737

@@ -2318,7 +2318,7 @@ def refresh_data(self, gid, sid, did, scid, vid):
23182318
res['rows'][0]['name'])
23192319

23202320
# Fetch the server details like hostname, port, roles etc
2321-
server = get_accessible_server(sid)
2321+
server = get_server(sid)
23222322

23232323
if server is None:
23242324
return make_json_response(
@@ -2436,9 +2436,7 @@ def check_utility_exists(self, gid, sid, did, scid, vid):
24362436
Returns:
24372437
None
24382438
"""
2439-
server = Server.query.filter_by(
2440-
id=sid, user_id=current_user.id
2441-
).first()
2439+
server = get_server(sid)
24422440

24432441
if server is None:
24442442
return make_json_response(

‎web/pgadmin/browser/server_groups/servers/tests/test_server_data_isolation.py‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
##########################################################################
2+
#
3+
# pgAdmin 4 - PostgreSQL Tools
4+
#
5+
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
6+
# This software is released under the PostgreSQL Licence
7+
#
8+
##########################################################################
9+
10+
"""Tests for server data isolation between users in server mode."""
11+
12+
import json
13+
import config
14+
from pgadmin.utils.route import BaseTestGenerator
15+
from regression.python_test_utils import test_utils as utils
16+
from regression.test_setup import config_data
17+
from regression.python_test_utils.test_utils import \
18+
create_user_wise_test_client
19+
from . import utils as servers_utils
20+
21+
test_user_details = None
22+
if config.SERVER_MODE:
23+
test_user_details = \
24+
config_data['pgAdmin4_test_non_admin_credentials']
25+
26+
27+
class ServerDataIsolationGetTestCase(BaseTestGenerator):
28+
"""Verify that a non-admin user cannot access another user's
29+
private (non-shared) server by ID."""
30+
31+
scenarios = [
32+
('User B gets 410 for User A private server',
33+
dict(is_positive_test=False)),
34+
]
35+
36+
def setUp(self):
37+
if not config.SERVER_MODE:
38+
self.skipTest(
39+
'Data isolation tests only apply to server mode.'
40+
)
41+
42+
# Create a private (non-shared) server as the admin user
43+
self.server['shared'] = False
44+
url = "/browser/server/obj/{0}/".format(utils.SERVER_GROUP)
45+
response = self.tester.post(
46+
url,
47+
data=json.dumps(self.server),
48+
content_type='html/json'
49+
)
50+
self.assertEqual(response.status_code, 200)
51+
response_data = json.loads(response.data.decode('utf-8'))
52+
self.assertIn('node', response_data)
53+
self.server_id = response_data['node']['_id']
54+
55+
@create_user_wise_test_client(test_user_details)
56+
def runTest(self):
57+
"""Non-admin user should NOT be able to GET another user's
58+
private server."""
59+
if not self.server_id:
60+
raise Exception("Server not found to test isolation")
61+
62+
url = '/browser/server/obj/{0}/{1}'.format(
63+
utils.SERVER_GROUP, self.server_id)
64+
response = self.tester.get(url, follow_redirects=True)
65+
# Expect 410 Gone (server not accessible to this user)
66+
self.assertIn(
67+
response.status_code, [404, 410],
68+
'Non-admin user should not access another user\'s '
69+
'private server. Got status {0}'.format(
70+
response.status_code)
71+
)
72+
73+
def tearDown(self):
74+
# Clean up with the admin tester (which owns the server)
75+
utils.delete_server_with_api(
76+
self.__class__.tester, self.server_id)
77+
78+
79+
class SharedServerAccessTestCase(BaseTestGenerator):
80+
"""Verify that a shared server IS accessible by a non-admin
81+
user (positive test — shared servers should work after the
82+
isolation fixes)."""
83+
84+
scenarios = [
85+
('User B can access shared server from User A',
86+
dict(is_positive_test=True)),
87+
]
88+
89+
def setUp(self):
90+
if not config.SERVER_MODE:
91+
self.skipTest(
92+
'Data isolation tests only apply to server mode.'
93+
)
94+
95+
# Create a shared server as the admin user
96+
self.server['shared'] = True
97+
url = "/browser/server/obj/{0}/".format(utils.SERVER_GROUP)
98+
response = self.tester.post(
99+
url,
100+
data=json.dumps(self.server),
101+
content_type='html/json'
102+
)
103+
response_data = json.loads(response.data.decode('utf-8'))
104+
self.server_id = response_data['node']['_id']
105+
106+
@create_user_wise_test_client(test_user_details)
107+
def runTest(self):
108+
"""Non-admin user SHOULD be able to GET a shared server."""
109+
if not self.server_id:
110+
raise Exception("Server not found to test shared access")
111+
112+
url = '/browser/server/obj/{0}/{1}'.format(
113+
utils.SERVER_GROUP, self.server_id)
114+
response = self.tester.get(url, follow_redirects=True)
115+
self.assertEqual(
116+
response.status_code, 200,
117+
'Non-admin user should be able to access shared server.'
118+
' Got status {0}'.format(response.status_code)
119+
)
120+
121+
def tearDown(self):
122+
utils.delete_server_with_api(
123+
self.__class__.tester, self.server_id)

‎web/pgadmin/browser/server_groups/servers/utils.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,8 @@ def delete_adhoc_servers(sid=None):
692692
has_user = (has_request_context() and
693693
current_user and current_user.is_authenticated)
694694
if sid is not None:
695-
q = db.session.query(Server).filter(Server.id == sid)
695+
q = db.session.query(Server).filter(
696+
Server.id == sid, Server.is_adhoc == 1)
696697
if has_user:
697698
q = q.filter(Server.user_id == current_user.id)
698699
q.delete()

0 commit comments

Comments
 (0)