fix enterprise-level SAML configuration

[gontzess]

Summary

When SAML is configured at the GitHub Enterprise level (not org level), the org-level SAML GraphQL query returns an error. This change:

1. Catches the "Enterprise SAML identity provider is available" error in hasSAML() and treats org-level SAML as disabled instead of failing

2. When --enterprises config is set and org-level SAML is disabled, loads SAML identity data from the enterprise consumed-licenses endpoint

3. Uses enterprise SAML data (saml_name_id, verified_domain_emails) for user email enrichment when org-level SAML is unavailable

Testing

Tested against a GitHub Enterprise with Okta SAML configured:

Feature	Result
Enterprise SAML data loading	Debug logs show users_with_saml_data: 1
User email enrichment	User email populated from enterprise SAML
Sync completion	Completes successfully
CI checks	lint/test pass

Note: Cannot fully replicate the original error locally because our test org's samlIdentityProvider returns null. Customer validation recommended.

Test plan

• Code compiles, linting passes
• All unit tests pass
• Enterprise SAML data loading works (verified via debug logs)
• Sync completes successfully with enterprise SAML configured
• Customer validation in environment with conflicting SAML state

Fixes: CXH-918

[linear]

CXH-918 GitHub connector fails with Enterprise SAML org error

[coderabbitai]

Walkthrough

The changes introduce enterprise-level SAML handling for user data enrichment in the connector package. A new enterpriseUserSAML type is added, and the userResourceType is extended with a custom HTTP client, enterprise configuration, and SAML data caching. The builder now accepts these additional dependencies, and new methods load and resolve enterprise SAML user information.

Changes

Cohort / File(s)	Summary
Enterprise SAML Integration pkg/connector/user.go	Extended userResourceType with custom client, enterprises list, and SAML data cache. Added loadEnterpriseSAMLData() and getEnterpriseSAMLEmail() methods. Modified List() flow to load enterprise SAML data when org-level SAML is unavailable. Enhanced error handling in hasSAML() to treat Enterprise SAML configuration gracefully.
Builder Signature Update pkg/connector/connector.go, pkg/connector/user_test.go	Updated userBuilder() invocation and calls to accept customClient and enterprises parameters, expanding dependencies passed to the builder.
Type Definition pkg/connector/helpers.go	Added new enterpriseUserSAML type with SAMLNameID and VerifiedEmails fields to support enterprise SAML data structures.

Sequence Diagram

sequenceDiagram
    participant Client
    participant userResourceType
    participant customClient
    participant enterpriseSAML as Enterprise SAML Cache

    Client->>userResourceType: List(ctx)
    userResourceType->>userResourceType: Check org-level SAML enabled
    alt Org SAML disabled
        userResourceType->>customClient: loadEnterpriseSAMLData(ctx)
        customClient->>enterpriseSAML: Fetch and cache enterprise SAML data
        enterpriseSAML-->>userResourceType: SAML data loaded
    end
    userResourceType->>userResourceType: Iterate user logins
    loop For each user
        userResourceType->>enterpriseSAML: getEnterpriseSAMLEmail(login)
        enterpriseSAML-->>userResourceType: primaryEmail, extraEmails
        userResourceType->>userResourceType: Enrich user with email data
    end
    userResourceType-->>Client: Return enriched users

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 With enterprises in sight,
SAML data shines so bright,
Custom clients hop along,
User emails now ring strong,
Enterprise support takes flight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding support for enterprise-level SAML configuration when org-level SAML is unavailable.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@pkg/connector/user.go`:
- Around line 386-398: The function getEnterpriseSAMLEmail uses named return
values causing a no-named-returns lint failure; change its signature from named
returns to unnamed returns (return types string, []string), introduce local
variables (e.g., primaryEmailLocal, extraEmailsLocal) instead of assigning to
named returns, update all early returns to return explicit values ("" or nil) or
return primaryEmailLocal, extraEmailsLocal at the end, and update the branch
that sets samlData.SAMLNameID and appends samlData.VerifiedEmails to use those
local vars; keep references to enterpriseSAMLData, samlData, SAMLNameID, and
VerifiedEmails to locate the changes.
- Around line 343-349: The current error handling in the enterprise SAML loader
logs the error (l.Warn(..., zap.String("enterprise", enterprise),
zap.Error(err))) then uses "return nil", which aborts the entire loader and
skips remaining enterprises; change this to skip only the failing enterprise by
replacing the early return with a loop continue (or otherwise continue to the
next enterprise in the surrounding iteration) so that other enterprises are
still processed when loadEnterpriseConsumedLicenses/SAML loading fails for one.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Don’t stop processing other enterprises on a single load error.
return nil exits the loader, so later enterprises are skipped when multiple are configured. Prefer skipping just the failing enterprise.

♻️ Suggested fix

-			if err != nil {
-				l.Warn("failed to load enterprise consumed licenses for SAML data",
-					zap.String("enterprise", enterprise),
-					zap.Error(err))
-				// Don't fail the sync, just continue without enterprise SAML data
-				return nil
-			}
+			if err != nil {
+				l.Warn("failed to load enterprise consumed licenses for SAML data",
+					zap.String("enterprise", enterprise),
+					zap.Error(err))
+				// Don't fail the sync; skip this enterprise and continue.
+				break
+			}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if err != nil {
	l.Warn("failed to load enterprise consumed licenses for SAML data",
	zap.String("enterprise", enterprise),
	zap.Error(err))
	// Don't fail the sync, just continue without enterprise SAML data
	return nil
	}
	if err != nil {
	l.Warn("failed to load enterprise consumed licenses for SAML data",
	zap.String("enterprise", enterprise),
	zap.Error(err))
	// Don't fail the sync; skip this enterprise and continue.
	break
	}

🤖 Prompt for AI Agents

In `@pkg/connector/user.go` around lines 343 - 349, The current error handling in
the enterprise SAML loader logs the error (l.Warn(..., zap.String("enterprise",
enterprise), zap.Error(err))) then uses "return nil", which aborts the entire
loader and skips remaining enterprises; change this to skip only the failing
enterprise by replacing the early return with a loop continue (or otherwise
continue to the next enterprise in the surrounding iteration) so that other
enterprises are still processed when loadEnterpriseConsumedLicenses/SAML loading
fails for one.

[gontzess]

Closing in favor of #112 which was merged. Our PR included additional enterprise SAML data fallback functionality that can be added in a follow-up if needed.