synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-13 21:36:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Use membership endpoint to ensure user exists in team (#3129)

Browse Source 
This was using the incorrect GitHub endpoint prior, which fetched a team
by slug. Any user in a GitHub organization can view all teams, so this
didn't block signups like intended.

I've verified this API returns an error when the calling user is not a
member  of the team requested.

Fixes #3105.
This commit is contained in:
Kyle Carberry
2022-07-22 13:54:08 -05:00
committed by GitHub
parent 471564df7d
commit fd4954b4e5
3 changed files with 22 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cli/server.go
View File

@ -796,8 +796,8 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
}) })
return memberships, err return memberships, err
}, },
Team: func(ctx context.Context, client *http.Client, org, teamSlug string) (*github.Team, error) { TeamMembership: func(ctx context.Context, client *http.Client, org, teamSlug, username string) (*github.Membership, error) {
team, _, err := github.NewClient(client).Teams.GetTeamBySlug(ctx, org, teamSlug) team, _, err := github.NewClient(client).Teams.GetTeamMembershipBySlug(ctx, org, teamSlug, username)
return team, err return team, err
}, },
}, nil }, nil

23
coderd/userauth.go
View File

@ -29,7 +29,7 @@ type GithubOAuth2Config struct {
AuthenticatedUser func(ctx context.Context, client *http.Client) (*github.User, error) AuthenticatedUser func(ctx context.Context, client *http.Client) (*github.User, error)
ListEmails func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) ListEmails func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error)
ListOrganizationMemberships func(ctx context.Context, client *http.Client) ([]*github.Membership, error) ListOrganizationMemberships func(ctx context.Context, client *http.Client) ([]*github.Membership, error)
Team func(ctx context.Context, client *http.Client, org, team string) (*github.Team, error) TeamMembership func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error)
AllowSignups bool AllowSignups bool
AllowOrganizations []string AllowOrganizations []string
@ -72,9 +72,18 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
return return
} }
ghUser, err := api.GithubOAuth2Config.AuthenticatedUser(r.Context(), oauthClient)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching authenticated Github user.",
Detail: err.Error(),
})
return
}
// The default if no teams are specified is to allow all. // The default if no teams are specified is to allow all.
if len(api.GithubOAuth2Config.AllowTeams) > 0 { if len(api.GithubOAuth2Config.AllowTeams) > 0 {
var allowedTeam *github.Team var allowedTeam *github.Membership
for _, allowTeam := range api.GithubOAuth2Config.AllowTeams { for _, allowTeam := range api.GithubOAuth2Config.AllowTeams {
if allowTeam.Organization != *selectedMembership.Organization.Login { if allowTeam.Organization != *selectedMembership.Organization.Login {
// This needs to continue because multiple organizations // This needs to continue because multiple organizations
@ -82,7 +91,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
continue continue
} }
allowedTeam, err = api.GithubOAuth2Config.Team(r.Context(), oauthClient, allowTeam.Organization, allowTeam.Slug) allowedTeam, err = api.GithubOAuth2Config.TeamMembership(r.Context(), oauthClient, allowTeam.Organization, allowTeam.Slug, *ghUser.Login)
// The calling user may not have permission to the requested team! // The calling user may not have permission to the requested team!
if err != nil { if err != nil {
continue continue
@ -151,14 +160,6 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
// email to organization. // email to organization.
organizationID = organizations[0].ID organizationID = organizations[0].ID
} }
ghUser, err := api.GithubOAuth2Config.AuthenticatedUser(r.Context(), oauthClient)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching authenticated Github user.",
Detail: err.Error(),
})
return
}
var verifiedEmail *github.UserEmail var verifiedEmail *github.UserEmail
for _, email := range emails { for _, email := range emails {
if !email.GetPrimary() || !email.GetVerified() { if !email.GetPrimary() || !email.GetVerified() {

11
coderd/userauth_test.go
View File

@ -88,7 +88,12 @@ func TestUserOAuth2Github(t *testing.T) {
}, },
}}, nil }}, nil
}, },
Team: func(ctx context.Context, client *http.Client, org, team string) (*github.Team, error) { AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{
Login: github.String("kyle"),
}, nil
},
TeamMembership: func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error) {
return nil, xerrors.New("no perms") return nil, xerrors.New("no perms")
}, },
}, },
@ -222,8 +227,8 @@ func TestUserOAuth2Github(t *testing.T) {
}, },
}}, nil }}, nil
}, },
Team: func(ctx context.Context, client *http.Client, org, team string) (*github.Team, error) { TeamMembership: func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error) {
return &github.Team{}, nil return &github.Membership{}, nil
}, },
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
return &github.User{ return &github.User{