Fix the github oauth2 implementation with multiple emails

If an account had multiple emails, the current implementation would
always select the first one regardless of any other settings. This fixes
it so it only chooses the primary email address that is verified.

This also fixes the generic oauth2 to require verified and primary to be
true if they are present. If they are not present, they are not
required.

CHANGELOG.md

@@ -31,6 +31,7 @@
 1.  [#3128](https://github.com/influxdata/chronograf/pull/3128): Fix type error bug in Kapacitor Alert Config page and persist deleting of team and recipient in OpsGenieConfig
 1.  [#3149](https://github.com/influxdata/chronograf/pull/3149): Fixes errors caused by switching query tabs in CEO
 1.  [#3162](https://github.com/influxdata/chronograf/pull/3162): Only send threshold value to parent on blur
+1.  [#3168](https://github.com/influxdata/chronograf/pull/3168): Fix the github oauth2 implementation with multiple emails
 
 ## v1.4.3.1 [2018-04-02]
 

oauth2/generic.go

@@ -165,6 +165,28 @@ type UserEmail struct {
 	Verified *bool   `json:"verified,omitempty"`
 }
 
+// GetPrimary returns if the email is the primary email.
+// If primary is not present, all emails are considered the primary.
+func (u *UserEmail) GetPrimary() bool {
+	if u == nil {
+		return false
+	} else if u.Primary == nil {
+		return true
+	}
+	return *u.Primary
+}
+
+// GetVerified returns if the email has been verified.
+// If verified is not present, all emails are considered verified.
+func (u *UserEmail) GetVerified() bool {
+	if u == nil {
+		return false
+	} else if u.Verified == nil {
+		return true
+	}
+	return *u.Verified
+}
+
 // getPrimaryEmail gets the private email account for the authenticated user.
 func (g *Generic) getPrimaryEmail(client *http.Client) (string, error) {
 	emailsEndpoint := g.APIURL + "/emails"
@@ -189,7 +211,7 @@ func (g *Generic) getPrimaryEmail(client *http.Client) (string, error) {
 
 func (g *Generic) primaryEmail(emails []*UserEmail) (string, error) {
 	for _, m := range emails {
-		if m != nil && m.Primary != nil && m.Verified != nil && m.Email != nil {
+		if m != nil && m.GetPrimary() && m.GetVerified() && m.Email != nil {
 			return *m.Email, nil
 		}
 	}

oauth2/generic_test.go

@@ -155,7 +155,9 @@ func TestGenericPrincipalIDDomain(t *testing.T) {
 		Primary  bool   `json:"primary"`
 		Verified bool   `json:"verified"`
 	}{
-		{"martymcfly@pinheads.rok", true, false},
+		{"mcfly@example.com", false, true},
+		{"martymcspelledwrong@example.com", false, false},
+		{"martymcfly@pinheads.rok", true, true},
 	}
 	mockAPI := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/" {

oauth2/github.go

@@ -26,22 +26,22 @@ type Github struct {
 	Logger       chronograf.Logger
 }
 
-// Name is the name of the provider
+// Name is the name of the provider.
 func (g *Github) Name() string {
 	return "github"
 }
 
-// ID returns the github application client id
+// ID returns the github application client id.
 func (g *Github) ID() string {
 	return g.ClientID
 }
 
-// Secret returns the github application client secret
+// Secret returns the github application client secret.
 func (g *Github) Secret() string {
 	return g.ClientSecret
 }
 
-// Scopes for github is only the email addres and possible organizations if
+// Scopes for github is only the email address and possible organizations if
 // we are filtering by organizations.
 func (g *Github) Scopes() []string {
 	scopes := []string{"user:email"}
@@ -51,7 +51,7 @@ func (g *Github) Scopes() []string {
 	return scopes
 }
 
-// Config is the Github OAuth2 exchange information and endpoints
+// Config is the Github OAuth2 exchange information and endpoints.
 func (g *Github) Config() *oauth2.Config {
 	return &oauth2.Config{
 		ClientID:     g.ID(),
@@ -122,7 +122,7 @@ func logResponseError(log chronograf.Logger, resp *github.Response, err error) {
 	}
 }
 
-// isMember makes sure that the user is in one of the required organizations
+// isMember makes sure that the user is in one of the required organizations.
 func isMember(requiredOrgs []string, userOrgs []*github.Organization) bool {
 	for _, requiredOrg := range requiredOrgs {
 		for _, userOrg := range userOrgs {
@@ -134,7 +134,7 @@ func isMember(requiredOrgs []string, userOrgs []*github.Organization) bool {
 	return false
 }
 
-// getOrganizations gets all organization for the currently authenticated user
+// getOrganizations gets all organization for the currently authenticated user.
 func getOrganizations(client *github.Client, log chronograf.Logger) ([]*github.Organization, error) {
 	// Get all pages of results
 	var allOrgs []*github.Organization
@@ -175,9 +175,23 @@ func getPrimaryEmail(client *github.Client, log chronograf.Logger) (string, erro
 
 func primaryEmail(emails []*github.UserEmail) (string, error) {
 	for _, m := range emails {
-		if m != nil && m.Primary != nil && m.Verified != nil && m.Email != nil {
+		if m != nil && getPrimary(m) && getVerified(m) && m.Email != nil {
 			return *m.Email, nil
 		}
 	}
 	return "", errors.New("No primary email address")
 }
+
+func getPrimary(m *github.UserEmail) bool {
+	if m == nil || m.Primary == nil {
+		return false
+	}
+	return *m.Primary
+}
+
+func getVerified(m *github.UserEmail) bool {
+	if m == nil || m.Verified == nil {
+		return false
+	}
+	return *m.Verified
+}

oauth2/github_test.go

@@ -18,7 +18,9 @@ func TestGithubPrincipalID(t *testing.T) {
 		Primary  bool   `json:"primary"`
 		Verified bool   `json:"verified"`
 	}{
-		{"martymcfly@example.com", true, false},
+		{"mcfly@example.com", false, true},
+		{"martymcspelledwrong@example.com", false, false},
+		{"martymcfly@example.com", true, true},
 	}
 	mockAPI := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 		if r.URL.Path != "/user/emails" {
@@ -50,8 +52,8 @@ func TestGithubPrincipalID(t *testing.T) {
 		t.Fatal("Unexpected error while retrieiving PrincipalID: err:", err)
 	}
 
-	if email != expected[0].Email {
-		t.Fatal("Retrieved email was not as expected. Want:", expected[0].Email, "Got:", email)
+	if got, want := email, "martymcfly@example.com"; got != want {
+		t.Fatal("Retrieved email was not as expected. Want:", want, "Got:", got)
 	}
 }
 
@@ -63,7 +65,7 @@ func TestGithubPrincipalIDOrganization(t *testing.T) {
 		Primary  bool   `json:"primary"`
 		Verified bool   `json:"verified"`
 	}{
-		{"martymcfly@example.com", true, false},
+		{"martymcfly@example.com", true, true},
 	}
 	expectedOrg := []struct {
 		Login string `json:"login"`