From 4be6ef4ab3684b116bc85f3aa18117601e2c8526 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 12:47:53 +0200 Subject: [PATCH 01/12] changelog: adds note about closing #12286 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e415b6eb9b6..12dcadc822e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * **Units**: W/m2 (energy), l/h (flow) and kPa (pressure) [#11233](https://github.com/grafana/grafana/pull/11233), thx [@flopp999](https://github.com/flopp999) * **Units**: Litre/min (flow) and milliLitre/min (flow) [#12282](https://github.com/grafana/grafana/pull/12282), thx [@flopp999](https://github.com/flopp999) * **Alerting**: Fix mobile notifications for Microsoft Teams alert notifier [#11484](https://github.com/grafana/grafana/pull/11484), thx [@manacker](https://github.com/manacker) +* **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) # 5.2.0-beta1 (2018-06-05) From b418e14bd975bca7d8afd8a236a909c8405a29f6 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 15 Jun 2018 13:40:42 +0200 Subject: [PATCH 02/12] make sure to use real ip when validating white listed ip's --- pkg/middleware/auth_proxy.go | 20 ++++++----- pkg/middleware/middleware_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index 144a0ae3a69..eff532b0da2 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -2,7 +2,6 @@ package middleware import ( "fmt" - "net" "net/mail" "reflect" "strings" @@ -29,7 +28,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { } // if auth proxy ip(s) defined, check if request comes from one of those - if err := checkAuthenticationProxy(ctx.Req.RemoteAddr, proxyHeaderValue); err != nil { + if err := checkAuthenticationProxy(ctx.RemoteAddr(), proxyHeaderValue); err != nil { ctx.Handle(407, "Proxy authentication required", err) return true } @@ -197,18 +196,23 @@ func checkAuthenticationProxy(remoteAddr string, proxyHeaderValue string) error return nil } - proxies := strings.Split(setting.AuthProxyWhitelist, ",") - sourceIP, _, err := net.SplitHostPort(remoteAddr) - if err != nil { - return err + // Multiple ip addresses? Right-most IP address is the IP address of the most recent proxy + if strings.Contains(remoteAddr, ",") { + sourceIPs := strings.Split(remoteAddr, ",") + remoteAddr = strings.TrimSpace(sourceIPs[len(sourceIPs)-1]) } + remoteAddr = strings.TrimPrefix(remoteAddr, "[") + remoteAddr = strings.TrimSuffix(remoteAddr, "]") + + proxies := strings.Split(setting.AuthProxyWhitelist, ",") + // Compare allowed IP addresses to actual address for _, proxyIP := range proxies { - if sourceIP == strings.TrimSpace(proxyIP) { + if remoteAddr == strings.TrimSpace(proxyIP) { return nil } } - return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, sourceIP) + return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, remoteAddr) } diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index b827751b1a5..0b50358ad73 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -293,6 +293,61 @@ func TestMiddlewareContext(t *testing.T) { }) }) + middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is not trusted", func(sc *scenarioContext) { + setting.AuthProxyEnabled = true + setting.AuthProxyHeaderName = "X-WEBAUTH-USER" + setting.AuthProxyHeaderProperty = "username" + setting.AuthProxyWhitelist = "192.168.1.1, 2001::23" + + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{OrgId: 4, UserId: 33} + return nil + }) + + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { + cmd.Result = &m.User{Id: 33} + return nil + }) + + sc.fakeReq("GET", "/") + sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") + sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.1, 192.168.1.2") + sc.exec() + + Convey("should return 407 status code", func() { + So(sc.resp.Code, ShouldEqual, 407) + So(sc.resp.Body.String(), ShouldContainSubstring, "Request for user (torkelo) from 192.168.1.2 is not from the authentication proxy") + }) + }) + + middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is trusted", func(sc *scenarioContext) { + setting.AuthProxyEnabled = true + setting.AuthProxyHeaderName = "X-WEBAUTH-USER" + setting.AuthProxyHeaderProperty = "username" + setting.AuthProxyWhitelist = "192.168.1.1, 2001::23" + + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{OrgId: 4, UserId: 33} + return nil + }) + + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { + cmd.Result = &m.User{Id: 33} + return nil + }) + + sc.fakeReq("GET", "/") + sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") + sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.2, 192.168.1.1") + sc.exec() + + Convey("Should init context with user info", func() { + So(sc.context.IsSignedIn, ShouldBeTrue) + So(sc.context.UserId, ShouldEqual, 33) + So(sc.context.OrgId, ShouldEqual, 4) + }) + }) + middlewareScenario("When session exists for previous user, create a new session", func(sc *scenarioContext) { setting.AuthProxyEnabled = true setting.AuthProxyHeaderName = "X-WEBAUTH-USER" From c02dd7462a1f651b2cd92f4935872b9999f129a3 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 15 Jun 2018 15:48:25 +0200 Subject: [PATCH 03/12] cloudwatch: handle invalid time range --- pkg/tsdb/cloudwatch/cloudwatch.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 499a3ed6e03..8af97575ae9 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -3,6 +3,7 @@ package cloudwatch import ( "context" "errors" + "fmt" "regexp" "sort" "strconv" @@ -144,6 +145,10 @@ func (e *CloudWatchExecutor) executeQuery(ctx context.Context, parameters *simpl return nil, err } + if endTime.Before(startTime) { + return nil, fmt.Errorf("Invalid time range: End time can't be before start time") + } + params := &cloudwatch.GetMetricStatisticsInput{ Namespace: aws.String(query.Namespace), MetricName: aws.String(query.MetricName), From da91b91b4bf32efdfd1946c419578a767b9b2de8 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 20:49:14 +0200 Subject: [PATCH 04/12] transactions: start sessions and transactions at the same place this make it possible for handler to use `withSession` when transactions is not nedded and `inTransactionCtx` if its needed without knowing who owns the session/transaction --- pkg/services/sqlstore/session.go | 19 ++++++++++++++----- pkg/services/sqlstore/transactions.go | 21 +++++++++------------ pkg/services/sqlstore/transactions_test.go | 6 +----- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index fdee2c76b0c..c85346231e4 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -22,21 +22,30 @@ func newSession() *DBSession { return &DBSession{Session: x.NewSession()} } -func startSession(ctx context.Context) *DBSession { +func startSession(ctx context.Context, engine *xorm.Engine, beginTran bool) (*DBSession, error) { value := ctx.Value(ContextSessionName) var sess *DBSession sess, ok := value.(*DBSession) if !ok { - newSess := newSession() - return newSess + newSess := &DBSession{Session: engine.NewSession()} + if beginTran { + err := newSess.Begin() + if err != nil { + return nil, err + } + } + return newSess, nil } - return sess + return sess, nil } func withDbSession(ctx context.Context, callback dbTransactionFunc) error { - sess := startSession(ctx) + sess, err := startSession(ctx, x, false) + if err != nil { + return err + } return callback(sess) } diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index f72b0bb8500..3e7634dc196 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -14,16 +14,16 @@ func (ss *SqlStore) InTransaction(ctx context.Context, fn func(ctx context.Conte } func (ss *SqlStore) inTransactionWithRetry(ctx context.Context, fn func(ctx context.Context) error, retry int) error { - sess := startSession(ctx) - defer sess.Close() - - if err := sess.Begin(); err != nil { + sess, err := startSession(ctx, ss.engine, true) + if err != nil { return err } + defer sess.Close() + withValue := context.WithValue(ctx, ContextSessionName, sess) - err := fn(withValue) + err = fn(withValue) // special handling of database locked errors for sqlite, then we can retry 3 times if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { @@ -60,16 +60,13 @@ func inTransactionWithRetry(callback dbTransactionFunc, retry int) error { } func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, retry int) error { - var err error - - sess := startSession(ctx) - - defer sess.Close() - - if err = sess.Begin(); err != nil { + sess, err := startSession(ctx, x, true) + if err != nil { return err } + defer sess.Close() + err = callback(sess) // special handling of database locked errors for sqlite, then we can retry 3 times diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go index 2575229aad5..937649921ba 100644 --- a/pkg/services/sqlstore/transactions_test.go +++ b/pkg/services/sqlstore/transactions_test.go @@ -5,7 +5,6 @@ import ( "errors" "testing" - "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" @@ -18,11 +17,9 @@ type testQuery struct { var ProvokedError = errors.New("testing error.") func TestTransaction(t *testing.T) { - InitTestDB(t) + ss := InitTestDB(t) Convey("InTransaction asdf asdf", t, func() { - ss := SqlStore{log: log.New("test-logger")} - cmd := &models.AddApiKeyCommand{Key: "secret-key", Name: "key", OrgId: 1} err := AddApiKey(cmd) @@ -50,7 +47,6 @@ func TestTransaction(t *testing.T) { } return ProvokedError - }) So(err, ShouldEqual, ProvokedError) From 1181e967992990c119e0194c8e7c55dfed46b0d6 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 21:23:57 +0200 Subject: [PATCH 05/12] merge create user handlers --- pkg/services/sqlstore/dashboard_test.go | 3 +- pkg/services/sqlstore/org_test.go | 11 +- pkg/services/sqlstore/team_test.go | 3 +- pkg/services/sqlstore/user.go | 146 +++++++++++------------- pkg/services/sqlstore/user_auth_test.go | 3 +- pkg/services/sqlstore/user_test.go | 3 +- 6 files changed, 82 insertions(+), 87 deletions(-) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 6d7c7a93e47..e4aecf0391d 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" "time" @@ -389,7 +390,7 @@ func createUser(name string, role string, isAdmin bool) m.User { setting.AutoAssignOrgRole = role currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin} - err := CreateUser(¤tUserCmd) + err := CreateUser(context.Background(), ¤tUserCmd) So(err, ShouldBeNil) q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id} diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 63b20aa6e86..f41b449de96 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "testing" "time" @@ -22,9 +23,9 @@ func TestAccountDataAccess(t *testing.T) { ac1cmd := m.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"} ac2cmd := m.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name"} - err := CreateUser(&ac1cmd) + err := CreateUser(context.Background(), &ac1cmd) So(err, ShouldBeNil) - err = CreateUser(&ac2cmd) + err = CreateUser(context.Background(), &ac2cmd) So(err, ShouldBeNil) q1 := m.GetUserOrgListQuery{UserId: ac1cmd.Result.Id} @@ -43,8 +44,8 @@ func TestAccountDataAccess(t *testing.T) { ac1cmd := m.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"} ac2cmd := m.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name", IsAdmin: true} - err := CreateUser(&ac1cmd) - err = CreateUser(&ac2cmd) + err := CreateUser(context.Background(), &ac1cmd) + err = CreateUser(context.Background(), &ac2cmd) So(err, ShouldBeNil) ac1 := ac1cmd.Result @@ -182,7 +183,7 @@ func TestAccountDataAccess(t *testing.T) { Convey("Given an org user with dashboard permissions", func() { ac3cmd := m.CreateUserCommand{Login: "ac3", Email: "ac3@test.com", Name: "ac3 name", IsAdmin: false} - err := CreateUser(&ac3cmd) + err := CreateUser(context.Background(), &ac3cmd) So(err, ShouldBeNil) ac3 := ac3cmd.Result diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index f4b022906da..abaa973957d 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -22,7 +23,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err := CreateUser(userCmd) + err := CreateUser(context.Background(), userCmd) So(err, ShouldBeNil) userIds = append(userIds, userCmd.Result.Id) } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 252499d5fdc..4448e973e99 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -16,7 +16,7 @@ import ( ) func init() { - bus.AddHandler("sql", CreateUser) + //bus.AddHandler("sql", CreateUser) bus.AddHandler("sql", GetUserById) bus.AddHandler("sql", UpdateUser) bus.AddHandler("sql", ChangeUserPassword) @@ -31,7 +31,7 @@ func init() { bus.AddHandler("sql", DeleteUser) bus.AddHandler("sql", UpdateUserPermissions) bus.AddHandler("sql", SetUserHelpFlag) - bus.AddHandlerCtx("sql", CreateUserCtx) + bus.AddHandlerCtx("sql", CreateUser) } func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error) { @@ -81,90 +81,80 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error return org.Id, nil } -func internalCreateUser(sess *DBSession, cmd *m.CreateUserCommand) error { - orgId, err := getOrgIdForNewUser(cmd, sess) - if err != nil { - return err - } - - if cmd.Email == "" { - cmd.Email = cmd.Login - } - - // create user - user := m.User{ - Email: cmd.Email, - Name: cmd.Name, - Login: cmd.Login, - Company: cmd.Company, - IsAdmin: cmd.IsAdmin, - OrgId: orgId, - EmailVerified: cmd.EmailVerified, - Created: time.Now(), - Updated: time.Now(), - LastSeenAt: time.Now().AddDate(-10, 0, 0), - } - - if len(cmd.Password) > 0 { - user.Salt = util.GetRandomString(10) - user.Rands = util.GetRandomString(10) - user.Password = util.EncodePassword(cmd.Password, user.Salt) - } - - sess.UseBool("is_admin") - - if _, err := sess.Insert(&user); err != nil { - return err - } - - sess.publishAfterCommit(&events.UserCreated{ - Timestamp: user.Created, - Id: user.Id, - Name: user.Name, - Login: user.Login, - Email: user.Email, - }) - - cmd.Result = user - - // create org user link - if !cmd.SkipOrgSetup { - orgUser := m.OrgUser{ - OrgId: orgId, - UserId: user.Id, - Role: m.ROLE_ADMIN, - Created: time.Now(), - Updated: time.Now(), +func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { + return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { + orgId, err := getOrgIdForNewUser(cmd, sess) + if err != nil { + return err } - if setting.AutoAssignOrg && !user.IsAdmin { - if len(cmd.DefaultOrgRole) > 0 { - orgUser.Role = m.RoleType(cmd.DefaultOrgRole) - } else { - orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) + if cmd.Email == "" { + cmd.Email = cmd.Login + } + + // create user + user := m.User{ + Email: cmd.Email, + Name: cmd.Name, + Login: cmd.Login, + Company: cmd.Company, + IsAdmin: cmd.IsAdmin, + OrgId: orgId, + EmailVerified: cmd.EmailVerified, + Created: time.Now(), + Updated: time.Now(), + LastSeenAt: time.Now().AddDate(-10, 0, 0), + } + + if len(cmd.Password) > 0 { + user.Salt = util.GetRandomString(10) + user.Rands = util.GetRandomString(10) + user.Password = util.EncodePassword(cmd.Password, user.Salt) + } + + sess.UseBool("is_admin") + + if _, err := sess.Insert(&user); err != nil { + return err + } + + sess.publishAfterCommit(&events.UserCreated{ + Timestamp: user.Created, + Id: user.Id, + Name: user.Name, + Login: user.Login, + Email: user.Email, + }) + + cmd.Result = user + + // create org user link + if !cmd.SkipOrgSetup { + orgUser := m.OrgUser{ + OrgId: orgId, + UserId: user.Id, + Role: m.ROLE_ADMIN, + Created: time.Now(), + Updated: time.Now(), + } + + if setting.AutoAssignOrg && !user.IsAdmin { + if len(cmd.DefaultOrgRole) > 0 { + orgUser.Role = m.RoleType(cmd.DefaultOrgRole) + } else { + orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) + } + } + + if _, err = sess.Insert(&orgUser); err != nil { + return err } } - if _, err = sess.Insert(&orgUser); err != nil { - return err - } - } - - return nil -} - -func CreateUserCtx(ctx context.Context, cmd *m.CreateUserCommand) error { - return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { - return internalCreateUser(sess, cmd) + return nil }, 0) } -func CreateUser(cmd *m.CreateUserCommand) error { - return inTransaction(func(sess *DBSession) error { - return internalCreateUser(sess, cmd) - }) -} - func GetUserById(query *m.GetUserByIdQuery) error { user := new(m.User) has, err := x.Id(query.Id).Get(user) diff --git a/pkg/services/sqlstore/user_auth_test.go b/pkg/services/sqlstore/user_auth_test.go index 882e0c7afa5..5ad93dc7a3b 100644 --- a/pkg/services/sqlstore/user_auth_test.go +++ b/pkg/services/sqlstore/user_auth_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -22,7 +23,7 @@ func TestUserAuth(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err = CreateUser(cmd) + err = CreateUser(context.Background(), cmd) So(err, ShouldBeNil) users = append(users, cmd.Result) } diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 2830733c96a..3597b6ad0c1 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -24,7 +25,7 @@ func TestUserDataAccess(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err = CreateUser(cmd) + err = CreateUser(context.Background(), cmd) So(err, ShouldBeNil) users = append(users, cmd.Result) } From 4c5fe68e7ef8c78f1572cb30730317358390d2bb Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 21:57:13 +0200 Subject: [PATCH 06/12] adds inTransactionCtx that calls inTransactionWithRetryCtx --- pkg/services/sqlstore/transactions.go | 4 ++++ pkg/services/sqlstore/user.go | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index 3e7634dc196..eccd37f9a43 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -100,3 +100,7 @@ func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, func inTransaction(callback dbTransactionFunc) error { return inTransactionWithRetry(callback, 0) } + +func inTransactionCtx(ctx context.Context, callback dbTransactionFunc) error { + return inTransactionWithRetryCtx(ctx, callback, 0) +} diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 4448e973e99..d32d51e0d0c 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -82,7 +82,7 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error } func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { - return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { + return inTransactionCtx(ctx, func(sess *DBSession) error { orgId, err := getOrgIdForNewUser(cmd, sess) if err != nil { return err @@ -152,7 +152,7 @@ func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { } return nil - }, 0) + }) } func GetUserById(query *m.GetUserByIdQuery) error { From 6782be80fd5e4c0d0b2e435a266315fb940f30d1 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 16 Jun 2018 16:59:15 +0200 Subject: [PATCH 07/12] changelog: adds note about closing #12199 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12dcadc822e..7c5133c5485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * **Units**: Litre/min (flow) and milliLitre/min (flow) [#12282](https://github.com/grafana/grafana/pull/12282), thx [@flopp999](https://github.com/flopp999) * **Alerting**: Fix mobile notifications for Microsoft Teams alert notifier [#11484](https://github.com/grafana/grafana/pull/11484), thx [@manacker](https://github.com/manacker) * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) +* **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) # 5.2.0-beta1 (2018-06-05) From a2ff7629e06979c5467d852c7ed2710db5e56572 Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Sun, 17 Jun 2018 22:38:37 -0700 Subject: [PATCH 08/12] Include the vendor directory when copying source in to Docker (#12305) This change updates the `.dockerignore` file to no longer contain the `vendor/` directory. When a Go project provides a `vendor/` directory within the repository, the best practice is to build that project using their vendored dependencies. By putting it in the `.dockerignore` file we prevent consumers from easily doing that. The `vendor/` directory is used to include all of the dependencies needed to build a project. This makes it so that we can reproducibly build the project, at any given commit, because the dependencies will always be present. Also, using the vendor directory avoids us needing to continually re-download all the dependencies, and it protects us from build failures if GitHub is down or a dependency gets removed or renamed. In addition to the change above, this also removes an extra `/tmp` entry from the `.dockerignore` file. Fixes #12304 Signed-off-by: Tim Heckman --- .dockerignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.dockerignore b/.dockerignore index c79fe777899..e50dfd86aa3 100644 --- a/.dockerignore +++ b/.dockerignore @@ -11,8 +11,5 @@ dump.rdb node_modules /local /tmp -/vendor *.yml *.md -/vendor -/tmp From ab9f0e8edda9c07be93ef904cc23ca2a17352a23 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 09:03:30 +0200 Subject: [PATCH 09/12] changelog: add notes about closing #10707 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c5133c5485..7120fed47ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * **Alerting**: Fix mobile notifications for Microsoft Teams alert notifier [#11484](https://github.com/grafana/grafana/pull/11484), thx [@manacker](https://github.com/manacker) * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) * **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) +* **Auth Proxy**: Whitelist proxy IP address instead of client IP address [#10707](https://github.com/grafana/grafana/issues/10707) # 5.2.0-beta1 (2018-06-05) From 6d48d0a80c8ce59b9dc782a623dab4e3fcefcbb4 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 13 Jun 2018 18:01:50 +0200 Subject: [PATCH 10/12] set current org when adding/removing user to org To not get into a situation where a user has a current organization assign which he is not a member of we try to always make sure that a user has a valid current organization assigned. --- pkg/services/sqlstore/org_test.go | 16 ++++- pkg/services/sqlstore/org_users.go | 64 ++++++++++++++++++- pkg/services/sqlstore/user.go | 18 ++++-- pkg/services/sqlstore/user_test.go | 14 ++-- .../features/admin/admin_edit_user_ctrl.ts | 2 + 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 63b20aa6e86..dcf45032198 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -150,7 +150,7 @@ func TestAccountDataAccess(t *testing.T) { }) Convey("Can set using org", func() { - cmd := m.SetUsingOrgCommand{UserId: ac2.Id, OrgId: ac1.Id} + cmd := m.SetUsingOrgCommand{UserId: ac2.Id, OrgId: ac1.OrgId} err := SetUsingOrg(&cmd) So(err, ShouldBeNil) @@ -159,13 +159,25 @@ func TestAccountDataAccess(t *testing.T) { err := GetSignedInUser(&query) So(err, ShouldBeNil) - So(query.Result.OrgId, ShouldEqual, ac1.Id) + So(query.Result.OrgId, ShouldEqual, ac1.OrgId) So(query.Result.Email, ShouldEqual, "ac2@test.com") So(query.Result.Name, ShouldEqual, "ac2 name") So(query.Result.Login, ShouldEqual, "ac2") So(query.Result.OrgName, ShouldEqual, "ac1@test.com") So(query.Result.OrgRole, ShouldEqual, "Viewer") }) + + Convey("Should set last org as current when removing user from current", func() { + remCmd := m.RemoveOrgUserCommand{OrgId: ac1.OrgId, UserId: ac2.Id} + err := RemoveOrgUser(&remCmd) + So(err, ShouldBeNil) + + query := m.GetSignedInUserQuery{UserId: ac2.Id} + err = GetSignedInUser(&query) + + So(err, ShouldBeNil) + So(query.Result.OrgId, ShouldEqual, ac2.OrgId) + }) }) Convey("Cannot delete last admin org user", func() { diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 0b991c73c55..aad72cdacb4 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -20,7 +20,14 @@ func init() { func AddOrgUser(cmd *m.AddOrgUserCommand) error { return inTransaction(func(sess *DBSession) error { // check if user exists - if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, cmd.UserId); err != nil { + var user m.User + if exists, err := sess.Id(cmd.UserId).Get(&user); err != nil { + return err + } else if !exists { + return m.ErrUserNotFound + } + + if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, user.Id); err != nil { return err } else if len(res) == 1 { return m.ErrOrgUserAlreadyAdded @@ -41,7 +48,26 @@ func AddOrgUser(cmd *m.AddOrgUserCommand) error { } _, err := sess.Insert(&entity) - return err + if err != nil { + return err + } + + var userOrgs []*m.UserOrgDTO + sess.Table("org_user") + sess.Join("INNER", "org", "org_user.org_id=org.id") + sess.Where("org_user.user_id=? AND org_user.org_id=?", user.Id, user.OrgId) + sess.Cols("org.name", "org_user.role", "org_user.org_id") + err = sess.Find(&userOrgs) + + if err != nil { + return err + } + + if len(userOrgs) == 0 { + return setUsingOrgInTransaction(sess, user.Id, cmd.OrgId) + } + + return nil }) } @@ -110,6 +136,14 @@ func GetOrgUsers(query *m.GetOrgUsersQuery) error { func RemoveOrgUser(cmd *m.RemoveOrgUserCommand) error { return inTransaction(func(sess *DBSession) error { + // check if user exists + var user m.User + if exists, err := sess.Id(cmd.UserId).Get(&user); err != nil { + return err + } else if !exists { + return m.ErrUserNotFound + } + deletes := []string{ "DELETE FROM org_user WHERE org_id=? and user_id=?", "DELETE FROM dashboard_acl WHERE org_id=? and user_id = ?", @@ -123,6 +157,32 @@ func RemoveOrgUser(cmd *m.RemoveOrgUserCommand) error { } } + var userOrgs []*m.UserOrgDTO + sess.Table("org_user") + sess.Join("INNER", "org", "org_user.org_id=org.id") + sess.Where("org_user.user_id=?", user.Id) + sess.Cols("org.name", "org_user.role", "org_user.org_id") + err := sess.Find(&userOrgs) + + if err != nil { + return err + } + + hasCurrentOrgSet := false + for _, userOrg := range userOrgs { + if user.OrgId == userOrg.OrgId { + hasCurrentOrgSet = true + break + } + } + + if !hasCurrentOrgSet && len(userOrgs) > 0 { + err = setUsingOrgInTransaction(sess, user.Id, userOrgs[0].OrgId) + if err != nil { + return err + } + } + return validateOneAdminLeftInOrg(cmd.OrgId, sess) }) } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index e7aa8da837a..ad86323c0d8 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -290,16 +290,20 @@ func SetUsingOrg(cmd *m.SetUsingOrgCommand) error { } return inTransaction(func(sess *DBSession) error { - user := m.User{ - Id: cmd.UserId, - OrgId: cmd.OrgId, - } - - _, err := sess.Id(cmd.UserId).Update(&user) - return err + return setUsingOrgInTransaction(sess, cmd.UserId, cmd.OrgId) }) } +func setUsingOrgInTransaction(sess *DBSession, userID int64, orgID int64) error { + user := m.User{ + Id: userID, + OrgId: orgID, + } + + _, err := sess.Id(userID).Update(&user) + return err +} + func GetUserProfile(query *m.GetUserProfileQuery) error { var user m.User has, err := x.Id(query.UserId).Get(&user) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 2830733c96a..076e88c2bb3 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -96,33 +96,33 @@ func TestUserDataAccess(t *testing.T) { }) Convey("when a user is an org member and has been assigned permissions", func() { - err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[0].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId}) + err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[1].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId, UserId: users[1].Id}) So(err, ShouldBeNil) - testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT}) + testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[1].Id, Permission: m.PERMISSION_EDIT}) So(err, ShouldBeNil) - err = SavePreferences(&m.SavePreferencesCommand{UserId: users[0].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) + err = SavePreferences(&m.SavePreferencesCommand{UserId: users[1].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) So(err, ShouldBeNil) Convey("when the user is deleted", func() { - err = DeleteUser(&m.DeleteUserCommand{UserId: users[0].Id}) + err = DeleteUser(&m.DeleteUserCommand{UserId: users[1].Id}) So(err, ShouldBeNil) Convey("Should delete connected org users and permissions", func() { - query := &m.GetOrgUsersQuery{OrgId: 1} + query := &m.GetOrgUsersQuery{OrgId: users[0].OrgId} err = GetOrgUsersForTest(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) - permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: 1} + permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: users[0].OrgId} err = GetDashboardAclInfoList(permQuery) So(err, ShouldBeNil) So(len(permQuery.Result), ShouldEqual, 0) - prefsQuery := &m.GetPreferencesQuery{OrgId: users[0].OrgId, UserId: users[0].Id} + prefsQuery := &m.GetPreferencesQuery{OrgId: users[0].OrgId, UserId: users[1].Id} err = GetPreferences(prefsQuery) So(err, ShouldBeNil) diff --git a/public/app/features/admin/admin_edit_user_ctrl.ts b/public/app/features/admin/admin_edit_user_ctrl.ts index 8203c7399c1..1d4fb9cf19a 100644 --- a/public/app/features/admin/admin_edit_user_ctrl.ts +++ b/public/app/features/admin/admin_edit_user_ctrl.ts @@ -75,6 +75,7 @@ export class AdminEditUserCtrl { $scope.removeOrgUser = function(orgUser) { backendSrv.delete('/api/orgs/' + orgUser.orgId + '/users/' + $scope.user_id).then(function() { + $scope.getUser($scope.user_id); $scope.getUserOrgs($scope.user_id); }); }; @@ -108,6 +109,7 @@ export class AdminEditUserCtrl { $scope.newOrg.loginOrEmail = $scope.user.login; backendSrv.post('/api/orgs/' + orgInfo.id + '/users/', $scope.newOrg).then(function() { + $scope.getUser($scope.user_id); $scope.getUserOrgs($scope.user_id); }); }; From a7383479574a73cf4c0d87658e36ae0fccf3ac9c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 11:04:16 +0200 Subject: [PATCH 11/12] snapshot: copy correct props when creating a snapshot --- public/app/features/dashboard/share_snapshot_ctrl.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/app/features/dashboard/share_snapshot_ctrl.ts b/public/app/features/dashboard/share_snapshot_ctrl.ts index aa146dcad63..7d5bd112dfd 100644 --- a/public/app/features/dashboard/share_snapshot_ctrl.ts +++ b/public/app/features/dashboard/share_snapshot_ctrl.ts @@ -123,6 +123,9 @@ export class ShareSnapshotCtrl { enable: annotation.enable, iconColor: annotation.iconColor, snapshotData: annotation.snapshotData, + type: annotation.type, + builtIn: annotation.builtIn, + hide: annotation.hide, }; }) .value(); From b72c45f7355152804e16fa93cd325ebb92b00595 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 11:22:55 +0200 Subject: [PATCH 12/12] changelog: add notes about closing #11076 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7120fed47ad..ba1e81e946e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) * **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) * **Auth Proxy**: Whitelist proxy IP address instead of client IP address [#10707](https://github.com/grafana/grafana/issues/10707) +* **User Management**: Make sure that a user always has a current org assigned [#11076](https://github.com/grafana/grafana/issues/11076) # 5.2.0-beta1 (2018-06-05)