From 1bd3d59ff8307689063f30f8b50c1b1f824a577d Mon Sep 17 00:00:00 2001 From: Sonja Happ Date: Tue, 10 Sep 2019 12:24:03 +0200 Subject: [PATCH] increase code coverage of dashboard tests, do not allow updating scenario ID of dashboard --- routes/dashboard/dashboard_endpoints.go | 16 ++--- routes/dashboard/dashboard_methods.go | 1 - routes/dashboard/dashboard_test.go | 86 ++++++++++++++++++++++-- routes/dashboard/dashboard_validators.go | 14 ++-- 4 files changed, 91 insertions(+), 26 deletions(-) diff --git a/routes/dashboard/dashboard_endpoints.go b/routes/dashboard/dashboard_endpoints.go index 54883de..3dfd8dd 100644 --- a/routes/dashboard/dashboard_endpoints.go +++ b/routes/dashboard/dashboard_endpoints.go @@ -78,15 +78,14 @@ func addDashboard(c *gin.Context) { newDashboard := req.createDashboard() // Check if user is allowed to modify scenario specified in request - ok, _ := scenario.CheckPermissions(c, common.Create, "body", int(newDashboard.ScenarioID)) + ok, _ := scenario.CheckPermissions(c, common.Update, "body", int(newDashboard.ScenarioID)) if !ok { return } // add dashboard to DB and add association to scenario err := newDashboard.addToScenario() - if err != nil { - common.DBError(c, err) + if common.DBError(c, err) { return } @@ -126,16 +125,11 @@ func updateDashboard(c *gin.Context) { return } // Create the updatedDashboard from oldDashboard - updatedDashboard, err := req.updatedDashboard(oldDashboard) - if err != nil { - common.BadRequestError(c, err.Error()) - return - } + updatedDashboard := req.updatedDashboard(oldDashboard) // update the dashboard in the DB - err = oldDashboard.update(updatedDashboard) - if err != nil { - common.DBError(c, err) + err := oldDashboard.update(updatedDashboard) + if common.DBError(c, err) { return } diff --git a/routes/dashboard/dashboard_methods.go b/routes/dashboard/dashboard_methods.go index 2747a8e..d4a3b7f 100644 --- a/routes/dashboard/dashboard_methods.go +++ b/routes/dashboard/dashboard_methods.go @@ -48,7 +48,6 @@ func (d *Dashboard) update(modifiedDab Dashboard) error { db := common.GetDB() - // TODO do we allow to update scenarioID here as well? err := db.Model(d).Updates(map[string]interface{}{ "Name": modifiedDab.Name, "Grid": modifiedDab.Grid, diff --git a/routes/dashboard/dashboard_test.go b/routes/dashboard/dashboard_test.go index a343259..04046a7 100644 --- a/routes/dashboard/dashboard_test.go +++ b/routes/dashboard/dashboard_test.go @@ -36,8 +36,11 @@ func addScenario(token string) (scenarioID uint) { Running: common.ScenarioA.Running, StartParameters: common.ScenarioA.StartParameters, } - _, resp, _ := common.TestEndpoint(router, token, + _, resp, err := common.TestEndpoint(router, token, "/api/scenarios", "POST", common.KeyModels{"scenario": newScenario}) + if err != nil { + panic(fmt.Sprint("The following error happend on POSTing a scenario: ", err.Error())) + } // Read newScenario's ID from the response newScenarioID, _ := common.GetResponseID(resp) @@ -105,7 +108,7 @@ func TestAddDashboard(t *testing.T) { assert.NoError(t, err) // try to POST a malformed dashboard - // Required fields are missing + // Required fields are missing (validation should fail) malformedNewDashboard := DashboardRequest{ Name: "ThisIsAMalformedDashboard", } @@ -115,6 +118,38 @@ func TestAddDashboard(t *testing.T) { assert.NoError(t, err) assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + // this should NOT work and return a bad request 400 status code + code, resp, err = common.TestEndpoint(router, token, + "/api/dashboards", "POST", "This is a test using plain text as body") + assert.NoError(t, err) + assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) + + // try to add a dashboard to a scenario that does not exist + // should return not found error + newDashboard.ScenarioID = scenarioID + 1 + code, resp, err = common.TestEndpoint(router, token, + "/api/dashboards", "POST", common.KeyModels{"dashboard": newDashboard}) + assert.NoError(t, err) + assert.Equalf(t, 404, code, "Response body: \n%v\n", resp) + + // try to get dashboard as a user that is not in the scenario (userB) + token, err = common.AuthenticateForTest(router, + "/api/authenticate", "POST", common.UserBCredentials) + assert.NoError(t, err) + + // this should fail with unprocessable entity + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards/%v", newDashboardID), "GET", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // try to add a dashboard to a scenario to which the user has no access + // this should give an unprocessable entity error + newDashboard.ScenarioID = scenarioID + code, resp, err = common.TestEndpoint(router, token, + "/api/dashboards", "POST", common.KeyModels{"dashboard": newDashboard}) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) } func TestUpdateDashboard(t *testing.T) { @@ -174,6 +209,11 @@ func TestUpdateDashboard(t *testing.T) { assert.NoError(t, err) assert.Equalf(t, 404, code, "Response body: \n%v\n", resp) + // try to update with a malformed body, should return a bad request error + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards/%v", newDashboardID), "PUT", "This is the body of a malformed update request.") + assert.NoError(t, err) + assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) } func TestDeleteDashboard(t *testing.T) { @@ -187,7 +227,6 @@ func TestDeleteDashboard(t *testing.T) { assert.NoError(t, err) scenarioID := addScenario(token) - fmt.Println(scenarioID) // test POST dashboards/ $newDashboard newDashboard := DashboardRequest{ @@ -204,6 +243,28 @@ func TestDeleteDashboard(t *testing.T) { newDashboardID, err := common.GetResponseID(resp) assert.NoError(t, err) + // try to delete a dashboard from a scenario to which the user has no access + token, err = common.AuthenticateForTest(router, + "/api/authenticate", "POST", common.UserBCredentials) + assert.NoError(t, err) + + // this should fail with unprocessable entity + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards/%v", newDashboardID), "DELETE", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // authenticate as normal user + token, err = common.AuthenticateForTest(router, + "/api/authenticate", "POST", common.UserACredentials) + assert.NoError(t, err) + + // try to delete a dashboard that does not exist; should return a not found error + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards/%v", newDashboardID+1), "DELETE", nil) + assert.NoError(t, err) + assert.Equalf(t, 404, code, "Response body: \n%v\n", resp) + // Count the number of all the dashboards returned for scenario initialNumber, err := common.LengthOfResponse(router, token, fmt.Sprintf("/api/dashboards?scenarioID=%v", scenarioID), "GET", nil) @@ -239,7 +300,6 @@ func TestGetAllDashboardsOfScenario(t *testing.T) { assert.NoError(t, err) scenarioID := addScenario(token) - fmt.Println(scenarioID) // Count the number of all the dashboards returned for scenario initialNumber, err := common.LengthOfResponse(router, token, @@ -274,4 +334,22 @@ func TestGetAllDashboardsOfScenario(t *testing.T) { assert.NoError(t, err) assert.Equal(t, initialNumber+2, finalNumber) + + // try to get all dashboards of a scenario that does not exist (should fail with not found) + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards?scenarioID=%v", scenarioID+1), "GET", nil) + assert.NoError(t, err) + assert.Equalf(t, 404, code, "Response body: \n%v\n", resp) + + // try to get all dashboards as a user that does not belong to scenario + token, err = common.AuthenticateForTest(router, + "/api/authenticate", "POST", common.UserBCredentials) + assert.NoError(t, err) + + // this should fail with unprocessable entity + code, resp, err = common.TestEndpoint(router, token, + fmt.Sprintf("/api/dashboards?scenarioID=%v", scenarioID), "GET", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + } diff --git a/routes/dashboard/dashboard_validators.go b/routes/dashboard/dashboard_validators.go index e013cf9..dc92996 100644 --- a/routes/dashboard/dashboard_validators.go +++ b/routes/dashboard/dashboard_validators.go @@ -13,9 +13,8 @@ type validNewDashboard struct { } type validUpdatedDashboard struct { - Name string `form:"Name" validate:"omitempty"` - Grid int `form:"Grid" validate:"omitempty"` - ScenarioID uint `form:"ScenarioID" validate:"omitempty"` + Name string `form:"Name" validate:"omitempty"` + Grid int `form:"Grid" validate:"omitempty"` } type addDashboardRequest struct { @@ -48,7 +47,7 @@ func (r *addDashboardRequest) createDashboard() Dashboard { return s } -func (r *updateDashboardRequest) updatedDashboard(oldDashboard Dashboard) (Dashboard, error) { +func (r *updateDashboardRequest) updatedDashboard(oldDashboard Dashboard) Dashboard { // Use the old Dashboard as a basis for the updated Dashboard `s` s := oldDashboard @@ -60,10 +59,5 @@ func (r *updateDashboardRequest) updatedDashboard(oldDashboard Dashboard) (Dashb s.Grid = r.Grid } - if r.ScenarioID != 0 { - // TODO do we allow this case? - //s.ScenarioID = r.ScenarioID - } - - return s, nil + return s }