From d1b8f13c6606ebc4652613abe2273129828a7e4e Mon Sep 17 00:00:00 2001 From: danielbh Date: Sat, 1 Dec 2018 12:28:10 -0500 Subject: [PATCH] feat: #11067 prevent removing last grafana admin permissions --- pkg/api/admin_users.go | 6 ++++ pkg/api/admin_users_test.go | 50 ++++++++++++++++++++++++++++++ pkg/models/user.go | 3 +- pkg/services/sqlstore/user.go | 26 +++++++++++++++- pkg/services/sqlstore/user_test.go | 26 ++++++++++++++++ 5 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 pkg/api/admin_users_test.go diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index dc3d390dda9..efc760d2b51 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -76,6 +76,7 @@ func AdminUpdateUserPassword(c *m.ReqContext, form dtos.AdminUpdateUserPasswordF c.JsonOK("User password updated") } +// PUT /api/admin/users/:id/permissions func AdminUpdateUserPermissions(c *m.ReqContext, form dtos.AdminUpdateUserPermissionsForm) { userID := c.ParamsInt64(":id") @@ -85,6 +86,11 @@ func AdminUpdateUserPermissions(c *m.ReqContext, form dtos.AdminUpdateUserPermis } if err := bus.Dispatch(&cmd); err != nil { + if err == m.ErrLastGrafanaAdmin { + c.JsonApiErr(400, m.ErrLastGrafanaAdmin.Error(), nil) + return + } + c.JsonApiErr(500, "Failed to update user permissions", err) return } diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go new file mode 100644 index 00000000000..0b94a64b3fb --- /dev/null +++ b/pkg/api/admin_users_test.go @@ -0,0 +1,50 @@ +package api + +import ( + "testing" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" + + . "github.com/smartystreets/goconvey/convey" +) + +func TestAdminApiEndpoint(t *testing.T) { + role := m.ROLE_ADMIN + Convey("Given a server admin attempts to remove themself as an admin", t, func() { + + updateCmd := dtos.AdminUpdateUserPermissionsForm{ + IsGrafanaAdmin: false, + } + + bus.AddHandler("test", func(cmd *m.UpdateUserPermissionsCommand) error { + return m.ErrLastGrafanaAdmin + }) + + putAdminScenario("When calling PUT on", "/api/admin/users/1/permissions", "/api/admin/users/:id/permissions", role, updateCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 400) + }) + }) +} + +func putAdminScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.AdminUpdateUserPermissionsForm, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := setupScenarioContext(url) + sc.defaultHandler = Wrap(func(c *m.ReqContext) { + sc.context = c + sc.context.UserId = TestUserID + sc.context.OrgId = TestOrgID + sc.context.OrgRole = role + + AdminUpdateUserPermissions(c, cmd) + }) + + sc.m.Put(routePattern, sc.defaultHandler) + + fn(sc) + }) +} diff --git a/pkg/models/user.go b/pkg/models/user.go index e3c7b556d35..69031e40338 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -7,7 +7,8 @@ import ( // Typed errors var ( - ErrUserNotFound = errors.New("User not found") + ErrUserNotFound = errors.New("User not found") + ErrLastGrafanaAdmin = errors.New("Cannot remove last grafana admin") ) type Password string diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 99a77ecabc3..a3ccb93b30c 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -504,8 +504,18 @@ func UpdateUserPermissions(cmd *m.UpdateUserPermissionsCommand) error { user.IsAdmin = cmd.IsGrafanaAdmin sess.UseBool("is_admin") + _, err := sess.ID(user.Id).Update(&user) - return err + if err != nil { + return err + } + + // validate that after update there is at least one server admin + if err := validateOneAdminLeft(sess); err != nil { + return err + } + + return nil }) } @@ -522,3 +532,17 @@ func SetUserHelpFlag(cmd *m.SetUserHelpFlagCommand) error { return err }) } + +func validateOneAdminLeft(sess *DBSession) error { + // validate that there is an admin user left + count, err := sess.Where("is_admin=?", true).Count(&m.User{}) + if err != nil { + return err + } + + if count == 0 { + return m.ErrLastGrafanaAdmin + } + + return nil +} diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index b26dd235772..627f2ab1ca5 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -155,6 +155,32 @@ func TestUserDataAccess(t *testing.T) { }) }) }) + + Convey("Given one grafana admin user", func() { + var err error + createUserCmd := &m.CreateUserCommand{ + Email: fmt.Sprint("admin", "@test.com"), + Name: fmt.Sprint("admin"), + Login: fmt.Sprint("admin"), + IsAdmin: true, + } + err = CreateUser(context.Background(), createUserCmd) + So(err, ShouldBeNil) + + Convey("Cannot make themselves a non-admin", func() { + updateUserPermsCmd := m.UpdateUserPermissionsCommand{IsGrafanaAdmin: false, UserId: 1} + updatePermsError := UpdateUserPermissions(&updateUserPermsCmd) + + So(updatePermsError, ShouldEqual, m.ErrLastGrafanaAdmin) + + query := m.GetUserByIdQuery{Id: createUserCmd.Result.Id} + getUserError := GetUserById(&query) + + So(getUserError, ShouldBeNil) + + So(query.Result.IsAdmin, ShouldEqual, true) + }) + }) }) }