From 6d48d0a80c8ce59b9dc782a623dab4e3fcefcbb4 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 13 Jun 2018 18:01:50 +0200 Subject: [PATCH] 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); }); };