HMS-4562: Allow to keep or unset password in Customization.Users

Password is not returned in API response at all Empty string for password or ssh key can be used for removal Nil value for password or ssh key will keep previous value. Signed-off-by: Andrea Waltlova <[email protected]>
osbuild · Sep 7, 2024 · cd806cc · cd806cc
1 parent 13dc171
commit cd806cc
Show file tree

Hide file tree

Showing 8 changed files with 497 additions and 84 deletions.
diff --git a/internal/clients/composer/client.go b/internal/clients/composer/client.go
@@ -146,7 +146,3 @@ func (cc *ComposerClient) CloneCompose(id uuid.UUID, clone CloneComposeBody) (*h
 func (cc *ComposerClient) CloneStatus(id uuid.UUID) (*http.Response, error) {
 	return cc.request("GET", fmt.Sprintf("%s/clones/%s", cc.composerURL, id), nil, nil)
 }
-
-func (cu *User) IsRedacted() bool {
-	return cu.Password == nil || *cu.Password == "<REDACTED>"
-}
diff --git a/internal/v1/api.go b/internal/v1/api.go
diff --git a/internal/v1/api.yaml b/internal/v1/api.yaml
@@ -1883,7 +1883,10 @@ components:
       type: object
       required:
         - name
-        # One of (password, ssh_key) must be set, validator takes care of it
+      description: |
+        At least one of password, ssh_key must be set, validator takes care of it.
+        On update empty string can be used to remove password or ssh_key,
+        but at least one of them still must be present.
       properties:
         name:
           type: string
@@ -1897,6 +1900,8 @@ components:
           example: "$6$G91SvTj7uVp3xhqj$zVa8nqnJTlewniDII5dmvsBJnj3kloL3CXWdPDu9.e677VoRQd5zB6GKwkDvfGLoRR7NTl5nXLnJywk6IPIvS."
           description: |
             Plaintext passwords are also supported, they will be hashed and stored using the SHA-512 algorithm.
+            The password is never returned in the response.
+            Empty string can be used to remove the password during update but only with ssh_key set.
     Filesystem:
       type: object
       required:

diff --git a/internal/v1/handler.go b/internal/v1/handler.go
@@ -268,8 +268,9 @@ func (h *Handlers) GetComposeStatus(ctx echo.Context, composeId uuid.UUID) error
 		return err
 	}
 	if composeRequest.Customizations != nil && composeRequest.Customizations.Users != nil {
-		for _, u := range *composeRequest.Customizations.Users {
-			u.RedactPassword()
+		users := *composeRequest.Customizations.Users
+		for i := range users {
+			users[i].RedactPassword()
 		}
 	}
 

diff --git a/internal/v1/handler_blueprints.go b/internal/v1/handler_blueprints.go
@@ -25,6 +25,7 @@ import (
 
 var (
 	blueprintNameRegex         = regexp.MustCompile(`\S+`)
+	customizationUserNameRegex = regexp.MustCompile(`\S+`)
 	blueprintInvalidNameDetail = "The blueprint name must contain at least two characters."
 )
 
@@ -36,15 +37,10 @@ type BlueprintBody struct {
 
 func (u *User) CryptPassword() error {
 	// Prevent empty and already hashed password  from being hashed
-	if u.Password == nil || (len(*u.Password) == 0 || crypt.PasswordIsCrypted(*u.Password)) {
+	if u.Password == nil || len(*u.Password) == 0 || crypt.PasswordIsCrypted(*u.Password) {
 		return nil
 	}
 
-	if u.IsRedacted() {
-		return errors.New(
-			"password is redacted and thus can't be hashed and stored in database, use plaintext password or already hashed password",
-		)
-	}
 	pw, err := crypt.CryptSHA512(*u.Password)
 	if err != nil {
 		return err
@@ -53,15 +49,9 @@ func (u *User) CryptPassword() error {
 	return nil
 }
 
+// Set password to nil if it is not nil
 func (u *User) RedactPassword() {
-	redactedPassword := "<REDACTED>"
-	if u.Password != nil {
-		*u.Password = redactedPassword
-	}
-}
-
-func (u *User) IsRedacted() bool {
-	return u.Password == nil || *u.Password == "<REDACTED>"
+	u.Password = nil
 }
 
 func (bb *BlueprintBody) CryptPasswords() error {
@@ -84,6 +74,58 @@ func (bb *BlueprintBody) RedactPasswords() {
 	}
 }
 
+// Merges Password or SshKey from other User struct to this User struct if it is not set
+func (u *User) MergeExisting(other User) {
+	if u.Password == nil {
+		u.Password = other.Password
+	}
+	if u.SshKey == nil {
+		u.SshKey = other.SshKey
+	}
+}
+
+// User must have name and non-empty password or ssh key
+func (u *User) Valid() error {
+	validName := customizationUserNameRegex.MatchString(u.Name)
+	validPassword := u.Password != nil && len(*u.Password) > 0
+	validSshKey := u.SshKey != nil && len(*u.SshKey) > 0
+	if !validName || !(validPassword || validSshKey) {
+		return fmt.Errorf("User ('%s') must have a name and either a password or an SSH key set.", u.Name)
+	}
+	return nil
+}
+
+func (u *User) MergeForUpdate(userData []User) error {
+	// If both password and ssh_key in request user we don't need to fetch user from DB
+	if !(u.Password != nil && len(*u.Password) > 0 && u.SshKey != nil && len(*u.SshKey) > 0) {
+		eui := slices.IndexFunc(userData, func(eu User) bool {
+			return eu.Name == u.Name
+		})
+
+		if eui == -1 { // User not found in DB
+			err := u.Valid()
+			if err != nil {
+				return err
+			}
+		} else {
+			u.MergeExisting(userData[eui])
+		}
+	}
+
+	// If there is empty string in password or ssh_key, it means that we should remove it (set to nil)
+	if u.Password != nil && *u.Password == "" {
+		u.Password = nil
+	}
+	if u.SshKey != nil && *u.SshKey == "" {
+		u.SshKey = nil
+	}
+
+	if err := u.Valid(); err != nil {
+		return err
+	}
+	return nil
+}
+
 // Util function used to create and update Blueprint from API request (WRITE)
 func BlueprintFromAPI(cbr CreateBlueprintRequest) (BlueprintBody, error) {
 	bb := BlueprintBody{
@@ -105,7 +147,14 @@ func BlueprintFromEntry(be *db.BlueprintEntry) (BlueprintBody, error) {
 	if err != nil {
 		return BlueprintBody{}, err
 	}
+	return result, nil
+}
 
+func BlueprintFromEntryWithRedactedPasswords(be *db.BlueprintEntry) (BlueprintBody, error) {
+	result, err := BlueprintFromEntry(be)
+	if err != nil {
+		return BlueprintBody{}, err
+	}
 	result.RedactPasswords()
 	return result, nil
 }
@@ -122,16 +171,6 @@ func (h *Handlers) CreateBlueprint(ctx echo.Context) error {
 		return err
 	}
 
-	blueprint, err := BlueprintFromAPI(blueprintRequest)
-	if err != nil {
-		return err
-	}
-
-	body, err := json.Marshal(blueprint)
-	if err != nil {
-		return err
-	}
-
 	var metadata []byte
 	if blueprintRequest.Metadata != nil {
 		metadata, err = json.Marshal(blueprintRequest.Metadata)
@@ -157,20 +196,31 @@ func (h *Handlers) CreateBlueprint(ctx echo.Context) error {
 		desc = *blueprintRequest.Description
 	}
 
-	if blueprintRequest.Customizations.Users != nil {
-		for _, user := range *blueprintRequest.Customizations.Users {
+	users := blueprintRequest.Customizations.Users
+	if users != nil {
+		for _, user := range *users {
 			// Make sure every user has either ssh key or password set
-			if user.Password == nil && user.SshKey == nil {
+			if err := user.Valid(); err != nil {
 				return ctx.JSON(http.StatusUnprocessableEntity, HTTPErrorList{
 					Errors: []HTTPError{{
 						Title:  "Invalid user",
-						Detail: "User must have either a password or an SSH key set.",
+						Detail: err.Error(),
 					}},
 				})
 			}
 		}
 	}
 
+	blueprint, err := BlueprintFromAPI(blueprintRequest)
+	if err != nil {
+		return err
+	}
+
+	body, err := json.Marshal(blueprint)
+	if err != nil {
+		return err
+	}
+
 	err = h.server.db.InsertBlueprint(ctx.Request().Context(), id, versionId, userID.OrgID(), userID.AccountNumber(), blueprintRequest.Name, desc, body, metadata)
 	if err != nil {
 		ctx.Logger().Errorf("Error inserting id into db: %s", err.Error())
@@ -214,7 +264,7 @@ func (h *Handlers) GetBlueprint(ctx echo.Context, id openapi_types.UUID, params
 		return err
 	}
 
-	blueprint, err := BlueprintFromEntry(blueprintEntry)
+	blueprint, err := BlueprintFromEntryWithRedactedPasswords(blueprintEntry)
 	if err != nil {
 		return err
 	}
@@ -246,7 +296,7 @@ func (h *Handlers) ExportBlueprint(ctx echo.Context, id openapi_types.UUID) erro
 		return err
 	}
 
-	blueprint, err := BlueprintFromEntry(blueprintEntry)
+	blueprint, err := BlueprintFromEntryWithRedactedPasswords(blueprintEntry)
 	if err != nil {
 		return err
 	}
@@ -277,6 +327,37 @@ func (h *Handlers) UpdateBlueprint(ctx echo.Context, blueprintId uuid.UUID) erro
 		return err
 	}
 
+	if !blueprintNameRegex.MatchString(blueprintRequest.Name) {
+		return ctx.JSON(http.StatusUnprocessableEntity, HTTPErrorList{
+			Errors: []HTTPError{{
+				Title:  "Invalid blueprint name",
+				Detail: blueprintInvalidNameDetail,
+			}},
+		})
+	}
+
+	if blueprintRequest.Customizations.Users != nil {
+		be, err := h.server.db.GetBlueprint(ctx.Request().Context(), blueprintId, userID.OrgID(), nil)
+		if err != nil {
+			return err
+		}
+		eb, err := BlueprintFromEntry(be)
+		if err != nil {
+			return err
+		}
+		for i := range *blueprintRequest.Customizations.Users {
+			err := (*blueprintRequest.Customizations.Users)[i].MergeForUpdate(*eb.Customizations.Users)
+			if err != nil {
+				return ctx.JSON(http.StatusUnprocessableEntity, HTTPErrorList{
+					Errors: []HTTPError{{
+						Title:  "Invalid user",
+						Detail: err.Error(),
+					}},
+				})
+			}
+		}
+	}
+
 	blueprint, err := BlueprintFromAPI(blueprintRequest)
 	if err != nil {
 		return ctx.JSON(http.StatusUnprocessableEntity, HTTPErrorList{
@@ -286,20 +367,12 @@ func (h *Handlers) UpdateBlueprint(ctx echo.Context, blueprintId uuid.UUID) erro
 			}},
 		})
 	}
+
 	body, err := json.Marshal(blueprint)
 	if err != nil {
 		return err
 	}
 
-	if !blueprintNameRegex.MatchString(blueprintRequest.Name) {
-		return ctx.JSON(http.StatusUnprocessableEntity, HTTPErrorList{
-			Errors: []HTTPError{{
-				Title:  "Invalid blueprint name",
-				Detail: blueprintInvalidNameDetail,
-			}},
-		})
-	}
-
 	versionId := uuid.New()
 	desc := ""
 	if blueprintRequest.Description != nil {
@@ -335,7 +408,7 @@ func (h *Handlers) ComposeBlueprint(ctx echo.Context, id openapi_types.UUID) err
 	if err != nil {
 		return err
 	}
-	blueprint, err := BlueprintFromEntry(blueprintEntry)
+	blueprint, err := BlueprintFromEntryWithRedactedPasswords(blueprintEntry)
 	if err != nil {
 		return err
 	}