Skip to content

feat(auth): add external group mappings table, resolver, and admin CRUD - #6741

Open
jonpspri wants to merge 3 commits into
feat/5898-jwt-trust-configfrom
feat/5976-external-group-mappings
Open

jonpspri wants to merge 3 commits into
feat/5898-jwt-trust-configfrom
feat/5976-external-group-mappings

Conversation

@jonpspri

@jonpspri jonpspri commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Adds the external_group_mappings layer for trust-mode Layer-1 scoping. One idempotent migration (e5f6a7b8c9d0, parent bf2998718ea1) creates the table: issuer, tenant (nullable), external_group_id, cf_team_id (FK email_teams.id), cf_role (plain String(255) validated against the roles table at write time — NOT a foreign key, because roles.name carries only a partial unique index), validation_status, last_validated_at, timestamps, and the unique key (issuer, tenant, external_group_id) with the one-group-one-team-one-role decision documented in the migration comment. The ExternalGroupMapping ORM mirrors it.

The resolver resolve_external_groups_to_teams(issuer, tenant, groups, db) in the new mcpgateway/utils/trusted_claims.py returns (team_ids, role_names); unmapped groups contribute nothing (fail-closed). Raw external group IDs never reach token_teams. Admin CRUD lives under /admin/external-group-mappings (RBAC-scoped, sibling convention), with deny paths tested: 401 unauthenticated, 403 insufficient permission, 400 unknown team or role, 409 duplicate, 404 missing. The Graph existence validator ships as an injectable seam, disabled by default (returns valid); WARN-AND-ALLOW semantics documented — the real client lands with #5977.

Visibility-gate and e2e team-isolation/no-mapping tests land as xfail(strict=True, reason="Requires trust branch from #5900") — 404-not-403 per _check_agent_access.

Tested with:

  • uv run pytest tests/unit/mcpgateway/ -k "group_mapping or external_group or visibility_gate" -q — 34 passed, 4 xfailed (TDD: collection ImportErrors first)
  • Migration up/down/up on SQLite: single head e5f6a7b8c9d0; columns verified in the migrated DB. Postgres runs in the Epic 2 gate's docker stacks (constructs used are portable).
  • make ruff — all checks passed
  • make test — 23268 passed, 879 skipped, 9 xfailed

Acceptance criteria of #5976 are met. Risk to existing users: none — a new table and new endpoints; no existing table or path changed.

Stack: B.4 of epic #5885 (base: #6740).

Closes #5976

@jonpspri
jonpspri added this pull request to stack #6729 September 9, 2026 23:38
@jonpspri
jonpspri marked this pull request as ready for review September 9, 2026 23:39
@jonpspri
jonpspri force-pushed the feat/5976-external-group-mappings branch from c6f2cad to 711a223 Compare September 10, 2026 06:22
@jonpspri
jonpspri removed this pull request from stack #6729 September 12, 2026 08:50
@jonpspri
jonpspri force-pushed the feat/5976-external-group-mappings branch from 711a223 to 7e69c55 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/5976-external-group-mappings branch 2 times, most recently from dcb8093 to d3978f1 Compare September 12, 2026 09:48
@jonpspri
jonpspri force-pushed the feat/5976-external-group-mappings branch 2 times, most recently from 2063d23 to e0f5198 Compare September 12, 2026 17:20
@jonpspri
jonpspri force-pushed the feat/5976-external-group-mappings branch from e0f5198 to 5edaeba Compare September 12, 2026 17:35
Signed-off-by: Jonathan Springer <jps@s390x.com>
…umns head

PR #6735 added migration a824749abd27 (user_id columns on user_roles and
email_team_members) parented at bf2998718ea1, which made e5f6a7b8c9d0 a
sibling head. Re-parent e5f6a7b8c9d0 onto a824749abd27 to restore the
single linear chain: bf2998718ea1 -> a824749abd27 -> e5f6a7b8c9d0.

Signed-off-by: Jonathan Springer <jps@s390x.com>
…rnal identity cache

The plain unique constraint on (issuer, tenant, external_group_id) treats
NULL tenants as distinct on SQLite and PostgreSQL, permitting duplicate
(issuer, external_group_id) rows when tenant IS NULL and weakening the
one-group-to-one-team rule. Add a partial unique index (idempotent
migration plus model declaration) for the NULL-tenant case and an
application-level pre-check in mapping create/update that raises the
existing 409 conflict.

Wire the existing invalidate_external_identity_cache() into every
successful mapping create/update/delete so mutations take effect for
cached external token identities immediately.

Signed-off-by: Jonathan Springer <jps@s390x.com>
@jonpspri
jonpspri force-pushed the feat/5976-external-group-mappings branch from 422c646 to b29c785 Compare September 12, 2026 21:11
@jonpspri

Copy link
Copy Markdown
Collaborator Author

Requirement note (remediation) — three changes

  1. New partial unique index WHERE tenant IS NULL (migration b7c8d9e0f1a2). The service also checks for duplicates on create and update.
  2. Create, update, and delete now invalidate the external-identity cache.
  3. e5f6a7b8c9d0 was re-parented onto a824749abd27. The chain keeps one head.

See the note on #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.

[FEATURE]: External group to team mapping seam + in-core mapping table

1 participant