From 553d1ae8d4bd0c96b1a93e56a3034acae7ee719a Mon Sep 17 00:00:00 2001 From: linxiaotao Date: Thu, 31 Aug 2023 17:31:12 +0800 Subject: [PATCH 1/6] fix-issue1758: create audit plan failed details: create audit plan failed due to audit plan name too long direct reason: The length of the jwt token exceeds the upper limit of varchar(255) defined in the data table, resulting in insert failure. deep reason: the length of apn is too long,resulting in the generated jwt token being too long. solution: Use MD5 to hash the name and apn that may be too long to make the name and apn for generating each token become fixed-length, thereby controlling the length of the token. @linxiaotao --- sqle/api/controller/v1/audit_plan.go | 4 +- sqle/api/middleware/jwt.go | 2 +- sqle/api/middleware/jwt_test.go | 59 ++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/sqle/api/controller/v1/audit_plan.go b/sqle/api/controller/v1/audit_plan.go index 8f66f5ca2e..118182bde1 100644 --- a/sqle/api/controller/v1/audit_plan.go +++ b/sqle/api/controller/v1/audit_plan.go @@ -318,8 +318,8 @@ func CreateAuditPlan(c echo.Context) error { // generate token j := utils.NewJWT(utils.JWTSecretKey) - t, err := j.CreateToken(currentUserName, time.Now().Add(tokenExpire).Unix(), - utils.WithAuditPlanName(req.Name)) + t, err := j.CreateToken(utils.Md5(currentUserName), time.Now().Add(tokenExpire).Unix(), + utils.WithAuditPlanName(utils.Md5(req.Name))) if err != nil { return controller.JSONBaseErrorReq(c, errors.New(errors.DataConflict, err)) } diff --git a/sqle/api/middleware/jwt.go b/sqle/api/middleware/jwt.go index 7749c053fd..f070eb3b0e 100644 --- a/sqle/api/middleware/jwt.go +++ b/sqle/api/middleware/jwt.go @@ -64,7 +64,7 @@ func ScannerVerifier() echo.MiddlewareFunc { } projectName := c.Param("project_name") apnInParam := c.Param("audit_plan_name") - if apnInToken != apnInParam { + if apnInToken != utils.Md5(apnInParam) { return echo.NewHTTPError(http.StatusInternalServerError, errAuditPlanMisMatch.Error()) } diff --git a/sqle/api/middleware/jwt_test.go b/sqle/api/middleware/jwt_test.go index d93b368090..12a1e4033e 100644 --- a/sqle/api/middleware/jwt_test.go +++ b/sqle/api/middleware/jwt_test.go @@ -146,3 +146,62 @@ func TestScannerVerifier(t *testing.T) { assert.NoError(t, err) } } + +func TestScannerVerifierIssue1758(t *testing.T) { + e := echo.New() + + jwt := utils.NewJWT(utils.JWTSecretKey) + apName120 := "aaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbbbbbaaaaaaaaaabbbbbbb120" + projectName := "default" + userName := "admin" + assert.Equal(t, 120, len(apName120)) + h := func(c echo.Context) error { + return c.HTML(http.StatusOK, "hello, world") + } + + mw := ScannerVerifier() + newContextFunc := func(token, apName string) (echo.Context, *httptest.ResponseRecorder) { + req := httptest.NewRequest(http.MethodGet, "/:audit_plan_name/", nil) + req.Header.Set(echo.HeaderAuthorization, token) + res := httptest.NewRecorder() + ctx := e.NewContext(req, res) + ctx.SetParamNames("audit_plan_name", "project_name") + ctx.SetParamValues(apName, projectName) + return ctx, res + } + { // test check success + token, err := jwt.CreateToken(utils.Md5(userName), time.Now().Add(1*time.Hour).Unix(), utils.WithAuditPlanName(utils.Md5(apName120))) + assert.NoError(t, err) + + mockDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + assert.NoError(t, err) + model.InitMockStorage(mockDB) + mock.ExpectQuery("SELECT `audit_plans`.* FROM `audit_plans` LEFT JOIN projects ON projects.id = audit_plans.project_id WHERE `audit_plans`.`deleted_at` IS NULL AND ((projects.name = ? AND audit_plans.name = ?))"). + WithArgs(projectName, apName120). + WillReturnRows(sqlmock.NewRows([]string{"name", "token"}).AddRow(userName, token)) + mock.ExpectClose() + + ctx, res := newContextFunc(token, apName120) //这里模拟上下文不需要哈希 + err = mw(h)(ctx) + assert.NoError(t, err) + assert.Contains(t, res.Body.String(), "hello, world") + + mockDB.Close() + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + } + { // test audit plan name don't match the token + token, err := jwt.CreateToken(utils.Md5(userName), time.Now().Add(1*time.Hour).Unix(), utils.WithAuditPlanName(utils.Md5(apName120))) + assert.NoError(t, err) + ctx, _ := newContextFunc(token, fmt.Sprintf("%s_modified", apName120)) + err = mw(h)(ctx) + assert.Contains(t, err.Error(), errAuditPlanMisMatch.Error()) + } + { // test unknown token + token, err := jwt.CreateToken(utils.Md5(userName), time.Now().Add(1*time.Hour).Unix()) + assert.NoError(t, err) + ctx, _ := newContextFunc(token, apName120) + err = mw(h)(ctx) + assert.Contains(t, err.Error(), "unknown token") + } +} From d2c7bb4c6852dda240cff4d1abff4157521aec09 Mon Sep 17 00:00:00 2001 From: linxiaotao Date: Mon, 4 Sep 2023 09:29:30 +0800 Subject: [PATCH 2/6] Compatible with previous versions of Token If the value determines the apn calculated by MD5, it will cause the token that was not generated according to MD5 to be invalidated, so it is also allowed to resolve the apn and the apn in the URL all the time @linxiaotao --- sqle/api/middleware/jwt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqle/api/middleware/jwt.go b/sqle/api/middleware/jwt.go index f070eb3b0e..3554c19024 100644 --- a/sqle/api/middleware/jwt.go +++ b/sqle/api/middleware/jwt.go @@ -64,7 +64,7 @@ func ScannerVerifier() echo.MiddlewareFunc { } projectName := c.Param("project_name") apnInParam := c.Param("audit_plan_name") - if apnInToken != utils.Md5(apnInParam) { + if apnInToken != apnInParam || apnInToken != utils.Md5(apnInParam) { return echo.NewHTTPError(http.StatusInternalServerError, errAuditPlanMisMatch.Error()) } From 198afbad6753f627f7cbf0766177e7bbad2645dd Mon Sep 17 00:00:00 2001 From: linxiaotao Date: Mon, 4 Sep 2023 09:34:09 +0800 Subject: [PATCH 3/6] Modify the logical calculation symbol @linxiaotao --- sqle/api/middleware/jwt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqle/api/middleware/jwt.go b/sqle/api/middleware/jwt.go index 3554c19024..e1ac909708 100644 --- a/sqle/api/middleware/jwt.go +++ b/sqle/api/middleware/jwt.go @@ -64,7 +64,7 @@ func ScannerVerifier() echo.MiddlewareFunc { } projectName := c.Param("project_name") apnInParam := c.Param("audit_plan_name") - if apnInToken != apnInParam || apnInToken != utils.Md5(apnInParam) { + if apnInToken != apnInParam && apnInToken != utils.Md5(apnInParam) { return echo.NewHTTPError(http.StatusInternalServerError, errAuditPlanMisMatch.Error()) } From e4660b041d9b6c510bd4125ba296978f6b82c5d8 Mon Sep 17 00:00:00 2001 From: linxiaotao Date: Thu, 7 Sep 2023 11:07:14 +0800 Subject: [PATCH 4/6] docs: add comments add comments for preprocessing code modifications of the auditplan token @linxiaotao --- sqle/api/controller/v1/audit_plan.go | 2 ++ sqle/api/middleware/jwt.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/sqle/api/controller/v1/audit_plan.go b/sqle/api/controller/v1/audit_plan.go index 118182bde1..bb70c8ccfe 100644 --- a/sqle/api/controller/v1/audit_plan.go +++ b/sqle/api/controller/v1/audit_plan.go @@ -317,6 +317,8 @@ func CreateAuditPlan(c echo.Context) error { } // generate token + // 为了控制JWT Token的长度，保证其长度不超过数据表定义的长度上限(255字符) + // 因此使用MD5算法将变长的 currentUserName 和 Name 转换为到固定长度 j := utils.NewJWT(utils.JWTSecretKey) t, err := j.CreateToken(utils.Md5(currentUserName), time.Now().Add(tokenExpire).Unix(), utils.WithAuditPlanName(utils.Md5(req.Name))) diff --git a/sqle/api/middleware/jwt.go b/sqle/api/middleware/jwt.go index e1ac909708..9171d8d625 100644 --- a/sqle/api/middleware/jwt.go +++ b/sqle/api/middleware/jwt.go @@ -64,6 +64,8 @@ func ScannerVerifier() echo.MiddlewareFunc { } projectName := c.Param("project_name") apnInParam := c.Param("audit_plan_name") + // 由于对生成的JWT Token的负载使用MD5算法进行预处理，因此在验证的时候也需要对param中的apn使用MD5处理 + // 为了兼容老版本的JWT Token需要增加不经MD5处理的apnInParam和apnInToken的判断 if apnInToken != apnInParam && apnInToken != utils.Md5(apnInParam) { return echo.NewHTTPError(http.StatusInternalServerError, errAuditPlanMisMatch.Error()) } From b3e145145ba5afd28e92a6fd75bb2d4eb55e4ff1 Mon Sep 17 00:00:00 2001 From: Winfred <31072467+winfredLIN@users.noreply.github.com> Date: Thu, 7 Sep 2023 11:15:55 +0800 Subject: [PATCH 5/6] docs: change comment --- sqle/api/controller/v1/audit_plan.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqle/api/controller/v1/audit_plan.go b/sqle/api/controller/v1/audit_plan.go index bb70c8ccfe..3df46b1f9e 100644 --- a/sqle/api/controller/v1/audit_plan.go +++ b/sqle/api/controller/v1/audit_plan.go @@ -318,7 +318,7 @@ func CreateAuditPlan(c echo.Context) error { // generate token // 为了控制JWT Token的长度，保证其长度不超过数据表定义的长度上限(255字符) - // 因此使用MD5算法将变长的 currentUserName 和 Name 转换为到固定长度 + // 因此使用MD5算法将变长的 currentUserName 和 Name 转换为固定长度 j := utils.NewJWT(utils.JWTSecretKey) t, err := j.CreateToken(utils.Md5(currentUserName), time.Now().Add(tokenExpire).Unix(), utils.WithAuditPlanName(utils.Md5(req.Name))) From 0ed270d51edf8c6f31045a85e57b6e8a4e95b261 Mon Sep 17 00:00:00 2001 From: linxiaotao Date: Thu, 7 Sep 2023 11:54:49 +0800 Subject: [PATCH 6/6] test: ScannerVerifier function using MD5 add test that compatible with older versions of tokens @linxiaotao --- sqle/api/middleware/jwt_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/sqle/api/middleware/jwt_test.go b/sqle/api/middleware/jwt_test.go index 12a1e4033e..965fea4eb2 100644 --- a/sqle/api/middleware/jwt_test.go +++ b/sqle/api/middleware/jwt_test.go @@ -204,4 +204,23 @@ func TestScannerVerifierIssue1758(t *testing.T) { err = mw(h)(ctx) assert.Contains(t, err.Error(), "unknown token") } + { // test old token + token, err := jwt.CreateToken(userName, time.Now().Add(1*time.Hour).Unix(), utils.WithAuditPlanName(apName120)) + assert.NoError(t, err) + mockDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + assert.NoError(t, err) + model.InitMockStorage(mockDB) + mock.ExpectQuery("SELECT `audit_plans`.* FROM `audit_plans` LEFT JOIN projects ON projects.id = audit_plans.project_id WHERE `audit_plans`.`deleted_at` IS NULL AND ((projects.name = ? AND audit_plans.name = ?))"). + WithArgs(projectName, apName120). + WillReturnRows(sqlmock.NewRows([]string{"name", "token"}).AddRow(userName, token)) + mock.ExpectClose() + + ctx, _ := newContextFunc(token, apName120) + err = mw(h)(ctx) + assert.NoError(t, err) + + mockDB.Close() + err = mock.ExpectationsWereMet() + assert.NoError(t, err) + } }