Skip to content

[CHORE]: Consolidate require_admin_auth and get_current_user_with_permissions for admin-only routes #6135

Description

@msureshkumar88

Type of Chore

  • Code maintenance / refactoring

Observation

Two admin authentication dependencies coexist with disjoint capabilities:

  • require_admin_auth (mcpgateway/utils/verify_credentials.py:1623) — supports HTTP Basic auth and the browser login redirect for unauthenticated UI requests. Returns a bare email string. Never consults the JWT teams claim.
  • get_current_user_with_permissions (mcpgateway/middleware/rbac.py:243) — resolves full Layer-1 scope context (token_teams, is_admin, etc.) needed by the scope-check helpers. Has no HTTP Basic path at all.

Neither dependency is a superset of the other, so a route that needs both HTTP Basic support and Layer-1 narrowing awareness can't use either one directly.

Where this bit us

/version (mcpgateway/version.py) needs to keep working for HTTP Basic-authenticated monitoring/scraper callers, so it correctly keeps _user=Depends(require_admin_auth) (see #6132's design spec for why swapping to get_current_user_with_permissions was considered and rejected — it would have turned every basic-auth call into a 401). But that meant the Layer-1 unrestricted-admin check couldn't be expressed as a clean dependency; it had to be written inline inside the handler body, calling is_unrestricted_platform_admin() directly against a fresh_db_session().

metrics_maintenance.py has the same shape at the router level (dependencies=[Depends(require_admin_auth)]), and would hit the identical limitation if it ever needed Layer-1 awareness.

Proposed direction

Give one dependency both capabilities — HTTP Basic support and token_teams/Layer-1 resolution — so admin-only routes stop having to choose between them, and stop needing hand-written in-handler scope checks as a workaround.

This is explicitly a maintenance item: no behavior change is being requested here, only removal of the fork that makes correct usage awkward. Any actual consolidation should preserve every existing caller's behavior (HTTP Basic, browser redirect, session-token DB authority, JWT narrowing) exactly.

Acceptance Criteria

Scenario: A single dependency serves both HTTP Basic and Layer-1-aware callers
  Given a route that needs both HTTP Basic auth support and token_teams narrowing
  When the route declares its auth dependency
  Then one dependency provides both, with no behavior change for existing
    require_admin_auth or get_current_user_with_permissions callers

Related

Follow-up to #5982, surfaced during #6132's implementation of /version's scope-check fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    apiREST API Related itemchoreLinting, formatting, dependency hygiene, or project maintenance choresrbacRole-based Access ControltriageIssues / Features awaiting triage

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions