Merge pull request #11148 from bergquist/v5.0.x

Cherry pick #11145 and #11127

pkg/services/alerting/extractor.go

@@ -143,10 +143,15 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
 
 		// validate
 		_, err = NewRuleFromDBAlert(alert)
-		if err == nil && alert.ValidToSave() {
+		if err != nil {
+			return nil, err
+		}
+
+		if alert.ValidToSave() {
 			alerts = append(alerts, alert)
 		} else {
-			return nil, err
+			e.log.Debug("Invalid Alert Data. Dashboard, Org or Panel ID is not correct", "alertName", alert.Name, "panelId", alert.PanelId)
+			return nil, m.ErrDashboardContainsInvalidAlertData
 		}
 	}
 

pkg/services/alerting/extractor_test.go

@@ -150,6 +150,22 @@ func TestAlertRuleExtraction(t *testing.T) {
 			})
 		})
 
+		Convey("Panel with id set to zero should return error", func() {
+			panelWithIdZero, err := ioutil.ReadFile("./test-data/panel-with-id-0.json")
+			So(err, ShouldBeNil)
+
+			dashJson, err := simplejson.NewJson([]byte(panelWithIdZero))
+			So(err, ShouldBeNil)
+			dash := m.NewDashboardFromJson(dashJson)
+			extractor := NewDashAlertExtractor(dash, 1)
+
+			_, err = extractor.GetAlerts()
+
+			Convey("panel with id 0 should return error", func() {
+				So(err, ShouldNotBeNil)
+			})
+		})
+
 		Convey("Parse alerts from dashboard without rows", func() {
 			json, err := ioutil.ReadFile("./test-data/v5-dashboard.json")
 			So(err, ShouldBeNil)

pkg/services/alerting/test-data/panel-with-id-0.json

@@ -0,0 +1,63 @@
+{
+  "id": 57,
+  "title": "Graphite 4",
+  "originalTitle": "Graphite 4",
+  "tags": ["graphite"],
+  "rows": [
+  {
+    "panels": [
+    {
+      "title": "Active desktop users",
+      "id": 0,
+      "editable": true,
+      "type": "graph",
+      "targets": [
+      {
+        "refId": "A",
+        "target": "aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)"
+      }
+      ],
+      "datasource": null,
+      "alert": {
+        "name": "name1",
+        "message": "desc1",
+        "handler": 1,
+        "frequency": "60s",
+        "conditions": [
+        {
+          "type": "query",
+          "query": {"params": ["A", "5m", "now"]},
+          "reducer": {"type": "avg", "params": []},
+          "evaluator": {"type": ">", "params": [100]}
+        }
+        ]
+      }
+    },
+    {
+      "title": "Active mobile users",
+      "id": 4,
+      "targets": [
+        {"refId": "A", "target": ""},
+        {"refId": "B", "target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}
+      ],
+      "datasource": "graphite2",
+      "alert": {
+        "name": "name2",
+        "message": "desc2",
+        "handler": 0,
+        "frequency": "60s",
+        "severity": "warning",
+        "conditions": [
+        {
+          "type": "query",
+          "query":  {"params": ["B", "5m", "now"]},
+          "reducer": {"type": "avg", "params": []},
+          "evaluator": {"type": ">", "params": [100]}
+        }
+        ]
+      }
+    }
+    ]
+  }
+]
+      }

pkg/social/generic_oauth.go

@@ -180,6 +180,7 @@ type UserInfoJson struct {
 
 func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token) (*BasicUserInfo, error) {
 	var data UserInfoJson
+	var err error
 
 	if s.extractToken(&data, token) != true {
 		response, err := HttpGet(client, s.apiUrl)
@@ -193,20 +194,17 @@ func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token)
 		}
 	}
 
-	name, err := s.extractName(data)
-	if err != nil {
-		return nil, err
+	name := s.extractName(&data)
+
+	email := s.extractEmail(&data)
+	if email == "" {
+		email, err = s.FetchPrivateEmail(client)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	email, err := s.extractEmail(data, client)
-	if err != nil {
-		return nil, err
-	}
-
-	login, err := s.extractLogin(data, email)
-	if err != nil {
-		return nil, err
-	}
+	login := s.extractLogin(&data, email)
 
 	userInfo := &BasicUserInfo{
 		Name:  name,
@@ -251,49 +249,55 @@ func (s *SocialGenericOAuth) extractToken(data *UserInfoJson, token *oauth2.Toke
 		return false
 	}
 
+	email := s.extractEmail(data)
+	if email == "" {
+		s.log.Debug("No email found in id_token", "json", string(payload), "data", data)
+		return false
+	}
+
 	s.log.Debug("Received id_token", "json", string(payload), "data", data)
 	return true
 }
 
-func (s *SocialGenericOAuth) extractEmail(data UserInfoJson, client *http.Client) (string, error) {
+func (s *SocialGenericOAuth) extractEmail(data *UserInfoJson) string {
 	if data.Email != "" {
-		return data.Email, nil
+		return data.Email
 	}
 
 	if data.Attributes["email:primary"] != nil {
-		return data.Attributes["email:primary"][0], nil
+		return data.Attributes["email:primary"][0]
 	}
 
 	if data.Upn != "" {
 		emailAddr, emailErr := mail.ParseAddress(data.Upn)
 		if emailErr == nil {
-			return emailAddr.Address, nil
+			return emailAddr.Address
 		}
 	}
 
-	return s.FetchPrivateEmail(client)
+	return ""
 }
 
-func (s *SocialGenericOAuth) extractLogin(data UserInfoJson, email string) (string, error) {
+func (s *SocialGenericOAuth) extractLogin(data *UserInfoJson, email string) string {
 	if data.Login != "" {
-		return data.Login, nil
+		return data.Login
 	}
 
 	if data.Username != "" {
-		return data.Username, nil
+		return data.Username
 	}
 
-	return email, nil
+	return email
 }
 
-func (s *SocialGenericOAuth) extractName(data UserInfoJson) (string, error) {
+func (s *SocialGenericOAuth) extractName(data *UserInfoJson) string {
 	if data.Name != "" {
-		return data.Name, nil
+		return data.Name
 	}
 
 	if data.DisplayName != "" {
-		return data.DisplayName, nil
+		return data.DisplayName
 	}
 
-	return "", nil
+	return ""
 }