From 699f9095e2be00fcf903dac0e14995f0690616f6 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Wed, 14 Jun 2017 23:45:30 +0200 Subject: [PATCH] WIP: remove permissions when deleting global user --- pkg/api/dashboard_acl.go | 4 +- pkg/models/dashboard_acl.go | 2 +- pkg/services/sqlstore/dashboard_acl.go | 4 + pkg/services/sqlstore/dashboard_acl_test.go | 9 ++ pkg/services/sqlstore/user.go | 3 + pkg/services/sqlstore/user_test.go | 168 +++++++++++++------- 6 files changed, 130 insertions(+), 60 deletions(-) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index ca29a6e66e8..80f0b0df532 100644 --- a/pkg/api/dashboard_acl.go +++ b/pkg/api/dashboard_acl.go @@ -36,8 +36,8 @@ func PostDashboardAcl(c *middleware.Context, cmd m.AddOrUpdateDashboardPermissio cmd.DashboardId = c.ParamsInt64(":id") if err := bus.Dispatch(&cmd); err != nil { - if err == m.ErrDashboardPermissionAlreadyAdded { - return ApiError(409, "Permission for user/user group already exists", err) + if err == m.ErrDashboardPermissionUserOrUserGroupEmpty { + return ApiError(409, err.Error(), err) } return ApiError(500, "Failed to create permission", err) } diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 9249da12d3b..08d22ef5ee1 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -24,7 +24,7 @@ func (p PermissionType) String() string { // Typed errors var ( - ErrDashboardPermissionAlreadyAdded = errors.New("A permission for this user/user group already exists.") + ErrDashboardPermissionUserOrUserGroupEmpty = errors.New("User id and user group id cannot both be empty for a dashboard permission.") ) // Dashboard ACL model diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 65237aa2744..d3e80cc108b 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -17,6 +17,10 @@ func init() { func AddOrUpdateDashboardPermission(cmd *m.AddOrUpdateDashboardPermissionCommand) error { return inTransaction(func(sess *DBSession) error { + if cmd.UserId == 0 && cmd.UserGroupId == 0 { + return m.ErrDashboardPermissionUserOrUserGroupEmpty + } + if res, err := sess.Query("SELECT 1 from "+dialect.Quote("dashboard_acl")+" WHERE dashboard_id =? and (user_group_id=? or user_id=?)", cmd.DashboardId, cmd.UserGroupId, cmd.UserId); err != nil { return err } else if len(res) == 1 { diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 4a2c2798c98..b6781c4aa6d 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -16,6 +16,15 @@ func TestDashboardAclDataAccess(t *testing.T) { savedFolder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") childDash := insertTestDashboard("2 test dash", 1, savedFolder.Id, false, "prod", "webapp") + Convey("When adding dashboard permission with userId and userGroupId set to 0", func() { + err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{ + OrgId: 1, + DashboardId: savedFolder.Id, + PermissionType: m.PERMISSION_EDIT, + }) + So(err, ShouldEqual, m.ErrDashboardPermissionUserOrUserGroupEmpty) + }) + Convey("Should be able to add dashboard permission", func() { err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{ OrgId: 1, diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 177f465dc96..af49e8dafe2 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -396,6 +396,9 @@ func DeleteUser(cmd *m.DeleteUserCommand) error { deletes := []string{ "DELETE FROM star WHERE user_id = ?", "DELETE FROM " + dialect.Quote("user") + " WHERE id = ?", + "DELETE FROM org_user WHERE user_id = ?", + "DELETE FROM dashboard_acl WHERE user_id = ?", + "DELETE FROM preferences WHERE user_id = ?", } for _, sql := range deletes { diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index decb4682552..03819e3959a 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -14,80 +14,134 @@ func TestUserDataAccess(t *testing.T) { Convey("Testing DB", t, func() { InitTestDB(t) - var err error - for i := 0; i < 5; i++ { - err = CreateUser(&models.CreateUserCommand{ - Email: fmt.Sprint("user", i, "@test.com"), - Name: fmt.Sprint("user", i), - Login: fmt.Sprint("loginuser", i), + Convey("Given 5 users", func() { + var err error + var cmd *models.CreateUserCommand + users := []models.User{} + for i := 0; i < 5; i++ { + cmd = &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + } + err = CreateUser(cmd) + So(err, ShouldBeNil) + users = append(users, cmd.Result) + } + + Convey("Can return the first page of users and a total count", func() { + query := models.SearchUsersQuery{Query: "", Page: 1, Limit: 3} + err = SearchUsers(&query) + + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 3) + So(query.Result.TotalCount, ShouldEqual, 5) }) - So(err, ShouldBeNil) - } - Convey("Can return the first page of users and a total count", func() { - query := models.SearchUsersQuery{Query: "", Page: 1, Limit: 3} - err = SearchUsers(&query) + Convey("Can return the second page of users and a total count", func() { + query := models.SearchUsersQuery{Query: "", Page: 2, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 3) - So(query.Result.TotalCount, ShouldEqual, 5) - }) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 2) + So(query.Result.TotalCount, ShouldEqual, 5) + }) - Convey("Can return the second page of users and a total count", func() { - query := models.SearchUsersQuery{Query: "", Page: 2, Limit: 3} - err = SearchUsers(&query) + Convey("Can return list of users matching query on user name", func() { + query := models.SearchUsersQuery{Query: "use", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 2) - So(query.Result.TotalCount, ShouldEqual, 5) - }) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 3) + So(query.Result.TotalCount, ShouldEqual, 5) - Convey("Can return list of users matching query on user name", func() { - query := models.SearchUsersQuery{Query: "use", Page: 1, Limit: 3} - err = SearchUsers(&query) + query = models.SearchUsersQuery{Query: "ser1", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 3) - So(query.Result.TotalCount, ShouldEqual, 5) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 1) + So(query.Result.TotalCount, ShouldEqual, 1) - query = models.SearchUsersQuery{Query: "ser1", Page: 1, Limit: 3} - err = SearchUsers(&query) + query = models.SearchUsersQuery{Query: "USER1", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 1) - So(query.Result.TotalCount, ShouldEqual, 1) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 1) + So(query.Result.TotalCount, ShouldEqual, 1) - query = models.SearchUsersQuery{Query: "USER1", Page: 1, Limit: 3} - err = SearchUsers(&query) + query = models.SearchUsersQuery{Query: "idontexist", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 1) - So(query.Result.TotalCount, ShouldEqual, 1) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 0) + So(query.Result.TotalCount, ShouldEqual, 0) + }) - query = models.SearchUsersQuery{Query: "idontexist", Page: 1, Limit: 3} - err = SearchUsers(&query) + Convey("Can return list of users matching query on email", func() { + query := models.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 0) - So(query.Result.TotalCount, ShouldEqual, 0) - }) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 1) + So(query.Result.TotalCount, ShouldEqual, 1) + }) - Convey("Can return list of users matching query on email", func() { - query := models.SearchUsersQuery{Query: "ser1@test.com", Page: 1, Limit: 3} - err = SearchUsers(&query) + Convey("Can return list of users matching query on login name", func() { + query := models.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3} + err = SearchUsers(&query) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 1) - So(query.Result.TotalCount, ShouldEqual, 1) - }) + So(err, ShouldBeNil) + So(len(query.Result.Users), ShouldEqual, 1) + So(query.Result.TotalCount, ShouldEqual, 1) + }) - Convey("Can return list of users matching query on login name", func() { - query := models.SearchUsersQuery{Query: "loginuser1", Page: 1, Limit: 3} - err = SearchUsers(&query) + Convey("when a user is an org member and has been assigned permissions", func() { + err = AddOrgUser(&models.AddOrgUserCommand{LoginOrEmail: users[0].Login, Role: models.ROLE_VIEWER, OrgId: users[0].OrgId}) + So(err, ShouldBeNil) - So(err, ShouldBeNil) - So(len(query.Result.Users), ShouldEqual, 1) - So(query.Result.TotalCount, ShouldEqual, 1) + err = AddOrUpdateDashboardPermission(&models.AddOrUpdateDashboardPermissionCommand{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, PermissionType: models.PERMISSION_EDIT}) + So(err, ShouldBeNil) + + err = SavePreferences(&models.SavePreferencesCommand{UserId: users[0].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) + So(err, ShouldBeNil) + + Convey("when the user is deleted", func() { + err = DeleteUser(&models.DeleteUserCommand{UserId: users[0].Id}) + So(err, ShouldBeNil) + + Convey("Should delete connected org users and permissions", func() { + query := &models.GetOrgUsersQuery{OrgId: 1} + err = GetOrgUsersForTest(query) + So(err, ShouldBeNil) + + So(len(query.Result), ShouldEqual, 1) + + permQuery := &models.GetDashboardPermissionsQuery{DashboardId: 1} + err = GetDashboardPermissions(permQuery) + So(err, ShouldBeNil) + + So(len(permQuery.Result), ShouldEqual, 0) + + prefsQuery := &models.GetPreferencesQuery{OrgId: users[0].OrgId, UserId: users[0].Id} + err = GetPreferences(prefsQuery) + So(err, ShouldBeNil) + + So(prefsQuery.Result.OrgId, ShouldEqual, 0) + So(prefsQuery.Result.UserId, ShouldEqual, 0) + }) + }) + }) }) }) } + +func GetOrgUsersForTest(query *models.GetOrgUsersQuery) error { + query.Result = make([]*models.OrgUserDTO, 0) + sess := x.Table("org_user") + sess.Join("LEFT ", "user", fmt.Sprintf("org_user.user_id=%s.id", x.Dialect().Quote("user"))) + sess.Where("org_user.org_id=?", query.OrgId) + sess.Cols("org_user.org_id", "org_user.user_id", "user.email", "user.login", "org_user.role") + + err := sess.Find(&query.Result) + return err +}