Skip to content

Commit 7061250

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. Adds centralized get_server() helper with only_owned parameter for write operations, scopes all server/group queries, adds access check in connection_manager(), and includes Alembic migration for DebuggerFunctionArguments user_id column.
1 parent 872d5ac commit 7061250

File tree

17 files changed

+567
-134
lines changed

17 files changed

+567
-134
lines changed

‎web/migrations/versions/add_user_id_to_debugger_func_args_.py‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
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+
# 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)
102+
index_stmts = [
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)'),
118+
]
119+
for table_name, stmt in index_stmts:
120+
if inspector.has_table(table_name):
121+
op.execute(stmt)
122+
123+
124+
def downgrade():
125+
# pgAdmin only upgrades, downgrade not implemented.
126+
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_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_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_server_group(gid)
425430

426431
if not group:
427432
return gone(

0 commit comments

Comments
 (0)