From 0c0cf36ab8ccee1511bd1d116962f0c1ad91a560 Mon Sep 17 00:00:00 2001 From: Ieva Date: Mon, 27 Jun 2022 19:40:12 +0300 Subject: [PATCH] Access control: role checks before updates (#51449) * add role checks * linting --- pkg/api/org_invite.go | 3 ++ pkg/api/org_users.go | 24 +++++---- pkg/api/org_users_test.go | 106 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/pkg/api/org_invite.go b/pkg/api/org_invite.go index 970ac732d95..44d6359e7f7 100644 --- a/pkg/api/org_invite.go +++ b/pkg/api/org_invite.go @@ -38,6 +38,9 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response { if !inviteDto.Role.IsValid() { return response.Error(400, "Invalid role specified", nil) } + if !c.OrgRole.Includes(inviteDto.Role) && !c.IsGrafanaAdmin { + return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) + } // first try get existing user userQuery := models.GetUserByLoginQuery{LoginOrEmail: inviteDto.LoginOrEmail} diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 7d07bc086a2..aa10fe8b354 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -21,7 +21,7 @@ func (hs *HTTPServer) AddOrgUserToCurrentOrg(c *models.ReqContext) response.Resp return response.Error(http.StatusBadRequest, "bad request data", err) } cmd.OrgId = c.OrgId - return hs.addOrgUserHelper(c.Req.Context(), cmd) + return hs.addOrgUserHelper(c, cmd) } // POST /api/orgs/:orgId/users @@ -36,16 +36,19 @@ func (hs *HTTPServer) AddOrgUser(c *models.ReqContext) response.Response { if err != nil { return response.Error(http.StatusBadRequest, "orgId is invalid", err) } - return hs.addOrgUserHelper(c.Req.Context(), cmd) + return hs.addOrgUserHelper(c, cmd) } -func (hs *HTTPServer) addOrgUserHelper(ctx context.Context, cmd models.AddOrgUserCommand) response.Response { +func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd models.AddOrgUserCommand) response.Response { if !cmd.Role.IsValid() { return response.Error(400, "Invalid role specified", nil) } + if !c.OrgRole.Includes(cmd.Role) && !c.IsGrafanaAdmin { + return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) + } userQuery := models.GetUserByLoginQuery{LoginOrEmail: cmd.LoginOrEmail} - err := hs.SQLStore.GetUserByLogin(ctx, &userQuery) + err := hs.SQLStore.GetUserByLogin(c.Req.Context(), &userQuery) if err != nil { return response.Error(404, "User not found", nil) } @@ -54,7 +57,7 @@ func (hs *HTTPServer) addOrgUserHelper(ctx context.Context, cmd models.AddOrgUse cmd.UserId = userToAdd.Id - if err := hs.SQLStore.AddOrgUser(ctx, &cmd); err != nil { + if err := hs.SQLStore.AddOrgUser(c.Req.Context(), &cmd); err != nil { if errors.Is(err, models.ErrOrgUserAlreadyAdded) { return response.JSON(409, util.DynMap{ "message": "User is already member of this organization", @@ -217,7 +220,7 @@ func (hs *HTTPServer) UpdateOrgUserForCurrentOrg(c *models.ReqContext) response. if err != nil { return response.Error(http.StatusBadRequest, "userId is invalid", err) } - return hs.updateOrgUserHelper(c.Req.Context(), cmd) + return hs.updateOrgUserHelper(c, cmd) } // PATCH /api/orgs/:orgId/users/:userId @@ -235,14 +238,17 @@ func (hs *HTTPServer) UpdateOrgUser(c *models.ReqContext) response.Response { if err != nil { return response.Error(http.StatusBadRequest, "userId is invalid", err) } - return hs.updateOrgUserHelper(c.Req.Context(), cmd) + return hs.updateOrgUserHelper(c, cmd) } -func (hs *HTTPServer) updateOrgUserHelper(ctx context.Context, cmd models.UpdateOrgUserCommand) response.Response { +func (hs *HTTPServer) updateOrgUserHelper(c *models.ReqContext, cmd models.UpdateOrgUserCommand) response.Response { if !cmd.Role.IsValid() { return response.Error(400, "Invalid role specified", nil) } - if err := hs.SQLStore.UpdateOrgUser(ctx, &cmd); err != nil { + if !c.OrgRole.Includes(cmd.Role) && !c.IsGrafanaAdmin { + return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) + } + if err := hs.SQLStore.UpdateOrgUser(c.Req.Context(), &cmd); err != nil { if errors.Is(err, models.ErrLastOrgAdmin) { return response.Error(400, "Cannot change role so that there is no organization admin left", nil) } diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index 3e46d269c79..1a11c61cbd5 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -570,6 +570,112 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { } } +func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) { + type accessControlTestCase2 struct { + expectedCode int + desc string + url string + method string + permissions []accesscontrol.Permission + input string + } + tests := []accessControlTestCase2{ + { + expectedCode: http.StatusOK, + desc: "org viewer with the correct permissions can add a user as a viewer to his org", + url: "/api/org/users", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersAdd, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(models.ROLE_VIEWER) + `"}`, + }, + { + expectedCode: http.StatusForbidden, + desc: "org viewer with the correct permissions cannot add a user as an editor to his org", + url: "/api/org/users", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersAdd, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(models.ROLE_EDITOR) + `"}`, + }, + { + expectedCode: http.StatusOK, + desc: "org viewer with the correct permissions can add a user as a viewer to his org", + url: "/api/orgs/1/users", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersAdd, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(models.ROLE_VIEWER) + `"}`, + }, + { + expectedCode: http.StatusForbidden, + desc: "org viewer with the correct permissions cannot add a user as an editor to his org", + url: "/api/orgs/1/users", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersAdd, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"loginOrEmail": "` + testAdminOrg2.Login + `", "role": "` + string(models.ROLE_EDITOR) + `"}`, + }, + { + expectedCode: http.StatusOK, + desc: "org viewer with the correct permissions can update a user's role to a viewer in his org", + url: fmt.Sprintf("/api/org/users/%d", testEditorOrg1.UserId), + method: http.MethodPatch, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"role": "` + string(models.ROLE_VIEWER) + `"}`, + }, + { + expectedCode: http.StatusForbidden, + desc: "org viewer with the correct permissions cannot update a user's role to a viewer in his org", + url: fmt.Sprintf("/api/org/users/%d", testEditorOrg1.UserId), + method: http.MethodPatch, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"role": "` + string(models.ROLE_EDITOR) + `"}`, + }, + { + expectedCode: http.StatusOK, + desc: "org viewer with the correct permissions can update a user's role to a viewer in his org", + url: fmt.Sprintf("/api/orgs/1/users/%d", testEditorOrg1.UserId), + method: http.MethodPatch, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"role": "` + string(models.ROLE_VIEWER) + `"}`, + }, + { + expectedCode: http.StatusForbidden, + desc: "org viewer with the correct permissions cannot update a user's role to a viewer in his org", + url: fmt.Sprintf("/api/orgs/1/users/%d", testEditorOrg1.UserId), + method: http.MethodPatch, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: accesscontrol.ScopeUsersAll}}, + input: `{"role": "` + string(models.ROLE_EDITOR) + `"}`, + }, + { + expectedCode: http.StatusOK, + desc: "org viewer with the correct permissions can invite a user as a viewer in his org", + url: "/api/org/invites", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionUsersCreate}}, + input: `{"loginOrEmail": "newUserEmail@test.com", "sendEmail": false, "role": "` + string(models.ROLE_VIEWER) + `"}`, + }, + { + expectedCode: http.StatusForbidden, + desc: "org viewer with the correct permissions cannot invite a user as an editor in his org", + url: "/api/org/invites", + method: http.MethodPost, + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionUsersCreate}}, + input: `{"loginOrEmail": "newUserEmail@test.com", "sendEmail": false, "role": "` + string(models.ROLE_EDITOR) + `"}`, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + sc := setupHTTPServer(t, true, true) + setInitCtxSignedInViewer(sc.initCtx) + setupOrgUsersDBForAccessControlTests(t, sc.db) + setAccessControlPermissions(sc.acmock, test.permissions, sc.initCtx.OrgId) + + input := strings.NewReader(test.input) + response := callAPI(sc.server, test.method, test.url, input, t) + assert.Equal(t, test.expectedCode, response.Code) + }) + } +} + func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { url := "/api/orgs/%v/users/%v" type testCase struct {