From 88eefd0aeb87888628e215ee81ae588625af5f35 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 3 Mar 2023 14:01:11 +0100 Subject: [PATCH] [bugfix] Clamp admin report limit <1 to 100 (#1583) * [bugfix] Clamp report limit <1 to 100 * add + update tests --- docs/api/swagger.yaml | 2 +- internal/api/client/admin/reportsget.go | 7 ++-- internal/api/client/admin/reportsget_test.go | 36 +++++++++++++++++--- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/api/swagger.yaml b/docs/api/swagger.yaml index 783e1147e..e9719f43e 100644 --- a/docs/api/swagger.yaml +++ b/docs/api/swagger.yaml @@ -3763,7 +3763,7 @@ paths: name: min_id type: string - default: 20 - description: Number of reports to return. If less than 1, will be clamped to 1. If more than 100, will be clamped to 100. + description: Number of reports to return. If more than 100 or less than 1, will be clamped to 100. in: query name: limit type: integer diff --git a/internal/api/client/admin/reportsget.go b/internal/api/client/admin/reportsget.go index b41877b84..58a8d3163 100644 --- a/internal/api/client/admin/reportsget.go +++ b/internal/api/client/admin/reportsget.go @@ -97,8 +97,7 @@ import ( // type: integer // description: >- // Number of reports to return. -// If less than 1, will be clamped to 1. -// If more than 100, will be clamped to 100. +// If more than 100 or less than 1, will be clamped to 100. // default: 20 // in: query // @@ -163,9 +162,7 @@ func (m *Module) ReportsGETHandler(c *gin.Context) { } // normalize - if i <= 0 { - i = 1 - } else if i >= 100 { + if i < 1 || i > 100 { i = 100 } limit = i diff --git a/internal/api/client/admin/reportsget_test.go b/internal/api/client/admin/reportsget_test.go index 706dcbc44..6628a4286 100644 --- a/internal/api/client/admin/reportsget_test.go +++ b/internal/api/client/admin/reportsget_test.go @@ -124,7 +124,7 @@ func (suite *ReportsGetTestSuite) getReports( return resp, result.Header.Get("Link"), nil } -func (suite *ReportsGetTestSuite) TestReportsGet1() { +func (suite *ReportsGetTestSuite) TestReportsGetAll() { testAccount := suite.testAccounts["admin_account"] testToken := suite.testTokens["admin_account"] testUser := suite.testUsers["admin_account"] @@ -515,7 +515,7 @@ func (suite *ReportsGetTestSuite) TestReportsGet1() { suite.Equal(`; rel="next", ; rel="prev"`, link) } -func (suite *ReportsGetTestSuite) TestReportsGet2() { +func (suite *ReportsGetTestSuite) TestReportsGetCreatedByAccount() { testAccount := suite.testAccounts["admin_account"] testToken := suite.testTokens["admin_account"] testUser := suite.testUsers["admin_account"] @@ -716,7 +716,7 @@ func (suite *ReportsGetTestSuite) TestReportsGet2() { suite.Equal(`; rel="next", ; rel="prev"`, link) } -func (suite *ReportsGetTestSuite) TestReportsGet3() { +func (suite *ReportsGetTestSuite) TestReportsGetTargetAccount() { testAccount := suite.testAccounts["admin_account"] testToken := suite.testTokens["admin_account"] testUser := suite.testUsers["admin_account"] @@ -917,7 +917,7 @@ func (suite *ReportsGetTestSuite) TestReportsGet3() { suite.Equal(`; rel="next", ; rel="prev"`, link) } -func (suite *ReportsGetTestSuite) TestReportsGet4() { +func (suite *ReportsGetTestSuite) TestReportsGetResolvedTargetAccount() { testAccount := suite.testAccounts["admin_account"] testToken := suite.testTokens["admin_account"] testUser := suite.testUsers["admin_account"] @@ -935,7 +935,7 @@ func (suite *ReportsGetTestSuite) TestReportsGet4() { suite.Empty(link) } -func (suite *ReportsGetTestSuite) TestReportsGet6() { +func (suite *ReportsGetTestSuite) TestReportsGetNotAdmin() { testAccount := suite.testAccounts["local_account_1"] testToken := suite.testTokens["local_account_1"] testUser := suite.testUsers["local_account_1"] @@ -945,6 +945,32 @@ func (suite *ReportsGetTestSuite) TestReportsGet6() { suite.Empty(reports) } +func (suite *ReportsGetTestSuite) TestReportsGetZeroLimit() { + testAccount := suite.testAccounts["admin_account"] + testToken := suite.testTokens["admin_account"] + testUser := suite.testUsers["admin_account"] + + reports, link, err := suite.getReports(testAccount, testToken, testUser, http.StatusOK, "", nil, "", "", "", "", "", 0) + suite.NoError(err) + suite.Len(reports, 2) + + // Limit in Link header should be set to 100 + suite.Equal(`; rel="next", ; rel="prev"`, link) +} + +func (suite *ReportsGetTestSuite) TestReportsGetHighLimit() { + testAccount := suite.testAccounts["admin_account"] + testToken := suite.testTokens["admin_account"] + testUser := suite.testUsers["admin_account"] + + reports, link, err := suite.getReports(testAccount, testToken, testUser, http.StatusOK, "", nil, "", "", "", "", "", 2000) + suite.NoError(err) + suite.Len(reports, 2) + + // Limit in Link header should be set to 100 + suite.Equal(`; rel="next", ; rel="prev"`, link) +} + func TestReportsGetTestSuite(t *testing.T) { suite.Run(t, &ReportsGetTestSuite{}) }