Skip to content

fix(github): optimize team membership check to prevent OAuth timeout for users in many teams #4632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vishwa5772 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(github): optimize team membership check to prevent OAuth timeout for users in many teams #4632

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 11 additions & 4 deletions connector/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,14 @@ func formatTeamName(org string, team string) string {
func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client, userName string) ([]string, error) {
groups := make([]string, 0)
var inOrgNoTeams bool

// Fetch all user teams once to avoid redundant API calls across multiple orgs.
// This prevents fetching the same team data repeatedly when iterating through orgs.
allTeamsByOrg, err := c.userOrgTeams(ctx, client)
if err != nil {
return nil, fmt.Errorf("github: get teams: %v", err)
}

for _, org := range c.orgs {
inOrg, err := c.userInOrg(ctx, client, userName, org.Name)
if err != nil {
Expand All @@ -352,10 +360,9 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client
continue
}

teams, err := c.teamsForOrg(ctx, client, org.Name)
if err != nil {
return nil, err
}
// Use cached teams from the single fetch above instead of fetching per-org
teams := allTeamsByOrg[org.Name]

// User is in at least one org. User is authorized if no teams are specified
// in config; include all teams in claim. Otherwise filter out teams not in
// 'teams' list in config.
Expand Down