Skip to content

Commit d9a22aa

Browse files
asheshvasheshv
committed
fix: enforce data isolation in server mode
In server mode, multiple users share one pgAdmin instance. Several code paths loaded objects belonging to all users without filtering, allowing cross-user information disclosure. Changes: - Add centralized get_accessible_server() and related helpers in utils/server_access.py. Single-query check: owned OR shared OR admin. Desktop mode bypasses (single user). - Replace ~30 unscoped Server.query.filter_by(id=sid) callsites across servers, sqleditor, schema_diff, erd, psql, import_export, views, workspaces with get_accessible_server(sid) + None checks. - Add access check in connection_manager() — the real security boundary for ~50+ check_precondition-decorated endpoints. - Scope ServerGroup.query.all() to current user + shared groups. Add user check to ServerGroup.properties() and nodes(gid). - Replace Server.shared listing patterns with get_user_server_query() using proper shared server semantics. - Add user_id to DebuggerFunctionArguments model + composite PK. Alembic migration handles SQLite (recreate) and PostgreSQL (add column, backfill from server owner, recreate PK). SCHEMA_VERSION bumped to 50. - Scope disconnect_from_all_servers(), delete_adhoc_servers(), get_servers_with_saved_passwords() to current user. Guard delete_adhoc_servers with has_request_context() for app startup. - Add indexes: server(user_id), server(servergroup_id), sharedserver(user_id), sharedserver(osid), servergroup(user_id). - Fix old migration ca00ec32581b to use raw SQL instead of model import (prevents breakage when model shape changes).
1 parent d59fcf3 commit d9a22aa

17 files changed

Lines changed: 531 additions & 122 deletions

File tree

‎web/migrations/versions/add_user_id_to_debugger_func_args_.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+
"""Add user_id to debugger_function_arguments and indexes for data isolation
11+
12+
Revision ID: add_user_id_dbg_args
13+
Revises: add_tools_ai_perm
14+
Create Date: 2026-04-08
15+
16+
"""
17+
from alembic import op
18+
import sqlalchemy as sa
19+
20+
# revision identifiers, used by Alembic.
21+
revision = 'add_user_id_dbg_args'
22+
down_revision = 'add_tools_ai_perm'
23+
branch_labels = None
24+
depends_on = None
25+
26+
27+
def upgrade():
28+
conn = op.get_bind()
29+
dialect = conn.dialect.name
30+
31+
# --- DebuggerFunctionArguments: add user_id to composite PK ---
32+
if dialect == 'sqlite':
33+
# SQLite cannot ALTER composite PKs. Recreate the table.
34+
# Existing debugger argument data is ephemeral (cached function
35+
# args) so dropping is acceptable.
36+
op.execute(
37+
'DROP TABLE IF EXISTS debugger_function_arguments'
38+
)
39+
op.create_table(
40+
'debugger_function_arguments',
41+
sa.Column('user_id', sa.Integer(),
42+
sa.ForeignKey('user.id'), nullable=False),
43+
sa.Column('server_id', sa.Integer(), nullable=False),
44+
sa.Column('database_id', sa.Integer(), nullable=False),
45+
sa.Column('schema_id', sa.Integer(), nullable=False),
46+
sa.Column('function_id', sa.Integer(), nullable=False),
47+
sa.Column('arg_id', sa.Integer(), nullable=False),
48+
sa.Column('is_null', sa.Integer(), nullable=False),
49+
sa.Column('is_expression', sa.Integer(), nullable=False),
50+
sa.Column('use_default', sa.Integer(), nullable=False),
51+
sa.Column('value', sa.String(), nullable=True),
52+
sa.PrimaryKeyConstraint(
53+
'user_id', 'server_id', 'database_id',
54+
'schema_id', 'function_id', 'arg_id'
55+
),
56+
sa.CheckConstraint('is_null >= 0 AND is_null <= 1'),
57+
sa.CheckConstraint(
58+
'is_expression >= 0 AND is_expression <= 1'),
59+
sa.CheckConstraint(
60+
'use_default >= 0 AND use_default <= 1'),
61+
)
62+
else:
63+
# PostgreSQL: add column, backfill from server owner, recreate
64+
# PK using batch_alter_table for portability.
65+
op.add_column(
66+
'debugger_function_arguments',
67+
sa.Column('user_id', sa.Integer(),
68+
sa.ForeignKey('user.id'), nullable=True)
69+
)
70+
# Backfill: assign user_id from the server's owner
71+
op.execute(
72+
'UPDATE debugger_function_arguments '
73+
'SET user_id = s.user_id '
74+
'FROM server s '
75+
'WHERE debugger_function_arguments.server_id = s.id'
76+
)
77+
# Delete orphans (rows with no matching server)
78+
op.execute(
79+
'DELETE FROM debugger_function_arguments '
80+
'WHERE user_id IS NULL'
81+
)
82+
op.alter_column(
83+
'debugger_function_arguments', 'user_id', nullable=False
84+
)
85+
# Recreate PK with user_id using batch_alter_table
86+
with op.batch_alter_table(
87+
'debugger_function_arguments'
88+
) as batch:
89+
batch.drop_constraint(
90+
'debugger_function_arguments_pkey', type_='primary'
91+
)
92+
batch.create_primary_key(
93+
'debugger_function_arguments_pkey',
94+
['user_id', 'server_id', 'database_id',
95+
'schema_id', 'function_id', 'arg_id']
96+
)
97+
98+
# --- 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.
102+
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)',
113+
]
114+
for stmt in index_stmts:
115+
try:
116+
op.execute(stmt)
117+
except Exception:
118+
pass
119+
120+
121+
def downgrade():
122+
# pgAdmin only upgrades, downgrade not implemented.
123+
pass

‎web/migrations/versions/ca00ec32581b_.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
1616
"""
1717
from alembic import op
18-
from sqlalchemy.orm.session import Session
19-
from pgadmin.model import DebuggerFunctionArguments
2018

2119
# revision identifiers, used by Alembic.
2220
revision = 'ca00ec32581b'
@@ -26,11 +24,10 @@
2624

2725

2826
def upgrade():
29-
session = Session(bind=op.get_bind())
30-
31-
debugger_records = session.query(DebuggerFunctionArguments).all()
32-
if debugger_records:
33-
session.delete(debugger_records)
27+
# Use raw SQL instead of importing the model class, because
28+
# model changes in later migrations (e.g. adding user_id) would
29+
# cause this migration to fail on fresh databases.
30+
op.execute('DELETE FROM debugger_function_arguments')
3431

3532

3633
def downgrade():

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
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, \
29+
get_server_groups_for_user
2830

2931

3032
def get_icon_css_class(group_id, group_user_id,
@@ -286,7 +288,7 @@ def update(self, gid):
286288
def properties(self, gid):
287289
"""Update the server-group properties"""
288290

289-
sg = ServerGroup.query.filter(ServerGroup.id == gid).first()
291+
sg = get_accessible_server_group(gid)
290292

291293
if sg is None:
292294
return make_json_response(
@@ -296,7 +298,8 @@ def properties(self, gid):
296298
)
297299
else:
298300
return ajax_response(
299-
response={'id': sg.id, 'name': sg.name, 'user_id': sg.user_id},
301+
response={'id': sg.id, 'name': sg.name,
302+
'user_id': sg.user_id},
300303
status=200
301304
)
302305

@@ -373,8 +376,9 @@ def dependents(self, gid):
373376
@staticmethod
374377
def get_all_server_groups():
375378
"""
376-
Returns the list of server groups to show in server mode and
377-
if there is any shared server in the group.
379+
Returns the list of server groups to show in server mode.
380+
Includes groups owned by the user and groups containing
381+
shared servers accessible to this user.
378382
:return: server groups
379383
"""
380384

@@ -383,17 +387,18 @@ def get_all_server_groups():
383387
pref = Preferences.module('browser')
384388
hide_shared_server = pref.preference('hide_shared_server').get()
385389

386-
server_groups = ServerGroup.query.all()
387-
groups = []
388-
for group in server_groups:
389-
if hide_shared_server and \
390-
ServerGroupModule.has_shared_server(group.id) and \
391-
group.user_id != current_user.id:
392-
continue
393-
if group.user_id == current_user.id or \
394-
ServerGroupModule.has_shared_server(group.id):
390+
server_groups = get_server_groups_for_user()
391+
392+
if hide_shared_server:
393+
groups = []
394+
for group in server_groups:
395+
if group.user_id != current_user.id and \
396+
ServerGroupModule.has_shared_server(group.id):
397+
continue
395398
groups.append(group)
396-
return groups
399+
return groups
400+
401+
return server_groups
397402

398403
@pga_login_required
399404
def nodes(self, gid=None):
@@ -421,7 +426,7 @@ def nodes(self, gid=None):
421426
)
422427
)
423428
else:
424-
group = ServerGroup.query.filter(ServerGroup.id == gid).first()
429+
group = get_accessible_server_group(gid)
425430

426431
if not group:
427432
return gone(

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

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +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
4850

4951

5052
def has_any(data, keys):
@@ -245,8 +247,7 @@ def get_nodes(self, gid, object_filters):
245247
"""Return a JSON document listing the server groups for the user"""
246248

247249
hide_shared_server = get_preferences()
248-
servers = Server.query.filter(
249-
or_(Server.user_id == current_user.id, Server.shared),
250+
servers = get_user_server_query().filter(
250251
Server.servergroup_id == gid, Server.is_adhoc == 0)
251252

252253
driver = get_driver(PG_DEFAULT_DRIVER)
@@ -560,9 +561,7 @@ def nodes(self, gid):
560561
Return a JSON document listing the servers under this server group
561562
for the user.
562563
"""
563-
servers = Server.query.filter(
564-
or_(Server.user_id == current_user.id,
565-
Server.shared),
564+
servers = get_user_server_query().filter(
566565
Server.servergroup_id == gid, Server.is_adhoc == 0)
567566

568567
driver = get_driver(PG_DEFAULT_DRIVER)
@@ -627,24 +626,22 @@ def nodes(self, gid):
627626
@pga_login_required
628627
def node(self, gid, sid):
629628
"""Return a JSON document listing the server groups for the user"""
630-
server = Server.query.filter_by(id=sid).first()
631-
632-
if server.shared and server.user_id != current_user.id:
633-
shared_server = ServerModule.get_shared_server(server, gid)
634-
server = ServerModule.get_shared_server_properties(server,
635-
shared_server)
629+
server = get_accessible_server(sid)
636630

637631
if server is None:
638632
return make_json_response(
639633
status=410,
640634
success=0,
641635
errormsg=gettext(
642-
gettext(
643-
"Could not find the server with id# {0}."
644-
).format(sid)
645-
)
636+
"Could not find the server with id# {0}."
637+
).format(sid)
646638
)
647639

640+
if server.shared and server.user_id != current_user.id:
641+
shared_server = ServerModule.get_shared_server(server, gid)
642+
server = ServerModule.get_shared_server_properties(server,
643+
shared_server)
644+
648645
manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(server.id)
649646
conn = manager.connection()
650647
connected = conn.connected()
@@ -754,7 +751,7 @@ def delete(self, gid, sid):
754751
@pga_login_required
755752
def update(self, gid, sid):
756753
"""Update the server settings"""
757-
server = Server.query.filter_by(id=sid).first()
754+
server = get_accessible_server(sid)
758755
sharedserver = None
759756

760757
if server is None:
@@ -956,13 +953,10 @@ def list(self, gid, object_filters):
956953
"""
957954
Return list of attributes of all servers.
958955
"""
959-
servers = Server.query.filter(
960-
or_(Server.user_id == current_user.id, Server.shared),
956+
servers = get_user_server_query().filter(
961957
Server.servergroup_id == gid,
962958
Server.is_adhoc == 0).order_by(Server.name)
963-
sg = ServerGroup.query.filter_by(
964-
id=gid
965-
).first()
959+
sg = get_accessible_server_group(gid)
966960
res = []
967961

968962
driver = get_driver(PG_DEFAULT_DRIVER)
@@ -1002,8 +996,7 @@ def list(self, gid, object_filters):
1002996
def properties(self, gid, sid):
1003997
"""Return list of attributes of a server"""
1004998

1005-
server = Server.query.filter_by(
1006-
id=sid).first()
999+
server = get_accessible_server(sid)
10071000

10081001
if server is None:
10091002
return make_json_response(
@@ -1395,7 +1388,12 @@ def supported_servers(self, **kwargs):
13951388

13961389
def connect_status(self, gid, sid):
13971390
"""Check and return the connection status."""
1398-
server = Server.query.filter_by(id=sid).first()
1391+
server = get_accessible_server(sid)
1392+
if server is None:
1393+
return make_json_response(
1394+
status=410, success=0,
1395+
errormsg=self.not_found_error_msg()
1396+
)
13991397
manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
14001398
conn = manager.connection()
14011399
connected = conn.connected()
@@ -1464,7 +1462,10 @@ def connect(self, gid, sid, is_qt=False, server=None):
14641462
# function in that case no need to fetch the server detail based on
14651463
# sid.
14661464
if server is None:
1467-
server = Server.query.filter_by(id=sid).first()
1465+
server = get_accessible_server(sid)
1466+
1467+
if server is None:
1468+
return bad_request(self.not_found_error_msg())
14681469

14691470
shared_server = None
14701471
if server.shared and server.user_id != current_user.id:
@@ -1474,8 +1475,6 @@ def connect(self, gid, sid, is_qt=False, server=None):
14741475
sess.expunge(server)
14751476
server = ServerModule.get_shared_server_properties(server,
14761477
shared_server)
1477-
if server is None:
1478-
return bad_request(self.not_found_error_msg())
14791478

14801479
# Return if username is blank and the server is shared
14811480
if server.username is None and not server.service and \
@@ -1693,7 +1692,7 @@ def connect(self, gid, sid, is_qt=False, server=None):
16931692
def disconnect(self, gid, sid):
16941693
"""Disconnect the Server."""
16951694

1696-
server = Server.query.filter_by(id=sid).first()
1695+
server = get_accessible_server(sid)
16971696
if server is None:
16981697
return bad_request(self.not_found_error_msg())
16991698

@@ -1818,7 +1817,7 @@ def change_password(self, gid, sid):
18181817
raise CryptKeyMissing
18191818

18201819
# Fetch Server Details
1821-
server = Server.query.filter_by(id=sid).first()
1820+
server = get_accessible_server(sid)
18221821
if server is None:
18231822
return bad_request(self.not_found_error_msg())
18241823

@@ -2108,7 +2107,7 @@ def clear_saved_password(self, gid, sid):
21082107
:return:
21092108
"""
21102109
try:
2111-
server = Server.query.filter_by(id=sid).first()
2110+
server = get_accessible_server(sid)
21122111
shared_server = None
21132112
if server is None:
21142113
return make_json_response(
@@ -2165,7 +2164,7 @@ def clear_sshtunnel_password(self, gid, sid):
21652164
:return:
21662165
"""
21672166
try:
2168-
server = Server.query.filter_by(id=sid).first()
2167+
server = get_accessible_server(sid)
21692168
if server is None:
21702169
return make_json_response(
21712170
success=0,

0 commit comments

Comments
 (0)