Fix Github authentication

Author: mig5

pkg/cfg/oauth.go

@@ -250,13 +250,13 @@ func setDefaultsGitHub() {
 		GenOAuth.TokenURL = github.Endpoint.TokenURL
 	}
 	if GenOAuth.UserInfoURL == "" {
-		GenOAuth.UserInfoURL = "https://api.github.com/user?access_token="
+		GenOAuth.UserInfoURL = "https://api.github.com/user"
 	}
 	if GenOAuth.UserTeamURL == "" {
-		GenOAuth.UserTeamURL = "https://api.github.com/orgs/:org_id/teams/:team_slug/memberships/:username?access_token="
+		GenOAuth.UserTeamURL = "https://api.github.com/orgs/:org_id/teams/:team_slug/memberships/:username"
 	}
 	if GenOAuth.UserOrgURL == "" {
-		GenOAuth.UserOrgURL = "https://api.github.com/orgs/:org_id/members/:username?access_token="
+		GenOAuth.UserOrgURL = "https://api.github.com/orgs/:org_id/members/:username"
 	}
 	if len(GenOAuth.Scopes) == 0 {
 		// https://github.com/vouch/vouch-proxy/issues/63

pkg/providers/github/github.go

@@ -37,17 +37,13 @@ func (Provider) Configure() {
 }
 
 // GetUserInfo github user info, calls github api for org and teams
-// https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-authorization-options-for-oauth-apps/
 func (me Provider) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens, opts ...oauth2.AuthCodeOption) (rerr error) {
-	client, ptoken, err := me.PrepareTokensAndClient(r, ptokens, true)
+	client, _, err := me.PrepareTokensAndClient(r, ptokens, true, opts...)
 	if err != nil {
-		// http.Error(w, err.Error(), http.StatusBadRequest)
 		return err
 	}
-	log.Debugf("ptoken.AccessToken: %s", ptoken.AccessToken)
-	userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL + ptoken.AccessToken)
+	userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL)
 	if err != nil {
-		// http.Error(w, err.Error(), http.StatusBadRequest)
 		return err
 	}
 	defer func() {
@@ -99,9 +95,9 @@ func (me Provider) GetUserInfo(r *http.Request, user *structs.User, customClaims
 				var err error
 				isMember := false
 				if team != "" {
-					isMember, err = getTeamMembershipStateFromGitHub(client, user, org, team, ptoken)
+					isMember, err = getTeamMembershipStateFromGitHub(client, user, org, team)
 				} else {
-					isMember, err = getOrgMembershipStateFromGitHub(client, user, org, ptoken)
+					isMember, err = getOrgMembershipStateFromGitHub(client, user, org)
 				}
 				if err != nil {
 					return err
@@ -121,9 +117,9 @@ func (me Provider) GetUserInfo(r *http.Request, user *structs.User, customClaims
 	return nil
 }
 
-func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, orgID string, ptoken *oauth2.Token) (isMember bool, rerr error) {
+func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, orgID string) (isMember bool, rerr error) {
 	replacements := strings.NewReplacer(":org_id", orgID, ":username", user.Username)
-	orgMembershipResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserOrgURL) + ptoken.AccessToken)
+	orgMembershipResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserOrgURL))
 	if err != nil {
 		log.Error(err)
 		return false, err
@@ -149,9 +145,9 @@ func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, or
 	}
 }
 
-func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgID string, team string, ptoken *oauth2.Token) (isMember bool, rerr error) {
+func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgID string, team string) (isMember bool, rerr error) {
 	replacements := strings.NewReplacer(":org_id", orgID, ":team_slug", team, ":username", user.Username)
-	membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL) + ptoken.AccessToken)
+	membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL))
 	if err != nil {
 		log.Error(err)
 		return false, err

pkg/providers/github/github_test.go

@@ -105,7 +105,7 @@ func TestGetTeamMembershipStateFromGitHubActive(t *testing.T) {
 	setUp()
 	mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}"))
 
-	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token)
+	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1")
 
 	assert.Nil(t, err)
 	assert.True(t, isMember)
@@ -115,7 +115,7 @@ func TestGetTeamMembershipStateFromGitHubInactive(t *testing.T) {
 	setUp()
 	mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"inactive\"}"))
 
-	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token)
+	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1")
 
 	assert.Nil(t, err)
 	assert.False(t, isMember)
@@ -125,7 +125,7 @@ func TestGetTeamMembershipStateFromGitHubNotAMember(t *testing.T) {
 	setUp()
 	mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte(""))
 
-	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token)
+	isMember, err := getTeamMembershipStateFromGitHub(client, user, "org1", "team1")
 
 	assert.Nil(t, err)
 	assert.False(t, isMember)
@@ -135,12 +135,12 @@ func TestGetOrgMembershipStateFromGitHubNotFound(t *testing.T) {
 	setUp()
 	mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte(""))
 
-	isMember, err := getOrgMembershipStateFromGitHub(client, user, "myorg", token)
+	isMember, err := getOrgMembershipStateFromGitHub(client, user, "myorg")
 
 	assert.Nil(t, err)
 	assert.False(t, isMember)
 
-	expectedOrgMembershipURL := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken
+	expectedOrgMembershipURL := "https://api.github.com/orgs/myorg/members/" + user.Username
 	assertURLCalled(t, expectedOrgMembershipURL)
 }
 
@@ -151,12 +151,12 @@ func TestGetOrgMembershipStateFromGitHubNoOrgAccess(t *testing.T) {
 	mockResponse(regexMatcher(".*orgs/myorg/members.*"), http.StatusFound, map[string]string{"Location": location}, []byte(""))
 	mockResponse(regexMatcher(".*orgs/myorg/public_members.*"), http.StatusNoContent, map[string]string{}, []byte(""))
 
-	isMember, err := getOrgMembershipStateFromGitHub(client, user, "myorg", token)
+	isMember, err := getOrgMembershipStateFromGitHub(client, user, "myorg")
 
 	assert.Nil(t, err)
 	assert.True(t, isMember)
 
-	expectedOrgMembershipURL := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken
+	expectedOrgMembershipURL := "https://api.github.com/orgs/myorg/members/" + user.Username
 	assertURLCalled(t, expectedOrgMembershipURL)
 
 	expectedOrgPublicMembershipURL := "https://api.github.com/orgs/myorg/public_members/" + user.Username
@@ -178,7 +178,7 @@ func TestGetUserInfo(t *testing.T) {
 		Login:   "myusername",
 		Picture: "avatar-url",
 	})
-	mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent)
+	mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL), http.StatusOK, map[string]string{}, userInfoContent)
 
 	cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myOtherOrg", "myorg/myteam")
 
@@ -194,6 +194,6 @@ func TestGetUserInfo(t *testing.T) {
 	assert.Equal(t, "myusername", user.Username)
 	assert.Equal(t, []string{"myOtherOrg", "myorg/myteam"}, user.TeamMemberships)
 
-	expectedTeamMembershipURL := "https://api.github.com/orgs/myorg/teams/myteam/memberships/myusername?access_token=" + token.AccessToken
+	expectedTeamMembershipURL := "https://api.github.com/orgs/myorg/teams/myteam/memberships/myusername"
 	assertURLCalled(t, expectedTeamMembershipURL)
 }