Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions fixtures/scenarios.json
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
}
]
}
]
3 changes: 3 additions & 0 deletions internal/notifiers/github_slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
41 changes: 27 additions & 14 deletions internal/okta/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
25 changes: 14 additions & 11 deletions internal/okta/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
})
}
}
Expand All @@ -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
}

Expand Down
30 changes: 19 additions & 11 deletions internal/okta/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down