Datasource API: #32556 resolve readonly datasources can be modified (#44186)

* Check if datasource is read-only when making an update
* Standardize api returning a 404 if datasource is not found while making an update

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>, Jesse Weaver<pianohacker@gmail.com>
This commit is contained in:
Jeff Levin 2022-01-21 14:22:43 -09:00 committed by GitHub
parent 79ec3ec54c
commit c8154b9fe2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 76 additions and 3 deletions

View File

@ -87,6 +87,7 @@ func (hs *HTTPServer) getDataSourceAccessControlMetadata(c *models.ReqContext, d
return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "datasources", dsIDs)[key], nil
}
// GET /api/datasources/:id
func (hs *HTTPServer) GetDataSourceById(c *models.ReqContext) response.Response {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
@ -123,6 +124,7 @@ func (hs *HTTPServer) GetDataSourceById(c *models.ReqContext) response.Response
return response.JSON(200, &dto)
}
// DELETE /api/datasources/:id
func (hs *HTTPServer) DeleteDataSourceById(c *models.ReqContext) response.Response {
id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64)
if err != nil {
@ -220,6 +222,7 @@ func (hs *HTTPServer) DeleteDataSourceByUID(c *models.ReqContext) response.Respo
})
}
// DELETE /api/datasources/name/:name
func (hs *HTTPServer) DeleteDataSourceByName(c *models.ReqContext) response.Response {
name := web.Params(c.Req)[":name"]
@ -265,6 +268,7 @@ func validateURL(tp string, u string) response.Response {
return nil
}
// POST /api/datasources/
func AddDataSource(c *models.ReqContext) response.Response {
cmd := models.AddDataSourceCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
@ -293,6 +297,7 @@ func AddDataSource(c *models.ReqContext) response.Response {
})
}
// PUT /api/datasources/:id
func (hs *HTTPServer) UpdateDataSource(c *models.ReqContext) response.Response {
cmd := models.UpdateDataSourceCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
@ -309,6 +314,18 @@ func (hs *HTTPServer) UpdateDataSource(c *models.ReqContext) response.Response {
return resp
}
ds, err := getRawDataSourceById(c.Req.Context(), cmd.Id, cmd.OrgId)
if err != nil {
if errors.Is(err, models.ErrDataSourceNotFound) {
return response.Error(404, "Data source not found", nil)
}
return response.Error(500, "Failed to update datasource", err)
}
if ds.ReadOnly {
return response.Error(403, "Cannot update read-only data source", nil)
}
err = hs.fillWithSecureJSONData(c.Req.Context(), &cmd)
if err != nil {
return response.Error(500, "Failed to update datasource", err)

View File

@ -12,13 +12,12 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/bus"
)
const (
@ -192,6 +191,16 @@ func TestAPI_Datasources_AccessControl(t *testing.T) {
Type: "postgresql",
Access: "Proxy",
}
testDatasourceReadOnly := models.DataSource{
Id: 4,
Uid: "testUID",
OrgId: testOrgID,
Name: "test",
Url: "http://localhost:5432",
Type: "postgresql",
Access: "Proxy",
ReadOnly: true,
}
getDatasourceStub := func(ctx context.Context, query *models.GetDataSourceQuery) error {
result := testDatasource
result.Id = query.Id
@ -211,6 +220,21 @@ func TestAPI_Datasources_AccessControl(t *testing.T) {
cmd.Result = &testDatasource
return nil
}
updateDatasourceReadOnlyStub := func(ctx context.Context, cmd *models.UpdateDataSourceCommand) error {
cmd.Result = &testDatasourceReadOnly
return nil
}
getDatasourceNotFoundStub := func(ctx context.Context, cmd *models.GetDataSourceQuery) error {
cmd.Result = nil
return models.ErrDataSourceNotFound
}
getDatasourceReadOnlyStub := func(ctx context.Context, query *models.GetDataSourceQuery) error {
query.Result = &testDatasourceReadOnly
return nil
}
deleteDatasourceStub := func(ctx context.Context, cmd *models.DeleteDataSourceCommand) error {
cmd.DeletedDatasourcesCount = 1
return nil
@ -233,13 +257,28 @@ func TestAPI_Datasources_AccessControl(t *testing.T) {
})
return bytes.NewReader(s)
}
type acTestCaseWithHandler struct {
busStubs []bus.HandlerFunc
body func() io.Reader
accessControlTestCase
}
tests := []acTestCaseWithHandler{
{
busStubs: []bus.HandlerFunc{getDatasourceNotFoundStub, updateDatasourceStub},
body: updateDatasourceBody,
accessControlTestCase: accessControlTestCase{
expectedCode: http.StatusNotFound,
desc: "DatasourcesPut should return 404 if datasource not found",
url: fmt.Sprintf("/api/datasources/%v", "12345678"),
method: http.MethodPut,
permissions: []*accesscontrol.Permission{
{
Action: ActionDatasourcesWrite,
Scope: ScopeDatasourcesAll,
},
},
},
},
{
busStubs: []bus.HandlerFunc{getDatasourcesStub},
accessControlTestCase: accessControlTestCase{
@ -304,6 +343,22 @@ func TestAPI_Datasources_AccessControl(t *testing.T) {
permissions: []*accesscontrol.Permission{{Action: "wrong"}},
},
},
{
busStubs: []bus.HandlerFunc{getDatasourceReadOnlyStub, updateDatasourceReadOnlyStub},
body: updateDatasourceBody,
accessControlTestCase: accessControlTestCase{
expectedCode: http.StatusForbidden,
desc: "DatasourcesPut should return 403 for read only datasource",
url: fmt.Sprintf("/api/datasources/%v", testDatasourceReadOnly.Id),
method: http.MethodPut,
permissions: []*accesscontrol.Permission{
{
Action: ActionDatasourcesWrite,
Scope: fmt.Sprintf("datasources:id:%v", testDatasourceReadOnly.Id),
},
},
},
},
{
busStubs: []bus.HandlerFunc{getDatasourceStub, deleteDatasourceStub},
accessControlTestCase: accessControlTestCase{
@ -505,6 +560,7 @@ func TestAPI_Datasources_AccessControl(t *testing.T) {
} else {
sc.req, err = http.NewRequest(test.method, test.url, nil)
}
assert.NoError(t, err)
sc.exec()