From 84d6e67e6b7268cbbb2a035ccbe0a023317f161f Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Wed, 7 Mar 2018 16:20:05 +0100 Subject: [PATCH 1/2] alerting: fixes validation error when saving alerts in dash If a panelId in the dashboard json is set to zero then the validation silently fails. Instead of returning an error, it just ignores alerts and saves the dashboard. (cherry picked from commit d96fbb486f6f7ea452a314398ad56eb0e8130177) --- pkg/services/alerting/extractor.go | 9 ++- pkg/services/alerting/extractor_test.go | 16 +++++ .../alerting/test-data/panel-with-id-0.json | 63 +++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 pkg/services/alerting/test-data/panel-with-id-0.json diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index a609824cbc8..5206c81642e 100644 --- a/pkg/services/alerting/extractor.go +++ b/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 } } diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 71f3026025d..f8b678e66bd 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/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) diff --git a/pkg/services/alerting/test-data/panel-with-id-0.json b/pkg/services/alerting/test-data/panel-with-id-0.json new file mode 100644 index 00000000000..d1f314a4f55 --- /dev/null +++ b/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]} + } + ] + } + } + ] + } +] + } From f736e7aeb2960e2059540cf05cb815ea2a73f31a Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Tue, 6 Mar 2018 14:21:36 -0500 Subject: [PATCH 2/2] only use jwt token if it contains an email address (cherry picked from commit 9d005c50a2e6be45bf216a909f2abdf911225330) --- pkg/social/generic_oauth.go | 54 ++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/social/generic_oauth.go b/pkg/social/generic_oauth.go index 2411e0006a2..b92d64ad9fc 100644 --- a/pkg/social/generic_oauth.go +++ b/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 "" }