fix: remove admin bypass from server access helpers (#9933) by asheshv · Pull Request #10006 · pgadmin-org/pgadmin4

asheshv · 2026-06-06T02:59:39Z

Summary

Removes the four _is_admin() bypasses in web/pgadmin/utils/server_access.py that auto-granted Administrators visibility into every user's private server groups and servers. Drops the now-unused helper.
Updates docstrings to make the new policy explicit: the Administrator role manages pgAdmin itself, it does not inherit other users' database connections. Cross-user visibility requires explicit sharing (Server.shared=True).
Adds a regression test (AdminCannotSeeOtherUserGroupTestCase) — admin attempts to GET a non-admin's private server group → expects HTTP 410.

Background

The four bypasses were introduced in 9a76ed8 ("fix: enforce data isolation and harden shared servers in server mode") as part of the broader data-isolation work, on the assumption that Administrator implied "see everything." That assumption was wrong — pgAdmin's Administrator role exists for managing pgAdmin (users, preferences, via tools/user_management), not for accessing other users' configured databases.

The original get_all_server_groups() (pre-9.15) had no admin bypass; this PR restores that semantics through the centralized helpers without re-introducing the unfiltered queries the 9a76ed8 work consolidated.

The existing data-isolation tests only covered the non-admin → admin direction, which is why the regression slipped through CI. This PR adds the symmetric admin → non-admin case so it cannot regress silently again.

Test plan

python regression/runtests.py --pkg browser.server_groups.tests --modules test_sg_data_isolation → 3/3 pass (includes the new admin-direction case)
python regression/runtests.py --pkg browser.server_groups.servers.tests --modules test_server_data_isolation → 6/6 pass (existing tests not broken)
Confirmed the new test correctly fails against unfixed master (AssertionError: 200 != 410)
pycodestyle --config=.pycodestyle clean on the changed files
Manual verification in server mode: admin logged in sees only their own + shared server groups

Summary by CodeRabbit

Tests
- Added a regression test ensuring administrators cannot fetch other users' private server groups in server mode.
Bug Fixes
- Tightened access control: administrators no longer bypass visibility rules — they can only see resources they own or resources explicitly shared.

In server mode, administrators were auto-granted visibility into every user's private server groups and servers via four _is_admin() bypasses in server_access.py. This made the Object Explorer show a separate top-level "Servers" entry per user when logged in as admin, exposing private connections the admin should have no access to. The Administrator role in pgAdmin governs management of pgAdmin itself (users, preferences) — it is not intended to inherit other users' database credentials and connection state. Cross-user visibility requires explicit sharing (Server.shared=True), same as for any user. Remove the admin bypass from get_server, get_server_group, get_server_groups_for_user, and get_user_server_query. Drop the now- unused _is_admin() helper. Update docstrings to make the policy explicit. Add a regression test (admin attempts to fetch a non-admin user's private server group → expect HTTP 410). The original isolation test only covered non-admin → admin, which is why the regression introduced by 9a76ed8 was not caught.

coderabbitai · 2026-06-06T02:59:53Z

PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e189617f-f3f7-4c01-ae33-4fe5b9eb7e6e

📥 Commits

Reviewing files that changed from the base of the PR and between a375db8 and 8bea081.

📒 Files selected for processing (1)

web/pgadmin/utils/server_access.py

🚧 Files skipped from review as they are similar to previous changes (1)

web/pgadmin/utils/server_access.py

Walkthrough

Remove Administrator bypass from server and server-group access checks so visibility is limited to resources owned by the current user or explicitly shared; add a regression test that asserts an admin cannot fetch another user’s private server group (GET returns 410).

Changes

Admin Data Isolation

Layer / File(s)	Summary
Access control isolation enforcement `web/pgadmin/utils/server_access.py`	Remove `_is_admin()` helper and update `get_server`, `get_server_group`, `get_server_groups_for_user`, and `get_user_server_query` to enforce owner-or-shared visibility only; update docstrings to reflect the change.
Regression test for admin isolation `web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py`	Add `AdminCannotSeeOtherUserGroupTestCase` that creates a private server group as a non-admin, then verifies an admin GET of that group returns `410`; includes setup and teardown cleanup.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing admin bypass logic from server access helper functions to enforce data isolation between users.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Removes implicit Administrator-role bypasses in centralized server/server-group access helpers to restore strict per-user data isolation in server mode, and adds a regression test to prevent admins from seeing other users’ private server groups unless explicitly shared.

Changes:

Removed Administrator “see everything” bypass logic from web/pgadmin/utils/server_access.py.
Updated helper docstrings to clarify that the Administrator role does not inherit other users’ configured connections; sharing requires Server.shared=True.
Added a regression test ensuring an admin cannot GET another user’s private server group (expects HTTP 410).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
web/pgadmin/utils/server_access.py	Removes admin bypasses from server access helpers and updates policy docstrings.
web/pgadmin/browser/server_groups/tests/test_sg_data_isolation.py	Adds regression coverage for admin→non-admin server group isolation (expects 410).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

All call sites of get_server() return HTTP 410 Gone when the helper returns None (e.g. servers/__init__.py:1507-1511, debugger/__init__.py: 1808-1812). The docstring incorrectly suggested 404 — predating this PR but adjacent to the lines being touched.

asheshv requested a review from Copilot June 6, 2026 03:01

Copilot started reviewing on behalf of asheshv June 6, 2026 03:01 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

Comment thread web/pgadmin/utils/server_access.py

asheshv merged commit 1487059 into pgadmin-org:master Jun 6, 2026
33 checks passed

asheshv mentioned this pull request Jun 6, 2026

fix: filter server groups by ownership and sharing status #10001

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove admin bypass from server access helpers (#9933)#10006

fix: remove admin bypass from server access helpers (#9933)#10006
asheshv merged 2 commits into
pgadmin-org:masterfrom
asheshv:fix/9933-admin-data-isolation

asheshv commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 6, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

asheshv commented Jun 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

asheshv commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 6, 2026 •

edited

Loading