Skip to content

Commit 9cbffbf

Browse files
crivetimihaiYosiefeyob
authored andcommitted
fix(rbac): resolve SSO role assignment FK constraint violation on granted_by='sso_system' (#3502)
* fix(rbac): resolve SSO role assignment FK constraint violation on granted_by='sso_system' (#3484) Add grant_source column to UserRole to track role assignment provenance (e.g. 'sso', 'manual', 'bootstrap') separately from the granted_by FK. SSO role sync now uses granted_by=user_email (satisfying the FK to email_users) with grant_source='sso' to distinguish SSO-granted roles for revocation logic. Closes #3484 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sso): rewrite Keycloak issuer URL to public_base_url for id_token verification Keycloak discovery fetches OIDC metadata via the internal base_url (e.g. http://keycloak:8080), but tokens issued through the browser flow contain the public-facing issuer (e.g. http://localhost:8180). The authorization_url was already rewritten to public_base_url but the issuer was not, causing id_token verification to fail with "Invalid issuer". Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs: update CHANGELOG and Keycloak tutorial for SSO fixes Add grant_source provenance tracking and both SSO fixes to RC2 CHANGELOG. Update Keycloak tutorial with split-URL issuer troubleshooting and SSO_KEYCLOAK_PUBLIC_BASE_URL description. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(rbac): expose grant_source in UserRoleResponse API schema Add grant_source field to UserRoleResponse so /rbac/my/roles and other role endpoints return the provenance of each assignment. Update Entra ID tutorial example to match the full response shape. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>
1 parent fd02ac2 commit 9cbffbf

15 files changed

+173
-27
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ This release **tightens production defaults** and adds **defense-in-depth contro
180180
* Admins can now set email verification status on users via API (`PUT /auth/email/admin/users/{email}`) and Admin UI checkbox
181181
* `@require_permission("teams.join")` added to `accept_team_invitation`, `request_to_join_team`, and `leave_team` endpoints
182182

183+
#### **🏷️ Role Assignment Provenance Tracking** ([#3484](https://github.com/IBM/mcp-context-forge/issues/3484), [#3502](https://github.com/IBM/mcp-context-forge/pull/3502))
184+
* New `grant_source` column on `user_roles` table tracks the origin of role assignments (`'sso'`, `'manual'`, `'bootstrap'`, `'auto'`)
185+
* SSO role sync uses `grant_source='sso'` to distinguish SSO-granted roles from manually or auto-assigned roles, enabling correct revocation without affecting non-SSO roles
186+
* Alembic migration `e1f2a3b4c5d6` adds the column (nullable, backward-compatible with existing rows)
187+
183188
### Fixed
184189

185190
#### **🔐 Security** (S-01, S-02, S-03, A-02, A-05, A-06, O-01, O-05, O-10, O-17, U-05, C-03, C-04, C-07, C-09, C-10, C-11, C-14, C-15, C-28, C-29, EXTRA-01)
@@ -210,6 +215,8 @@ This release **tightens production defaults** and adds **defense-in-depth contro
210215
* **GitHub SSO email-claim compatibility** - GitHub logins no longer fail when `/user` omits `email_verified`; explicit false verification claims are still denied (O-03 follow-up)
211216
* **SSO approval-state hardening** - expired pending approvals no longer fall through to user creation; approval statuses now fail closed (O-04)
212217
* **SSO scope policy enforcement** - requested scopes are normalized and constrained to provider policy; invalid scopes rejected with HTTP 400 (O-06)
218+
* **SSO role assignment FK constraint violation** - `_sync_user_roles()` used `granted_by='sso_system'` which violated the `user_roles.granted_by` foreign key to `email_users.email` on PostgreSQL; now uses `granted_by=<user_email>` with a new `grant_source` column to track provenance ([#3484](https://github.com/IBM/mcp-context-forge/issues/3484), [#3502](https://github.com/IBM/mcp-context-forge/pull/3502))
219+
* **Keycloak SSO issuer mismatch** - Keycloak OIDC discovery rewrote `authorization_url` to `public_base_url` but not `issuer`; tokens issued via browser flow contain the public-facing issuer, causing `id_token` verification to fail with "Invalid issuer" when `SSO_KEYCLOAK_PUBLIC_BASE_URL` differs from `SSO_KEYCLOAK_BASE_URL` ([#3502](https://github.com/IBM/mcp-context-forge/pull/3502))
213220
* **OAuth grant fallback removal** - `authorization_code` no longer falls back to `client_credentials` in non-interactive token retrieval (O-11)
214221
* **SSO callback session binding** - state is bound to browser session marker and callback requires matching session binding (O-14)
215222
* **OAuth authorize/status ownership checks** - gateway visibility/team/owner checks now enforced consistently on authorize/status endpoints (O-16)

docs/docs/architecture/adr/034-sso-admin-sync-config-precedence.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ if should_be_admin and not user.is_admin:
3030
- The `is_admin` flag is a **platform-level override** that grants `["*"]` permissions via JWT scopes
3131
- Manual admin grants via Admin UI/API are intentional decisions by platform administrators
3232
- SSO should enhance access control, not unexpectedly revoke access
33-
- The RBAC system (`platform_admin` role) already handles group-based role revocation with proper tracking (`granted_by='sso_system'`)
33+
- The RBAC system (`platform_admin` role) already handles group-based role revocation with proper tracking (`grant_source='sso'`)
3434
- To revoke admin access, administrators should use the Admin UI/API explicitly
3535

3636
**Trade-offs**:

docs/docs/manage/sso-entra-role-mapping.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ ContextForge includes a comprehensive RBAC system with the following default rol
8080
- Revokes SSO-granted roles no longer in groups
8181
- Assigns new roles based on current groups
8282
- Preserves manually assigned roles
83-
- Maintains audit trail with `granted_by='sso_system'`
83+
- Maintains audit trail with `grant_source='sso'` and `granted_by=<user_email>`
8484

8585
6. **Admin Status Synchronization** (`authenticate_or_create_user()`)
8686

@@ -242,7 +242,7 @@ When a new user logs in via EntraID SSO:
242242
2. Groups are mapped to roles via `_map_groups_to_roles()`
243243
3. Roles are assigned via `_sync_user_roles()`
244244
4. User is created with `is_admin` flag if in admin groups
245-
5. RBAC roles are assigned with `granted_by='sso_system'`
245+
5. RBAC roles are assigned with `grant_source='sso'` (self-granted by the user)
246246

247247
### On User Login
248248

@@ -259,8 +259,8 @@ When an existing user logs in:
259259
### Manual Role Management
260260

261261
- Admins can manually assign additional roles via the Admin UI
262-
- Manually assigned roles (not granted by `sso_system`) are preserved
263-
- Only SSO-granted roles are synchronized on login
262+
- Manually assigned roles (without `grant_source='sso'`) are preserved
263+
- Only SSO-granted roles (`grant_source='sso'`) are synchronized on login
264264

265265
## Token Claims
266266

docs/docs/manage/sso-keycloak-tutorial.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ SSO_KEYCLOAK_CLIENT_ID=mcp-gateway-prod
403403
|----------|----------|---------|-------------|
404404
| `SSO_KEYCLOAK_ENABLED` | Yes | `false` | Enable Keycloak SSO provider |
405405
| `SSO_KEYCLOAK_BASE_URL` | Yes | - | Base URL of Keycloak instance |
406-
| `SSO_KEYCLOAK_PUBLIC_BASE_URL` | No | _unset_ | Browser-facing Keycloak URL used for authorization redirects when gateway uses an internal URL |
406+
| `SSO_KEYCLOAK_PUBLIC_BASE_URL` | No | _unset_ | Browser-facing Keycloak URL used for authorization redirects and issuer verification when gateway uses an internal URL |
407407
| `SSO_KEYCLOAK_REALM` | Yes | `master` | Keycloak realm name |
408408
| `SSO_KEYCLOAK_CLIENT_ID` | Yes | - | OAuth client ID from Keycloak |
409409
| `SSO_KEYCLOAK_CLIENT_SECRET` | Yes | - | OAuth client secret from Keycloak |
@@ -823,6 +823,17 @@ SSO_KEYCLOAK_BASE_URL=https://keycloak.yourcompany.com/auth
823823
SSO_KEYCLOAK_BASE_URL=https://keycloak.yourcompany.com
824824
```
825825

826+
**Split-URL deployments** (e.g., Docker Compose with reverse proxy): When the gateway
827+
reaches Keycloak via an internal URL (`SSO_KEYCLOAK_BASE_URL=http://keycloak:8080`) but
828+
users authenticate via a different public URL, set `SSO_KEYCLOAK_PUBLIC_BASE_URL` to the
829+
browser-facing URL. The gateway rewrites both the authorization URL and the issuer to
830+
the public base so that `id_token` issuer verification succeeds:
831+
832+
```bash
833+
SSO_KEYCLOAK_BASE_URL=http://keycloak:8080 # Internal (token, userinfo, JWKS)
834+
SSO_KEYCLOAK_PUBLIC_BASE_URL=http://localhost:8180 # Browser-facing (auth URL, issuer)
835+
```
836+
826837
### Roles Not Appearing in JWT
827838

828839
**Problem**: User roles not included in JWT token

docs/docs/manage/sso-microsoft-entra-id-tutorial.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -601,12 +601,20 @@ After configuration, test role assignment:
601601
curl -H "Authorization: Bearer YOUR_TOKEN" \
602602
http://localhost:8000/rbac/my/roles
603603

604-
# Should return assigned roles:
604+
# Should return assigned roles (abbreviated):
605605
[
606606
{
607+
"id": "...",
608+
"user_email": "user@example.com",
609+
"role_id": "...",
607610
"role_name": "developer",
608611
"scope": "team",
609-
"granted_by": "sso_system"
612+
"scope_id": null,
613+
"granted_by": "user@example.com",
614+
"granted_at": "2026-02-20T21:20:20Z",
615+
"expires_at": null,
616+
"is_active": true,
617+
"grant_source": "sso"
610618
}
611619
]
612620
```
@@ -643,7 +651,7 @@ Roles are automatically synchronized:
643651

644652
- Admins can manually assign additional roles via Admin UI
645653
- Manually assigned roles are preserved during sync
646-
- Only SSO-granted roles (granted_by='sso_system') are synchronized
654+
- Only SSO-granted roles (`grant_source='sso'`) are synchronized
647655

648656
### 8.5.7 Troubleshooting Role Mapping
649657

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# pylint: disable=no-member
2+
"""add grant_source to user_roles
3+
4+
Revision ID: e1f2a3b4c5d6
5+
Revises: d9e0f1a2b3c4
6+
Create Date: 2026-03-05
7+
8+
Adds a grant_source column to user_roles to track the origin of role
9+
assignments (e.g. 'sso', 'manual', 'bootstrap', 'auto'). This replaces
10+
the previous pattern of using granted_by='sso_system' which violated the
11+
FK constraint on granted_by -> email_users.email.
12+
"""
13+
14+
# Standard
15+
from typing import Sequence, Union
16+
17+
# Third-Party
18+
from alembic import op
19+
import sqlalchemy as sa
20+
21+
# revision identifiers, used by Alembic.
22+
revision: str = "e1f2a3b4c5d6"
23+
down_revision: Union[str, Sequence[str], None] = "d9e0f1a2b3c4"
24+
branch_labels: Union[str, Sequence[str], None] = None
25+
depends_on: Union[str, Sequence[str], None] = None
26+
27+
28+
def upgrade() -> None:
29+
"""Add grant_source column to user_roles table."""
30+
inspector = sa.inspect(op.get_bind())
31+
32+
if "user_roles" not in inspector.get_table_names():
33+
return
34+
35+
columns = [col["name"] for col in inspector.get_columns("user_roles")]
36+
if "grant_source" in columns:
37+
return
38+
39+
op.add_column("user_roles", sa.Column("grant_source", sa.String(50), nullable=True))
40+
41+
42+
def downgrade() -> None:
43+
"""Remove grant_source column from user_roles table."""
44+
inspector = sa.inspect(op.get_bind())
45+
46+
if "user_roles" not in inspector.get_table_names():
47+
return
48+
49+
columns = [col["name"] for col in inspector.get_columns("user_roles")]
50+
if "grant_source" not in columns:
51+
return
52+
53+
op.drop_column("user_roles", "grant_source")

mcpgateway/db.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,7 @@ class UserRole(Base):
978978
granted_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, default=utc_now)
979979
expires_at: Mapped[Optional[datetime]] = mapped_column(DateTime(timezone=True), nullable=True)
980980
is_active: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True)
981+
grant_source: Mapped[Optional[str]] = mapped_column(String(50), nullable=True, default=None)
981982

982983
# Relationships
983984
role: Mapped["Role"] = relationship("Role", back_populates="user_assignments")

mcpgateway/schemas.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6701,6 +6701,7 @@ class UserRoleResponse(BaseModel):
67016701
granted_at: datetime = Field(..., description="When role was granted")
67026702
expires_at: Optional[datetime] = Field(None, description="Optional expiration")
67036703
is_active: bool = Field(..., description="Whether assignment is active")
6704+
grant_source: Optional[str] = Field(None, description="Origin of the grant (e.g., 'sso', 'manual', 'bootstrap', 'auto')")
67046705

67056706

67066707
class PermissionCheckRequest(BaseModel):

mcpgateway/services/role_service.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,9 @@ async def delete_role(self, role_id: str) -> bool:
526526
logger.info(f"Deleted role: {role.name} (id: {role.id})")
527527
return True
528528

529-
async def assign_role_to_user(self, user_email: str, role_id: str, scope: str, scope_id: Optional[str], granted_by: str, expires_at: Optional[datetime] = None) -> UserRole:
529+
async def assign_role_to_user(
530+
self, user_email: str, role_id: str, scope: str, scope_id: Optional[str], granted_by: str, expires_at: Optional[datetime] = None, grant_source: Optional[str] = None
531+
) -> UserRole:
530532
"""Assign a role to a user.
531533
532534
Args:
@@ -536,6 +538,7 @@ async def assign_role_to_user(self, user_email: str, role_id: str, scope: str, s
536538
scope_id: Team ID if team-scoped
537539
granted_by: Email of user granting the role
538540
expires_at: Optional expiration datetime
541+
grant_source: Origin of the grant (e.g., 'sso', 'manual', 'bootstrap', 'auto')
539542
540543
Returns:
541544
UserRole: The role assignment
@@ -619,7 +622,7 @@ async def assign_role_to_user(self, user_email: str, role_id: str, scope: str, s
619622
raise ValueError("User already has this role assignment")
620623

621624
# Create the assignment
622-
user_role = UserRole(user_email=user_email, role_id=role_id, scope=scope, scope_id=scope_id, granted_by=granted_by, expires_at=expires_at)
625+
user_role = UserRole(user_email=user_email, role_id=role_id, scope=scope, scope_id=scope_id, granted_by=granted_by, expires_at=expires_at, grant_source=grant_source)
623626

624627
self.db.add(user_role)
625628
self.db.commit()

mcpgateway/services/sso_service.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1893,9 +1893,9 @@ async def _sync_user_roles(self, user_email: str, role_assignments: List[Dict[st
18931893

18941894
role_service = RoleService(self.db)
18951895

1896-
# Get current SSO-granted roles (granted_by='sso_system')
1896+
# Get current SSO-granted roles
18971897
current_roles = await role_service.list_user_roles(user_email, include_expired=False)
1898-
sso_roles = [r for r in current_roles if r.granted_by == "sso_system"]
1898+
sso_roles = [r for r in current_roles if getattr(r, "grant_source", None) == "sso"]
18991899

19001900
# Build set of desired role assignments
19011901
desired_roles = {(r["role_name"], r["scope"], r.get("scope_id")) for r in role_assignments}
@@ -1921,7 +1921,9 @@ async def _sync_user_roles(self, user_email: str, role_assignments: List[Dict[st
19211921

19221922
if not existing or not existing.is_active:
19231923
# Assign role to user
1924-
await role_service.assign_role_to_user(user_email=user_email, role_id=role.id, scope=assignment["scope"], scope_id=assignment.get("scope_id"), granted_by="sso_system")
1924+
await role_service.assign_role_to_user(
1925+
user_email=user_email, role_id=role.id, scope=assignment["scope"], scope_id=assignment.get("scope_id"), granted_by=user_email, grant_source="sso"
1926+
)
19251927
logger.info(f"Assigned SSO role '{role.name}' to {user_email}")
19261928

19271929
except Exception as e:

0 commit comments

Comments
 (0)