From 1609ab3d36de2f85e7c381e1758d5989ddfa03b6 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Wed, 7 Feb 2024 13:47:07 +0100
Subject: [PATCH 1/2] osbuild: test user creation

---
 pkg/osbuild/users_stage_test.go | 44 ++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/pkg/osbuild/users_stage_test.go b/pkg/osbuild/users_stage_test.go
index 30b3bc15b1..866364d206 100644
--- a/pkg/osbuild/users_stage_test.go
+++ b/pkg/osbuild/users_stage_test.go
@@ -4,9 +4,11 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/osbuild/images/pkg/customizations/users"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/osbuild/images/internal/common"
+	"github.com/osbuild/images/pkg/customizations/users"
 )
 
 func TestNewUsersStage(t *testing.T) {
@@ -57,3 +59,43 @@ func TestNewUsersStageOptionsPassword(t *testing.T) {
 	// homer's password should still be nil (locked account)
 	assert.Nil(t, options.Users["homer"].Password)
 }
+
+func TestGenUsersStageSameAsNewUsersStageOptions(t *testing.T) {
+	users := []users.User{
+		{
+			Name: "user1", UID: common.ToPtr(1000), GID: common.ToPtr(1000),
+			Groups:      []string{"grp1"},
+			Description: common.ToPtr("some-descr"),
+			Home:        common.ToPtr("/home/user1"),
+			Shell:       common.ToPtr("/bin/zsh"),
+			Key:         common.ToPtr("some-key"),
+		},
+	}
+	expected := &UsersStageOptions{
+		Users: map[string]UsersStageOptionsUser{
+			"user1": {
+				UID:         common.ToPtr(1000),
+				GID:         common.ToPtr(1000),
+				Groups:      []string{"grp1"},
+				Description: common.ToPtr("some-descr"),
+				Home:        common.ToPtr("/home/user1"),
+				Shell:       common.ToPtr("/bin/zsh"),
+				Key:         common.ToPtr("some-key")},
+		},
+	}
+
+	// check that NewUsersStageOptions creates the expected user options
+	opts, err := NewUsersStageOptions(users, false)
+	require.Nil(t, err)
+	assert.Equal(t, opts, expected)
+
+	// check that GenUsersStage creates the expected user options
+	st, err := GenUsersStage(users, false)
+	require.Nil(t, err)
+	usrStageOptions := st.Options.(*UsersStageOptions)
+	assert.Equal(t, usrStageOptions, expected)
+
+	// and (for good measure, not really needed) check that both gen
+	// the same
+	assert.Equal(t, usrStageOptions, opts)
+}

From 996573496a7e909ca3db5ea34f933a4e4d664d8e Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Sun, 4 Feb 2024 18:37:05 +0100
Subject: [PATCH 2/2] osbuild: use New{Group,Users}StageOptions() in
 Gen{Group,Users}Stage()

We currently duplicate the code that generates the stage options
for users and groups. AFAICT there is no need for this so this
commit just call the existing functions instead of duplicating them.
If we want to remove New{Group,Users}StageOptions() later (which
a previous commit message indicates) we can still do this by just
unexporting or moving the helper.
---
 pkg/osbuild/groups_stage.go | 10 +---------
 pkg/osbuild/users_stage.go  | 38 +++----------------------------------
 2 files changed, 4 insertions(+), 44 deletions(-)

diff --git a/pkg/osbuild/groups_stage.go b/pkg/osbuild/groups_stage.go
index 7f89823484..4af420ead2 100644
--- a/pkg/osbuild/groups_stage.go
+++ b/pkg/osbuild/groups_stage.go
@@ -36,13 +36,5 @@ func NewGroupsStageOptions(groups []users.Group) *GroupsStageOptions {
 }
 
 func GenGroupsStage(groups []users.Group) *Stage {
-	options := &GroupsStageOptions{
-		Groups: make(map[string]GroupsStageOptionsGroup, len(groups)),
-	}
-	for _, group := range groups {
-		options.Groups[group.Name] = GroupsStageOptionsGroup{
-			GID: group.GID,
-		}
-	}
-	return NewGroupsStage(options)
+	return NewGroupsStage(NewGroupsStageOptions(groups))
 }
diff --git a/pkg/osbuild/users_stage.go b/pkg/osbuild/users_stage.go
index 6d2a6ef989..d86538374c 100644
--- a/pkg/osbuild/users_stage.go
+++ b/pkg/osbuild/users_stage.go
@@ -71,41 +71,9 @@ func NewUsersStageOptions(userCustomizations []users.User, omitKey bool) (*Users
 }
 
 func GenUsersStage(users []users.User, omitKey bool) (*Stage, error) {
-	options := &UsersStageOptions{
-		Users: make(map[string]UsersStageOptionsUser, len(users)),
+	options, err := NewUsersStageOptions(users, omitKey)
+	if err != nil {
+		return nil, err
 	}
-
-	for _, user := range users {
-		// Don't hash empty passwords, set to nil to lock account
-		if user.Password != nil && len(*user.Password) == 0 {
-			user.Password = nil
-		}
-
-		// Hash non-empty un-hashed passwords
-		if user.Password != nil && !crypt.PasswordIsCrypted(*user.Password) {
-			cryptedPassword, err := crypt.CryptSHA512(*user.Password)
-			if err != nil {
-				return nil, err
-			}
-
-			user.Password = &cryptedPassword
-		}
-
-		userOptions := UsersStageOptionsUser{
-			UID:         user.UID,
-			GID:         user.GID,
-			Groups:      user.Groups,
-			Description: user.Description,
-			Home:        user.Home,
-			Shell:       user.Shell,
-			Password:    user.Password,
-			Key:         nil,
-		}
-		if !omitKey {
-			userOptions.Key = user.Key
-		}
-		options.Users[user.Name] = userOptions
-	}
-
 	return NewUsersStage(options), nil
 }