From 2e747ec18ae709f417e50515115c0dbef3095bf6 Mon Sep 17 00:00:00 2001 From: Steffen Hanikel Date: Thu, 10 Sep 2026 18:24:44 +0200 Subject: [PATCH] feat(users): fall back to verified-domain emails when SAML lookup is unavailable --- README.md | 2 + pkg/connector/helpers.go | 12 ++++++ pkg/connector/user.go | 31 ++++++++++++++ pkg/connector/user_test.go | 83 ++++++++++++++++++++++++++++++++++++++ test/mocks/github.go | 4 ++ 5 files changed, 132 insertions(+) diff --git a/README.md b/README.md index e1ae9d78..cc90ed19 100644 --- a/README.md +++ b/README.md @@ -113,3 +113,5 @@ Org: Repo: - Administrator: Read and Write - This permission implies Metadata: Read + +When SAML identity lookup is unavailable (SAML configured at the enterprise level while authenticating as a GitHub App, or no SAML at all), the connector falls back to each member's verified-domain emails. This requires the organization to have verified domains and the Members read permission. Users without an email on a verified domain keep an empty email. diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index ece84762..85675f19 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -291,6 +291,18 @@ type listUsersQuery struct { } } +type verifiedDomainEmailsQuery struct { + User struct { + OrganizationVerifiedDomainEmails []string `graphql:"organizationVerifiedDomainEmails(login: $orgLoginName)"` + } `graphql:"user(login: $userName)"` + RateLimit struct { + Limit int + Cost int + Remaining int + ResetAt githubv4.DateTime + } +} + type hasSAMLQuery struct { Organization struct { SamlIdentityProvider struct { diff --git a/pkg/connector/user.go b/pkg/connector/user.go index 705507a5..3c30692c 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -286,6 +286,37 @@ func (u *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, op // no SAML enrichment } + if !isEmail(userEmail) { + // Enterprise-level SAML hides identities from org-scoped credentials; verified-domain emails stay readable. + q := verifiedDomainEmailsQuery{} + variables := map[string]interface{}{ + "orgLoginName": githubv4.String(orgName), + "userName": githubv4.String(ghUser.GetLogin()), + } + if err := u.graphqlClient.Query(ctx, &q, variables); err != nil { + l.Warn("failed to fetch verified domain emails", zap.String("login", ghUser.GetLogin()), zap.Error(err)) + } else { + for _, email := range q.User.OrganizationVerifiedDomainEmails { + switch { + case !isEmail(email) || email == userEmail: + case !isEmail(userEmail): + userEmail = email + default: + extraEmails = append(extraEmails, email) + } + } + lastGraphQLRateLimit = &struct { + Limit int + Remaining int + ResetAt githubv4.DateTime + }{ + Limit: q.RateLimit.Limit, + Remaining: q.RateLimit.Remaining, + ResetAt: q.RateLimit.ResetAt, + } + } + } + ur, err := userResource(ctx, ghUser, userEmail, extraEmails) if err != nil { return nil, nil, err diff --git a/pkg/connector/user_test.go b/pkg/connector/user_test.go index e2e4d0e5..db9ed929 100644 --- a/pkg/connector/user_test.go +++ b/pkg/connector/user_test.go @@ -2,13 +2,20 @@ package connector import ( "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" "testing" "github.com/conductorone/baton-github/test" "github.com/conductorone/baton-github/test/mocks" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/pagination" resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" "github.com/google/go-github/v69/github" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/require" ) @@ -57,3 +64,79 @@ func TestUsersList(t *testing.T) { require.Equal(t, *githubUser.Login, users[0].Id.Resource) }) } + +func mockGraphQLWithoutSAML(t *testing.T, verifiedDomainEmails []string) *githubv4.Client { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + var payload map[string]any + switch { + case strings.Contains(string(body), "organizationVerifiedDomainEmails"): + payload = map[string]any{"data": map[string]any{ + "user": map[string]any{"organizationVerifiedDomainEmails": verifiedDomainEmails}, + "rateLimit": map[string]any{"limit": 5000, "cost": 1, "remaining": 4999, "resetAt": "2030-01-01T00:00:00Z"}, + }} + default: + payload = map[string]any{"data": map[string]any{ + "organization": map[string]any{"samlIdentityProvider": map[string]any{"id": "", "ssoUrl": ""}}, + }} + } + require.NoError(t, json.NewEncoder(w).Encode(payload)) + })) + t.Cleanup(server.Close) + return githubv4.NewEnterpriseClient(server.URL, server.Client()) +} + +func TestUsersListVerifiedDomainEmailFallback(t *testing.T) { + ctx := context.Background() + + listUsers := func(t *testing.T, verifiedDomainEmails []string) *v2.Resource { + mgh := mocks.NewMockGitHub() + githubOrganization, _, _, githubUser, _, _ := mgh.Seed() + githubUser.Email = nil + mgh.SetUser(*githubUser) + + organization, err := organizationResource(ctx, githubOrganization, nil, false) + require.NoError(t, err) + + githubClient := github.NewClient(mgh.Server()) + client := UserBuilder( + githubClient, + mockGraphQLWithoutSAML(t, verifiedDomainEmails), + newOrgNameCache(githubClient), + []string{organization.DisplayName}, + nil, + nil, + ) + + users, _, err := client.List(ctx, organization.Id, resourceSdk.SyncOpAttrs{ + PageToken: pagination.Token{}, + Session: &noOpSessionStore{}, + }) + require.NoError(t, err) + require.Len(t, users, 1) + return users[0] + } + + t.Run("uses the first verified-domain email as primary and keeps the rest", func(t *testing.T) { + user := listUsers(t, []string{"not-an-email", "alice@verified.example", "alice@other.example"}) + + trait, err := resourceSdk.GetUserTrait(user) + require.NoError(t, err) + require.Len(t, trait.Emails, 2) + require.Equal(t, "alice@verified.example", trait.Emails[0].Address) + require.True(t, trait.Emails[0].IsPrimary) + require.Equal(t, "alice@other.example", trait.Emails[1].Address) + require.False(t, trait.Emails[1].IsPrimary) + }) + + t.Run("leaves the email empty when the org has no verified-domain email for the member", func(t *testing.T) { + user := listUsers(t, []string{}) + + trait, err := resourceSdk.GetUserTrait(user) + require.NoError(t, err) + for _, email := range trait.Emails { + require.Empty(t, email.Address) + } + }) +} diff --git a/test/mocks/github.go b/test/mocks/github.go index 129e119f..16b0d78a 100644 --- a/test/mocks/github.go +++ b/test/mocks/github.go @@ -150,6 +150,10 @@ func (mgh MockGitHub) Seed() ( return &githubOrganization, &githubRepository, &githubTeam, &githubUser, orgRole, nil } +func (mgh MockGitHub) SetUser(user github.User) { + mgh.users[user.GetID()] = user +} + func getResource[T interface{}]( w http.ResponseWriter, idStr string,