Skip to content

feat(auth): resolve service-principal group membership via Graph for app-only trust tokens - #6757

Open
jonpspri wants to merge 3 commits into
docs/5906-trust-mode-docs-gatefrom
feat/6756-app-only-graph-lookup
Open

feat(auth): resolve service-principal group membership via Graph for app-only trust tokens#6757
jonpspri wants to merge 3 commits into
docs/5906-trust-mode-docs-gatefrom
feat/6756-app-only-graph-lookup

Conversation

@jonpspri

Copy link
Copy Markdown
Collaborator

Adds app-only (client-credentials) trust-token group resolution to JWT-trust mode (issue #6756, epic #5885).

App-only Entra tokens carry idtyp="app", a roles claim, and no groups claim. Entra emits no overage markers for them, so the #5977 overage dispatch never triggers — and /users/{oid}/getMemberObjects would fail for a service principal (a service principal is not a user). Under jwt_trust_overage_policy="graph_lookup" this PR resolves the service principal's security groups through POST /servicePrincipals/{oid}/getMemberObjects and maps them through external_group_mappings.

Changes:

  • detect_app_only_token(payload): True when idtyp == "app" (mcpgateway/utils/trusted_claims.py).
  • EntraGraphClient endpoint selection: /servicePrincipals/{oid}/getMemberObjects for app-only tokens; /users/{oid}/getMemberObjects unchanged for user tokens. Client-credentials grant only; the inbound bearer token is never used. Same oid-keyed Redis cache, TTL bounded by exp.
  • Trust-path dispatch (get_current_user): idtyp == "app" AND groups absent AND graph_lookup -> Graph resolve (cached) -> payload copy -> resolve_external_groups_to_teams. Under fail_closed (default) and proceed_without_groups an app token without groups authenticates with token_teams=[]; the app-role path (Admin claim feeds both admin tracks atomically + parity tests #5902) stays intact.
  • make_trusted_test_jwt: new idtyp kwarg; default output unchanged.

TDD: 7 new tests in tests/unit/mcpgateway/test_entra_graph_client.py (red first: ImportError on the new symbols). URL capture asserts /servicePrincipals/{id} is called and /users/ never is for app tokens; fail_closed pins a roles=["viewer"] app token (authenticated, token_teams=[], viewer role granted); Graph failure under graph_lookup -> 401; Redis read error -> cache miss -> live call. All 11 pre-existing tests in the file pass unmodified (user-token overage regression).

Gate:

  • make ruff — All checks passed!
  • make test — 23411 passed, 879 skipped, 2 xfailed. (Two earlier full-suite runs each flaked on the pre-existing wall-clock benchmark test_trust_p99_within_2x_default under load; it passes in isolation and in the final green run. This PR adds no timing tests.)
  • Pre-commit hooks on commit: ruff check/format, interrogate, bandit, IBM detect-secrets — all pass.

Note: the commit also carries the pre-staged .secrets.baseline regeneration (line-number bookkeeping for existing is_secret: false entries), which was already in the index from the stack work.

Risk to existing users: none — every new branch is trust-mode + graph_lookup gated; default mode and the user-token overage path verified green in the full suite.

Stack: B.15 of epic #5885 (base: #6755).

Closes #6756

@jonpspri
jonpspri added this pull request to stack #6729 September 10, 2026 07:50
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from b4dc120 to 054ffd8 Compare September 10, 2026 08:06
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 054ffd8 to 6c8f36b Compare September 10, 2026 14:20
@jonpspri
jonpspri removed this pull request from stack #6729 September 12, 2026 08:50
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 6c8f36b to ff2fc67 Compare September 12, 2026 09:07
@jonpspri
jonpspri added this pull request to stack #6798 September 12, 2026 09:08
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from ff2fc67 to 5ee3ba0 Compare September 12, 2026 09:20
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 5ee3ba0 to a880b4b Compare September 12, 2026 09:48
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch 2 times, most recently from 590012e to 899809c Compare September 12, 2026 16:45
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 899809c to 3a6e02d Compare September 12, 2026 17:20
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 3a6e02d to 44f883b Compare September 12, 2026 17:35
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 44f883b to 9a59c6d Compare September 12, 2026 17:54
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch 3 times, most recently from dda407f to 6e9e93c Compare September 12, 2026 18:52
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from 6e9e93c to 1418007 Compare September 12, 2026 19:12
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch 2 times, most recently from 5fd66d7 to f92aa3c Compare September 12, 2026 20:26
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch 2 times, most recently from 64a6ec4 to e2f1c59 Compare September 12, 2026 21:11
…app-only trust tokens

Signed-off-by: Jonathan Springer <jps@s390x.com>
@jonpspri
jonpspri force-pushed the feat/6756-app-only-graph-lookup branch from e2f1c59 to d06f96c Compare September 12, 2026 21:54
…group mappings

Replace the disabled group_exists_validator stub (#5976) with the real
Microsoft Graph validator (#5977): Entra issuers resolve the SSO provider
record for the issuer (same issuer->provider resolution as the trust-mode
overage path) and GET /v1.0/groups/<id> with an app-only token. Graph 200
-> valid, 404 -> graph_group_not_found (recorded, not rejected; the
resolver fails closed at read time), any other failure -> unknown under
the existing warn-and-allow contract. Non-Entra issuers and issuers
without app-only credentials keep the disabled-stub posture (valid) with
a log line. The Entra hosts set is single-sourced in entra_graph_client
and aliased by OAuthManager._ENTRA_HOSTS. The validator seam stays
module-level injectable; sync callables remain supported.

Signed-off-by: Jonathan Springer <jps@s390x.com>
…ycle; refresh identity-domains trust row

Signed-off-by: Jonathan Springer <jps@s390x.com>
@jonpspri

Copy link
Copy Markdown
Collaborator Author

Requirement note (remediation) — validator wired

The group-existence validator is real now (5412c7719). Status semantics: 200 gives "valid"; 404 gives "graph_group_not_found" (row allowed, resolver fails closed at read); errors give "unknown" with a warning; non-Microsoft issuers get no call.

Also on this PR: a lazy import broke an import cycle that the remediation set introduced (aae0b9707), and the identity-domains doc row was refreshed. See the notes on #5977 and #5976.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant