Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8b76e40

Browse files
authored
fix: Fetch GitHub teams by name for performance (#2955)
In large organizations with thousands of teams, looping took >5s.This fetches organizations by team name, which should be very fast!
1 parent7e9819f commit8b76e40

File tree

4 files changed

+20
-55
lines changed

4 files changed

+20
-55
lines changed

‎cli/server.go

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -770,24 +770,9 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
770770
})
771771
returnmemberships,err
772772
},
773-
ListTeams:func(ctx context.Context,client*http.Client,orgstring) ([]*github.Team,error) {
774-
opt:=&github.ListOptions{
775-
// This is the maximum amount per-page that GitHub allows.
776-
PerPage:100,
777-
}
778-
varallTeams []*github.Team
779-
for {
780-
teams,resp,err:=github.NewClient(client).Teams.ListTeams(ctx,org,opt)
781-
iferr!=nil {
782-
returnnil,err
783-
}
784-
allTeams=append(allTeams,teams...)
785-
ifresp.NextPage==0 {
786-
break
787-
}
788-
opt.Page=resp.NextPage
789-
}
790-
returnallTeams,nil
773+
Team:func(ctx context.Context,client*http.Client,org,teamSlugstring) (*github.Team,error) {
774+
team,_,err:=github.NewClient(client).Teams.GetTeamBySlug(ctx,org,teamSlug)
775+
returnteam,err
791776
},
792777
},nil
793778
}

‎coderd/userauth.go

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type GithubOAuth2Config struct {
2929
AuthenticatedUserfunc(ctx context.Context,client*http.Client) (*github.User,error)
3030
ListEmailsfunc(ctx context.Context,client*http.Client) ([]*github.UserEmail,error)
3131
ListOrganizationMembershipsfunc(ctx context.Context,client*http.Client) ([]*github.Membership,error)
32-
ListTeamsfunc(ctx context.Context,client*http.Client,orgstring) ([]*github.Team,error)
32+
Teamfunc(ctx context.Context,client*http.Client,org,teamstring) (*github.Team,error)
3333

3434
AllowSignupsbool
3535
AllowOrganizations []string
@@ -74,31 +74,20 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
7474

7575
// The default if no teams are specified is to allow all.
7676
iflen(api.GithubOAuth2Config.AllowTeams)>0 {
77-
teams,err:=api.GithubOAuth2Config.ListTeams(r.Context(),oauthClient,*selectedMembership.Organization.Login)
78-
iferr!=nil {
79-
httpapi.Write(rw,http.StatusInternalServerError, codersdk.Response{
80-
Message:"Failed to fetch teams from GitHub.",
81-
Detail:err.Error(),
82-
})
83-
return
84-
}
85-
8677
varallowedTeam*github.Team
87-
for_,team:=rangeteams {
88-
for_,allowTeam:=rangeapi.GithubOAuth2Config.AllowTeams {
89-
ifallowTeam.Organization!=*selectedMembership.Organization.Login {
90-
// This needs to continue because multiple organizations
91-
// could exist in the allow/team listings.
92-
continue
93-
}
94-
ifallowTeam.Slug!=*team.Slug {
95-
continue
96-
}
97-
allowedTeam=team
98-
break
78+
for_,allowTeam:=rangeapi.GithubOAuth2Config.AllowTeams {
79+
ifallowTeam.Organization!=*selectedMembership.Organization.Login {
80+
// This needs to continue because multiple organizations
81+
// could exist in the allow/team listings.
82+
continue
9983
}
100-
}
10184

85+
allowedTeam,err=api.GithubOAuth2Config.Team(r.Context(),oauthClient,allowTeam.Organization,allowTeam.Slug)
86+
// The calling user may not have permission to the requested team!
87+
iferr!=nil {
88+
continue
89+
}
90+
}
10291
ifallowedTeam==nil {
10392
httpapi.Write(rw,http.StatusUnauthorized, codersdk.Response{
10493
Message:fmt.Sprintf("You aren't a member of an authorized team in the %s Github organization!",*selectedMembership.Organization.Login),

‎coderd/userauth_test.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/google/go-github/v43/github"
1010
"github.com/stretchr/testify/require"
1111
"golang.org/x/oauth2"
12+
"golang.org/x/xerrors"
1213

1314
"github.com/coder/coder/coderd"
1415
"github.com/coder/coder/coderd/coderdtest"
@@ -87,10 +88,8 @@ func TestUserOAuth2Github(t *testing.T) {
8788
},
8889
}},nil
8990
},
90-
ListTeams:func(ctx context.Context,client*http.Client,orgstring) ([]*github.Team,error) {
91-
return []*github.Team{{
92-
Slug:github.String("nope"),
93-
}},nil
91+
Team:func(ctx context.Context,client*http.Client,org,teamstring) (*github.Team,error) {
92+
returnnil,xerrors.New("no perms")
9493
},
9594
},
9695
})
@@ -223,10 +222,8 @@ func TestUserOAuth2Github(t *testing.T) {
223222
},
224223
}},nil
225224
},
226-
ListTeams:func(ctx context.Context,client*http.Client,orgstring) ([]*github.Team,error) {
227-
return []*github.Team{{
228-
Slug:github.String("frontend"),
229-
}},nil
225+
Team:func(ctx context.Context,client*http.Client,org,teamstring) (*github.Team,error) {
226+
return&github.Team{},nil
230227
},
231228
AuthenticatedUser:func(ctx context.Context,client*http.Client) (*github.User,error) {
232229
return&github.User{

‎go.sum

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1880,8 +1880,6 @@ github.com/zclconf/go-cty v1.10.0 h1:mp9ZXQeIcN8kAwuqorjH+Q+njbJKjLrvB2yIh4q7U+0
18801880
github.com/zclconf/go-ctyv1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk=
18811881
github.com/zclconf/go-cty-debugv0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8=
18821882
github.com/zeebo/assertv1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
1883-
github.com/zeebo/errsv1.2.2 h1:5NFypMTuSdoySVTqlNs1dEoU21QVamMQJxW/Fii5O7g=
1884-
github.com/zeebo/errsv1.2.2/go.mod h1:sgbWHsvVuTPHcqJJGQ1WhI5KbWlHYz+2+2C/LSEtCw4=
18851883
github.com/zeebo/errsv1.3.0 h1:hmiaKqgYZzcVgRL1Vkc1Mn2914BbzB0IBxs+ebeutGs=
18861884
github.com/zeebo/errsv1.3.0/go.mod h1:sgbWHsvVuTPHcqJJGQ1WhI5KbWlHYz+2+2C/LSEtCw4=
18871885
github.com/zenazn/gojiv0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
@@ -2912,10 +2910,6 @@ sigs.k8s.io/structured-merge-diff/v4 v4.1.2/go.mod h1:j/nl6xW8vLS49O8YvXW1ocPhZa
29122910
sigs.k8s.io/yamlv1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
29132911
sigs.k8s.io/yamlv1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
29142912
software.sslmate.com/src/go-pkcs12v0.0.0-20210415151418-c5206de65a78 h1:SqYE5+A2qvRhErbsXFfUEUmpWEKxxRSMgGLkvRAFOV4=
2915-
storj.io/drpcv0.0.30 h1:jqPe4T9KEu3CDBI05A2hCMgMSHLtd/E0N0yTF9QreIE=
2916-
storj.io/drpcv0.0.30/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg=
2917-
storj.io/drpcv0.0.32 h1:5p5ZwsK/VOgapaCu+oxaPVwO6UwIs+iwdMiD50+R4PI=
2918-
storj.io/drpcv0.0.32/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg=
29192913
storj.io/drpcv0.0.33-0.20220622181519-9206537a4db7 h1:6jIp39oQGZMjfrG3kiafK2tcL0Fbprh2kvaoJNfhvuM=
29202914
storj.io/drpcv0.0.33-0.20220622181519-9206537a4db7/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg=
29212915
tailscale.comv1.26.2 h1:EBR0DXblI2Rx3mPe/YU29oZbQLnC8BtJYUTufmEygUY=

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp