From e8d13b4635ac3a604d7a23c2a31bd3bae9b4c9d8 Mon Sep 17 00:00:00 2001 From: Josh Bendson Date: Mon, 26 Jan 2026 22:55:59 -0600 Subject: [PATCH 1/3] feat: Add SSO group mapping with optional display names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements configurable mapping of external SSO provider groups (Entra, Keycloak) to internal CIDX groups (admins, powerusers, users) during JIT user provisioning. Key Features: - Parse groups from ID token instead of userinfo endpoint - Support list-based group mapping format with optional display names - First-match strategy with graceful fallback to "users" group - Backward compatibility with old dict format (auto-migrated) - Fixed group_manager injection bug during OIDC config reload - Removed insecure ID token logging Configuration Format: [ { "external_group_id": "guid-or-name", // Required: matches token "external_group_name": "Display Name", // Optional: for docs/UI "cidx_group": "admins" // Required: target group } ] Behavior: - Mapping matches + group exists → assign to mapped group - Mapping matches + group missing → fallback to "users" (with warning) - No mappings or no match → fallback to "users" - Existing users not reassigned on re-login (AC3) Changes: - config_manager.py: Add group_mappings config with backward compatibility - sso_provisioning_hook.py: Implement matching logic and fallback behavior - oidc_provider.py: Parse ID token for groups, remove insecure logging - oidc_manager.py: Pass groups from token to provisioning hook - config_service.py: Handle both dict and list mapping formats - routes.py: Fix group_manager injection during config reload - config_section.html: Display mappings with optional names in UI - Tests: Update for ID token parsing and new mapping format Security: Removed raw ID token payload logging (only logs claim names) --- .../server/auth/oidc/oidc_manager.py | 25 ++- .../server/auth/oidc/oidc_provider.py | 87 ++++++--- src/code_indexer/server/auth/oidc/routes.py | 10 +- .../server/services/config_service.py | 26 +++ .../server/services/sso_provisioning_hook.py | 118 ++++++++--- .../server/utils/config_manager.py | 17 +- src/code_indexer/server/web/routes.py | 19 ++ .../templates/partials/config_section.html | 35 ++++ .../oidc/test_oidc_provider_http_calls.py | 134 +++++++++++++ .../unit/server/test_sso_provisioning_hook.py | 184 ++++++++++++++++++ 10 files changed, 599 insertions(+), 56 deletions(-) diff --git a/src/code_indexer/server/auth/oidc/oidc_manager.py b/src/code_indexer/server/auth/oidc/oidc_manager.py index d3c3f2d6..322d3218 100644 --- a/src/code_indexer/server/auth/oidc/oidc_manager.py +++ b/src/code_indexer/server/auth/oidc/oidc_manager.py @@ -60,7 +60,7 @@ def is_enabled(self): """Check if OIDC is enabled in configuration.""" return self.config.enabled - def _ensure_group_membership(self, username: str) -> None: + def _ensure_group_membership(self, username: str, external_groups=None) -> None: """Ensure user has group membership via SSO provisioning hook. Story #708: SSO Auto-Provisioning with Default Group Assignment @@ -68,8 +68,14 @@ def _ensure_group_membership(self, username: str) -> None: - AC3: Existing users' membership is NOT changed - AC6: Errors are logged but do not block authentication + Group Mapping Support: + - External groups from SSO provider can be mapped to CIDX groups via configuration + - First matched group is used for assignment + - Falls back to "users" group if no mappings match + Args: username: The user's username to provision + external_groups: Optional list of external group names from OIDC provider """ import logging @@ -88,7 +94,12 @@ def _ensure_group_membership(self, username: str) -> None: ensure_user_group_membership, ) - result = ensure_user_group_membership(username, self.group_manager) + # Get group_mappings from config + group_mappings = self.config.group_mappings or {} + + result = ensure_user_group_membership( + username, self.group_manager, external_groups, group_mappings + ) if result: logger.debug( f"SSO provisioning completed for user {username}", @@ -191,7 +202,9 @@ async def match_or_create_user(self, user_info): extra={"correlation_id": get_correlation_id()}, ) # Story #708: Ensure group membership on every SSO login - self._ensure_group_membership(existing_user.username) + self._ensure_group_membership( + existing_user.username, user_info.groups + ) return existing_user else: # Stale OIDC link (defensive check - should be cleaned up on user deletion) @@ -219,7 +232,9 @@ async def match_or_create_user(self, user_info): email=user_info.email, ) # Story #708: Ensure group membership on every SSO login - self._ensure_group_membership(existing_user.username) + self._ensure_group_membership( + existing_user.username, user_info.groups + ) return existing_user # Create new user via JIT provisioning if enabled @@ -281,6 +296,6 @@ async def match_or_create_user(self, user_info): ) # Story #708: Ensure group membership for new JIT-provisioned user - self._ensure_group_membership(new_user.username) + self._ensure_group_membership(new_user.username, user_info.groups) return new_user diff --git a/src/code_indexer/server/auth/oidc/oidc_provider.py b/src/code_indexer/server/auth/oidc/oidc_provider.py index b47e1dd0..0711d8e9 100644 --- a/src/code_indexer/server/auth/oidc/oidc_provider.py +++ b/src/code_indexer/server/auth/oidc/oidc_provider.py @@ -2,7 +2,7 @@ from code_indexer.server.middleware.correlation import get_correlation_id from dataclasses import dataclass -from typing import Optional +from typing import Optional, List @dataclass @@ -20,6 +20,7 @@ class OIDCUserInfo: email: Optional[str] = None email_verified: bool = False username: Optional[str] = None + groups: Optional[List[str]] = None class OIDCProvider: @@ -110,39 +111,53 @@ async def exchange_code_for_token(self, code, code_verifier, redirect_uri): return tokens - async def get_user_info(self, access_token): - import httpx + async def get_user_info(self, access_token, id_token): + """Parse ID token to extract user information and claims. + + ID token contains all necessary user claims including groups. + This approach works universally with Entra, Keycloak, and other OIDC providers. + + Args: + access_token: OAuth access token (kept for backward compatibility) + id_token: OIDC ID token (JWT) containing user claims + + Returns: + OIDCUserInfo object with user claims including groups + """ + import base64 + import json import logging logger = logging.getLogger(__name__) - # Construct userinfo endpoint (typically from discovery, but fallback to standard path) - # Use userinfo endpoint from discovery metadata (preferred) or fallback - if self._metadata and self._metadata.userinfo_endpoint: - userinfo_endpoint = self._metadata.userinfo_endpoint - else: - userinfo_endpoint = ( - f"{self.config.issuer_url}/protocol/openid-connect/userinfo" - ) + # Parse ID token JWT (format: header.payload.signature) + if not id_token: + raise Exception("ID token is required but was not provided") - # Fetch user info from userinfo endpoint - headers = {"Authorization": f"Bearer {access_token}"} try: - async with httpx.AsyncClient() as client: - response = await client.get(userinfo_endpoint, headers=headers) - response.raise_for_status() # Raise HTTPStatusError for 4xx/5xx - data = response.json() # Not async in httpx - except httpx.HTTPStatusError as e: - raise Exception( - f"Failed to get user info: HTTP {e.response.status_code} - {e.response.text}" - ) from e - except httpx.RequestError as e: - raise Exception(f"Failed to connect to userinfo endpoint: {str(e)}") from e + parts = id_token.split('.') + if len(parts) != 3: + raise Exception(f"Invalid ID token format: expected 3 parts, got {len(parts)}") + + # Decode payload (base64url decode with padding) + payload = parts[1] + # Add padding if needed (base64 requires length to be multiple of 4) + padding = 4 - (len(payload) % 4) + if padding != 4: + payload += '=' * padding + + data = json.loads(base64.urlsafe_b64decode(payload)) + logger.info( + f"Parsed ID token with claims: {list(data.keys())}", + extra={"correlation_id": get_correlation_id()}, + ) + except Exception as e: + raise Exception(f"Failed to parse ID token: {e}") from e - # Validate userinfo response has required fields + # Validate ID token has required fields if "sub" not in data or not data["sub"]: raise Exception( - "Invalid userinfo response: missing or empty sub (subject) claim" + "Invalid ID token: missing or empty sub (subject) claim" ) # Log claim extraction for debugging @@ -151,7 +166,7 @@ async def get_user_info(self, access_token): extra={"correlation_id": get_correlation_id()}, ) logger.info( - f"Available claims in userinfo: {list(data.keys())}", + f"Available claims in ID token: {list(data.keys())}", extra={"correlation_id": get_correlation_id()}, ) @@ -167,12 +182,32 @@ async def get_user_info(self, access_token): extra={"correlation_id": get_correlation_id()}, ) + # Extract groups from configured groups_claim + groups_value = data.get(self.config.groups_claim) + logger.info( + f"Groups claim '{self.config.groups_claim}' raw value: {groups_value} (type: {type(groups_value).__name__})", + extra={"correlation_id": get_correlation_id()}, + ) + if groups_value and isinstance(groups_value, list): + groups_list = [str(g) for g in groups_value] + logger.info( + f"Extracted {len(groups_list)} groups from '{self.config.groups_claim}' claim: {groups_list}", + extra={"correlation_id": get_correlation_id()}, + ) + else: + groups_list = [] + logger.info( + f"No groups found in '{self.config.groups_claim}' claim or claim value is not a list", + extra={"correlation_id": get_correlation_id()}, + ) + # Create OIDCUserInfo from response user_info = OIDCUserInfo( subject=data.get("sub", ""), email=email_value, email_verified=data.get("email_verified", False), username=username_value, + groups=groups_list if groups_list else None, ) return user_info diff --git a/src/code_indexer/server/auth/oidc/routes.py b/src/code_indexer/server/auth/oidc/routes.py index f08367a7..e81c6115 100644 --- a/src/code_indexer/server/auth/oidc/routes.py +++ b/src/code_indexer/server/auth/oidc/routes.py @@ -56,8 +56,14 @@ async def sso_callback(code: str, state: str, request: Request): code, code_verifier, callback_url ) - # Get user info from provider - user_info = await oidc_manager.provider.get_user_info(tokens["access_token"]) + # Parse ID token to get user info (includes groups for Entra/Keycloak) + if "id_token" not in tokens: + raise HTTPException(status_code=500, detail="ID token not returned by provider") + + user_info = await oidc_manager.provider.get_user_info( + tokens["access_token"], + tokens["id_token"] + ) # Match or create user (email-based) user = await oidc_manager.match_or_create_user(user_info) diff --git a/src/code_indexer/server/services/config_service.py b/src/code_indexer/server/services/config_service.py index 554b0c7b..9a1e4b0f 100644 --- a/src/code_indexer/server/services/config_service.py +++ b/src/code_indexer/server/services/config_service.py @@ -191,6 +191,8 @@ def get_all_settings(self) -> Dict[str, Any]: "require_email_verification": config.oidc_provider_config.require_email_verification, "enable_jit_provisioning": config.oidc_provider_config.enable_jit_provisioning, "default_role": config.oidc_provider_config.default_role, + "groups_claim": config.oidc_provider_config.groups_claim, + "group_mappings": config.oidc_provider_config.group_mappings, }, # SCIP workspace cleanup (Story #647, Story #15 AC2: moved to scip_config) "scip_cleanup": { @@ -580,6 +582,30 @@ def _update_oidc_setting(self, config: ServerConfig, key: str, value: Any) -> No oidc.enable_jit_provisioning = value in ["true", True] elif key == "default_role": oidc.default_role = str(value) + elif key == "groups_claim": + oidc.groups_claim = str(value) + elif key == "group_mappings": + # Parse JSON string, dict (old format), or list (new format) + import json + + if isinstance(value, (dict, list)): + oidc.group_mappings = value + elif isinstance(value, str): + try: + parsed = json.loads(value) + if not isinstance(parsed, (dict, list)): + raise ValueError( + f"Invalid JSON for group_mappings: must be dict or list, got {type(parsed)}" + ) + oidc.group_mappings = parsed + except json.JSONDecodeError: + raise ValueError( + f"Invalid JSON for group_mappings: {value}. Expected format: [{{'external_group_id': 'guid', 'cidx_group': 'admins'}}]" + ) + else: + raise ValueError( + f"Invalid type for group_mappings: {type(value)}. Expected dict, list, or JSON string" + ) else: raise ValueError(f"Unknown OIDC setting: {key}") diff --git a/src/code_indexer/server/services/sso_provisioning_hook.py b/src/code_indexer/server/services/sso_provisioning_hook.py index 5e28486b..9a7ca180 100644 --- a/src/code_indexer/server/services/sso_provisioning_hook.py +++ b/src/code_indexer/server/services/sso_provisioning_hook.py @@ -11,10 +11,15 @@ - Existing users' group membership is NOT changed on re-login - assigned_by is set to "system:sso-provisioning" for auto-provisioned users - Errors are logged but do not block authentication + +Group Mapping Support: +- External groups from SSO provider can be mapped to CIDX groups via configuration +- First matched group is used for assignment +- Falls back to "users" group if no mappings match """ import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Optional, List, Dict from .constants import DEFAULT_GROUP_USERS @@ -48,16 +53,59 @@ class SSOProvisioningHook: have a group membership. Existing users are not modified. """ - def __init__(self, group_manager: "GroupAccessManager"): + def __init__( + self, + group_manager: "GroupAccessManager", + group_mappings: Optional[List[Dict[str, str]]] = None, + ): """ Initialize the SSO provisioning hook. Args: group_manager: The GroupAccessManager instance for group operations + group_mappings: Optional list of group mapping objects with: + - external_group_id: The external group identifier (GUID or name) + - external_group_name: Optional display name for documentation + - cidx_group: Target CIDX group name """ self.group_manager = group_manager + self.group_mappings = group_mappings or [] + + def _determine_target_group( + self, external_groups: Optional[List[str]] + ) -> str: + """ + Determine target CIDX group based on external groups and configured mappings. - def ensure_group_membership(self, user_id: str) -> bool: + Args: + external_groups: List of external group identifiers from OIDC provider + + Returns: + CIDX group name to assign user to (first match or DEFAULT_GROUP_USERS) + """ + if not external_groups or not self.group_mappings: + return DEFAULT_GROUP_USERS + + # Find first matching group mapping (new list format) + for external_group_id in external_groups: + for mapping in self.group_mappings: + if mapping.get("external_group_id") == external_group_id: + cidx_group = mapping.get("cidx_group") + external_name = mapping.get("external_group_name", external_group_id) + logger.debug( + f"Matched external group '{external_name}' ({external_group_id}) to CIDX group '{cidx_group}'" + ) + return cidx_group + + # No matches found, use default + logger.debug( + f"No group mappings matched for external groups {external_groups}, using default '{DEFAULT_GROUP_USERS}'" + ) + return DEFAULT_GROUP_USERS + + def ensure_group_membership( + self, user_id: str, external_groups: Optional[List[str]] = None + ) -> bool: """ Ensure the user has a group membership, defaulting to users group. @@ -65,8 +113,13 @@ def ensure_group_membership(self, user_id: str) -> bool: AC3: Existing users' membership is NOT changed AC6: Errors are logged but do not block authentication + Group Mapping: + - If external_groups provided and group_mappings configured, assigns to first matched group + - Falls back to "users" group if no match or no mappings + Args: user_id: The user's unique identifier (from SSO token sub claim) + external_groups: Optional list of external group names from OIDC provider Returns: True if user has membership (existing or newly created), @@ -83,36 +136,52 @@ def ensure_group_membership(self, user_id: str) -> bool: ) return True - # New user - assign to "users" group - users_group = self.group_manager.get_group_by_name(DEFAULT_GROUP_USERS) - - if users_group is None: - # PRECONDITION VIOLATION: Database not properly initialized - # Per Anti-Fallback principle, fail loudly instead of silent degradation - raise SystemConfigurationError( - f"SSO provisioning failed: '{DEFAULT_GROUP_USERS}' group not found. " - "Database may not be properly initialized. " - "Run database initialization to create default groups." - ) - - # AC1: Assign new user to users group + # New user - determine target group based on mappings + target_group_name = self._determine_target_group(external_groups) + target_group = self.group_manager.get_group_by_name(target_group_name) + + # If target group doesn't exist, fallback to default users group + if target_group is None: + if target_group_name != DEFAULT_GROUP_USERS: + logger.warning( + f"Configured group '{target_group_name}' not found, falling back to '{DEFAULT_GROUP_USERS}' group for user '{user_id}'" + ) + target_group_name = DEFAULT_GROUP_USERS + target_group = self.group_manager.get_group_by_name(target_group_name) + + # If fallback group also doesn't exist, database is not initialized + if target_group is None: + # PRECONDITION VIOLATION: Database not properly initialized + # Per Anti-Fallback principle, fail loudly instead of silent degradation + raise SystemConfigurationError( + f"SSO provisioning failed: '{DEFAULT_GROUP_USERS}' group not found. " + "Database may not be properly initialized. " + "Run database initialization to create default groups." + ) + + # AC1: Assign new user to determined group self.group_manager.assign_user_to_group( user_id=user_id, - group_id=users_group.id, + group_id=target_group.id, assigned_by=SSO_PROVISIONING_ASSIGNED_BY, ) # AC7 (Story #710): Log to audit trail for administrative actions + mapping_info = ( + f" (mapped from external groups: {external_groups})" + if external_groups and target_group_name != DEFAULT_GROUP_USERS + else "" + ) self.group_manager.log_audit( admin_id=SSO_PROVISIONING_ASSIGNED_BY, action_type="user_assign", target_type="user", target_id=user_id, - details=f"SSO auto-provisioned to '{DEFAULT_GROUP_USERS}' group", + details=f"SSO auto-provisioned to '{target_group_name}' group{mapping_info}", ) logger.info( - f"SSO auto-provisioned user '{user_id}' to '{DEFAULT_GROUP_USERS}' group" + f"SSO auto-provisioned user '{user_id}' to '{target_group_name}' group{mapping_info}" ) return True @@ -130,7 +199,10 @@ def ensure_group_membership(self, user_id: str) -> bool: def ensure_user_group_membership( - user_id: str, group_manager: "GroupAccessManager" + user_id: str, + group_manager: "GroupAccessManager", + external_groups: Optional[List[str]] = None, + group_mappings: Optional[List[Dict[str, str]]] = None, ) -> bool: """ Standalone function wrapper for SSO provisioning. @@ -141,10 +213,12 @@ def ensure_user_group_membership( Args: user_id: The user's unique identifier (from SSO token sub claim) group_manager: The GroupAccessManager instance + external_groups: Optional list of external group identifiers from OIDC provider + group_mappings: Optional list of group mapping objects Returns: True if user has membership (existing or newly created), False if provisioning failed """ - hook = SSOProvisioningHook(group_manager) - return hook.ensure_group_membership(user_id) + hook = SSOProvisioningHook(group_manager, group_mappings) + return hook.ensure_group_membership(user_id, external_groups) diff --git a/src/code_indexer/server/utils/config_manager.py b/src/code_indexer/server/utils/config_manager.py index 3d06ec93..bc2fa153 100644 --- a/src/code_indexer/server/utils/config_manager.py +++ b/src/code_indexer/server/utils/config_manager.py @@ -10,7 +10,7 @@ import os from dataclasses import dataclass, asdict from pathlib import Path -from typing import Optional +from typing import Optional, Dict, List, Union @dataclass @@ -124,9 +124,24 @@ class OIDCProviderConfig: enable_jit_provisioning: bool = True default_role: str = "normal_user" + # Group mapping configuration + groups_claim: str = "groups" # Claim name containing user's groups (e.g., "groups", "roles") + group_mappings: Optional[Union[Dict[str, str], List[Dict[str, str]]]] = None # Map external groups to CIDX groups + def __post_init__(self): if self.scopes is None: self.scopes = ["openid", "profile", "email"] + if self.group_mappings is None: + self.group_mappings = [] + elif isinstance(self.group_mappings, dict): + # Backward compatibility: convert old dict format to new list format + self.group_mappings = [ + { + "external_group_id": external_id, + "cidx_group": cidx_group, + } + for external_id, cidx_group in self.group_mappings.items() + ] @dataclass diff --git a/src/code_indexer/server/web/routes.py b/src/code_indexer/server/web/routes.py index 169375df..dd2bf0f8 100644 --- a/src/code_indexer/server/web/routes.py +++ b/src/code_indexer/server/web/routes.py @@ -711,6 +711,17 @@ async def delete_user( ) await db.commit() + # Clean up group membership (Bug fix: prevent orphaned group memberships) + try: + group_manager = _get_group_manager() + user_group = group_manager.get_user_group(username) + if user_group: + group_manager.remove_user_from_group(username, user_group.id) + logger.info(f"Cleaned up group membership for deleted user: {username}") + except RuntimeError: + # group_manager not available - skip cleanup + logger.warning(f"group_manager not available, skipped group cleanup for: {username}") + return _create_users_page_response( request, session, success_message=f"User '{username}' deleted successfully" ) @@ -3998,6 +4009,14 @@ async def _reload_oidc_configuration(): # Provider metadata will be discovered lazily on next SSO login attempt await oidc_manager.initialize() + # Inject GroupAccessManager into OIDCManager for SSO provisioning (Story #708) + if hasattr(app_module.app.state, "group_manager") and app_module.app.state.group_manager: + oidc_manager.group_manager = app_module.app.state.group_manager + logger.info( + "GroupAccessManager injected into reloaded OIDCManager for SSO auto-provisioning", + extra={"correlation_id": get_correlation_id()}, + ) + # Replace the old managers with new ones oidc_routes.oidc_manager = oidc_manager oidc_routes.state_manager = state_manager diff --git a/src/code_indexer/server/web/templates/partials/config_section.html b/src/code_indexer/server/web/templates/partials/config_section.html index 67632aac..1096275a 100644 --- a/src/code_indexer/server/web/templates/partials/config_section.html +++ b/src/code_indexer/server/web/templates/partials/config_section.html @@ -472,6 +472,31 @@

SSO Authentication

{{ config.oidc.username_claim }} + + Groups Claim + {{ config.oidc.groups_claim }} + Claim containing user's groups + + + Group Mappings + + {% if config.oidc.group_mappings %} +
+ {% for mapping in config.oidc.group_mappings %} + {% if mapping.external_group_name %} + {{ mapping.external_group_name }}
+ {{ mapping.external_group_id }} → {{ mapping.cidx_group }}
+ {% else %} + {{ mapping.external_group_id }} → {{ mapping.cidx_group }}
+ {% endif %} + {% endfor %} +
+ {% else %} + None configured + {% endif %} + + Maps external groups to CIDX groups + Use PKCE {{ 'Yes' if config.oidc.use_pkce else 'No' }} @@ -541,6 +566,16 @@

SSO Authentication

OIDC claim for username (used ONLY when creating new users via JIT provisioning) + +