SEP-1867: Resolve a single-user lookup through Grafana's org-users listing when the service account is org-scoped

[marcuscruz-percona]

Summary

• GrafanaUser.get_user() called Grafana's user-lookup endpoint unconditionally. That endpoint needs instance-scoped users:read, which only a Grafana Server Admin holds, so a service account scoped to a single org was refused and GET /api/users/{username} returned a permission error to a SEP admin instead of the record — the shape the PMM-embedded topology mints. A 403 from that lookup now falls back to GET /api/org/users, the listing the same token already reads for list_users().
• The fallback resolves the requested user's real org role. It matches a login before an email, since Grafana allows an email-shaped login and a single pass would resolve by listing order. Matching accepts either field for parity with the lookup endpoint's own loginOrEmail parameter, and is case-folded because Grafana treats logins case-insensitively. A miss is a flat 404 User not found, carrying none of Grafana's own error text.
• The server-admin path is untouched: the lookup stays the first and only call when it succeeds, with no added latency and no extra request. Only the 403 status triggers the fallback — 401, 404, 5xx, and an unreachable Grafana all propagate, so a bad or wrongly-scoped credential stays visible as an error instead of reading as a missing user. No route, SDK, or Casdoor change.

Review round two

Two things the first shape got wrong, both fixed in 52b6133:

• One role policy, not two. The fallback used to refuse a matched row whose role it could not rank (absent, "None", or undefined) with a 502. That contradicted the read path it was meant to mirror — the org-users listing serves such a row with the lowest role, and so does the server-admin lookup — so a Grafana user with RBAC "no basic role" could be listed successfully and refused when fetched singly. Roles now rank through _rank_org_role, shared by every record shape this provider maps, so the single lookup, the listing, and the login flow cannot disagree about one row. A role Grafana does not define still grants nothing, but is logged as schema drift rather than read as an access decision.
• The listing is validated, not cast. A payload off contract used to escape from inside the mapping as a KeyError/AttributeError and surface as an unhandled 500. Both read paths now go through _org_user_records(), which checks the shape before any field is read and reports a violation as an upstream error. This also removes the duplicated fetch-and-cast between the two callers. Behavior change beyond the reported bug: list_users() now reports a malformed Grafana payload as 502 instead of 500, and its previously KeyError-asserting test was rewritten.

Also folded in: the matching predicate moved next to the record type it reads, _from_org_scope renamed to _get_user_from_org_listing (it reads the network rather than mapping a record already in hand), :raises: contracts naming both upstream calls, and the route-level test for the path the fallback exists for.

Reviewed and kept

Four points a review round raised and this PR keeps deliberately, so a reviewer does not have to re-derive the reasoning:

• list_users() now reports a malformed Grafana payload as 502 rather than 500. Both read paths share _org_user_records(). Validating only the fallback would leave the duplicated fetch-and-cast in place and leave list_users() raising an unhandled error, so the shared reader was the deliberate choice. Disclosed in the changelog fragment.
• _rank_org_role is also used by the login flow's org loop. That reuse is the fix for the split policy — restricting it to the org-listing shapes would re-create the disagreement between read paths. Behaviour there is unchanged apart from the drift warning.
• Matching accepts an email and folds case, beyond login == username. Parity with the lookup endpoint's own loginOrEmail parameter. Login is matched across the whole listing before any email, because Grafana permits an email-shaped login: a single pass would resolve by listing order and could serve user B for a request naming user A.
• The route tests mount the route function on their own app. The application binds response_model at import to the configured provider's user model, which requires owner — a field a Grafana record does not carry — so a Grafana user cannot pass validation on the route as mounted there. Requests still exercise routing, the current-user dependency, the path parameter, and serialization.

Known, inherent limitations of the org-scoped topology, not defects this fix eliminates: org-user records carry no server-admin flag, so the fallback can never assert super_admin (the same asymmetry get_users() already documents), and it inherits get_org_users()'s @alru_cache(ttl=300), so a rename can resolve stale for up to 5 minutes on that already-cached call.

Tested

• Org-scoped service-account token (no Grafana Server Admin): signed in as a SEP admin, GET /api/users/{other_username} returns 200 with the record and the user's true org role, where it previously returned a permission error carrying Grafana's message
• Same token: GET /api/users/{other_user_email} returns 200 with the same record
• Same token, target user holding no org role in Grafana: GET /api/users/{username} returns 200 with role: none, matching what GET /api/users/ reports for the same user
• Same token: GET /api/users/{unknown_username} returns 404 User not found, with no Grafana text in the body
• Same token: GET /api/users/{own_username} returns 200, short-circuiting before any upstream call
• Same token, as a viewer: GET /api/users/{other_username} still returns 403 from the route's own gate
• Server-admin-scoped token: GET /api/users/{other_username} is identical to main, and debug logs show no /api/org/users call for that lookup

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• All tests pass locally (make test) — 10177 passed, 0 failed, 424 skipped
• Pre-commit hooks pass (make run-pre-commit)
• Database migrations generated if models changed (make makemigrations) — N/A, no table models touched
• User-facing changes documented (README, inline help, UI text) — N/A, no UI or documented behavior changes
• Configuration changes documented with examples — N/A, no new settings
• Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A

[Copilot]

Pull request overview

Adds a focused Grafana org-scoped fallback while preserving the server-admin lookup path.

Changes:

• Falls back to organization users on lookup 403 responses.
• Resolves login/email matches and organization roles.
• Adds extensive tests and a changelog fragment.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
app/core/auth/providers/grafana/models.py	Implements the org-scoped fallback.
tests/app/core/auth/providers/grafana/test_models.py	Covers fallback behavior and errors.
changelog.d/SEP-1867.fixed.md	Documents the user-facing fix.

Suppressed comments (1)

app/core/auth/providers/grafana/models.py:490

• This helper directly calls get_org_users(), whose upstream failures propagate as HTTPException, but its :raises: block omits that path. Document it alongside the two locally raised errors.

        :raises HTTPNotFoundException: If no org user matches ``username``.
        :raises GrafanaException: If the matched row names no role this provider
            ranks.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
app/core/auth/providers/grafana
models.py
Project Total

This report was generated by python-coverage-comment-action