diff --git a/fixtures/scenarios.json b/fixtures/scenarios.json index b49a1e3..f2f3038 100644 --- a/fixtures/scenarios.json +++ b/fixtures/scenarios.json @@ -1054,5 +1054,101 @@ "description": "send sync report notification to slack (orphaned user check skipped)" } ] + }, + { + "name": "okta_sync_users_without_github_username", + "description": "Test that Okta users without GitHub username are skipped but tracked in sync report", + "event_type": "scheduled_event", + "event_payload": { + "action": "okta-sync" + }, + "expected_calls": [ + { + "service": "okta", + "method": "GET", + "path": "/api/v1/groups" + }, + { + "service": "github", + "method": "GET", + "path": "/orgs/*/teams/*" + }, + { + "service": "github", + "method": "PUT", + "path": "/orgs/acme-ghorg/teams/engineering/memberships/alice-gh" + }, + { + "service": "slack", + "method": "POST", + "path": "/chat.postMessage" + } + ], + "mock_responses": [ + { + "service": "github", + "method": "POST", + "path": "/app/installations/987654/access_tokens", + "status_code": 201, + "body": "{\"token\":\"ghs_mock_installation_token\",\"expires_at\":\"2099-12-31T23:59:59Z\"}", + "description": "github app installation token authentication" + }, + { + "service": "github", + "method": "GET", + "path": "/orgs/acme-ghorg/teams/engineering", + "status_code": 200, + "body": "{\"id\":1,\"name\":\"Engineering\",\"slug\":\"engineering\",\"description\":\"Engineering team\"}", + "description": "fetch engineering team details" + }, + { + "service": "github", + "method": "GET", + "path": "/orgs/acme-ghorg/teams/engineering/members", + "status_code": 200, + "body": "[]", + "description": "engineering team has no existing members" + }, + { + "service": "github", + "method": "PUT", + "path": "/orgs/acme-ghorg/teams/engineering/memberships/alice-gh", + "status_code": 200, + "body": "{\"state\":\"active\",\"role\":\"member\"}", + "description": "add alice-gh to team (only user with github username)" + }, + { + "service": "okta", + "method": "POST", + "path": "/oauth2/v1/token", + "status_code": 200, + "body": "{\"token_type\":\"Bearer\",\"expires_in\":3600,\"access_token\":\"mock-access-token\",\"scope\":\"okta.groups.read okta.users.read\"}", + "description": "okta oauth 2.0 token authentication" + }, + { + "service": "okta", + "method": "GET", + "path": "/api/v1/groups", + "status_code": 200, + "body": "[{\"id\":\"00g1234567890abcdef\",\"profile\":{\"name\":\"Engineering\",\"description\":\"Engineering team\"}}]", + "description": "fetch all okta groups (returns engineering group)" + }, + { + "service": "okta", + "method": "GET", + "path": "/api/v1/groups/00g1234567890abcdef/users", + "status_code": 200, + "body": "[{\"id\":\"00u1111111111111111\",\"status\":\"ACTIVE\",\"profile\":{\"email\":\"alice@example.com\",\"githubUsername\":\"alice-gh\"}},{\"id\":\"00u2222222222222222\",\"status\":\"ACTIVE\",\"profile\":{\"email\":\"bob@example.com\"}},{\"id\":\"00u3333333333333333\",\"status\":\"ACTIVE\",\"profile\":{\"email\":\"charlie@example.com\",\"githubUsername\":\"\"}}]", + "description": "fetch users in engineering group (alice has gh username, bob and charlie do not)" + }, + { + "service": "slack", + "method": "POST", + "path": "/chat.postMessage", + "status_code": 200, + "body": "{\"ok\":true,\"channel\":\"C01234TEST\",\"ts\":\"1234567890.123456\"}", + "description": "send sync report to slack (should include skipped users count)" + } + ] } ] diff --git a/internal/notifiers/github_slack.go b/internal/notifiers/github_slack.go index 3ce6b24..dbe2cfb 100644 --- a/internal/notifiers/github_slack.go +++ b/internal/notifiers/github_slack.go @@ -116,6 +116,9 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if len(report.MembersSkippedExternal) > 0 { sectionText += fmt.Sprintf("\n*Skipped (External):* %d members", len(report.MembersSkippedExternal)) } + if len(report.MembersSkippedNoGHUsername) > 0 { + sectionText += fmt.Sprintf("\n*Skipped (No GitHub Username):* %d members", len(report.MembersSkippedNoGHUsername)) + } if len(report.Errors) > 0 { sectionText += fmt.Sprintf("\n*Errors:* %d", len(report.Errors)) } diff --git a/internal/okta/client.go b/internal/okta/client.go index 82e7279..78240c1 100644 --- a/internal/okta/client.go +++ b/internal/okta/client.go @@ -127,34 +127,47 @@ func (c *Client) GetGroupByName(name string) (*okta.Group, error) { return nil, errors.Newf("group '%s' not found", name) } +// GroupMembersResult contains the results of fetching group members. +type GroupMembersResult struct { + Members []string + SkippedNoGitHubUsername []string +} + // GetGroupMembers fetches GitHub usernames for all active members of an Okta // group. only includes users with status "ACTIVE" to exclude -// suspended/deprovisioned users. falls back to email if GitHub username field -// is not set. -func (c *Client) GetGroupMembers(groupID string) ([]string, error) { +// suspended/deprovisioned users. skips users without a GitHub username in +// their profile and tracks them separately. +func (c *Client) GetGroupMembers(groupID string) (*GroupMembersResult, error) { users, _, err := c.client.Group.ListGroupUsers(c.ctx, groupID, &query.Params{}) if err != nil { return nil, errors.Wrapf(err, "failed to list members for group '%s'", groupID) } - logins := make([]string, 0, len(users)) + result := &GroupMembersResult{ + Members: make([]string, 0, len(users)), + SkippedNoGitHubUsername: []string{}, + } + for _, user := range users { if user.Status != "ACTIVE" { continue } - if user.Profile != nil { - githubUsername := (*user.Profile)[c.githubUserField] - if username, ok := githubUsername.(string); ok && username != "" { - logins = append(logins, username) - } else { - email := (*user.Profile)["email"] - if emailStr, ok := email.(string); ok && emailStr != "" { - logins = append(logins, emailStr) - } + if user.Profile == nil { + continue + } + + githubUsername := (*user.Profile)[c.githubUserField] + if username, ok := githubUsername.(string); ok && username != "" { + result.Members = append(result.Members, username) + } else { + email := (*user.Profile)["email"] + if emailStr, ok := email.(string); ok && emailStr != "" { + result.SkippedNoGitHubUsername = append( + result.SkippedNoGitHubUsername, emailStr) } } } - return logins, nil + return result, nil } diff --git a/internal/okta/groups.go b/internal/okta/groups.go index 4c8034a..6396ec8 100644 --- a/internal/okta/groups.go +++ b/internal/okta/groups.go @@ -11,9 +11,10 @@ import ( // GroupInfo contains Okta group details and member list. type GroupInfo struct { - ID string - Name string - Members []string + ID string + Name string + Members []string + SkippedNoGitHubUsername []string } // GetGroupsByPattern fetches all Okta groups matching a regex pattern. @@ -39,15 +40,16 @@ func (c *Client) GetGroupsByPattern(pattern string) ([]*GroupInfo, error) { } if re.MatchString(group.Profile.Name) { - members, err := c.GetGroupMembers(group.Id) + result, err := c.GetGroupMembers(group.Id) if err != nil { continue } matched = append(matched, &GroupInfo{ - ID: group.Id, - Name: group.Profile.Name, - Members: members, + ID: group.Id, + Name: group.Profile.Name, + Members: result.Members, + SkippedNoGitHubUsername: result.SkippedNoGitHubUsername, }) } } @@ -62,15 +64,16 @@ func (c *Client) GetGroupInfo(groupName string) (*GroupInfo, error) { return nil, err } - members, err := c.GetGroupMembers(group.Id) + result, err := c.GetGroupMembers(group.Id) if err != nil { return nil, err } return &GroupInfo{ - ID: group.Id, - Name: group.Profile.Name, - Members: members, + ID: group.Id, + Name: group.Profile.Name, + Members: result.Members, + SkippedNoGitHubUsername: result.SkippedNoGitHubUsername, }, nil } diff --git a/internal/okta/sync.go b/internal/okta/sync.go index 1c35ec0..bca0ca0 100644 --- a/internal/okta/sync.go +++ b/internal/okta/sync.go @@ -29,13 +29,14 @@ type SyncRule struct { // SyncReport contains the results of syncing a single Okta group to GitHub // team. type SyncReport struct { - Rule string - OktaGroup string - GitHubTeam string - MembersAdded []string - MembersRemoved []string - MembersSkippedExternal []string - Errors []string + Rule string + OktaGroup string + GitHubTeam string + MembersAdded []string + MembersRemoved []string + MembersSkippedExternal []string + MembersSkippedNoGHUsername []string + Errors []string } // OrphanedUsersReport contains users who are org members but not in any synced @@ -215,10 +216,17 @@ func (s *Syncer) computeTeamName(oktaGroupName string, rule SyncRule) string { // creates team if missing and syncs members if enabled. func (s *Syncer) syncGroupToTeam(ctx context.Context, rule SyncRule, group *GroupInfo, teamName string) *SyncReport { report := &SyncReport{ - Rule: rule.Name, - OktaGroup: group.Name, - GitHubTeam: teamName, - Errors: []string{}, + Rule: rule.Name, + OktaGroup: group.Name, + GitHubTeam: teamName, + MembersSkippedNoGHUsername: group.SkippedNoGitHubUsername, + Errors: []string{}, + } + + if len(group.SkippedNoGitHubUsername) > 0 { + s.logger.Warn("okta users skipped due to missing github username", + slog.String("group", group.Name), + slog.Int("count", len(group.SkippedNoGitHubUsername))) } privacy := "closed"