Added strict validation for username and password in backend. (#4670)

* Added strict validation for username and password in backend. Signed-off-by: aryan <[email protected]> * fixed silly mistake Signed-off-by: aryan <[email protected]> * some requested changes. Signed-off-by: aryan <[email protected]> * modified tests Signed-off-by: aryan <[email protected]> * small change Signed-off-by: aryan <[email protected]> * Update message string in chaoscenter/authentication/api/handlers/doc.go Co-authored-by: Vedant Shrotria <[email protected]> Signed-off-by: Aryan Bhokare <[email protected]> * Update error message chaoscenter/authentication/pkg/utils/sanitizers.go Co-authored-by: Vedant Shrotria <[email protected]> Signed-off-by: Aryan Bhokare <[email protected]> * Modified swagger with requested changes. Signed-off-by: aryan <[email protected]> * added negative tests for requested functions and fixed some conflicts. Signed-off-by: aryan <[email protected]> --------- Signed-off-by: aryan <[email protected]> Signed-off-by: Aryan Bhokare <[email protected]> Co-authored-by: Vedant Shrotria <[email protected]> Co-authored-by: Saranya Jena <[email protected]>
litmuschaos · Jun 27, 2024 · 9d2c93a · 9d2c93a
1 parent 544d324
commit 9d2c93a
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 23 deletions.
diff --git a/chaoscenter/authentication/api/docs/docs.go b/chaoscenter/authentication/api/docs/docs.go
@@ -940,6 +940,12 @@ const docTemplate = `{
                             "$ref": "#/definitions/response.ErrInvalidRequest"
                         }
                     },
+                    "401": {
+                        "description": "Unauthorized",
+                        "schema": {
+                            "$ref": "#/definitions/response.ErrStrictUsernamePolicyViolation"
+                        }
+                    },
                     "500": {
                         "description": "Internal Server Error",
                         "schema": {
@@ -1214,7 +1220,20 @@ const docTemplate = `{
                 },
                 "message": {
                     "type": "string",
-                    "example": "Please ensure the password is 8 characters long and has 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"
+                    "example": "Please ensure the password is atleast 8 characters and atmost 16 characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"
+                }
+            }
+        },
+        "response.ErrStrictUsernamePolicyViolation": {
+            "type": "object",
+            "properties": {
+                "code": {
+                    "type": "integer",
+                    "example": 401
+                },
+                "message": {
+                    "type": "string",
+                    "example": "The username should be atleast 3 characters long and atmost 16 characters long."
                 }
             }
         },

diff --git a/chaoscenter/authentication/api/docs/swagger.json b/chaoscenter/authentication/api/docs/swagger.json
@@ -930,6 +930,12 @@
                             "$ref": "#/definitions/response.ErrInvalidRequest"
                         }
                     },
+                    "401": {
+                        "description": "Unauthorized",
+                        "schema": {
+                            "$ref": "#/definitions/response.ErrStrictUsernamePolicyViolation"
+                        }
+                    },
                     "500": {
                         "description": "Internal Server Error",
                         "schema": {
@@ -1204,7 +1210,20 @@
                 },
                 "message": {
                     "type": "string",
-                    "example": "Please ensure the password is 8 characters long and has 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"
+                    "example": "Please ensure the password is atleast 8 characters and atmost 16 characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"
+                }
+            }
+        },
+        "response.ErrStrictUsernamePolicyViolation": {
+            "type": "object",
+            "properties": {
+                "code": {
+                    "type": "integer",
+                    "example": 401
+                },
+                "message": {
+                    "type": "string",
+                    "example": "The username should be atleast 3 characters long and atmost 16 characters long."
                 }
             }
         },

diff --git a/chaoscenter/authentication/api/docs/swagger.yaml b/chaoscenter/authentication/api/docs/swagger.yaml
@@ -82,8 +82,19 @@ definitions:
         example: 401
         type: integer
       message:
-        example: Please ensure the password is 8 characters long and has 1 digit,
-          1 lowercase alphabet, 1 uppercase alphabet and 1 special character
+        example: Please ensure the password is atleast 8 characters and atmost 16
+          characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase
+          alphabet and 1 special character
+        type: string
+    type: object
+  response.ErrStrictUsernamePolicyViolation:
+    properties:
+      code:
+        example: 401
+        type: integer
+      message:
+        example: The username should be atleast 3 characters long and atmost 16 characters
+          long.
         type: string
     type: object
   response.ErrUnauthorized:
@@ -762,6 +773,10 @@ paths:
           description: Bad Request
           schema:
             $ref: '#/definitions/response.ErrInvalidRequest'
+        "401":
+          description: Unauthorized
+          schema:
+            $ref: '#/definitions/response.ErrStrictUsernamePolicyViolation'
         "500":
           description: Internal Server Error
           schema:

diff --git a/chaoscenter/authentication/api/handlers/doc.go b/chaoscenter/authentication/api/handlers/doc.go
@@ -97,7 +97,12 @@ type ErrUserDeactivated struct {
 
 type ErrStrictPasswordPolicyViolation struct {
 	Code    int    `json:"code" example:"401"`
-	Message string `json:"message" example:"Please ensure the password is 8 characters long and has 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"`
+	Message string `json:"message" example:"Please ensure the password is atleast 8 characters and atmost 16 characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"`
+}
+
+type ErrStrictUsernamePolicyViolation struct {
+	Code    int    `json:"code" example:"401"`
+	Message string `json:"message" example:"The username should be atleast 3 characters long and atmost 16 characters long."`
 }
 
 type ErrEmptyProjectName struct {

diff --git a/chaoscenter/authentication/api/handlers/rest/user_handlers.go b/chaoscenter/authentication/api/handlers/rest/user_handlers.go
@@ -28,6 +28,8 @@ const BearerSchema = "Bearer "
 //	@Failure		400	{object}	response.ErrInvalidRequest
 //	@Failure		401	{object}	response.ErrUnauthorized
 //	@Failure		400	{object}	response.ErrInvalidEmail
+//	@Failure		401	{object}	response.ErrStrictPasswordPolicyViolation
+//	@Failure		401	{object}	response.ErrStrictUsernamePolicyViolation
 //	@Failure		401	{object}	response.ErrUserExists
 //	@Failure		500	{object}	response.ErrServerError
 //	@Success		200	{object}	response.UserResponse{}
@@ -59,6 +61,12 @@ func CreateUser(service services.ApplicationService) gin.HandlerFunc {
 			c.JSON(utils.ErrorStatusCodes[utils.ErrInvalidRequest], presenter.CreateErrorResponse(utils.ErrInvalidRequest))
 			return
 		}
+		//username validation
+		err = utils.ValidateStrictUsername(userRequest.Username)
+		if err != nil {
+			c.JSON(utils.ErrorStatusCodes[utils.ErrStrictUsernamePolicyViolation], presenter.CreateErrorResponse(utils.ErrStrictUsernamePolicyViolation))
+			return
+		}
 
 		// Assigning UID to user
 		uID := uuid.Must(uuid.NewRandom()).String()
@@ -108,6 +116,8 @@ func CreateUser(service services.ApplicationService) gin.HandlerFunc {
 //	@Accept			json
 //	@Produce		json
 //	@Failure		400	{object}	response.ErrInvalidRequest
+//	@Failure		401	{object}	response.ErrStrictPasswordPolicyViolation
+//	@Failure		401	{object}	response.ErrStrictUsernamePolicyViolation
 //	@Failure		500	{object}	response.ErrServerError
 //	@Success		200	{object}	response.MessageResponse{}
 //	@Router			/update/details [post]
@@ -414,17 +424,17 @@ func UpdatePassword(service services.ApplicationService) gin.HandlerFunc {
 		}
 		username := c.MustGet("username").(string)
 		userPasswordRequest.Username = username
-		if utils.StrictPasswordPolicy {
+		if userPasswordRequest.NewPassword != "" {
 			err := utils.ValidateStrictPassword(userPasswordRequest.NewPassword)
 			if err != nil {
 				c.JSON(utils.ErrorStatusCodes[utils.ErrStrictPasswordPolicyViolation], presenter.CreateErrorResponse(utils.ErrStrictPasswordPolicyViolation))
 				return
 			}
-		}
-		if userPasswordRequest.NewPassword == "" {
+		} else {
 			c.JSON(utils.ErrorStatusCodes[utils.ErrInvalidRequest], presenter.CreateErrorResponse(utils.ErrInvalidRequest))
 			return
 		}
+
 		err = service.UpdatePassword(&userPasswordRequest, true)
 		if err != nil {
 			log.Info(err)
@@ -478,7 +488,7 @@ func ResetPassword(service services.ApplicationService) gin.HandlerFunc {
 			c.JSON(utils.ErrorStatusCodes[utils.ErrServerError], presenter.CreateErrorResponse(utils.ErrPasswordNotUpdated))
 		}
 
-		if utils.StrictPasswordPolicy {
+		if userPasswordRequest.NewPassword != "" {
 			err := utils.ValidateStrictPassword(userPasswordRequest.NewPassword)
 			if err != nil {
 				c.JSON(utils.ErrorStatusCodes[utils.ErrStrictPasswordPolicyViolation], presenter.CreateErrorResponse(utils.ErrStrictPasswordPolicyViolation))

diff --git a/chaoscenter/authentication/api/handlers/rest/user_handlers_test.go b/chaoscenter/authentication/api/handlers/rest/user_handlers_test.go
@@ -55,7 +55,7 @@ func TestCreateUser(t *testing.T) {
 			name: "successfully",
 			inputBody: &entities.User{
 				Username: "newUser",
-				Password: "validPassword123",
+				Password: "ValidPassword@1",
 				Email:    "[email protected]",
 				Name:     "John Doe",
 				Role:     entities.RoleUser,
@@ -68,7 +68,7 @@ func TestCreateUser(t *testing.T) {
 					Email:    "[email protected]",
 					Name:     "John Doe",
 					Role:     entities.RoleUser,
-				}, nil)
+				}, nil).Once()
 			},
 			expectedCode: 200,
 		},
@@ -80,8 +80,12 @@ func TestCreateUser(t *testing.T) {
 			c, _ := gin.CreateTestContext(w)
 			c.Set("role", tc.mockRole)
 			if tc.inputBody != nil {
-				b, _ := json.Marshal(tc.inputBody)
+				b, err := json.Marshal(tc.inputBody)
+				if err != nil {
+					t.Fatalf("could not marshal input body: %v", err)
+				}
 				c.Request = httptest.NewRequest(http.MethodPost, "/users", bytes.NewBuffer(b))
+				c.Request.Header.Set("Content-Type", "application/json")
 			}
 
 			tc.given()
@@ -477,13 +481,22 @@ func TestUpdatePassword(t *testing.T) {
 	}{
 		{
 			name:                 "Successfully update password",
-			givenBody:            `{"oldPassword":"oldPass", "newPassword":"newPass"}`,
+			givenBody:            `{"oldPassword":"oldPass@123", "newPassword":"newPass@123"}`,
 			givenUsername:        "testUser",
 			givenStrictPassword:  false,
 			givenServiceResponse: nil,
 			expectedCode:         http.StatusOK,
 			expectedOutput:       `{"message":"password has been updated successfully"}`,
 		},
+		{
+			name:                 "Invalid new password",
+			givenBody:            `{"oldPassword":"oldPass@123", "newPassword":"short"}`,
+			givenUsername:        "testUser",
+			givenStrictPassword:  false,
+			givenServiceResponse: errors.New("invalid password"),
+			expectedCode:         utils.ErrorStatusCodes[utils.ErrStrictPasswordPolicyViolation],
+			expectedOutput:       `{"error":"password_policy_violation","errorDescription":"Please ensure the password is atleast 8 characters long and atmost 16 characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character"}`,
+		},
 	}
 
 	for _, tt := range tests {
@@ -498,8 +511,8 @@ func TestUpdatePassword(t *testing.T) {
 
 			userPassword := entities.UserPassword{
 				Username:    tt.givenUsername,
-				OldPassword: "oldPass",
-				NewPassword: "newPass",
+				OldPassword: "oldPass@123",
+				NewPassword: "newPass@123",
 			}
 			user := &entities.User{
 				ID:             "testUID",
@@ -532,11 +545,11 @@ func TestResetPassword(t *testing.T) {
 		expectedCode int
 	}{
 		{
-			name: "Non-admin role",
+			name: "Admin role",
 			inputBody: &entities.UserPassword{
 				Username:    "testUser",
 				OldPassword: "",
-				NewPassword: "validPassword123",
+				NewPassword: "ValidPass@123",
 			},
 			mockRole:     "admin",
 			mockUID:      "testUID",
@@ -559,7 +572,7 @@ func TestResetPassword(t *testing.T) {
 			inputBody: &entities.UserPassword{
 				Username:    "testUser",
 				OldPassword: "",
-				NewPassword: "validPassword123",
+				NewPassword: "validPass@123",
 			},
 			mockRole:     "user",
 			mockUID:      "testUID",
@@ -581,6 +594,29 @@ func TestResetPassword(t *testing.T) {
 			mockUsername: "adminUser",
 			expectedCode: utils.ErrorStatusCodes[utils.ErrInvalidRequest],
 		},
+		{
+			name: "Admin role wrong password",
+			inputBody: &entities.UserPassword{
+				Username:    "testUser",
+				OldPassword: "",
+				NewPassword: "short",
+			},
+			mockRole:     "admin",
+			mockUID:      "testUID",
+			mockUsername: "adminUser",
+			given: func() {
+				user := &entities.User{
+					ID:             "testUID",
+					Username:       "testUser",
+					Email:          "[email protected]",
+					IsInitialLogin: false,
+				}
+				service.On("GetUser", "testUID").Return(user, nil)
+				service.On("IsAdministrator", mock.AnythingOfType("*entities.User")).Return(nil)
+				service.On("UpdatePassword", mock.AnythingOfType("*entities.UserPassword"), false).Return(nil)
+			},
+			expectedCode: utils.ErrorStatusCodes[utils.ErrStrictPasswordPolicyViolation],
+		},
 	}
 
 	for _, tt := range tests {

diff --git a/chaoscenter/authentication/pkg/utils/errors.go b/chaoscenter/authentication/pkg/utils/errors.go
@@ -10,6 +10,7 @@ var (
 	ErrServerError                   AppError = errors.New("server_error")
 	ErrInvalidRequest                AppError = errors.New("invalid_request")
 	ErrStrictPasswordPolicyViolation AppError = errors.New("password_policy_violation")
+	ErrStrictUsernamePolicyViolation AppError = errors.New("username_policy_violation")
 	ErrUnauthorized                  AppError = errors.New("unauthorized")
 	ErrUserExists                    AppError = errors.New("user_exists")
 	ErrUserNotFound                  AppError = errors.New("user does not exist")
@@ -32,6 +33,7 @@ var ErrorStatusCodes = map[AppError]int{
 	ErrUnauthorized:                  401,
 	ErrUserExists:                    401,
 	ErrStrictPasswordPolicyViolation: 401,
+	ErrStrictUsernamePolicyViolation: 401,
 	ErrUserNotFound:                  400,
 	ErrProjectNotFound:               400,
 	ErrUpdatingAdmin:                 400,
@@ -50,7 +52,8 @@ var ErrorDescriptions = map[AppError]string{
 	ErrInvalidRequest:                "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed",
 	ErrUnauthorized:                  "The user does not have requested authorization to access this resource",
 	ErrUserExists:                    "This username is already assigned to another user",
-	ErrStrictPasswordPolicyViolation: "Please ensure the password is 8 characters long and has 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character",
+	ErrStrictPasswordPolicyViolation: "Please ensure the password is atleast 8 characters long and atmost 16 characters long and has atleast 1 digit, 1 lowercase alphabet, 1 uppercase alphabet and 1 special character",
+	ErrStrictUsernamePolicyViolation: "The username should be atleast 3 characters long and atmost 16 characters long.",
 	ErrEmptyProjectName:              "Project name can't be empty",
 	ErrInvalidRole:                   "Role is invalid",
 	ErrProjectNotFound:               "This project does not exist",

diff --git a/chaoscenter/authentication/pkg/utils/sanitizers.go b/chaoscenter/authentication/pkg/utils/sanitizers.go
@@ -13,20 +13,25 @@ func SanitizeString(input string) string {
 
 /*
 ValidateStrictPassword represents and checks for the following patterns:
-- Input is at least 8 characters long
-- Input contains at least one special character
+- Input is at least 8 characters long and at most 16 characters long
+- Input contains at least one special character of these @$!%*?_&
 - Input contains at least one digit
 - Input contains at least one uppercase alphabet
 - Input contains at least one lowercase alphabet
 */
 func ValidateStrictPassword(input string) error {
 	if len(input) < 8 {
-		return fmt.Errorf("password is less than 8 characters")
+		return fmt.Errorf("password length is less than 8 characters")
 	}
+
+	if len(input) > 16 {
+		return fmt.Errorf("password length is more than 16 characters")
+	}
+
 	digits := `[0-9]{1}`
 	lowerAlphabets := `[a-z]{1}`
 	capitalAlphabets := `[A-Z]{1}`
-	specialCharacters := `[!@#~$%^&*()+|_]{1}`
+	specialCharacters := `[@$!%*?_&]{1}`
 	if b, err := regexp.MatchString(digits, input); !b || err != nil {
 		return fmt.Errorf("password does not contain digits")
 	}
@@ -41,3 +46,16 @@ func ValidateStrictPassword(input string) error {
 	}
 	return nil
 }
+
+// Username must start with a letter - ^[a-zA-Z]
+// Allow letters, digits, underscores, and hyphens - [a-zA-Z0-9_-]
+// Ensure the length of the username is between 3 and 16 characters (1 character is already matched above) - {2,15}$
+
+func ValidateStrictUsername(username string) error {
+	// Ensure username doesn't contain special characters (only letters, numbers, and underscores are allowed)
+	if matched, _ := regexp.MatchString(`^[a-zA-Z][a-zA-Z0-9_-]{2,15}$`, username); !matched {
+		return fmt.Errorf("username can only contain letters, numbers, and underscores")
+	}
+
+	return nil
+}