centralize oauth http calls, validate response status (#8470)

pkg/api/login_oauth.go

@@ -28,6 +28,7 @@ var (
 	ErrEmailNotAllowed       = errors.New("Required email domain not fulfilled")
 	ErrSignUpNotAllowed      = errors.New("Signup is not allowed for this adapter")
 	ErrUsersQuotaReached     = errors.New("Users quota reached")
+	ErrNoEmail               = errors.New("Login provider didn't return an email address")
 )
 
 func GenStateString() string {
@@ -134,6 +135,12 @@ func OAuthLogin(ctx *middleware.Context) {
 
 	ctx.Logger.Debug("OAuthLogin got user info", "userInfo", userInfo)
 
+	// validate that we got at least an email address
+	if userInfo.Email == "" {
+		redirectWithError(ctx, ErrNoEmail)
+		return
+	}
+
 	// validate that the email is allowed to login to grafana
 	if !connect.IsEmailAllowed(userInfo.Email) {
 		redirectWithError(ctx, ErrEmailNotAllowed)

pkg/social/common.go

@@ -2,7 +2,11 @@ package social
 
 import (
 	"fmt"
+	"io/ioutil"
+	"net/http"
 	"strings"
+
+	"github.com/grafana/grafana/pkg/log"
 )
 
 func isEmailAllowed(email string, allowedDomains []string) bool {
@@ -18,3 +22,25 @@ func isEmailAllowed(email string, allowedDomains []string) bool {
 
 	return valid
 }
+
+func HttpGet(client *http.Client, url string) ([]byte, error) {
+	r, err := client.Get(url)
+	if err != nil {
+		return nil, err
+	}
+
+	defer r.Body.Close()
+
+	body, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	if r.StatusCode >= 300 {
+		return nil, fmt.Errorf(string(body))
+	}
+
+	log.Trace("HTTP GET %s: %s %s", url, r.Status, string(body))
+
+	return body, nil
+}

pkg/social/generic_oauth.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -84,22 +83,14 @@ func (s *GenericOAuth) FetchPrivateEmail(client *http.Client) (string, error) {
 		IsConfirmed bool   `json:"is_confirmed"`
 	}
 
-	emailsUrl := fmt.Sprintf(s.apiUrl + "/emails")
-	r, err := client.Get(emailsUrl)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/emails"))
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("Error getting email address: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	body, err := ioutil.ReadAll(r.Body)
-	if err != nil {
-		return "", err
-	}
-
-	err = json.Unmarshal(body, records)
+	err = json.Unmarshal(body, &records)
 	if err != nil {
 		var data struct {
 			Values []Record `json:"values"`
@@ -107,7 +98,7 @@ func (s *GenericOAuth) FetchPrivateEmail(client *http.Client) (string, error) {
 
 		err = json.Unmarshal(body, &data)
 		if err != nil {
-			return "", err
+			return "", fmt.Errorf("Error getting email address: %s", err)
 		}
 
 		records = data.Values
@@ -129,18 +120,16 @@ func (s *GenericOAuth) FetchTeamMemberships(client *http.Client) ([]int, error)
 		Id int `json:"id"`
 	}
 
-	membershipUrl := fmt.Sprintf(s.apiUrl + "/teams")
-	r, err := client.Get(membershipUrl)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/teams"))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting team memberships: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	if err = json.NewDecoder(r.Body).Decode(&records); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &records)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting team memberships: %s", err)
 	}
 
 	var ids = make([]int, len(records))
@@ -156,18 +145,16 @@ func (s *GenericOAuth) FetchOrganizations(client *http.Client) ([]string, error)
 		Login string `json:"login"`
 	}
 
-	url := fmt.Sprintf(s.apiUrl + "/orgs")
-	r, err := client.Get(url)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/orgs"))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting organizations: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	if err = json.NewDecoder(r.Body).Decode(&records); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &records)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting organizations: %s", err)
 	}
 
 	var logins = make([]string, len(records))
@@ -188,16 +175,14 @@ func (s *GenericOAuth) UserInfo(client *http.Client) (*BasicUserInfo, error) {
 		Attributes  map[string][]string `json:"attributes"`
 	}
 
-	var err error
-	r, err := client.Get(s.apiUrl)
+	body, err := HttpGet(client, s.apiUrl)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
-	defer r.Body.Close()
-
-	if err = json.NewDecoder(r.Body).Decode(&data); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &data)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
 	userInfo := &BasicUserInfo{

pkg/social/github_oauth.go

@@ -85,18 +85,16 @@ func (s *SocialGithub) FetchPrivateEmail(client *http.Client) (string, error) {
 		Verified bool   `json:"verified"`
 	}
 
-	emailsUrl := fmt.Sprintf(s.apiUrl + "/emails")
-	r, err := client.Get(emailsUrl)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/emails"))
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("Error getting email address: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	if err = json.NewDecoder(r.Body).Decode(&records); err != nil {
-		return "", err
+	err = json.Unmarshal(body, &records)
+	if err != nil {
+		return "", fmt.Errorf("Error getting email address: %s", err)
 	}
 
 	var email = ""
@@ -114,18 +112,16 @@ func (s *SocialGithub) FetchTeamMemberships(client *http.Client) ([]int, error)
 		Id int `json:"id"`
 	}
 
-	membershipUrl := fmt.Sprintf(s.apiUrl + "/teams")
-	r, err := client.Get(membershipUrl)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/teams"))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting team memberships: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	if err = json.NewDecoder(r.Body).Decode(&records); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &records)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting team memberships: %s", err)
 	}
 
 	var ids = make([]int, len(records))
@@ -141,18 +137,16 @@ func (s *SocialGithub) FetchOrganizations(client *http.Client) ([]string, error)
 		Login string `json:"login"`
 	}
 
-	url := fmt.Sprintf(s.apiUrl + "/orgs")
-	r, err := client.Get(url)
+	body, err := HttpGet(client, fmt.Sprintf(s.apiUrl+"/orgs"))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting organizations: %s", err)
 	}
 
-	defer r.Body.Close()
-
 	var records []Record
 
-	if err = json.NewDecoder(r.Body).Decode(&records); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &records)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting organizations: %s", err)
 	}
 
 	var logins = make([]string, len(records))
@@ -170,16 +164,14 @@ func (s *SocialGithub) UserInfo(client *http.Client) (*BasicUserInfo, error) {
 		Email string `json:"email"`
 	}
 
-	var err error
-	r, err := client.Get(s.apiUrl)
+	body, err := HttpGet(client, s.apiUrl)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
-	defer r.Body.Close()
-
-	if err = json.NewDecoder(r.Body).Decode(&data); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &data)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
 	userInfo := &BasicUserInfo{

pkg/social/google_oauth.go

@@ -2,6 +2,7 @@ package social
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -34,16 +35,17 @@ func (s *SocialGoogle) UserInfo(client *http.Client) (*BasicUserInfo, error) {
 		Name  string `json:"name"`
 		Email string `json:"email"`
 	}
-	var err error
 
-	r, err := client.Get(s.apiUrl)
+	body, err := HttpGet(client, s.apiUrl)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
-	defer r.Body.Close()
-	if err = json.NewDecoder(r.Body).Decode(&data); err != nil {
-		return nil, err
+
+	err = json.Unmarshal(body, &data)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
+
 	return &BasicUserInfo{
 		Name:  data.Name,
 		Email: data.Email,

pkg/social/grafana_com_oauth.go

@@ -2,6 +2,7 @@ package social
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -57,16 +58,14 @@ func (s *SocialGrafanaCom) UserInfo(client *http.Client) (*BasicUserInfo, error)
 		Orgs  []OrgRecord `json:"orgs"`
 	}
 
-	var err error
-	r, err := client.Get(s.url + "/api/oauth2/user")
+	body, err := HttpGet(client, s.url+"/api/oauth2/user")
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
-	defer r.Body.Close()
-
-	if err = json.NewDecoder(r.Body).Decode(&data); err != nil {
-		return nil, err
+	err = json.Unmarshal(body, &data)
+	if err != nil {
+		return nil, fmt.Errorf("Error getting user info: %s", err)
 	}
 
 	userInfo := &BasicUserInfo{

public/views/500.html

@@ -4,33 +4,21 @@
     <meta charset="utf-8">
     <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
     <meta name="viewport" content="width=device-width">
-
     <title>Grafana - Error</title>
-
-		<link href='[[.AppSubUrl]]/public/css/fonts.min.css' rel='stylesheet' type='text/css'>
-
-		<link rel="stylesheet" href="[[.AppSubUrl]]/public/css/grafana.dark.min.css">
-
+    <link href='[[.AppSubUrl]]/public/css/fonts.min.css' rel='stylesheet' type='text/css'>
+    <link rel="stylesheet" href="[[.AppSubUrl]]/public/css/grafana.dark.min.css">
     <link rel="icon" type="image/png" href="[[.AppSubUrl]]/public/img/fav32.png">
-
-		<base href="[[.AppSubUrl]]/" />
-
+    <base href="[[.AppSubUrl]]/" />
   </head>
-
-	<body>
-
-		<div class="page-container">
-			<div class="page-header">
-				<h1>
-					Server side error :(
-				</h1>
-			</div>
-
-			<h4>[[.Title]]</h4>
-
-			<pre>[[.ErrorMsg]]</pre>
-
-		</div>
-	</body>
-
+  <body>
+    <div class="page-container">
+      <div class="page-header">
+        <h1>Server side error :(</h1>
+      </div>
+      <h4>[[.Title]]</h4>
+      [[if .ErrorMsg]]
+        <pre>[[.ErrorMsg]]</pre>
+      [[end]]
+    </div>
+  </body>
 </html>