From 5507bd6377bba0cd9bfa0a2a58589f25939ac8ab Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Thu, 23 Jan 2020 08:49:33 +0100 Subject: [PATCH 01/21] Check GitHub user team memberships and store them in User struct --- handlers/handlers.go | 44 ++++++++- handlers/handlers_test.go | 188 ++++++++++++++++++++++++++++++++++++++ pkg/cfg/cfg.go | 51 ++++++++--- pkg/structs/structs.go | 6 ++ 4 files changed, 274 insertions(+), 15 deletions(-) create mode 100644 handlers/handlers_test.go diff --git a/handlers/handlers.go b/handlers/handlers.go index c065a026..afdd64e2 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -538,7 +538,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust return getUserInfoFromOpenStax(client, user, customClaims, providerToken) } - if (providerToken.Extra("id_token") != nil) { + if providerToken.Extra("id_token") != nil { // Certain providers (eg. gitea) don't provide an id_token // and it's not neccessary for the authentication phase ptokens.PIdToken = providerToken.Extra("id_token").(string) @@ -677,13 +677,55 @@ func getUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims user.Name = ghUser.Name user.Username = ghUser.Username user.ID = ghUser.ID + // user = &ghUser.User + if cfg.Cfg.Org != "" && cfg.Cfg.TeamWhiteList != nil { + for _, team := range cfg.Cfg.TeamWhiteList { + e, isMember := getTeamMembershipStateFromGitHub(client, user, cfg.Cfg.Org, team, ptoken) + if e != nil { + return e + } else { + if isMember { + user.TeamMemberships = append(user.TeamMemberships, team) + } + } + } + } + log.Debug("getUserInfoFromGitHub") log.Debug(user) return nil } +func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, team string, ptoken *oauth2.Token) (rerr error, isMember bool) { + replacements := strings.NewReplacer(":org_id", orgId, ":team_slug", team, ":username", user.Username) + membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL) + ptoken.AccessToken) + if err != nil { + log.Error(err) + return err, false + } + defer func() { + if err := membershipStateResp.Body.Close(); err != nil { + rerr = err + } + }() + if membershipStateResp.StatusCode == 200 { + data, _ := ioutil.ReadAll(membershipStateResp.Body) + log.Infof("github team membership body: ", string(data)) + ghTeamState := structs.GitHubTeamMembershipState{} + if err = json.Unmarshal(data, &ghTeamState); err != nil { + log.Error(err) + return err, false + } + log.Debug("getTeamMembershipStateFromGitHub ghTeamState") + log.Debug(ghTeamState) + return nil, ghTeamState.State == "active" + } else { + return nil, false + } +} + func getUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { code := r.URL.Query().Get("code") diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go new file mode 100644 index 00000000..1091d283 --- /dev/null +++ b/handlers/handlers_test.go @@ -0,0 +1,188 @@ +package handlers + +import ( + "encoding/json" + "golang.org/x/oauth2" + "net/http" + "regexp" + + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/domains" + "github.com/vouch/vouch-proxy/pkg/structs" + + mockhttp "github.com/karupanerura/go-mock-http-response" + "github.com/stretchr/testify/assert" + "testing" +) + +type ReqMatcher func(*http.Request) bool + +type FunResponsePair struct { + matcher ReqMatcher + response *mockhttp.ResponseMock +} + +type Transport struct { + MockError error +} + +func (c *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + if c.MockError != nil { + return nil, c.MockError + } + for _, p := range mockedResponses { + if p.matcher(req) { + return p.response.MakeResponse(req), nil + } + } + return nil, nil +} + +func mockResponse(fun ReqMatcher, statusCode int, headers map[string]string, body []byte) { + mockedResponses = append(mockedResponses, FunResponsePair{matcher: fun, response: mockhttp.NewResponseMock(statusCode, headers, body)}) +} + +func regexMatcher(expr string) ReqMatcher { + return func(r *http.Request) bool { + matches, _ := regexp.Match(expr, []byte(r.URL.String())) + return matches + } +} + +func urlEquals(value string) ReqMatcher { + return func(r *http.Request) bool { + return r.URL.String() == value + } +} + +var ( + user *structs.User + token = &oauth2.Token{AccessToken: "123"} + mockedResponses = []FunResponsePair{} + client = &http.Client{Transport: &Transport{}} +) + +func init() { + setUp() +} + +func setUp() { + cfg.InitForTestPurposesWithProvider("github") + + cfg.Cfg.AllowAllUsers = false + cfg.Cfg.WhiteList = make([]string, 0) + cfg.Cfg.Domains = []string{"domain1"} + + domains.Refresh() + + mockedResponses = []FunResponsePair{} + + user = &structs.User{Username: "testuser", Email: "test@example.com"} +} + +func TestVerifyUserPositiveUserInWhiteList(t *testing.T) { + setUp() + cfg.Cfg.WhiteList = append(cfg.Cfg.WhiteList, user.Username) + + ok, err := VerifyUser(*user) + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserPositiveAllowAllUsers(t *testing.T) { + setUp() + cfg.Cfg.AllowAllUsers = true + + ok, err := VerifyUser(*user) + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserPositiveByEmail(t *testing.T) { + setUp() + cfg.Cfg.Domains = append(cfg.Cfg.Domains, "example.com") + domains.Refresh() + + ok, err := VerifyUser(*user) + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserPositiveNoDomainsConfigured(t *testing.T) { + setUp() + cfg.Cfg.Domains = make([]string, 0) + + ok, err := VerifyUser(*user) + + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserNegative(t *testing.T) { + setUp() + + ok, err := VerifyUser(*user) + + assert.False(t, ok) + assert.NotNil(t, err) +} + +func TestGetTeamMembershipStateFromGitHubActive(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.True(t, isMember) +} + +func TestGetTeamMembershipStateFromGitHubInactive(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"inactive\"}")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.False(t, isMember) +} + +func TestGetTeamMembershipStateFromGitHubNotAMember(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.False(t, isMember) +} + +func TestGetUserInfoFromGitHub(t *testing.T) { + setUp() + + userInfoContent, _ := json.Marshal(structs.GitHubUser{ + User: structs.User{ + Username: "test", + CreatedOn: 123, + Email: "email@example.com", + ID: 1, + LastUpdate: 123, + Name: "name", + }, + Login: "login", + Picture: "avatar-url", + }) + mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) + + cfg.Cfg.Org = "myorg" + + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myteam") + + mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + + err := getUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) + + assert.Nil(t, err) + assert.Equal(t, "login", user.Username) + assert.Equal(t, []string{"myteam"}, user.TeamMemberships) +} diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index b3f5929a..4745a3af 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "strconv" "strings" @@ -33,6 +34,8 @@ type config struct { HealthCheck bool `mapstructure:"healthCheck"` Domains []string `mapstructure:"domains"` WhiteList []string `mapstructure:"whitelist"` + Org string `mapstructure:"org"` + TeamWhiteList []string `mapstructure:"teamWhitelist"` AllowAllUsers bool `mapstructure:"allowAllUsers"` PublicAccess bool `mapstructure:"publicAccess"` JWT struct { @@ -84,6 +87,7 @@ type oauthConfig struct { RedirectURLs []string `mapstructure:"callback_urls"` Scopes []string `mapstructure:"scopes"` UserInfoURL string `mapstructure:"user_info_url"` + UserTeamURL string `mapstructure:"user_team_url"` PreferredDomain string `mapstructre:"preferredDomain"` } @@ -275,7 +279,13 @@ func setDevelopmentLogger() { // InitForTestPurposes is called by most *_testing.go files in Vouch Proxy func InitForTestPurposes() { - if err := os.Setenv(Branding.UCName+"_CONFIG", "../../config/test_config.yml"); err != nil { + InitForTestPurposesWithProvider("") +} + +func InitForTestPurposesWithProvider(provider string) { + _, b, _, _ := runtime.Caller(0) + basepath := filepath.Dir(b) + if err := os.Setenv(Branding.UCName+"_CONFIG", filepath.Join(basepath, "../../config/test_config.yml")); err != nil { log.Error(err) } // log.Debug("opening config") @@ -283,6 +293,12 @@ func InitForTestPurposes() { ParseConfig() SetDefaults() + // Needed to override the provider, which is otherwise set via yml + if provider != "" { + GenOAuth.Provider = provider + setProviderDefaults() + } + } // ParseConfig parse the config file @@ -563,19 +579,23 @@ func SetDefaults() { // OAuth defaults and client configuration err := UnmarshalKey("oauth", &GenOAuth) if err == nil { - if GenOAuth.Provider == Providers.Google { - setDefaultsGoogle() - // setDefaultsGoogle also configures the OAuthClient - } else if GenOAuth.Provider == Providers.GitHub { - setDefaultsGitHub() - configureOAuthClient() - } else if GenOAuth.Provider == Providers.ADFS { - setDefaultsADFS() - configureOAuthClient() - } else { - // IndieAuth, OIDC, OpenStax - configureOAuthClient() - } + setProviderDefaults() + } +} + +func setProviderDefaults() { + if GenOAuth.Provider == Providers.Google { + setDefaultsGoogle() + // setDefaultsGoogle also configures the OAuthClient + } else if GenOAuth.Provider == Providers.GitHub { + setDefaultsGitHub() + configureOAuthClient() + } else if GenOAuth.Provider == Providers.ADFS { + setDefaultsADFS() + configureOAuthClient() + } else { + // IndieAuth, OIDC, OpenStax + configureOAuthClient() } } @@ -615,6 +635,9 @@ func setDefaultsGitHub() { if GenOAuth.UserInfoURL == "" { GenOAuth.UserInfoURL = "https://api.github.com/user?access_token=" } + if GenOAuth.UserTeamURL == "" { + GenOAuth.UserTeamURL = "https://api.github.com/orgs/:org_id/teams/:team_slug/memberships/:username?access_token=" + } if len(GenOAuth.Scopes) == 0 { // https://github.com/vouch/vouch-proxy/issues/63 // https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ diff --git a/pkg/structs/structs.go b/pkg/structs/structs.go index cc564789..13a5ffbb 100644 --- a/pkg/structs/structs.go +++ b/pkg/structs/structs.go @@ -23,6 +23,8 @@ type User struct { // don't populate ID from json https://github.com/vouch/vouch-proxy/issues/185 ID int `json:"-" mapstructure:"id"` // jwt.StandardClaims + + TeamMemberships []string } // PrepareUserData implement PersonalData interface @@ -79,6 +81,10 @@ type GitHubUser struct { // jwt.StandardClaims } +type GitHubTeamMembershipState struct { + State string `json:"state"` +} + // PrepareUserData implement PersonalData interface func (u *GitHubUser) PrepareUserData() { // always use the u.Login as the u.Username From 7b0f0a3f180b7a3d897f2256c2f1c253c2295c75 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 09:27:39 +0100 Subject: [PATCH 02/21] Evaluate team memberships when verifying user --- handlers/handlers.go | 14 ++++++++++++++ handlers/handlers_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/handlers/handlers.go b/handlers/handlers.go index afdd64e2..6a0e0a7f 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -423,6 +423,20 @@ func VerifyUser(u interface{}) (ok bool, err error) { if !ok { err = fmt.Errorf("user.Username not found in WhiteList: %s", user.Username) } + } else if len(cfg.Cfg.TeamWhiteList) != 0 && cfg.Cfg.Org != "" { + for _, team := range user.TeamMemberships { + for _, wl := range cfg.Cfg.TeamWhiteList { + if team == wl { + log.Debugf("found user.TeamWhiteList in TeamWhiteList: %s for user %s", wl, user.Username) + ok = true + break + } + } + } + + if !ok { + err = fmt.Errorf("user.TeamMemberships %s not found in TeamWhiteList: %s for user %s", user.TeamMemberships, cfg.Cfg.TeamWhiteList, user.Username) + } } else if len(cfg.Cfg.Domains) != 0 && !domains.IsUnderManagement(user.Email) { err = fmt.Errorf("Email %s is not within a "+cfg.Branding.CcName+" managed domain", user.Email) // } else if !domains.IsUnderManagement(user.HostDomain) { diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 1091d283..0be1deb7 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -71,6 +71,8 @@ func setUp() { cfg.Cfg.AllowAllUsers = false cfg.Cfg.WhiteList = make([]string, 0) + cfg.Cfg.Org = "" + cfg.Cfg.TeamWhiteList = make([]string, 0) cfg.Cfg.Domains = []string{"domain1"} domains.Refresh() @@ -108,6 +110,28 @@ func TestVerifyUserPositiveByEmail(t *testing.T) { assert.Nil(t, err) } +func TestVerifyUserPositiveByTeam(t *testing.T) { + setUp() + cfg.Cfg.Org = "testorg" + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "team2", "team1") + + user.TeamMemberships = append(user.TeamMemberships, "team3") + user.TeamMemberships = append(user.TeamMemberships, "team1") + ok, err := VerifyUser(*user) + assert.True(t, ok) + assert.Nil(t, err) +} + +func TestVerifyUserNegativeByTeam(t *testing.T) { + setUp() + cfg.Cfg.Org = "testorg" + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "team1") + + ok, err := VerifyUser(*user) + assert.False(t, ok) + assert.NotNil(t, err) +} + func TestVerifyUserPositiveNoDomainsConfigured(t *testing.T) { setUp() cfg.Cfg.Domains = make([]string, 0) From 0b99ff8dbe2db46448db3c89f339e0cef052e790 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 09:37:43 +0100 Subject: [PATCH 03/21] Remover Org from config and use / as format for github team whitelist --- handlers/handlers.go | 32 +++++++++++++++++++++++--------- handlers/handlers_test.go | 17 ++++++----------- pkg/cfg/cfg.go | 1 - 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/handlers/handlers.go b/handlers/handlers.go index 6a0e0a7f..d1655982 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -423,7 +423,7 @@ func VerifyUser(u interface{}) (ok bool, err error) { if !ok { err = fmt.Errorf("user.Username not found in WhiteList: %s", user.Username) } - } else if len(cfg.Cfg.TeamWhiteList) != 0 && cfg.Cfg.Org != "" { + } else if len(cfg.Cfg.TeamWhiteList) != 0 { for _, team := range user.TeamMemberships { for _, wl := range cfg.Cfg.TeamWhiteList { if team == wl { @@ -694,15 +694,29 @@ func getUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims // user = &ghUser.User - if cfg.Cfg.Org != "" && cfg.Cfg.TeamWhiteList != nil { - for _, team := range cfg.Cfg.TeamWhiteList { - e, isMember := getTeamMembershipStateFromGitHub(client, user, cfg.Cfg.Org, team, ptoken) - if e != nil { - return e - } else { - if isMember { - user.TeamMemberships = append(user.TeamMemberships, team) + toOrgAndTeam := func(orgAndTeam string) (string, string) { + split := strings.Split(orgAndTeam, "/") + if len(split) != 2 { + return "", "" + } else { + return split[0], split[1] + } + } + + if len(cfg.Cfg.TeamWhiteList) != 0 { + for _, orgAndTeam := range cfg.Cfg.TeamWhiteList { + org, team := toOrgAndTeam(orgAndTeam) + if org != "" && team != "" { + e, isMember := getTeamMembershipStateFromGitHub(client, user, org, team, ptoken) + if e != nil { + return e + } else { + if isMember { + user.TeamMemberships = append(user.TeamMemberships, org+"/"+team) + } } + } else { + log.Warnf("Invalid org/team format in %s: must be written as /", orgAndTeam) } } } diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 0be1deb7..3e64cf06 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -71,7 +71,6 @@ func setUp() { cfg.Cfg.AllowAllUsers = false cfg.Cfg.WhiteList = make([]string, 0) - cfg.Cfg.Org = "" cfg.Cfg.TeamWhiteList = make([]string, 0) cfg.Cfg.Domains = []string{"domain1"} @@ -112,11 +111,10 @@ func TestVerifyUserPositiveByEmail(t *testing.T) { func TestVerifyUserPositiveByTeam(t *testing.T) { setUp() - cfg.Cfg.Org = "testorg" - cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "team2", "team1") + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "org1/team2", "org1/team1") - user.TeamMemberships = append(user.TeamMemberships, "team3") - user.TeamMemberships = append(user.TeamMemberships, "team1") + user.TeamMemberships = append(user.TeamMemberships, "org1/team3") + user.TeamMemberships = append(user.TeamMemberships, "org1/team1") ok, err := VerifyUser(*user) assert.True(t, ok) assert.Nil(t, err) @@ -124,8 +122,7 @@ func TestVerifyUserPositiveByTeam(t *testing.T) { func TestVerifyUserNegativeByTeam(t *testing.T) { setUp() - cfg.Cfg.Org = "testorg" - cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "team1") + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "org1/team1") ok, err := VerifyUser(*user) assert.False(t, ok) @@ -198,9 +195,7 @@ func TestGetUserInfoFromGitHub(t *testing.T) { }) mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) - cfg.Cfg.Org = "myorg" - - cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myteam") + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myorg/myteam") mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) @@ -208,5 +203,5 @@ func TestGetUserInfoFromGitHub(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "login", user.Username) - assert.Equal(t, []string{"myteam"}, user.TeamMemberships) + assert.Equal(t, []string{"myorg/myteam"}, user.TeamMemberships) } diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 4745a3af..49fbd066 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -34,7 +34,6 @@ type config struct { HealthCheck bool `mapstructure:"healthCheck"` Domains []string `mapstructure:"domains"` WhiteList []string `mapstructure:"whitelist"` - Org string `mapstructure:"org"` TeamWhiteList []string `mapstructure:"teamWhitelist"` AllowAllUsers bool `mapstructure:"allowAllUsers"` PublicAccess bool `mapstructure:"publicAccess"` From fd72962d759aef37093e4b292a95975baa72375a Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 09:45:08 +0100 Subject: [PATCH 04/21] Add test assertions on urls called --- handlers/handlers_test.go | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 3e64cf06..89f9ba89 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -2,13 +2,12 @@ package handlers import ( "encoding/json" - "golang.org/x/oauth2" - "net/http" - "regexp" - "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/domains" "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "net/http" + "regexp" mockhttp "github.com/karupanerura/go-mock-http-response" "github.com/stretchr/testify/assert" @@ -32,6 +31,7 @@ func (c *Transport) RoundTrip(req *http.Request) (*http.Response, error) { } for _, p := range mockedResponses { if p.matcher(req) { + requests = append(requests, req.URL.String()) return p.response.MakeResponse(req), nil } } @@ -55,10 +55,22 @@ func urlEquals(value string) ReqMatcher { } } +func assertUrlCalled(t *testing.T, url string) { + found := false + for _, requested_url := range requests { + if requested_url == url { + found = true + break + } + } + assert.True(t, found, "Expected %s to have been called", url) +} + var ( user *structs.User token = &oauth2.Token{AccessToken: "123"} mockedResponses = []FunResponsePair{} + requests []string client = &http.Client{Transport: &Transport{}} ) @@ -77,6 +89,7 @@ func setUp() { domains.Refresh() mockedResponses = []FunResponsePair{} + requests = make([]string, 0) user = &structs.User{Username: "testuser", Email: "test@example.com"} } @@ -190,7 +203,7 @@ func TestGetUserInfoFromGitHub(t *testing.T) { LastUpdate: 123, Name: "name", }, - Login: "login", + Login: "myusername", Picture: "avatar-url", }) mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) @@ -202,6 +215,9 @@ func TestGetUserInfoFromGitHub(t *testing.T) { err := getUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) assert.Nil(t, err) - assert.Equal(t, "login", user.Username) + assert.Equal(t, "myusername", user.Username) assert.Equal(t, []string{"myorg/myteam"}, user.TeamMemberships) + + expectedTeamMembershipUrl := "https://api.github.com/orgs/myorg/teams/myteam/memberships/myusername?access_token=" + token.AccessToken + assertUrlCalled(t, expectedTeamMembershipUrl) } From 4ad1f13bb048ac117aaf56bf9b85432655ec4369 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 19:15:55 +0100 Subject: [PATCH 05/21] Add org membership url to config and github defaults --- pkg/cfg/cfg.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 49fbd066..495f8142 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -87,6 +87,7 @@ type oauthConfig struct { Scopes []string `mapstructure:"scopes"` UserInfoURL string `mapstructure:"user_info_url"` UserTeamURL string `mapstructure:"user_team_url"` + UserOrgURL string `mapstructure:"user_org_url"` PreferredDomain string `mapstructre:"preferredDomain"` } @@ -637,6 +638,9 @@ func setDefaultsGitHub() { if GenOAuth.UserTeamURL == "" { GenOAuth.UserTeamURL = "https://api.github.com/orgs/:org_id/teams/:team_slug/memberships/:username?access_token=" } + if GenOAuth.UserOrgURL == "" { + GenOAuth.UserOrgURL = "https://api.github.com/orgs/:org_id/members/:username?access_token=" + } if len(GenOAuth.Scopes) == 0 { // https://github.com/vouch/vouch-proxy/issues/63 // https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ From 4a9d20abb8c5553dca53ef130afb0fef5b6c0351 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 19:16:24 +0100 Subject: [PATCH 06/21] Add method to check for github org membership --- handlers/handlers.go | 25 +++++++++++++++++++++++++ handlers/handlers_test.go | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/handlers/handlers.go b/handlers/handlers.go index d1655982..6574a41a 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -726,6 +726,31 @@ func getUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims return nil } +func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, ptoken *oauth2.Token) (rerr error, isMember bool) { + replacements := strings.NewReplacer(":org_id", orgId, ":username", user.Username) + orgMembershipResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserOrgURL) + ptoken.AccessToken) + if err != nil { + log.Error(err) + return err, false + } + + if orgMembershipResp.StatusCode == 302 { + log.Debug("Need to check public membership") + location := orgMembershipResp.Header.Get("Location") + if location != "" { + orgMembershipResp, err = client.Get(location) + } + } + + if orgMembershipResp.StatusCode == 204 { + return nil, true + } else if orgMembershipResp.StatusCode == 404 { + return nil, false + } else { + return nil, false + } +} + func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, team string, ptoken *oauth2.Token) (rerr error, isMember bool) { replacements := strings.NewReplacer(":org_id", orgId, ":team_slug", team, ":username", user.Username) membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL) + ptoken.AccessToken) diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 89f9ba89..86533e71 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -63,7 +63,7 @@ func assertUrlCalled(t *testing.T, url string) { break } } - assert.True(t, found, "Expected %s to have been called", url) + assert.True(t, found, "Expected %s to have been called, but got only %s", url, requests) } var ( @@ -191,6 +191,38 @@ func TestGetTeamMembershipStateFromGitHubNotAMember(t *testing.T) { assert.False(t, isMember) } +func TestGetOrgMembershipStateFromGitHubNotFound(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) + + err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) + + assert.Nil(t, err) + assert.False(t, isMember) + + expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken + assertUrlCalled(t, expectedOrgMembershipUrl) +} + +func TestGetOrgMembershipStateFromGitHubNoOrgAccess(t *testing.T) { + setUp() + location := "https://api.github.com/orgs/myorg/public_members/" + user.Username + + 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("")) + + err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) + + assert.Nil(t, err) + assert.True(t, isMember) + + expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken + assertUrlCalled(t, expectedOrgMembershipUrl) + + expectedOrgPublicMembershipUrl := "https://api.github.com/orgs/myorg/public_members/" + user.Username + assertUrlCalled(t, expectedOrgPublicMembershipUrl) +} + func TestGetUserInfoFromGitHub(t *testing.T) { setUp() From 6ad9c842c06d4547e653fcd16ae71e90bce93dc4 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Fri, 31 Jan 2020 19:26:45 +0100 Subject: [PATCH 07/21] Check for GitHub Org membership if no team qualified in TeamWhiteList value - i.e. only "" and not fully qualified team "/" is given in config's teamWhitelist --- handlers/handlers.go | 24 ++++++++++++++++++------ handlers/handlers_test.go | 7 ++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/handlers/handlers.go b/handlers/handlers.go index 6574a41a..ebfc2b7d 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -696,23 +696,35 @@ func getUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims toOrgAndTeam := func(orgAndTeam string) (string, string) { split := strings.Split(orgAndTeam, "/") - if len(split) != 2 { - return "", "" - } else { + if len(split) == 1 { + // only organization given + return orgAndTeam, "" + } else if len(split) == 2 { return split[0], split[1] + } else { + return "", "" } } if len(cfg.Cfg.TeamWhiteList) != 0 { for _, orgAndTeam := range cfg.Cfg.TeamWhiteList { org, team := toOrgAndTeam(orgAndTeam) - if org != "" && team != "" { - e, isMember := getTeamMembershipStateFromGitHub(client, user, org, team, ptoken) + if org != "" { + log.Info(org) + var ( + e error + isMember bool + ) + if team != "" { + e, isMember = getTeamMembershipStateFromGitHub(client, user, org, team, ptoken) + } else { + e, isMember = getOrgMembershipStateFromGitHub(client, user, org, ptoken) + } if e != nil { return e } else { if isMember { - user.TeamMemberships = append(user.TeamMemberships, org+"/"+team) + user.TeamMemberships = append(user.TeamMemberships, orgAndTeam) } } } else { diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index 86533e71..f5aa95ed 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -240,15 +240,16 @@ func TestGetUserInfoFromGitHub(t *testing.T) { }) mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) - cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myorg/myteam") + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myOtherOrg", "myorg/myteam") - mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + mockResponse(regexMatcher(".*teams.*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + mockResponse(regexMatcher(".*members.*"), http.StatusNoContent, map[string]string{}, []byte("")) err := getUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) assert.Nil(t, err) assert.Equal(t, "myusername", user.Username) - assert.Equal(t, []string{"myorg/myteam"}, user.TeamMemberships) + assert.Equal(t, []string{"myOtherOrg", "myorg/myteam"}, user.TeamMemberships) expectedTeamMembershipUrl := "https://api.github.com/orgs/myorg/teams/myteam/memberships/myusername?access_token=" + token.AccessToken assertUrlCalled(t, expectedTeamMembershipUrl) From 227cd5b48b5f5873ad4aabb5d4b6fcaf83733c3f Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Mon, 3 Feb 2020 13:26:05 +0100 Subject: [PATCH 08/21] Add documentation for teamWhitelist to github example config --- config/config.yml_example_github | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config/config.yml_example_github b/config/config.yml_example_github index 2935a691..5f74a975 100644 --- a/config/config.yml_example_github +++ b/config/config.yml_example_github @@ -15,6 +15,16 @@ vouch: # set allowAllUsers: true to use Vouch Proxy to just accept anyone who can authenticate at GitHub # allowAllUsers: true + # set teamWhitelist: to list of teams and/or GitHub organizations + # When putting an organization id without a slash, it will allow all (public) members from the organization. + # The client will try to read the private organization membership using the client credentials, if that's not possible + # due to access restriction, it will try to evaluate the publicly visible membership. + # Allowing members form a specific team can be configured by qualifying the team with the organization, separated by + # a slash. + # teamWhitelist: + # - myOrg + # - myOrg/myTeam + oauth: # create a new OAuth application at: # https://github.com/settings/applications/new From fd1019a2cd14b3c3e17674faa6faab485fa87a81 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 16:59:38 +0100 Subject: [PATCH 09/21] Move Github-related handler stuff to own package --- handlers/common/common.go | 35 ++++++ handlers/github/github.go | 152 ++++++++++++++++++++++++++ handlers/github/github_test.go | 189 +++++++++++++++++++++++++++++++++ handlers/handlers.go | 174 ++---------------------------- handlers/handlers_test.go | 165 +--------------------------- 5 files changed, 388 insertions(+), 327 deletions(-) create mode 100644 handlers/common/common.go create mode 100644 handlers/github/github.go create mode 100644 handlers/github/github_test.go diff --git a/handlers/common/common.go b/handlers/common/common.go new file mode 100644 index 00000000..6b9308fc --- /dev/null +++ b/handlers/common/common.go @@ -0,0 +1,35 @@ +package common + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" +) + +var ( + log = cfg.Cfg.Logger +) + +func MapClaims(claims []byte, customClaims *structs.CustomClaims) error { + // Create a struct that contains the claims that we want to store from the config. + var f interface{} + err := json.Unmarshal(claims, &f) + if err != nil { + log.Error("Error unmarshaling claims") + return err + } + m := f.(map[string]interface{}) + for k := range m { + var found = false + for _, e := range cfg.Cfg.Headers.Claims { + if k == e { + found = true + } + } + if found == false { + delete(m, k) + } + } + customClaims.Claims = m + return nil +} diff --git a/handlers/github/github.go b/handlers/github/github.go new file mode 100644 index 00000000..e3c7a45f --- /dev/null +++ b/handlers/github/github.go @@ -0,0 +1,152 @@ +package github + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "io/ioutil" + "net/http" + "strings" +) + +var ( + log = cfg.Cfg.Logger +) + +// github +// https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-authorization-options-for-oauth-apps/ +func GetUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { + + log.Errorf("ptoken.AccessToken: %s", ptoken.AccessToken) + userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL + ptoken.AccessToken) + if err != nil { + // http.Error(w, err.Error(), http.StatusBadRequest) + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + data, _ := ioutil.ReadAll(userinfo.Body) + log.Infof("github userinfo body: %s", string(data)) + if err = common.MapClaims(data, customClaims); err != nil { + log.Error(err) + return err + } + ghUser := structs.GitHubUser{} + if err = json.Unmarshal(data, &ghUser); err != nil { + log.Error(err) + return err + } + log.Debug("getUserInfoFromGitHub ghUser") + log.Debug(ghUser) + log.Debug("getUserInfoFromGitHub user") + log.Debug(user) + + ghUser.PrepareUserData() + user.Email = ghUser.Email + user.Name = ghUser.Name + user.Username = ghUser.Username + user.ID = ghUser.ID + + // user = &ghUser.User + + toOrgAndTeam := func(orgAndTeam string) (string, string) { + split := strings.Split(orgAndTeam, "/") + if len(split) == 1 { + // only organization given + return orgAndTeam, "" + } else if len(split) == 2 { + return split[0], split[1] + } else { + return "", "" + } + } + + if len(cfg.Cfg.TeamWhiteList) != 0 { + for _, orgAndTeam := range cfg.Cfg.TeamWhiteList { + org, team := toOrgAndTeam(orgAndTeam) + if org != "" { + log.Info(org) + var ( + e error + isMember bool + ) + if team != "" { + e, isMember = getTeamMembershipStateFromGitHub(client, user, org, team, ptoken) + } else { + e, isMember = getOrgMembershipStateFromGitHub(client, user, org, ptoken) + } + if e != nil { + return e + } else { + if isMember { + user.TeamMemberships = append(user.TeamMemberships, orgAndTeam) + } + } + } else { + log.Warnf("Invalid org/team format in %s: must be written as /", orgAndTeam) + } + } + } + + log.Debug("getUserInfoFromGitHub") + log.Debug(user) + return nil +} + +func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, ptoken *oauth2.Token) (rerr error, isMember bool) { + replacements := strings.NewReplacer(":org_id", orgId, ":username", user.Username) + orgMembershipResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserOrgURL) + ptoken.AccessToken) + if err != nil { + log.Error(err) + return err, false + } + + if orgMembershipResp.StatusCode == 302 { + log.Debug("Need to check public membership") + location := orgMembershipResp.Header.Get("Location") + if location != "" { + orgMembershipResp, err = client.Get(location) + } + } + + if orgMembershipResp.StatusCode == 204 { + return nil, true + } else if orgMembershipResp.StatusCode == 404 { + return nil, false + } else { + return nil, false + } +} + +func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, team string, ptoken *oauth2.Token) (rerr error, isMember bool) { + replacements := strings.NewReplacer(":org_id", orgId, ":team_slug", team, ":username", user.Username) + membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL) + ptoken.AccessToken) + if err != nil { + log.Error(err) + return err, false + } + defer func() { + if err := membershipStateResp.Body.Close(); err != nil { + rerr = err + } + }() + if membershipStateResp.StatusCode == 200 { + data, _ := ioutil.ReadAll(membershipStateResp.Body) + log.Infof("github team membership body: ", string(data)) + ghTeamState := structs.GitHubTeamMembershipState{} + if err = json.Unmarshal(data, &ghTeamState); err != nil { + log.Error(err) + return err, false + } + log.Debug("getTeamMembershipStateFromGitHub ghTeamState") + log.Debug(ghTeamState) + return nil, ghTeamState.State == "active" + } else { + return nil, false + } +} diff --git a/handlers/github/github_test.go b/handlers/github/github_test.go new file mode 100644 index 00000000..8f7c2c2d --- /dev/null +++ b/handlers/github/github_test.go @@ -0,0 +1,189 @@ +package github + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/domains" + "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "net/http" + "regexp" + + mockhttp "github.com/karupanerura/go-mock-http-response" + "github.com/stretchr/testify/assert" + "testing" +) + +type ReqMatcher func(*http.Request) bool + +type FunResponsePair struct { + matcher ReqMatcher + response *mockhttp.ResponseMock +} + +type Transport struct { + MockError error +} + +func (c *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + if c.MockError != nil { + return nil, c.MockError + } + for _, p := range mockedResponses { + if p.matcher(req) { + requests = append(requests, req.URL.String()) + return p.response.MakeResponse(req), nil + } + } + return nil, nil +} + +func mockResponse(fun ReqMatcher, statusCode int, headers map[string]string, body []byte) { + mockedResponses = append(mockedResponses, FunResponsePair{matcher: fun, response: mockhttp.NewResponseMock(statusCode, headers, body)}) +} + +func regexMatcher(expr string) ReqMatcher { + return func(r *http.Request) bool { + matches, _ := regexp.Match(expr, []byte(r.URL.String())) + return matches + } +} + +func urlEquals(value string) ReqMatcher { + return func(r *http.Request) bool { + return r.URL.String() == value + } +} + +func assertUrlCalled(t *testing.T, url string) { + found := false + for _, requested_url := range requests { + if requested_url == url { + found = true + break + } + } + assert.True(t, found, "Expected %s to have been called, but got only %s", url, requests) +} + +var ( + user *structs.User + token = &oauth2.Token{AccessToken: "123"} + mockedResponses = []FunResponsePair{} + requests []string + client = &http.Client{Transport: &Transport{}} +) + +func init() { + setUp() +} + +func setUp() { + cfg.InitForTestPurposesWithProvider("github") + + cfg.Cfg.AllowAllUsers = false + cfg.Cfg.WhiteList = make([]string, 0) + cfg.Cfg.TeamWhiteList = make([]string, 0) + cfg.Cfg.Domains = []string{"domain1"} + + domains.Refresh() + + mockedResponses = []FunResponsePair{} + requests = make([]string, 0) + + user = &structs.User{Username: "testuser", Email: "test@example.com"} +} + +func TestGetTeamMembershipStateFromGitHubActive(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.True(t, isMember) +} + +func TestGetTeamMembershipStateFromGitHubInactive(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"inactive\"}")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.False(t, isMember) +} + +func TestGetTeamMembershipStateFromGitHubNotAMember(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) + + err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) + + assert.Nil(t, err) + assert.False(t, isMember) +} + +func TestGetOrgMembershipStateFromGitHubNotFound(t *testing.T) { + setUp() + mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) + + err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) + + assert.Nil(t, err) + assert.False(t, isMember) + + expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken + assertUrlCalled(t, expectedOrgMembershipUrl) +} + +func TestGetOrgMembershipStateFromGitHubNoOrgAccess(t *testing.T) { + setUp() + location := "https://api.github.com/orgs/myorg/public_members/" + user.Username + + 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("")) + + err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) + + assert.Nil(t, err) + assert.True(t, isMember) + + expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken + assertUrlCalled(t, expectedOrgMembershipUrl) + + expectedOrgPublicMembershipUrl := "https://api.github.com/orgs/myorg/public_members/" + user.Username + assertUrlCalled(t, expectedOrgPublicMembershipUrl) +} + +func TestGetUserInfoFromGitHub(t *testing.T) { + setUp() + + userInfoContent, _ := json.Marshal(structs.GitHubUser{ + User: structs.User{ + Username: "test", + CreatedOn: 123, + Email: "email@example.com", + ID: 1, + LastUpdate: 123, + Name: "name", + }, + Login: "myusername", + Picture: "avatar-url", + }) + mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) + + cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myOtherOrg", "myorg/myteam") + + mockResponse(regexMatcher(".*teams.*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) + mockResponse(regexMatcher(".*members.*"), http.StatusNoContent, map[string]string{}, []byte("")) + + err := GetUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) + + assert.Nil(t, err) + 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 + assertUrlCalled(t, expectedTeamMembershipUrl) +} diff --git a/handlers/handlers.go b/handlers/handlers.go index ebfc2b7d..3a74cf74 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -6,6 +6,8 @@ import ( "encoding/base64" "encoding/json" "fmt" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/handlers/github" "html/template" "io/ioutil" "mime/multipart" @@ -567,7 +569,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust if cfg.GenOAuth.Provider == cfg.Providers.Google { return getUserInfoFromGoogle(client, user, customClaims) } else if cfg.GenOAuth.Provider == cfg.Providers.GitHub { - return getUserInfoFromGitHub(client, user, customClaims, providerToken) + return github.GetUserInfoFromGitHub(client, user, customClaims, providerToken) } else if cfg.GenOAuth.Provider == cfg.Providers.OIDC { return getUserInfoFromOpenID(client, user, customClaims, providerToken) } @@ -587,7 +589,7 @@ func getUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims }() data, _ := ioutil.ReadAll(userinfo.Body) log.Infof("OpenID userinfo body: %s", string(data)) - if err = mapClaims(data, customClaims); err != nil { + if err = common.MapClaims(data, customClaims); err != nil { log.Error(err) return err } @@ -611,7 +613,7 @@ func getUserInfoFromOpenStax(client *http.Client, user *structs.User, customClai }() data, _ := ioutil.ReadAll(userinfo.Body) log.Infof("OpenStax userinfo body: %s", string(data)) - if err = mapClaims(data, customClaims); err != nil { + if err = common.MapClaims(data, customClaims); err != nil { log.Error(err) return err } @@ -642,7 +644,7 @@ func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims }() data, _ := ioutil.ReadAll(userinfo.Body) log.Infof("google userinfo body: %s", string(data)) - if err = mapClaims(data, customClaims); err != nil { + if err = common.MapClaims(data, customClaims); err != nil { log.Error(err) return err } @@ -655,142 +657,6 @@ func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims return nil } -// github -// https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-authorization-options-for-oauth-apps/ -func getUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { - - log.Errorf("ptoken.AccessToken: %s", ptoken.AccessToken) - userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL + ptoken.AccessToken) - if err != nil { - // http.Error(w, err.Error(), http.StatusBadRequest) - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - data, _ := ioutil.ReadAll(userinfo.Body) - log.Infof("github userinfo body: %s", string(data)) - if err = mapClaims(data, customClaims); err != nil { - log.Error(err) - return err - } - ghUser := structs.GitHubUser{} - if err = json.Unmarshal(data, &ghUser); err != nil { - log.Error(err) - return err - } - log.Debug("getUserInfoFromGitHub ghUser") - log.Debug(ghUser) - log.Debug("getUserInfoFromGitHub user") - log.Debug(user) - - ghUser.PrepareUserData() - user.Email = ghUser.Email - user.Name = ghUser.Name - user.Username = ghUser.Username - user.ID = ghUser.ID - - // user = &ghUser.User - - toOrgAndTeam := func(orgAndTeam string) (string, string) { - split := strings.Split(orgAndTeam, "/") - if len(split) == 1 { - // only organization given - return orgAndTeam, "" - } else if len(split) == 2 { - return split[0], split[1] - } else { - return "", "" - } - } - - if len(cfg.Cfg.TeamWhiteList) != 0 { - for _, orgAndTeam := range cfg.Cfg.TeamWhiteList { - org, team := toOrgAndTeam(orgAndTeam) - if org != "" { - log.Info(org) - var ( - e error - isMember bool - ) - if team != "" { - e, isMember = getTeamMembershipStateFromGitHub(client, user, org, team, ptoken) - } else { - e, isMember = getOrgMembershipStateFromGitHub(client, user, org, ptoken) - } - if e != nil { - return e - } else { - if isMember { - user.TeamMemberships = append(user.TeamMemberships, orgAndTeam) - } - } - } else { - log.Warnf("Invalid org/team format in %s: must be written as /", orgAndTeam) - } - } - } - - log.Debug("getUserInfoFromGitHub") - log.Debug(user) - return nil -} - -func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, ptoken *oauth2.Token) (rerr error, isMember bool) { - replacements := strings.NewReplacer(":org_id", orgId, ":username", user.Username) - orgMembershipResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserOrgURL) + ptoken.AccessToken) - if err != nil { - log.Error(err) - return err, false - } - - if orgMembershipResp.StatusCode == 302 { - log.Debug("Need to check public membership") - location := orgMembershipResp.Header.Get("Location") - if location != "" { - orgMembershipResp, err = client.Get(location) - } - } - - if orgMembershipResp.StatusCode == 204 { - return nil, true - } else if orgMembershipResp.StatusCode == 404 { - return nil, false - } else { - return nil, false - } -} - -func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, orgId string, team string, ptoken *oauth2.Token) (rerr error, isMember bool) { - replacements := strings.NewReplacer(":org_id", orgId, ":team_slug", team, ":username", user.Username) - membershipStateResp, err := client.Get(replacements.Replace(cfg.GenOAuth.UserTeamURL) + ptoken.AccessToken) - if err != nil { - log.Error(err) - return err, false - } - defer func() { - if err := membershipStateResp.Body.Close(); err != nil { - rerr = err - } - }() - if membershipStateResp.StatusCode == 200 { - data, _ := ioutil.ReadAll(membershipStateResp.Body) - log.Infof("github team membership body: ", string(data)) - ghTeamState := structs.GitHubTeamMembershipState{} - if err = json.Unmarshal(data, &ghTeamState); err != nil { - log.Error(err) - return err, false - } - log.Debug("getTeamMembershipStateFromGitHub ghTeamState") - log.Debug(ghTeamState) - return nil, ghTeamState.State == "active" - } else { - return nil, false - } -} - func getUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { code := r.URL.Query().Get("code") @@ -848,7 +714,7 @@ func getUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims data, _ := ioutil.ReadAll(userinfo.Body) log.Infof("indieauth userinfo body: %s", string(data)) - if err = mapClaims(data, customClaims); err != nil { + if err = common.MapClaims(data, customClaims); err != nil { log.Error(err) return err } @@ -941,7 +807,7 @@ func getUserInfoFromADFS(r *http.Request, user *structs.User, customClaims *stru // data contains an access token, refresh token, and id token // Please note that in order for custom claims to work you MUST set allatclaims in ADFS to be passed // https://oktotechnologies.ca/2018/08/26/adfs-openidconnect-configuration/ - if err = mapClaims([]byte(idToken), customClaims); err != nil { + if err = common.MapClaims([]byte(idToken), customClaims); err != nil { log.Error(err) return err } @@ -991,27 +857,3 @@ func ok200(w http.ResponseWriter, r *http.Request) { log.Error(err) } } - -func mapClaims(claims []byte, customClaims *structs.CustomClaims) error { - // Create a struct that contains the claims that we want to store from the config. - var f interface{} - err := json.Unmarshal(claims, &f) - if err != nil { - log.Error("Error unmarshaling claims") - return err - } - m := f.(map[string]interface{}) - for k := range m { - var found = false - for _, e := range cfg.Cfg.Headers.Claims { - if k == e { - found = true - } - } - if found == false { - delete(m, k) - } - } - customClaims.Claims = m - return nil -} diff --git a/handlers/handlers_test.go b/handlers/handlers_test.go index f5aa95ed..1f9a6cc2 100644 --- a/handlers/handlers_test.go +++ b/handlers/handlers_test.go @@ -1,77 +1,17 @@ package handlers import ( - "encoding/json" + "github.com/stretchr/testify/assert" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/domains" "github.com/vouch/vouch-proxy/pkg/structs" "golang.org/x/oauth2" - "net/http" - "regexp" - - mockhttp "github.com/karupanerura/go-mock-http-response" - "github.com/stretchr/testify/assert" "testing" ) -type ReqMatcher func(*http.Request) bool - -type FunResponsePair struct { - matcher ReqMatcher - response *mockhttp.ResponseMock -} - -type Transport struct { - MockError error -} - -func (c *Transport) RoundTrip(req *http.Request) (*http.Response, error) { - if c.MockError != nil { - return nil, c.MockError - } - for _, p := range mockedResponses { - if p.matcher(req) { - requests = append(requests, req.URL.String()) - return p.response.MakeResponse(req), nil - } - } - return nil, nil -} - -func mockResponse(fun ReqMatcher, statusCode int, headers map[string]string, body []byte) { - mockedResponses = append(mockedResponses, FunResponsePair{matcher: fun, response: mockhttp.NewResponseMock(statusCode, headers, body)}) -} - -func regexMatcher(expr string) ReqMatcher { - return func(r *http.Request) bool { - matches, _ := regexp.Match(expr, []byte(r.URL.String())) - return matches - } -} - -func urlEquals(value string) ReqMatcher { - return func(r *http.Request) bool { - return r.URL.String() == value - } -} - -func assertUrlCalled(t *testing.T, url string) { - found := false - for _, requested_url := range requests { - if requested_url == url { - found = true - break - } - } - assert.True(t, found, "Expected %s to have been called, but got only %s", url, requests) -} - var ( - user *structs.User - token = &oauth2.Token{AccessToken: "123"} - mockedResponses = []FunResponsePair{} - requests []string - client = &http.Client{Transport: &Transport{}} + user *structs.User + token = &oauth2.Token{AccessToken: "123"} ) func init() { @@ -79,7 +19,7 @@ func init() { } func setUp() { - cfg.InitForTestPurposesWithProvider("github") + cfg.InitForTestPurposes() cfg.Cfg.AllowAllUsers = false cfg.Cfg.WhiteList = make([]string, 0) @@ -88,9 +28,6 @@ func setUp() { domains.Refresh() - mockedResponses = []FunResponsePair{} - requests = make([]string, 0) - user = &structs.User{Username: "testuser", Email: "test@example.com"} } @@ -160,97 +97,3 @@ func TestVerifyUserNegative(t *testing.T) { assert.False(t, ok) assert.NotNil(t, err) } - -func TestGetTeamMembershipStateFromGitHubActive(t *testing.T) { - setUp() - mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) - - err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) - - assert.Nil(t, err) - assert.True(t, isMember) -} - -func TestGetTeamMembershipStateFromGitHubInactive(t *testing.T) { - setUp() - mockResponse(regexMatcher(".*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"inactive\"}")) - - err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) - - assert.Nil(t, err) - assert.False(t, isMember) -} - -func TestGetTeamMembershipStateFromGitHubNotAMember(t *testing.T) { - setUp() - mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) - - err, isMember := getTeamMembershipStateFromGitHub(client, user, "org1", "team1", token) - - assert.Nil(t, err) - assert.False(t, isMember) -} - -func TestGetOrgMembershipStateFromGitHubNotFound(t *testing.T) { - setUp() - mockResponse(regexMatcher(".*"), http.StatusNotFound, map[string]string{}, []byte("")) - - err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) - - assert.Nil(t, err) - assert.False(t, isMember) - - expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken - assertUrlCalled(t, expectedOrgMembershipUrl) -} - -func TestGetOrgMembershipStateFromGitHubNoOrgAccess(t *testing.T) { - setUp() - location := "https://api.github.com/orgs/myorg/public_members/" + user.Username - - 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("")) - - err, isMember := getOrgMembershipStateFromGitHub(client, user, "myorg", token) - - assert.Nil(t, err) - assert.True(t, isMember) - - expectedOrgMembershipUrl := "https://api.github.com/orgs/myorg/members/" + user.Username + "?access_token=" + token.AccessToken - assertUrlCalled(t, expectedOrgMembershipUrl) - - expectedOrgPublicMembershipUrl := "https://api.github.com/orgs/myorg/public_members/" + user.Username - assertUrlCalled(t, expectedOrgPublicMembershipUrl) -} - -func TestGetUserInfoFromGitHub(t *testing.T) { - setUp() - - userInfoContent, _ := json.Marshal(structs.GitHubUser{ - User: structs.User{ - Username: "test", - CreatedOn: 123, - Email: "email@example.com", - ID: 1, - LastUpdate: 123, - Name: "name", - }, - Login: "myusername", - Picture: "avatar-url", - }) - mockResponse(urlEquals(cfg.GenOAuth.UserInfoURL+token.AccessToken), http.StatusOK, map[string]string{}, userInfoContent) - - cfg.Cfg.TeamWhiteList = append(cfg.Cfg.TeamWhiteList, "myOtherOrg", "myorg/myteam") - - mockResponse(regexMatcher(".*teams.*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) - mockResponse(regexMatcher(".*members.*"), http.StatusNoContent, map[string]string{}, []byte("")) - - err := getUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) - - assert.Nil(t, err) - 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 - assertUrlCalled(t, expectedTeamMembershipUrl) -} From cfb5be2b3de55606cd15907b26afdf363ba8f19b Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:01:45 +0100 Subject: [PATCH 10/21] Move IndieAuth-related handler stuff to own package --- handlers/handlers.go | 77 +---------------------------- handlers/indieauth/indieauth.go | 88 +++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 75 deletions(-) create mode 100644 handlers/indieauth/indieauth.go diff --git a/handlers/handlers.go b/handlers/handlers.go index 3a74cf74..2228d241 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -1,16 +1,15 @@ package handlers import ( - "bytes" "context" "encoding/base64" "encoding/json" "fmt" "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" + "github.com/vouch/vouch-proxy/handlers/indieauth" "html/template" "io/ioutil" - "mime/multipart" "net/http" "net/url" "path/filepath" @@ -536,7 +535,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust // indieauth sends the "me" setting in json back to the callback, so just pluck it from the callback if cfg.GenOAuth.Provider == cfg.Providers.IndieAuth { - return getUserInfoFromIndieAuth(r, user, customClaims) + return indieauth.GetUserInfoFromIndieAuth(r, user, customClaims) } else if cfg.GenOAuth.Provider == cfg.Providers.ADFS { return getUserInfoFromADFS(r, user, customClaims, ptokens) } @@ -657,78 +656,6 @@ func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims return nil } -func getUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { - - code := r.URL.Query().Get("code") - log.Errorf("ptoken.AccessToken: %s", code) - var b bytes.Buffer - w := multipart.NewWriter(&b) - // v.Set("code", code) - fw, err := w.CreateFormField("code") - if err != nil { - return err - } - if _, err = fw.Write([]byte(code)); err != nil { - return err - } - // v.Set("redirect_uri", cfg.GenOAuth.RedirectURL) - if fw, err = w.CreateFormField("redirect_uri"); err != nil { - return err - } - if _, err = fw.Write([]byte(cfg.GenOAuth.RedirectURL)); err != nil { - return err - } - // v.Set("client_id", cfg.GenOAuth.ClientID) - if fw, err = w.CreateFormField("client_id"); err != nil { - return err - } - if _, err = fw.Write([]byte(cfg.GenOAuth.ClientID)); err != nil { - return err - } - if err = w.Close(); err != nil { - log.Error("error closing writer.") - } - - req, err := http.NewRequest("POST", cfg.GenOAuth.AuthURL, &b) - if err != nil { - return err - } - req.Header.Set("Content-Type", w.FormDataContentType()) - req.Header.Set("Accept", "application/json") - - // v := url.Values{} - // userinfo, err := client.PostForm(cfg.GenOAuth.UserInfoURL, v) - - client := &http.Client{} - userinfo, err := client.Do(req) - - if err != nil { - // http.Error(w, err.Error(), http.StatusBadRequest) - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - - data, _ := ioutil.ReadAll(userinfo.Body) - log.Infof("indieauth userinfo body: %s", string(data)) - if err = common.MapClaims(data, customClaims); err != nil { - log.Error(err) - return err - } - iaUser := structs.IndieAuthUser{} - if err = json.Unmarshal(data, &iaUser); err != nil { - log.Error(err) - return err - } - iaUser.PrepareUserData() - user.Username = iaUser.Username - log.Debug(user) - return nil -} - // More info: https://developers.home-assistant.io/docs/en/auth_api.html func getUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { // Home assistant does not provide an API to query username, so we statically set it to "homeassistant" diff --git a/handlers/indieauth/indieauth.go b/handlers/indieauth/indieauth.go new file mode 100644 index 00000000..f1c1d9ae --- /dev/null +++ b/handlers/indieauth/indieauth.go @@ -0,0 +1,88 @@ +package indieauth + +import ( + "bytes" + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "io/ioutil" + "mime/multipart" + "net/http" +) + +var ( + log = cfg.Cfg.Logger +) + +func GetUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { + + code := r.URL.Query().Get("code") + log.Errorf("ptoken.AccessToken: %s", code) + var b bytes.Buffer + w := multipart.NewWriter(&b) + // v.Set("code", code) + fw, err := w.CreateFormField("code") + if err != nil { + return err + } + if _, err = fw.Write([]byte(code)); err != nil { + return err + } + // v.Set("redirect_uri", cfg.GenOAuth.RedirectURL) + if fw, err = w.CreateFormField("redirect_uri"); err != nil { + return err + } + if _, err = fw.Write([]byte(cfg.GenOAuth.RedirectURL)); err != nil { + return err + } + // v.Set("client_id", cfg.GenOAuth.ClientID) + if fw, err = w.CreateFormField("client_id"); err != nil { + return err + } + if _, err = fw.Write([]byte(cfg.GenOAuth.ClientID)); err != nil { + return err + } + if err = w.Close(); err != nil { + log.Error("error closing writer.") + } + + req, err := http.NewRequest("POST", cfg.GenOAuth.AuthURL, &b) + if err != nil { + return err + } + req.Header.Set("Content-Type", w.FormDataContentType()) + req.Header.Set("Accept", "application/json") + + // v := url.Values{} + // userinfo, err := client.PostForm(cfg.GenOAuth.UserInfoURL, v) + + client := &http.Client{} + userinfo, err := client.Do(req) + + if err != nil { + // http.Error(w, err.Error(), http.StatusBadRequest) + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + + data, _ := ioutil.ReadAll(userinfo.Body) + log.Infof("indieauth userinfo body: %s", string(data)) + if err = common.MapClaims(data, customClaims); err != nil { + log.Error(err) + return err + } + iaUser := structs.IndieAuthUser{} + if err = json.Unmarshal(data, &iaUser); err != nil { + log.Error(err) + return err + } + iaUser.PrepareUserData() + user.Username = iaUser.Username + log.Debug(user) + return nil +} From 82f7c18140549141f182088b2c5f22f9cd3b2336 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:03:25 +0100 Subject: [PATCH 11/21] Move ADFS-related handler stuff to own package --- handlers/adfs/adfs.go | 110 ++++++++++++++++++++++++++++++++++++++++++ handlers/handlers.go | 97 +------------------------------------ 2 files changed, 112 insertions(+), 95 deletions(-) create mode 100644 handlers/adfs/adfs.go diff --git a/handlers/adfs/adfs.go b/handlers/adfs/adfs.go new file mode 100644 index 00000000..4280e839 --- /dev/null +++ b/handlers/adfs/adfs.go @@ -0,0 +1,110 @@ +package adfs + +import ( + "encoding/base64" + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "io/ioutil" + "net/http" + "net/url" + "regexp" + "strconv" + "strings" +) + +type adfsTokenRes struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + IDToken string `json:"id_token"` + ExpiresIn int64 `json:"expires_in"` // relative seconds from now +} + +var ( + log = cfg.Cfg.Logger +) + +// More info: https://docs.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-scenarios-for-developers#supported-scenarios +func GetUserInfoFromADFS(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + code := r.URL.Query().Get("code") + log.Debugf("code: %s", code) + + formData := url.Values{} + formData.Set("code", code) + formData.Set("grant_type", "authorization_code") + formData.Set("resource", cfg.GenOAuth.RedirectURL) + formData.Set("client_id", cfg.GenOAuth.ClientID) + formData.Set("redirect_uri", cfg.GenOAuth.RedirectURL) + if cfg.GenOAuth.ClientSecret != "" { + formData.Set("client_secret", cfg.GenOAuth.ClientSecret) + } + req, err := http.NewRequest("POST", cfg.GenOAuth.TokenURL, strings.NewReader(formData.Encode())) + if err != nil { + return err + } + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + req.Header.Add("Content-Length", strconv.Itoa(len(formData.Encode()))) + req.Header.Set("Accept", "application/json") + + client := &http.Client{} + userinfo, err := client.Do(req) + + if err != nil { + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + + data, _ := ioutil.ReadAll(userinfo.Body) + tokenRes := adfsTokenRes{} + + if err := json.Unmarshal(data, &tokenRes); err != nil { + log.Errorf("oauth2: cannot fetch token: %v", err) + return nil + } + + ptokens.PAccessToken = string(tokenRes.AccessToken) + ptokens.PIdToken = string(tokenRes.IDToken) + + s := strings.Split(tokenRes.IDToken, ".") + if len(s) < 2 { + log.Error("jws: invalid token received") + return nil + } + + idToken, err := base64.RawURLEncoding.DecodeString(s[1]) + if err != nil { + log.Error(err) + return nil + } + log.Debugf("idToken: %+v", string(idToken)) + + adfsUser := structs.ADFSUser{} + json.Unmarshal([]byte(idToken), &adfsUser) + log.Infof("adfs adfsUser: %+v", adfsUser) + // data contains an access token, refresh token, and id token + // Please note that in order for custom claims to work you MUST set allatclaims in ADFS to be passed + // https://oktotechnologies.ca/2018/08/26/adfs-openidconnect-configuration/ + if err = common.MapClaims([]byte(idToken), customClaims); err != nil { + log.Error(err) + return err + } + adfsUser.PrepareUserData() + var rxEmail = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") + + if len(adfsUser.Email) == 0 { + // If the email is blank, we will try to determine if the UPN is an email. + if rxEmail.MatchString(adfsUser.UPN) { + // Set the email from UPN if there is a valid email present. + adfsUser.Email = adfsUser.UPN + } + } + user.Username = adfsUser.Username + user.Email = adfsUser.Email + log.Debugf("User Obj: %+v", user) + return nil +} diff --git a/handlers/handlers.go b/handlers/handlers.go index 2228d241..8e67765d 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -2,20 +2,18 @@ package handlers import ( "context" - "encoding/base64" "encoding/json" "fmt" + "github.com/vouch/vouch-proxy/handlers/adfs" "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" "github.com/vouch/vouch-proxy/handlers/indieauth" "html/template" "io/ioutil" "net/http" - "net/url" "path/filepath" "reflect" "regexp" - "strconv" "strings" "go.uber.org/zap" @@ -537,7 +535,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust if cfg.GenOAuth.Provider == cfg.Providers.IndieAuth { return indieauth.GetUserInfoFromIndieAuth(r, user, customClaims) } else if cfg.GenOAuth.Provider == cfg.Providers.ADFS { - return getUserInfoFromADFS(r, user, customClaims, ptokens) + return adfs.GetUserInfoFromADFS(r, user, customClaims, ptokens) } providerToken, err := cfg.OAuthClient.Exchange(context.TODO(), r.URL.Query().Get("code")) if err != nil { @@ -663,97 +661,6 @@ func getUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customCla return nil } -type adfsTokenRes struct { - AccessToken string `json:"access_token"` - TokenType string `json:"token_type"` - IDToken string `json:"id_token"` - ExpiresIn int64 `json:"expires_in"` // relative seconds from now -} - -// More info: https://docs.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-scenarios-for-developers#supported-scenarios -func getUserInfoFromADFS(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { - code := r.URL.Query().Get("code") - log.Debugf("code: %s", code) - - formData := url.Values{} - formData.Set("code", code) - formData.Set("grant_type", "authorization_code") - formData.Set("resource", cfg.GenOAuth.RedirectURL) - formData.Set("client_id", cfg.GenOAuth.ClientID) - formData.Set("redirect_uri", cfg.GenOAuth.RedirectURL) - if cfg.GenOAuth.ClientSecret != "" { - formData.Set("client_secret", cfg.GenOAuth.ClientSecret) - } - req, err := http.NewRequest("POST", cfg.GenOAuth.TokenURL, strings.NewReader(formData.Encode())) - if err != nil { - return err - } - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - req.Header.Add("Content-Length", strconv.Itoa(len(formData.Encode()))) - req.Header.Set("Accept", "application/json") - - client := &http.Client{} - userinfo, err := client.Do(req) - - if err != nil { - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - - data, _ := ioutil.ReadAll(userinfo.Body) - tokenRes := adfsTokenRes{} - - if err := json.Unmarshal(data, &tokenRes); err != nil { - log.Errorf("oauth2: cannot fetch token: %v", err) - return nil - } - - ptokens.PAccessToken = string(tokenRes.AccessToken) - ptokens.PIdToken = string(tokenRes.IDToken) - - s := strings.Split(tokenRes.IDToken, ".") - if len(s) < 2 { - log.Error("jws: invalid token received") - return nil - } - - idToken, err := base64.RawURLEncoding.DecodeString(s[1]) - if err != nil { - log.Error(err) - return nil - } - log.Debugf("idToken: %+v", string(idToken)) - - adfsUser := structs.ADFSUser{} - json.Unmarshal([]byte(idToken), &adfsUser) - log.Infof("adfs adfsUser: %+v", adfsUser) - // data contains an access token, refresh token, and id token - // Please note that in order for custom claims to work you MUST set allatclaims in ADFS to be passed - // https://oktotechnologies.ca/2018/08/26/adfs-openidconnect-configuration/ - if err = common.MapClaims([]byte(idToken), customClaims); err != nil { - log.Error(err) - return err - } - adfsUser.PrepareUserData() - var rxEmail = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") - - if len(adfsUser.Email) == 0 { - // If the email is blank, we will try to determine if the UPN is an email. - if rxEmail.MatchString(adfsUser.UPN) { - // Set the email from UPN if there is a valid email present. - adfsUser.Email = adfsUser.UPN - } - } - user.Username = adfsUser.Username - user.Email = adfsUser.Email - log.Debugf("User Obj: %+v", user) - return nil -} - // the standard error // this is captured by nginx, which converts the 401 into 302 to the login page func error401(w http.ResponseWriter, r *http.Request, ae AuthError) { From eacccd55d3f8d568c72100d486488b344cd2c144 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:05:10 +0100 Subject: [PATCH 12/21] Move HomeAssistant-related handler stuff to own package --- handlers/handlers.go | 10 ++-------- handlers/homeassistant/homeassistant.go | 13 +++++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 handlers/homeassistant/homeassistant.go diff --git a/handlers/handlers.go b/handlers/handlers.go index 8e67765d..0aa52a30 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -7,6 +7,7 @@ import ( "github.com/vouch/vouch-proxy/handlers/adfs" "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" + "github.com/vouch/vouch-proxy/handlers/homeassistant" "github.com/vouch/vouch-proxy/handlers/indieauth" "html/template" "io/ioutil" @@ -543,7 +544,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust } if cfg.GenOAuth.Provider == cfg.Providers.HomeAssistant { ptokens.PAccessToken = providerToken.Extra("access_token").(string) - return getUserInfoFromHomeAssistant(r, user, customClaims) + return homeassistant.GetUserInfoFromHomeAssistant(r, user, customClaims) } ptokens.PAccessToken = providerToken.AccessToken if cfg.GenOAuth.Provider == cfg.Providers.OpenStax { @@ -654,13 +655,6 @@ func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims return nil } -// More info: https://developers.home-assistant.io/docs/en/auth_api.html -func getUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { - // Home assistant does not provide an API to query username, so we statically set it to "homeassistant" - user.Username = "homeassistant" - return nil -} - // the standard error // this is captured by nginx, which converts the 401 into 302 to the login page func error401(w http.ResponseWriter, r *http.Request, ae AuthError) { diff --git a/handlers/homeassistant/homeassistant.go b/handlers/homeassistant/homeassistant.go new file mode 100644 index 00000000..1a0a7c41 --- /dev/null +++ b/handlers/homeassistant/homeassistant.go @@ -0,0 +1,13 @@ +package homeassistant + +import ( + "github.com/vouch/vouch-proxy/pkg/structs" + "net/http" +) + +// More info: https://developers.home-assistant.io/docs/en/auth_api.html +func GetUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { + // Home assistant does not provide an API to query username, so we statically set it to "homeassistant" + user.Username = "homeassistant" + return nil +} From d367704d3aa8995ac43688212202f137eb6d82ca Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:06:40 +0100 Subject: [PATCH 13/21] Move OpenStax-related handler stuff to own package --- handlers/handlers.go | 34 ++------------------------ handlers/openstax/openstax.go | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 handlers/openstax/openstax.go diff --git a/handlers/handlers.go b/handlers/handlers.go index 0aa52a30..9a159a7e 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -9,6 +9,7 @@ import ( "github.com/vouch/vouch-proxy/handlers/github" "github.com/vouch/vouch-proxy/handlers/homeassistant" "github.com/vouch/vouch-proxy/handlers/indieauth" + "github.com/vouch/vouch-proxy/handlers/openstax" "html/template" "io/ioutil" "net/http" @@ -549,7 +550,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust ptokens.PAccessToken = providerToken.AccessToken if cfg.GenOAuth.Provider == cfg.Providers.OpenStax { client := cfg.OAuthClient.Client(context.TODO(), providerToken) - return getUserInfoFromOpenStax(client, user, customClaims, providerToken) + return openstax.GetUserInfoFromOpenStax(client, user, customClaims, providerToken) } if providerToken.Extra("id_token") != nil { @@ -599,37 +600,6 @@ func getUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims return nil } -func getUserInfoFromOpenStax(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { - userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) - if err != nil { - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - data, _ := ioutil.ReadAll(userinfo.Body) - log.Infof("OpenStax userinfo body: %s", string(data)) - if err = common.MapClaims(data, customClaims); err != nil { - log.Error(err) - return err - } - oxUser := structs.OpenStaxUser{} - if err = json.Unmarshal(data, &oxUser); err != nil { - log.Error(err) - return err - } - - oxUser.PrepareUserData() - user.Email = oxUser.Email - user.Name = oxUser.Name - user.Username = oxUser.Username - user.ID = oxUser.ID - user.PrepareUserData() - return nil -} - func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) if err != nil { diff --git a/handlers/openstax/openstax.go b/handlers/openstax/openstax.go new file mode 100644 index 00000000..8052375c --- /dev/null +++ b/handlers/openstax/openstax.go @@ -0,0 +1,46 @@ +package openstax + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "io/ioutil" + "net/http" +) + +var ( + log = cfg.Cfg.Logger +) + +func GetUserInfoFromOpenStax(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { + userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) + if err != nil { + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + data, _ := ioutil.ReadAll(userinfo.Body) + log.Infof("OpenStax userinfo body: %s", string(data)) + if err = common.MapClaims(data, customClaims); err != nil { + log.Error(err) + return err + } + oxUser := structs.OpenStaxUser{} + if err = json.Unmarshal(data, &oxUser); err != nil { + log.Error(err) + return err + } + + oxUser.PrepareUserData() + user.Email = oxUser.Email + user.Name = oxUser.Name + user.Username = oxUser.Username + user.ID = oxUser.ID + user.PrepareUserData() + return nil +} From 2b6659df409364260020cdfec98019548fe00574 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:08:06 +0100 Subject: [PATCH 14/21] Move Google-related handler stuff to own package --- handlers/google/google.go | 39 +++++++++++++++++++++++++++++++++++++++ handlers/handlers.go | 28 ++-------------------------- 2 files changed, 41 insertions(+), 26 deletions(-) create mode 100644 handlers/google/google.go diff --git a/handlers/google/google.go b/handlers/google/google.go new file mode 100644 index 00000000..95643413 --- /dev/null +++ b/handlers/google/google.go @@ -0,0 +1,39 @@ +package google + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "io/ioutil" + "net/http" +) + +var ( + log = cfg.Cfg.Logger +) + +func GetUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { + userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) + if err != nil { + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + data, _ := ioutil.ReadAll(userinfo.Body) + log.Infof("google userinfo body: ", string(data)) + if err = common.MapClaims(data, customClaims); err != nil { + log.Error(err) + return err + } + if err = json.Unmarshal(data, user); err != nil { + log.Error(err) + return err + } + user.PrepareUserData() + + return nil +} diff --git a/handlers/handlers.go b/handlers/handlers.go index 9a159a7e..15540774 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -7,6 +7,7 @@ import ( "github.com/vouch/vouch-proxy/handlers/adfs" "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" + "github.com/vouch/vouch-proxy/handlers/google" "github.com/vouch/vouch-proxy/handlers/homeassistant" "github.com/vouch/vouch-proxy/handlers/indieauth" "github.com/vouch/vouch-proxy/handlers/openstax" @@ -566,7 +567,7 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust // make the "third leg" request back to provider to exchange the token for the userinfo client := cfg.OAuthClient.Client(context.TODO(), providerToken) if cfg.GenOAuth.Provider == cfg.Providers.Google { - return getUserInfoFromGoogle(client, user, customClaims) + return google.GetUserInfoFromGoogle(client, user, customClaims) } else if cfg.GenOAuth.Provider == cfg.Providers.GitHub { return github.GetUserInfoFromGitHub(client, user, customClaims, providerToken) } else if cfg.GenOAuth.Provider == cfg.Providers.OIDC { @@ -600,31 +601,6 @@ func getUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims return nil } -func getUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { - userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) - if err != nil { - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - data, _ := ioutil.ReadAll(userinfo.Body) - log.Infof("google userinfo body: %s", string(data)) - if err = common.MapClaims(data, customClaims); err != nil { - log.Error(err) - return err - } - if err = json.Unmarshal(data, user); err != nil { - log.Error(err) - return err - } - user.PrepareUserData() - - return nil -} - // the standard error // this is captured by nginx, which converts the 401 into 302 to the login page func error401(w http.ResponseWriter, r *http.Request, ae AuthError) { From fb73f16a3d9cfb145fb1f9cf6045b9d6caedd54d Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 17:09:25 +0100 Subject: [PATCH 15/21] Move OpenID-related handler stuff to own package --- handlers/handlers.go | 30 ++---------------------------- handlers/openid/openid.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 28 deletions(-) create mode 100644 handlers/openid/openid.go diff --git a/handlers/handlers.go b/handlers/handlers.go index 15540774..d9bc8f45 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -2,17 +2,15 @@ package handlers import ( "context" - "encoding/json" "fmt" "github.com/vouch/vouch-proxy/handlers/adfs" - "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" "github.com/vouch/vouch-proxy/handlers/google" "github.com/vouch/vouch-proxy/handlers/homeassistant" "github.com/vouch/vouch-proxy/handlers/indieauth" + "github.com/vouch/vouch-proxy/handlers/openid" "github.com/vouch/vouch-proxy/handlers/openstax" "html/template" - "io/ioutil" "net/http" "path/filepath" "reflect" @@ -571,36 +569,12 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust } else if cfg.GenOAuth.Provider == cfg.Providers.GitHub { return github.GetUserInfoFromGitHub(client, user, customClaims, providerToken) } else if cfg.GenOAuth.Provider == cfg.Providers.OIDC { - return getUserInfoFromOpenID(client, user, customClaims, providerToken) + return openid.GetUserInfoFromOpenID(client, user, customClaims, providerToken) } log.Error("we don't know how to look up the user info") return nil } -func getUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { - userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) - if err != nil { - return err - } - defer func() { - if err := userinfo.Body.Close(); err != nil { - rerr = err - } - }() - data, _ := ioutil.ReadAll(userinfo.Body) - log.Infof("OpenID userinfo body: %s", string(data)) - if err = common.MapClaims(data, customClaims); err != nil { - log.Error(err) - return err - } - if err = json.Unmarshal(data, user); err != nil { - log.Error(err) - return err - } - user.PrepareUserData() - return nil -} - // the standard error // this is captured by nginx, which converts the 401 into 302 to the login page func error401(w http.ResponseWriter, r *http.Request, ae AuthError) { diff --git a/handlers/openid/openid.go b/handlers/openid/openid.go new file mode 100644 index 00000000..719052d3 --- /dev/null +++ b/handlers/openid/openid.go @@ -0,0 +1,39 @@ +package openid + +import ( + "encoding/json" + "github.com/vouch/vouch-proxy/handlers/common" + "github.com/vouch/vouch-proxy/pkg/cfg" + "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "io/ioutil" + "net/http" +) + +var ( + log = cfg.Cfg.Logger +) + +func GetUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { + userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) + if err != nil { + return err + } + defer func() { + if err := userinfo.Body.Close(); err != nil { + rerr = err + } + }() + data, _ := ioutil.ReadAll(userinfo.Body) + log.Infof("OpenID userinfo body: %s", string(data)) + if err = common.MapClaims(data, customClaims); err != nil { + log.Error(err) + return err + } + if err = json.Unmarshal(data, user); err != nil { + log.Error(err) + return err + } + user.PrepareUserData() + return nil +} From 02dd4dadb3dc98ecd9f34373b283c3a704c143e4 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 20:12:11 +0100 Subject: [PATCH 16/21] Refactor to common parameters for different vendor methods --- handlers/common/common.go | 26 ++++++++++++++++++++ handlers/github/github.go | 8 +++++-- handlers/google/google.go | 6 ++++- handlers/handlers.go | 32 +++++-------------------- handlers/homeassistant/homeassistant.go | 8 ++++++- handlers/indieauth/indieauth.go | 2 +- handlers/openid/openid.go | 7 ++++-- handlers/openstax/openstax.go | 7 ++++-- 8 files changed, 61 insertions(+), 35 deletions(-) diff --git a/handlers/common/common.go b/handlers/common/common.go index 6b9308fc..cd706184 100644 --- a/handlers/common/common.go +++ b/handlers/common/common.go @@ -1,15 +1,41 @@ package common import ( + "context" "encoding/json" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/structs" + "golang.org/x/oauth2" + "net/http" ) var ( log = cfg.Cfg.Logger ) +func PrepareTokensAndClient(r *http.Request, ptokens *structs.PTokens, setpid bool) (error, *http.Client, *oauth2.Token) { + providerToken, err := cfg.OAuthClient.Exchange(context.TODO(), r.URL.Query().Get("code")) + if err != nil { + return err, nil, nil + } + ptokens.PAccessToken = providerToken.AccessToken + + if setpid { + if providerToken.Extra("id_token") != nil { + // Certain providers (eg. gitea) don't provide an id_token + // and it's not neccessary for the authentication phase + ptokens.PIdToken = providerToken.Extra("id_token").(string) + } else { + log.Debugf("id_token missing - may not be supported by this provider") + } + } + + log.Debugf("ptokens: %+v", ptokens) + + client := cfg.OAuthClient.Client(context.TODO(), providerToken) + return err, client, providerToken +} + func MapClaims(claims []byte, customClaims *structs.CustomClaims) error { // Create a struct that contains the claims that we want to store from the config. var f interface{} diff --git a/handlers/github/github.go b/handlers/github/github.go index e3c7a45f..db8a7367 100644 --- a/handlers/github/github.go +++ b/handlers/github/github.go @@ -17,8 +17,12 @@ var ( // github // https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-authorization-options-for-oauth-apps/ -func GetUserInfoFromGitHub(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { - +func GetUserInfoFromGitHub(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, client, ptoken := common.PrepareTokensAndClient(r, ptokens, true) + if err != nil { + // http.Error(w, err.Error(), http.StatusBadRequest) + return err + } log.Errorf("ptoken.AccessToken: %s", ptoken.AccessToken) userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL + ptoken.AccessToken) if err != nil { diff --git a/handlers/google/google.go b/handlers/google/google.go index 95643413..aecd5485 100644 --- a/handlers/google/google.go +++ b/handlers/google/google.go @@ -13,7 +13,11 @@ var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromGoogle(client *http.Client, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { +func GetUserInfoFromGoogle(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, client, _ := common.PrepareTokensAndClient(r, ptokens, true) + if err != nil { + return err + } userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) if err != nil { return err diff --git a/handlers/handlers.go b/handlers/handlers.go index d9bc8f45..302ca65b 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -1,7 +1,6 @@ package handlers import ( - "context" "fmt" "github.com/vouch/vouch-proxy/handlers/adfs" "github.com/vouch/vouch-proxy/handlers/github" @@ -534,42 +533,23 @@ func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.Cust // indieauth sends the "me" setting in json back to the callback, so just pluck it from the callback if cfg.GenOAuth.Provider == cfg.Providers.IndieAuth { - return indieauth.GetUserInfoFromIndieAuth(r, user, customClaims) + return indieauth.GetUserInfoFromIndieAuth(r, user, customClaims, ptokens) } else if cfg.GenOAuth.Provider == cfg.Providers.ADFS { return adfs.GetUserInfoFromADFS(r, user, customClaims, ptokens) } - providerToken, err := cfg.OAuthClient.Exchange(context.TODO(), r.URL.Query().Get("code")) - if err != nil { - return err - } if cfg.GenOAuth.Provider == cfg.Providers.HomeAssistant { - ptokens.PAccessToken = providerToken.Extra("access_token").(string) - return homeassistant.GetUserInfoFromHomeAssistant(r, user, customClaims) + return homeassistant.GetUserInfoFromHomeAssistant(r, user, customClaims, ptokens) } - ptokens.PAccessToken = providerToken.AccessToken if cfg.GenOAuth.Provider == cfg.Providers.OpenStax { - client := cfg.OAuthClient.Client(context.TODO(), providerToken) - return openstax.GetUserInfoFromOpenStax(client, user, customClaims, providerToken) - } - - if providerToken.Extra("id_token") != nil { - // Certain providers (eg. gitea) don't provide an id_token - // and it's not neccessary for the authentication phase - ptokens.PIdToken = providerToken.Extra("id_token").(string) - } else { - log.Debugf("id_token missing - may not be supported by this provider") + return openstax.GetUserInfoFromOpenStax(r, user, customClaims, ptokens) } - log.Debugf("ptokens: %+v", ptokens) - - // make the "third leg" request back to provider to exchange the token for the userinfo - client := cfg.OAuthClient.Client(context.TODO(), providerToken) if cfg.GenOAuth.Provider == cfg.Providers.Google { - return google.GetUserInfoFromGoogle(client, user, customClaims) + return google.GetUserInfoFromGoogle(r, user, customClaims, ptokens) } else if cfg.GenOAuth.Provider == cfg.Providers.GitHub { - return github.GetUserInfoFromGitHub(client, user, customClaims, providerToken) + return github.GetUserInfoFromGitHub(r, user, customClaims, ptokens) } else if cfg.GenOAuth.Provider == cfg.Providers.OIDC { - return openid.GetUserInfoFromOpenID(client, user, customClaims, providerToken) + return openid.GetUserInfoFromOpenID(r, user, customClaims, ptokens) } log.Error("we don't know how to look up the user info") return nil diff --git a/handlers/homeassistant/homeassistant.go b/handlers/homeassistant/homeassistant.go index 1a0a7c41..1695fae7 100644 --- a/handlers/homeassistant/homeassistant.go +++ b/handlers/homeassistant/homeassistant.go @@ -1,12 +1,18 @@ package homeassistant import ( + "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/pkg/structs" "net/http" ) // More info: https://developers.home-assistant.io/docs/en/auth_api.html -func GetUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { +func GetUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, _, providerToken := common.PrepareTokensAndClient(r, ptokens, false) + if err != nil { + return err + } + ptokens.PAccessToken = providerToken.Extra("access_token").(string) // Home assistant does not provide an API to query username, so we statically set it to "homeassistant" user.Username = "homeassistant" return nil diff --git a/handlers/indieauth/indieauth.go b/handlers/indieauth/indieauth.go index f1c1d9ae..f0b97135 100644 --- a/handlers/indieauth/indieauth.go +++ b/handlers/indieauth/indieauth.go @@ -15,7 +15,7 @@ var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims) (rerr error) { +func GetUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { code := r.URL.Query().Get("code") log.Errorf("ptoken.AccessToken: %s", code) diff --git a/handlers/openid/openid.go b/handlers/openid/openid.go index 719052d3..61a9a0b4 100644 --- a/handlers/openid/openid.go +++ b/handlers/openid/openid.go @@ -5,7 +5,6 @@ import ( "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/structs" - "golang.org/x/oauth2" "io/ioutil" "net/http" ) @@ -14,7 +13,11 @@ var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromOpenID(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { +func GetUserInfoFromOpenID(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, client, _ := common.PrepareTokensAndClient(r, ptokens, true) + if err != nil { + return err + } userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) if err != nil { return err diff --git a/handlers/openstax/openstax.go b/handlers/openstax/openstax.go index 8052375c..583cc47a 100644 --- a/handlers/openstax/openstax.go +++ b/handlers/openstax/openstax.go @@ -5,7 +5,6 @@ import ( "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/structs" - "golang.org/x/oauth2" "io/ioutil" "net/http" ) @@ -14,7 +13,11 @@ var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromOpenStax(client *http.Client, user *structs.User, customClaims *structs.CustomClaims, ptoken *oauth2.Token) (rerr error) { +func GetUserInfoFromOpenStax(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, client, _ := common.PrepareTokensAndClient(r, ptokens, false) + if err != nil { + return err + } userinfo, err := client.Get(cfg.GenOAuth.UserInfoURL) if err != nil { return err From 1244cbdc2642b97e68ea565274e9cfec03188644 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 20:51:29 +0100 Subject: [PATCH 17/21] Use strategy pattern-like switch to select vendor-specific handler --- handlers/adfs/adfs.go | 4 +- handlers/github/github.go | 8 +++- handlers/github/github_test.go | 12 +++--- handlers/google/google.go | 4 +- handlers/handlers.go | 49 +++++++++++++------------ handlers/homeassistant/homeassistant.go | 4 +- handlers/indieauth/indieauth.go | 6 ++- handlers/openid/openid.go | 4 +- handlers/openstax/openstax.go | 4 +- 9 files changed, 58 insertions(+), 37 deletions(-) diff --git a/handlers/adfs/adfs.go b/handlers/adfs/adfs.go index 4280e839..f738b339 100644 --- a/handlers/adfs/adfs.go +++ b/handlers/adfs/adfs.go @@ -14,6 +14,8 @@ import ( "strings" ) +type Handler struct{} + type adfsTokenRes struct { AccessToken string `json:"access_token"` TokenType string `json:"token_type"` @@ -26,7 +28,7 @@ var ( ) // More info: https://docs.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-scenarios-for-developers#supported-scenarios -func GetUserInfoFromADFS(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { code := r.URL.Query().Get("code") log.Debugf("code: %s", code) diff --git a/handlers/github/github.go b/handlers/github/github.go index db8a7367..501996ed 100644 --- a/handlers/github/github.go +++ b/handlers/github/github.go @@ -11,14 +11,18 @@ import ( "strings" ) +type Handler struct { + PrepareTokensAndClient func(*http.Request, *structs.PTokens, bool) (error, *http.Client, *oauth2.Token) +} + var ( log = cfg.Cfg.Logger ) // github // https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-authorization-options-for-oauth-apps/ -func GetUserInfoFromGitHub(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { - err, client, ptoken := common.PrepareTokensAndClient(r, ptokens, true) +func (me Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + err, client, ptoken := me.PrepareTokensAndClient(r, ptokens, true) if err != nil { // http.Error(w, err.Error(), http.StatusBadRequest) return err diff --git a/handlers/github/github_test.go b/handlers/github/github_test.go index 8f7c2c2d..f4ec0f10 100644 --- a/handlers/github/github_test.go +++ b/handlers/github/github_test.go @@ -2,15 +2,14 @@ package github import ( "encoding/json" + mockhttp "github.com/karupanerura/go-mock-http-response" + "github.com/stretchr/testify/assert" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/domains" "github.com/vouch/vouch-proxy/pkg/structs" "golang.org/x/oauth2" "net/http" "regexp" - - mockhttp "github.com/karupanerura/go-mock-http-response" - "github.com/stretchr/testify/assert" "testing" ) @@ -156,7 +155,7 @@ func TestGetOrgMembershipStateFromGitHubNoOrgAccess(t *testing.T) { assertUrlCalled(t, expectedOrgPublicMembershipUrl) } -func TestGetUserInfoFromGitHub(t *testing.T) { +func TestGetUserInfo(t *testing.T) { setUp() userInfoContent, _ := json.Marshal(structs.GitHubUser{ @@ -178,7 +177,10 @@ func TestGetUserInfoFromGitHub(t *testing.T) { mockResponse(regexMatcher(".*teams.*"), http.StatusOK, map[string]string{}, []byte("{\"state\": \"active\"}")) mockResponse(regexMatcher(".*members.*"), http.StatusNoContent, map[string]string{}, []byte("")) - err := GetUserInfoFromGitHub(client, user, &structs.CustomClaims{}, token) + handler := Handler{PrepareTokensAndClient: func(_ *http.Request, _ *structs.PTokens, _ bool) (error, *http.Client, *oauth2.Token) { + return nil, client, token + }} + err := handler.GetUserInfo(nil, user, &structs.CustomClaims{}, &structs.PTokens{}) assert.Nil(t, err) assert.Equal(t, "myusername", user.Username) diff --git a/handlers/google/google.go b/handlers/google/google.go index aecd5485..cfcd0d18 100644 --- a/handlers/google/google.go +++ b/handlers/google/google.go @@ -9,11 +9,13 @@ import ( "net/http" ) +type Handler struct{} + var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromGoogle(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { err, client, _ := common.PrepareTokensAndClient(r, ptokens, true) if err != nil { return err diff --git a/handlers/handlers.go b/handlers/handlers.go index 302ca65b..45f94222 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -3,6 +3,7 @@ package handlers import ( "fmt" "github.com/vouch/vouch-proxy/handlers/adfs" + "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/handlers/github" "github.com/vouch/vouch-proxy/handlers/google" "github.com/vouch/vouch-proxy/handlers/homeassistant" @@ -43,6 +44,10 @@ type AuthError struct { JWT string } +type Handler interface { + GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) error +} + const ( base64Bytes = 32 ) @@ -527,32 +532,30 @@ func CallbackHandler(w http.ResponseWriter, r *http.Request) { renderIndex(w, "/auth "+tokenstring) } -// TODO: put all getUserInfo logic into its own pkg - func getUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) error { + return getHandler().GetUserInfo(r, user, customClaims, ptokens) +} - // indieauth sends the "me" setting in json back to the callback, so just pluck it from the callback - if cfg.GenOAuth.Provider == cfg.Providers.IndieAuth { - return indieauth.GetUserInfoFromIndieAuth(r, user, customClaims, ptokens) - } else if cfg.GenOAuth.Provider == cfg.Providers.ADFS { - return adfs.GetUserInfoFromADFS(r, user, customClaims, ptokens) - } - if cfg.GenOAuth.Provider == cfg.Providers.HomeAssistant { - return homeassistant.GetUserInfoFromHomeAssistant(r, user, customClaims, ptokens) - } - if cfg.GenOAuth.Provider == cfg.Providers.OpenStax { - return openstax.GetUserInfoFromOpenStax(r, user, customClaims, ptokens) - } - - if cfg.GenOAuth.Provider == cfg.Providers.Google { - return google.GetUserInfoFromGoogle(r, user, customClaims, ptokens) - } else if cfg.GenOAuth.Provider == cfg.Providers.GitHub { - return github.GetUserInfoFromGitHub(r, user, customClaims, ptokens) - } else if cfg.GenOAuth.Provider == cfg.Providers.OIDC { - return openid.GetUserInfoFromOpenID(r, user, customClaims, ptokens) +func getHandler() Handler { + switch cfg.GenOAuth.Provider { + case cfg.Providers.IndieAuth: + return indieauth.Handler{} + case cfg.Providers.ADFS: + return adfs.Handler{} + case cfg.Providers.HomeAssistant: + return homeassistant.Handler{} + case cfg.Providers.OpenStax: + return openstax.Handler{} + case cfg.Providers.Google: + return google.Handler{} + case cfg.Providers.GitHub: + return github.Handler{common.PrepareTokensAndClient} + case cfg.Providers.OIDC: + return openid.Handler{} + default: + log.Error("we don't know how to look up the user info") + return nil } - log.Error("we don't know how to look up the user info") - return nil } // the standard error diff --git a/handlers/homeassistant/homeassistant.go b/handlers/homeassistant/homeassistant.go index 1695fae7..597e4582 100644 --- a/handlers/homeassistant/homeassistant.go +++ b/handlers/homeassistant/homeassistant.go @@ -6,8 +6,10 @@ import ( "net/http" ) +type Handler struct{} + // More info: https://developers.home-assistant.io/docs/en/auth_api.html -func GetUserInfoFromHomeAssistant(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { err, _, providerToken := common.PrepareTokensAndClient(r, ptokens, false) if err != nil { return err diff --git a/handlers/indieauth/indieauth.go b/handlers/indieauth/indieauth.go index f0b97135..53897d58 100644 --- a/handlers/indieauth/indieauth.go +++ b/handlers/indieauth/indieauth.go @@ -11,12 +11,14 @@ import ( "net/http" ) +type Handler struct{} + var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromIndieAuth(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { - +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { + // indieauth sends the "me" setting in json back to the callback, so just pluck it from the callback code := r.URL.Query().Get("code") log.Errorf("ptoken.AccessToken: %s", code) var b bytes.Buffer diff --git a/handlers/openid/openid.go b/handlers/openid/openid.go index 61a9a0b4..3b6025a3 100644 --- a/handlers/openid/openid.go +++ b/handlers/openid/openid.go @@ -9,11 +9,13 @@ import ( "net/http" ) +type Handler struct{} + var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromOpenID(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { err, client, _ := common.PrepareTokensAndClient(r, ptokens, true) if err != nil { return err diff --git a/handlers/openstax/openstax.go b/handlers/openstax/openstax.go index 583cc47a..60212668 100644 --- a/handlers/openstax/openstax.go +++ b/handlers/openstax/openstax.go @@ -9,11 +9,13 @@ import ( "net/http" ) +type Handler struct{} + var ( log = cfg.Cfg.Logger ) -func GetUserInfoFromOpenStax(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { +func (Handler) GetUserInfo(r *http.Request, user *structs.User, customClaims *structs.CustomClaims, ptokens *structs.PTokens) (rerr error) { err, client, _ := common.PrepareTokensAndClient(r, ptokens, false) if err != nil { return err From 6e05f94247b28f4493efa748e2d3a589a2d9a8ae Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Sun, 9 Feb 2020 21:13:07 +0100 Subject: [PATCH 18/21] Add org/team configuration relevant urls to github enterprise sample config --- config/config.yml_example_github_enterprise | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/config/config.yml_example_github_enterprise b/config/config.yml_example_github_enterprise index a1bea492..27b808d9 100644 --- a/config/config.yml_example_github_enterprise +++ b/config/config.yml_example_github_enterprise @@ -15,6 +15,15 @@ vouch: # instead of setting specific domains you may prefer to allow all users... # set allowAllUsers: true to use Vouch Proxy to just accept anyone who can authenticate at the configured provider # allowAllUsers: true + # set teamWhitelist: to list of teams and/or GitHub organizations + # When putting an organization id without a slash, it will allow all (public) members from the organization. + # The client will try to read the private organization membership using the client credentials, if that's not possible + # due to access restriction, it will try to evaluate the publicly visible membership. + # Allowing members form a specific team can be configured by qualifying the team with the organization, separated by + # a slash. + # teamWhitelist: + # - myOrg + # - myOrg/myTeam oauth: # create a new OAuth application at: @@ -25,6 +34,10 @@ oauth: auth_url: https://githubenterprise.yoursite.com/login/oauth/authorize token_url: https://githubenterprise.yoursite.com/login/oauth/access_token user_info_url: https://githubenterprise.yoursite.com/api/v3/user?access_token= + # relevant only if teamWhitelist is configured; colon-prefixed parts are parameters that + # will be replaced with the respective values. + user_team_url: https://githubenterprise.yoursite.com/api/v3/orgs/:org_id/teams/:team_slug/memberships/:username?access_token= + user_org_url: https://githubenterprise.yoursite.com/api/v3/orgs/:org_id/members/:username?access_token= # these GitHub OAuth defaults are set for you.. # scopes: # - user From 785ec9fb2d98a478f55ec1d6d74e0fd4eecfceef Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Mon, 10 Feb 2020 09:34:37 +0100 Subject: [PATCH 19/21] Add read:org scope if team whitelist is configured for github --- config/config.yml_example_github | 1 + config/config.yml_example_github_enterprise | 2 ++ pkg/cfg/cfg.go | 4 ++++ pkg/cfg/cfg_test.go | 16 ++++++++++++++++ 4 files changed, 23 insertions(+) diff --git a/config/config.yml_example_github b/config/config.yml_example_github index 5f74a975..0e7c051e 100644 --- a/config/config.yml_example_github +++ b/config/config.yml_example_github @@ -24,6 +24,7 @@ vouch: # teamWhitelist: # - myOrg # - myOrg/myTeam + # In case both vouch.teamWhitelist AND oauth.scopes is configured, make sure read:org scope is included oauth: # create a new OAuth application at: diff --git a/config/config.yml_example_github_enterprise b/config/config.yml_example_github_enterprise index 27b808d9..384e721b 100644 --- a/config/config.yml_example_github_enterprise +++ b/config/config.yml_example_github_enterprise @@ -24,6 +24,7 @@ vouch: # teamWhitelist: # - myOrg # - myOrg/myTeam + # In case both vouch.teamWhitelist AND oauth.scopes is configured, make sure read:org scope is included oauth: # create a new OAuth application at: @@ -41,3 +42,4 @@ oauth: # these GitHub OAuth defaults are set for you.. # scopes: # - user + # In case both vouch.teamWhitelist AND oauth.scopes is configured, make sure read:org scope is included diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 495f8142..b4f76ca0 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -645,6 +645,10 @@ func setDefaultsGitHub() { // https://github.com/vouch/vouch-proxy/issues/63 // https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ GenOAuth.Scopes = []string{"read:user"} + + if len(Cfg.TeamWhiteList) > 0 { + GenOAuth.Scopes = append(GenOAuth.Scopes, "read:org") + } } } diff --git a/pkg/cfg/cfg_test.go b/pkg/cfg/cfg_test.go index 40e1efda..fe2ce6ba 100644 --- a/pkg/cfg/cfg_test.go +++ b/pkg/cfg/cfg_test.go @@ -23,3 +23,19 @@ func TestConfigParsing(t *testing.T) { assert.NotEmpty(t, Cfg.JWT.MaxAge) } + +func TestSetGitHubDefaults(t *testing.T) { + InitForTestPurposesWithProvider("github") + + assert.Equal(t, []string{"read:user"}, GenOAuth.Scopes) +} + +func TestSetGitHubDefaultsWithTeamWhitelist(t *testing.T) { + InitForTestPurposesWithProvider("github") + Cfg.TeamWhiteList = append(Cfg.TeamWhiteList, "org/team") + GenOAuth.Scopes = []string{} + + setDefaultsGitHub() + assert.Contains(t, GenOAuth.Scopes, "read:user") + assert.Contains(t, GenOAuth.Scopes, "read:org") +} From e26ea0c3e88c2eac4c93b0f14b29eff0a54ce9b3 Mon Sep 17 00:00:00 2001 From: Eike Hartmann Date: Mon, 10 Feb 2020 21:44:30 +0100 Subject: [PATCH 20/21] Improve logging and error handling in github org/team membership retrieval --- handlers/github/github.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/handlers/github/github.go b/handlers/github/github.go index 501996ed..3d8f3f00 100644 --- a/handlers/github/github.go +++ b/handlers/github/github.go @@ -2,6 +2,7 @@ package github import ( "encoding/json" + "errors" "github.com/vouch/vouch-proxy/handlers/common" "github.com/vouch/vouch-proxy/pkg/cfg" "github.com/vouch/vouch-proxy/pkg/structs" @@ -123,11 +124,14 @@ func getOrgMembershipStateFromGitHub(client *http.Client, user *structs.User, or } if orgMembershipResp.StatusCode == 204 { + log.Debug("getOrgMembershipStateFromGitHub isMember: true") return nil, true } else if orgMembershipResp.StatusCode == 404 { + log.Debug("getOrgMembershipStateFromGitHub isMember: false") return nil, false } else { - return nil, false + log.Errorf("getOrgMembershipStateFromGitHub: unexpected status code %d", orgMembershipResp.StatusCode) + return errors.New("Unexpected response status " + orgMembershipResp.Status), false } } @@ -154,7 +158,11 @@ func getTeamMembershipStateFromGitHub(client *http.Client, user *structs.User, o log.Debug("getTeamMembershipStateFromGitHub ghTeamState") log.Debug(ghTeamState) return nil, ghTeamState.State == "active" - } else { + } else if membershipStateResp.StatusCode == 404 { + log.Debug("getTeamMembershipStateFromGitHub isMember: false") return nil, false + } else { + log.Errorf("getTeamMembershipStateFromGitHub: unexpected status code %d", membershipStateResp.StatusCode) + return errors.New("Unexpected response status " + membershipStateResp.Status), false } } From 35dbe19fe7e18f8b658bf674f0b07cc62780f541 Mon Sep 17 00:00:00 2001 From: Benjamin Foote Date: Wed, 11 Mar 2020 14:54:52 -0700 Subject: [PATCH 21/21] remove port before testing host --- pkg/domains/domains.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/domains/domains.go b/pkg/domains/domains.go index 1f768189..1b5527dc 100644 --- a/pkg/domains/domains.go +++ b/pkg/domains/domains.go @@ -23,8 +23,15 @@ func Refresh() { // TODO return all matches // Matches return the first match of the func Matches(s string) string { + if strings.Contains(s, ":") { + // then we have a port and we just want to check the host + split := strings.Split(s, ":") + log.Debugf("removing port from %s to test domain %s", s, split[0]) + s = split[0] + } + for i, v := range domains { - if s == v || strings.HasSuffix(s, "." + v) { + if s == v || strings.HasSuffix(s, "."+v) { log.Debugf("domain %s matched array value at [%d]=%v", s, i, v) return v }