From ccc3d88cee22355fee8eb940d74aab2f80db5f87 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Mon, 8 Jul 2019 10:02:16 -0700 Subject: [PATCH 01/19] Refactor: move dom utils to @grafana/ui (#17976) --- .../explore => packages/grafana-ui/src}/utils/dom.ts | 8 ++++---- packages/grafana-ui/src/utils/index.ts | 4 ++++ .../editor/KustoQueryField.tsx | 4 ++-- .../datasource/loki/components/LokiQueryFieldForm.tsx | 11 +++++------ .../prometheus/components/PromQueryField.tsx | 11 +++++------ 5 files changed, 20 insertions(+), 18 deletions(-) rename {public/app/features/explore => packages/grafana-ui/src}/utils/dom.ts (81%) diff --git a/public/app/features/explore/utils/dom.ts b/packages/grafana-ui/src/utils/dom.ts similarity index 81% rename from public/app/features/explore/utils/dom.ts rename to packages/grafana-ui/src/utils/dom.ts index 381c150e3f4..39582d493f7 100644 --- a/public/app/features/explore/utils/dom.ts +++ b/packages/grafana-ui/src/utils/dom.ts @@ -1,6 +1,6 @@ // Node.closest() polyfill if ('Element' in window && !Element.prototype.closest) { - Element.prototype.closest = function(this: any, s) { + Element.prototype.closest = function(this: any, s: string) { const matches = (this.document || this.ownerDocument).querySelectorAll(s); let el = this; let i; @@ -15,7 +15,7 @@ if ('Element' in window && !Element.prototype.closest) { }; } -export function getPreviousCousin(node, selector) { +export function getPreviousCousin(node: any, selector: string) { let sibling = node.parentElement.previousSibling; let el; while (sibling) { @@ -30,12 +30,12 @@ export function getPreviousCousin(node, selector) { export function getNextCharacter(global = window) { const selection = global.getSelection(); - if (!selection.anchorNode) { + if (!selection || !selection.anchorNode) { return null; } const range = selection.getRangeAt(0); const text = selection.anchorNode.textContent; const offset = range.startOffset; - return text.substr(offset, 1); + return text!.substr(offset, 1); } diff --git a/packages/grafana-ui/src/utils/index.ts b/packages/grafana-ui/src/utils/index.ts index 6ccf5a71107..0c973b08486 100644 --- a/packages/grafana-ui/src/utils/index.ts +++ b/packages/grafana-ui/src/utils/index.ts @@ -7,3 +7,7 @@ export * from './deprecationWarning'; export * from './validate'; export { getFlotPairs } from './flotPairs'; export * from './slate'; + +// Export with a namespace +import * as DOMUtil from './dom'; // includes Element.closest polyfil +export { DOMUtil }; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/editor/KustoQueryField.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/editor/KustoQueryField.tsx index fc4d1a44822..c49718056af 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/editor/KustoQueryField.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/editor/KustoQueryField.tsx @@ -4,7 +4,7 @@ import Plain from 'slate-plain-serializer'; import QueryField from './query_field'; import debounce from 'lodash/debounce'; -import { getNextCharacter } from 'app/features/explore/utils/dom'; +import { DOMUtil } from '@grafana/ui'; import { KEYWORDS, functionTokens, operatorTokens, grafanaMacros } from './kusto/kusto'; // import '../sass/editor.base.scss'; @@ -203,7 +203,7 @@ export default class KustoQueryField extends QueryField { // Modify suggestion based on context - const nextChar = getNextCharacter(); + const nextChar = DOMUtil.getNextCharacter(); if (suggestion.type === 'function') { if (!nextChar || nextChar !== '(') { suggestionText += '('; diff --git a/public/app/plugins/datasource/loki/components/LokiQueryFieldForm.tsx b/public/app/plugins/datasource/loki/components/LokiQueryFieldForm.tsx index 8646a7c6849..95d1b3d4cb3 100644 --- a/public/app/plugins/datasource/loki/components/LokiQueryFieldForm.tsx +++ b/public/app/plugins/datasource/loki/components/LokiQueryFieldForm.tsx @@ -10,13 +10,12 @@ import QueryField, { TypeaheadInput, QueryFieldState } from 'app/features/explor // Utils & Services // dom also includes Element polyfills -import { getNextCharacter, getPreviousCousin } from 'app/features/explore/utils/dom'; import BracesPlugin from 'app/features/explore/slate-plugins/braces'; // Types import { LokiQuery } from '../types'; import { TypeaheadOutput, HistoryItem } from 'app/types/explore'; -import { DataSourceApi, ExploreQueryFieldProps, DataSourceStatus } from '@grafana/ui'; +import { DataSourceApi, ExploreQueryFieldProps, DataSourceStatus, DOMUtil } from '@grafana/ui'; import { AbsoluteTimeRange } from '@grafana/data'; function getChooserText(hasSyntax: boolean, hasLogLabels: boolean, datasourceStatus: DataSourceStatus) { @@ -36,7 +35,7 @@ function willApplySuggestion(suggestion: string, { typeaheadContext, typeaheadTe // Modify suggestion based on context switch (typeaheadContext) { case 'context-labels': { - const nextChar = getNextCharacter(); + const nextChar = DOMUtil.getNextCharacter(); if (!nextChar || nextChar === '}' || nextChar === ',') { suggestion += '='; } @@ -48,7 +47,7 @@ function willApplySuggestion(suggestion: string, { typeaheadContext, typeaheadTe if (!typeaheadText.match(/^(!?=~?"|")/)) { suggestion = `"${suggestion}`; } - if (getNextCharacter() !== '"') { + if (DOMUtil.getNextCharacter() !== '"') { suggestion = `${suggestion}"`; } break; @@ -130,9 +129,9 @@ export class LokiQueryFieldForm extends React.PureComponent Date: Mon, 8 Jul 2019 10:12:02 -0700 Subject: [PATCH 02/19] Refactor: fix range util imports (#17988) --- .../src/components/TimePicker/time.ts | 18 +++++++++++++----- .../dashboard/panel_editor/QueryOptions.tsx | 15 +++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/grafana-ui/src/components/TimePicker/time.ts b/packages/grafana-ui/src/components/TimePicker/time.ts index eb9bb0b0b23..7d90e04db6b 100644 --- a/packages/grafana-ui/src/components/TimePicker/time.ts +++ b/packages/grafana-ui/src/components/TimePicker/time.ts @@ -1,7 +1,15 @@ -import { TimeRange, TIME_FORMAT, RawTimeRange, TimeZone } from '@grafana/data'; -import { describeTimeRange } from '@grafana/data/src/utils/rangeutil'; -import { dateMath } from '@grafana/data'; -import { isDateTime, dateTime, DateTime, toUtc } from '@grafana/data'; +import { + TimeRange, + TIME_FORMAT, + RawTimeRange, + TimeZone, + rangeUtil, + dateMath, + isDateTime, + dateTime, + DateTime, + toUtc, +} from '@grafana/data'; export const rawToTimeRange = (raw: RawTimeRange, timeZone?: TimeZone): TimeRange => { const from = stringToDateTimeType(raw.from, false, timeZone); @@ -32,7 +40,7 @@ export const stringToDateTimeType = (value: string | DateTime, roundUp?: boolean }; export const mapTimeRangeToRangeString = (timeRange: RawTimeRange): string => { - return describeTimeRange(timeRange); + return rangeUtil.describeTimeRange(timeRange); }; export const isValidTimeString = (text: string) => dateMath.isValid(text); diff --git a/public/app/features/dashboard/panel_editor/QueryOptions.tsx b/public/app/features/dashboard/panel_editor/QueryOptions.tsx index e47b12cdb55..a10ec31fae7 100644 --- a/public/app/features/dashboard/panel_editor/QueryOptions.tsx +++ b/public/app/features/dashboard/panel_editor/QueryOptions.tsx @@ -2,12 +2,19 @@ import React, { PureComponent, ChangeEvent, FocusEvent } from 'react'; // Utils -import { isValidTimeSpan } from '@grafana/data/src/utils/rangeutil'; +import { rangeUtil } from '@grafana/data'; // Components -import { DataSourceSelectItem, EventsWithValidation, Input, InputStatus, Switch, ValidationEvents } from '@grafana/ui'; +import { + DataSourceSelectItem, + EventsWithValidation, + Input, + InputStatus, + Switch, + ValidationEvents, + FormLabel, +} from '@grafana/ui'; import { DataSourceOption } from './DataSourceOption'; -import { FormLabel } from '@grafana/ui'; // Types import { PanelModel } from '../state'; @@ -19,7 +26,7 @@ const timeRangeValidationEvents: ValidationEvents = { if (!value) { return true; } - return isValidTimeSpan(value); + return rangeUtil.isValidTimeSpan(value); }, errorMessage: 'Not a valid timespan', }, From 78ca55f3d73019f2e942e8cec2987c32274e3f5f Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki Date: Tue, 9 Jul 2019 09:37:24 +0300 Subject: [PATCH 03/19] Fix: Break redirect loop if oauth_auto_login = true and OAuth login fails (#17974) * Add tests for login view * Fix OAuth auto login redirect loop login_error cookie is only set when the OAuth login fails for some reason. Therefore, the login view should return immediately if a login_error cookie exists before trying to login the user using OAuth again. * Fix test Use 'index-template' instead of 'index' for testing * Add some comments --- pkg/api/common_test.go | 20 ++++++ pkg/api/login.go | 14 ++++- pkg/api/login_test.go | 135 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 pkg/api/login_test.go diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 0764fb0bfd6..590074f7193 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -93,6 +93,26 @@ func (sc *scenarioContext) fakeReqWithParams(method, url string, queryParams map return sc } +func (sc *scenarioContext) fakeReqNoAssertions(method, url string) *scenarioContext { + sc.resp = httptest.NewRecorder() + req, _ := http.NewRequest(method, url, nil) + sc.req = req + + return sc +} + +func (sc *scenarioContext) fakeReqNoAssertionsWithCookie(method, url string, cookie http.Cookie) *scenarioContext { + sc.resp = httptest.NewRecorder() + http.SetCookie(sc.resp, &cookie) + + req, _ := http.NewRequest(method, url, nil) + req.Header = http.Header{"Cookie": sc.resp.Header()["Set-Cookie"]} + + sc.req = req + + return sc +} + type scenarioContext struct { m *macaron.Macaron context *m.ReqContext diff --git a/pkg/api/login.go b/pkg/api/login.go index 37df4613212..61a6299b935 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -21,8 +21,14 @@ const ( LoginErrorCookieName = "login_error" ) +var setIndexViewData = (*HTTPServer).setIndexViewData + +var getViewIndex = func() string { + return ViewIndex +} + func (hs *HTTPServer) LoginView(c *models.ReqContext) { - viewData, err := hs.setIndexViewData(c) + viewData, err := setIndexViewData(hs, c) if err != nil { c.Handle(500, "Failed to get settings", err) return @@ -41,8 +47,14 @@ func (hs *HTTPServer) LoginView(c *models.ReqContext) { viewData.Settings["samlEnabled"] = hs.Cfg.SAMLEnabled if loginError, ok := tryGetEncryptedCookie(c, LoginErrorCookieName); ok { + //this cookie is only set whenever an OAuth login fails + //therefore the loginError should be passed to the view data + //and the view should return immediately before attempting + //to login again via OAuth and enter to a redirect loop deleteCookie(c, LoginErrorCookieName) viewData.Settings["loginError"] = loginError + c.HTML(200, getViewIndex(), viewData) + return } if tryOAuthAutoLogin(c) { diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go new file mode 100644 index 00000000000..ab28848a43d --- /dev/null +++ b/pkg/api/login_test.go @@ -0,0 +1,135 @@ +package api + +import ( + "encoding/hex" + "errors" + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" + "github.com/stretchr/testify/assert" + "io/ioutil" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func mockSetIndexViewData() { + setIndexViewData = func(*HTTPServer, *models.ReqContext) (*dtos.IndexViewData, error) { + data := &dtos.IndexViewData{ + User: &dtos.CurrentUser{}, + Settings: map[string]interface{}{}, + NavTree: []*dtos.NavLink{}, + } + return data, nil + } +} + +func resetSetIndexViewData() { + setIndexViewData = (*HTTPServer).setIndexViewData +} + +func mockViewIndex() { + getViewIndex = func() string { + return "index-template" + } +} + +func resetViewIndex() { + getViewIndex = func() string { + return ViewIndex + } +} + +func getBody(resp *httptest.ResponseRecorder) (string, error) { + responseData, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + return string(responseData), nil +} + +func TestLoginErrorCookieApiEndpoint(t *testing.T) { + mockSetIndexViewData() + defer resetSetIndexViewData() + + mockViewIndex() + defer resetViewIndex() + + sc := setupScenarioContext("/login") + hs := &HTTPServer{ + Cfg: setting.NewCfg(), + } + + sc.defaultHandler = Wrap(func(w http.ResponseWriter, c *models.ReqContext) { + hs.LoginView(c) + }) + + setting.OAuthService = &setting.OAuther{} + setting.OAuthService.OAuthInfos = make(map[string]*setting.OAuthInfo) + setting.LoginCookieName = "grafana_session" + setting.SecretKey = "login_testing" + + setting.OAuthService = &setting.OAuther{} + setting.OAuthService.OAuthInfos = make(map[string]*setting.OAuthInfo) + setting.OAuthService.OAuthInfos["github"] = &setting.OAuthInfo{ + ClientId: "fake", + ClientSecret: "fakefake", + Enabled: true, + AllowSignup: true, + Name: "github", + } + setting.OAuthAutoLogin = true + + oauthError := errors.New("User not a member of one of the required organizations") + encryptedError, _ := util.Encrypt([]byte(oauthError.Error()), setting.SecretKey) + cookie := http.Cookie{ + Name: LoginErrorCookieName, + MaxAge: 60, + Value: hex.EncodeToString(encryptedError), + HttpOnly: true, + Path: setting.AppSubUrl + "/", + Secure: hs.Cfg.CookieSecure, + SameSite: hs.Cfg.CookieSameSite, + } + sc.m.Get(sc.url, sc.defaultHandler) + sc.fakeReqNoAssertionsWithCookie("GET", sc.url, cookie).exec() + assert.Equal(t, sc.resp.Code, 200) + + responseString, err := getBody(sc.resp) + assert.Nil(t, err) + assert.True(t, strings.Contains(responseString, oauthError.Error())) +} + +func TestLoginOAuthRedirect(t *testing.T) { + mockSetIndexViewData() + defer resetSetIndexViewData() + + sc := setupScenarioContext("/login") + hs := &HTTPServer{ + Cfg: setting.NewCfg(), + } + + sc.defaultHandler = Wrap(func(c *models.ReqContext) { + hs.LoginView(c) + }) + + setting.OAuthService = &setting.OAuther{} + setting.OAuthService.OAuthInfos = make(map[string]*setting.OAuthInfo) + setting.OAuthService.OAuthInfos["github"] = &setting.OAuthInfo{ + ClientId: "fake", + ClientSecret: "fakefake", + Enabled: true, + AllowSignup: true, + Name: "github", + } + setting.OAuthAutoLogin = true + sc.m.Get(sc.url, sc.defaultHandler) + sc.fakeReqNoAssertions("GET", sc.url).exec() + + assert.Equal(t, sc.resp.Code, 307) + location, ok := sc.resp.Header()["Location"] + assert.True(t, ok) + assert.Equal(t, location[0], "/login/github") +} From 332920954e904518b3fa10e0b161eb4583ac8f70 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Tue, 9 Jul 2019 10:15:52 +0100 Subject: [PATCH 04/19] SAML: Show SAML login button even if OAuth is disabled (#17993) * Move the SAML button outside of the oauth div * Don't attempt to search cookies with an empty name --- pkg/middleware/middleware.go | 4 ++++ public/app/partials/login.html | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 49ec9f54b2a..d4a0b2da2aa 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -182,6 +182,10 @@ func initContextWithBasicAuth(ctx *models.ReqContext, orgId int64) bool { } func initContextWithToken(authTokenService models.UserTokenService, ctx *models.ReqContext, orgID int64) bool { + if setting.LoginCookieName == "" { + return false + } + rawToken := ctx.GetCookie(setting.LoginCookieName) if rawToken == "" { return false diff --git a/public/app/partials/login.html b/public/app/partials/login.html index 8acc0527fdb..78ff178c151 100644 --- a/public/app/partials/login.html +++ b/public/app/partials/login.html @@ -45,6 +45,10 @@
+