Skip to content

Commit 0effbec

Browse files
sgtojsgtoj
committed
feat: handle okta users missing github username value
1 parent 2556999 commit 0effbec

File tree

5 files changed

+159
-36
lines changed

5 files changed

+159
-36
lines changed

‎fixtures/scenarios.json‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,5 +1054,101 @@
10541054
"description": "send sync report notification to slack (orphaned user check skipped)"
10551055
}
10561056
]
1057+
},
1058+
{
1059+
"name": "okta_sync_users_without_github_username",
1060+
"description": "Test that Okta users without GitHub username are skipped but tracked in sync report",
1061+
"event_type": "scheduled_event",
1062+
"event_payload": {
1063+
"action": "okta-sync"
1064+
},
1065+
"expected_calls": [
1066+
{
1067+
"service": "okta",
1068+
"method": "GET",
1069+
"path": "/api/v1/groups"
1070+
},
1071+
{
1072+
"service": "github",
1073+
"method": "GET",
1074+
"path": "/orgs/*/teams/*"
1075+
},
1076+
{
1077+
"service": "github",
1078+
"method": "PUT",
1079+
"path": "/orgs/acme-ghorg/teams/engineering/memberships/alice-gh"
1080+
},
1081+
{
1082+
"service": "slack",
1083+
"method": "POST",
1084+
"path": "/chat.postMessage"
1085+
}
1086+
],
1087+
"mock_responses": [
1088+
{
1089+
"service": "github",
1090+
"method": "POST",
1091+
"path": "/app/installations/987654/access_tokens",
1092+
"status_code": 201,
1093+
"body": "{\"token\":\"ghs_mock_installation_token\",\"expires_at\":\"2099-12-31T23:59:59Z\"}",
1094+
"description": "github app installation token authentication"
1095+
},
1096+
{
1097+
"service": "github",
1098+
"method": "GET",
1099+
"path": "/orgs/acme-ghorg/teams/engineering",
1100+
"status_code": 200,
1101+
"body": "{\"id\":1,\"name\":\"Engineering\",\"slug\":\"engineering\",\"description\":\"Engineering team\"}",
1102+
"description": "fetch engineering team details"
1103+
},
1104+
{
1105+
"service": "github",
1106+
"method": "GET",
1107+
"path": "/orgs/acme-ghorg/teams/engineering/members",
1108+
"status_code": 200,
1109+
"body": "[]",
1110+
"description": "engineering team has no existing members"
1111+
},
1112+
{
1113+
"service": "github",
1114+
"method": "PUT",
1115+
"path": "/orgs/acme-ghorg/teams/engineering/memberships/alice-gh",
1116+
"status_code": 200,
1117+
"body": "{\"state\":\"active\",\"role\":\"member\"}",
1118+
"description": "add alice-gh to team (only user with github username)"
1119+
},
1120+
{
1121+
"service": "okta",
1122+
"method": "POST",
1123+
"path": "/oauth2/v1/token",
1124+
"status_code": 200,
1125+
"body": "{\"token_type\":\"Bearer\",\"expires_in\":3600,\"access_token\":\"mock-access-token\",\"scope\":\"okta.groups.read okta.users.read\"}",
1126+
"description": "okta oauth 2.0 token authentication"
1127+
},
1128+
{
1129+
"service": "okta",
1130+
"method": "GET",
1131+
"path": "/api/v1/groups",
1132+
"status_code": 200,
1133+
"body": "[{\"id\":\"00g1234567890abcdef\",\"profile\":{\"name\":\"Engineering\",\"description\":\"Engineering team\"}}]",
1134+
"description": "fetch all okta groups (returns engineering group)"
1135+
},
1136+
{
1137+
"service": "okta",
1138+
"method": "GET",
1139+
"path": "/api/v1/groups/00g1234567890abcdef/users",
1140+
"status_code": 200,
1141+
"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\":\"\"}}]",
1142+
"description": "fetch users in engineering group (alice has gh username, bob and charlie do not)"
1143+
},
1144+
{
1145+
"service": "slack",
1146+
"method": "POST",
1147+
"path": "/chat.postMessage",
1148+
"status_code": 200,
1149+
"body": "{\"ok\":true,\"channel\":\"C01234TEST\",\"ts\":\"1234567890.123456\"}",
1150+
"description": "send sync report to slack (should include skipped users count)"
1151+
}
1152+
]
10571153
}
10581154
]

‎internal/notifiers/github_slack.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync
116116
if len(report.MembersSkippedExternal) > 0 {
117117
sectionText += fmt.Sprintf("\n*Skipped (External):* %d members", len(report.MembersSkippedExternal))
118118
}
119+
if len(report.MembersSkippedNoGHUsername) > 0 {
120+
sectionText += fmt.Sprintf("\n*Skipped (No GitHub Username):* %d members", len(report.MembersSkippedNoGHUsername))
121+
}
119122
if len(report.Errors) > 0 {
120123
sectionText += fmt.Sprintf("\n*Errors:* %d", len(report.Errors))
121124
}

‎internal/okta/client.go‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,34 +127,47 @@ func (c *Client) GetGroupByName(name string) (*okta.Group, error) {
127127
return nil, errors.Newf("group '%s' not found", name)
128128
}
129129

130+
// GroupMembersResult contains the results of fetching group members.
131+
type GroupMembersResult struct {
132+
Members []string
133+
SkippedNoGitHubUsername []string
134+
}
135+
130136
// GetGroupMembers fetches GitHub usernames for all active members of an Okta
131137
// group. only includes users with status "ACTIVE" to exclude
132-
// suspended/deprovisioned users. falls back to email if GitHub username field
133-
// is not set.
134-
func (c *Client) GetGroupMembers(groupID string) ([]string, error) {
138+
// suspended/deprovisioned users. skips users without a GitHub username in
139+
// their profile and tracks them separately.
140+
func (c *Client) GetGroupMembers(groupID string) (*GroupMembersResult, error) {
135141
users, _, err := c.client.Group.ListGroupUsers(c.ctx, groupID, &query.Params{})
136142
if err != nil {
137143
return nil, errors.Wrapf(err, "failed to list members for group '%s'", groupID)
138144
}
139145

140-
logins := make([]string, 0, len(users))
146+
result := &GroupMembersResult{
147+
Members: make([]string, 0, len(users)),
148+
SkippedNoGitHubUsername: []string{},
149+
}
150+
141151
for _, user := range users {
142152
if user.Status != "ACTIVE" {
143153
continue
144154
}
145155

146-
if user.Profile != nil {
147-
githubUsername := (*user.Profile)[c.githubUserField]
148-
if username, ok := githubUsername.(string); ok && username != "" {
149-
logins = append(logins, username)
150-
} else {
151-
email := (*user.Profile)["email"]
152-
if emailStr, ok := email.(string); ok && emailStr != "" {
153-
logins = append(logins, emailStr)
154-
}
156+
if user.Profile == nil {
157+
continue
158+
}
159+
160+
githubUsername := (*user.Profile)[c.githubUserField]
161+
if username, ok := githubUsername.(string); ok && username != "" {
162+
result.Members = append(result.Members, username)
163+
} else {
164+
email := (*user.Profile)["email"]
165+
if emailStr, ok := email.(string); ok && emailStr != "" {
166+
result.SkippedNoGitHubUsername = append(
167+
result.SkippedNoGitHubUsername, emailStr)
155168
}
156169
}
157170
}
158171

159-
return logins, nil
172+
return result, nil
160173
}

‎internal/okta/groups.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ import (
1111

1212
// GroupInfo contains Okta group details and member list.
1313
type GroupInfo struct {
14-
ID string
15-
Name string
16-
Members []string
14+
ID string
15+
Name string
16+
Members []string
17+
SkippedNoGitHubUsername []string
1718
}
1819

1920
// GetGroupsByPattern fetches all Okta groups matching a regex pattern.
@@ -39,15 +40,16 @@ func (c *Client) GetGroupsByPattern(pattern string) ([]*GroupInfo, error) {
3940
}
4041

4142
if re.MatchString(group.Profile.Name) {
42-
members, err := c.GetGroupMembers(group.Id)
43+
result, err := c.GetGroupMembers(group.Id)
4344
if err != nil {
4445
continue
4546
}
4647

4748
matched = append(matched, &GroupInfo{
48-
ID: group.Id,
49-
Name: group.Profile.Name,
50-
Members: members,
49+
ID: group.Id,
50+
Name: group.Profile.Name,
51+
Members: result.Members,
52+
SkippedNoGitHubUsername: result.SkippedNoGitHubUsername,
5153
})
5254
}
5355
}
@@ -62,15 +64,16 @@ func (c *Client) GetGroupInfo(groupName string) (*GroupInfo, error) {
6264
return nil, err
6365
}
6466

65-
members, err := c.GetGroupMembers(group.Id)
67+
result, err := c.GetGroupMembers(group.Id)
6668
if err != nil {
6769
return nil, err
6870
}
6971

7072
return &GroupInfo{
71-
ID: group.Id,
72-
Name: group.Profile.Name,
73-
Members: members,
73+
ID: group.Id,
74+
Name: group.Profile.Name,
75+
Members: result.Members,
76+
SkippedNoGitHubUsername: result.SkippedNoGitHubUsername,
7477
}, nil
7578
}
7679

‎internal/okta/sync.go‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@ type SyncRule struct {
2929
// SyncReport contains the results of syncing a single Okta group to GitHub
3030
// team.
3131
type SyncReport struct {
32-
Rule string
33-
OktaGroup string
34-
GitHubTeam string
35-
MembersAdded []string
36-
MembersRemoved []string
37-
MembersSkippedExternal []string
38-
Errors []string
32+
Rule string
33+
OktaGroup string
34+
GitHubTeam string
35+
MembersAdded []string
36+
MembersRemoved []string
37+
MembersSkippedExternal []string
38+
MembersSkippedNoGHUsername []string
39+
Errors []string
3940
}
4041

4142
// 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 {
215216
// creates team if missing and syncs members if enabled.
216217
func (s *Syncer) syncGroupToTeam(ctx context.Context, rule SyncRule, group *GroupInfo, teamName string) *SyncReport {
217218
report := &SyncReport{
218-
Rule: rule.Name,
219-
OktaGroup: group.Name,
220-
GitHubTeam: teamName,
221-
Errors: []string{},
219+
Rule: rule.Name,
220+
OktaGroup: group.Name,
221+
GitHubTeam: teamName,
222+
MembersSkippedNoGHUsername: group.SkippedNoGitHubUsername,
223+
Errors: []string{},
224+
}
225+
226+
if len(group.SkippedNoGitHubUsername) > 0 {
227+
s.logger.Warn("okta users skipped due to missing github username",
228+
slog.String("group", group.Name),
229+
slog.Int("count", len(group.SkippedNoGitHubUsername)))
222230
}
223231

224232
privacy := "closed"

0 commit comments

Comments
 (0)