osutil: move to useradd from adduser (#13236)

* osutil: move to useradd from adduser adduser is perl based tool. Moving to useradd removes this dependency. Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> * osutil: useradd: minor fixes as recomended in PR Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> * osutil: useradd: remove redundant userTool variable Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> * osutil: useradd: set default tool and alter if needed Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> * osutil: move to useradd: improve comment text Co-authored-by: Maciej Borzecki <maciek.borzecki@gmail.com> * osutil: remove --badname from the useradd call as it was doing nothing for our use-case. The regex used by snapd is already sufficient and a lot more strict. Remove + from allowed characters in usernames, both normal and system, as they were not allowed by adduser anyway. * osutil: fix typo of available * osutil: remove obsolete part of the comment Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> * osutil: mention disabled passwords for useradd Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com> --------- Signed-off-by: Ondrej Kubik <ondrej.kubik@canonical.com> Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com> Co-authored-by: Maciej Borzecki <maciek.borzecki@gmail.com> Co-authored-by: Philip Meulengracht <philip.meulengracht@canonical.com> Co-authored-by: Maciej Borzecki <maciej.borzecki@canonical.com>
2026-03-13 11:15:47 -07:00 · 2024-02-08 19:07:59 +00:00
parent 1446ec6a56
commit 99d5b808cd
3 changed files with 102 additions and 7 deletions
--- a/osutil/export_test.go
+++ b/osutil/export_test.go
@@ -149,6 +149,14 @@ func MockLookPath(new func(string) (string, error)) (restore func()) {
 	}
 }

+func MockhasAddUserExecutable(new func() bool) (restore func()) {
+	old := hasAddUserExecutable
+	hasAddUserExecutable = new
+	return func() {
+		hasAddUserExecutable = old
+	}
+}
+
 func SetAtomicFileRenamed(aw *AtomicFile, renamed bool) {
 	aw.renamed = renamed
 }
--- a/osutil/user.go
+++ b/osutil/user.go
@@ -60,16 +60,19 @@ type AddUserOptions struct {
 // We check the (user)name ourselves, adduser is a bit too
 // strict (i.e. no `.`) - this regexp is in sync with that SSO
 // allows as valid usernames.
+// On systems where there are no adduser, this is the regex that verifies
+// users being created, and serves as a replacement for the regex that adduser
+// was providing.
 //
 // IsValidUsername define what is valid for a "system-user" assertion.
-var IsValidUsername = regexp.MustCompile(`^[a-z0-9][-a-z0-9+._]*$`).MatchString
+var IsValidUsername = regexp.MustCompile(`^[a-z0-9][-a-z0-9._]*$`).MatchString

 // IsValidSnapSystemUsername defines what is valid for the
 // "system-usernames" stanza in the snap.yaml.
 //
 // Unlike a normal username a system usernames can be encloused in "_"
 // (e.g. _username_ is valid)
-var IsValidSnapSystemUsername = regexp.MustCompile(`^([_][-a-z0-9+._]+[_]|[a-z0-9][-a-z0-9+._]*)$`).MatchString
+var IsValidSnapSystemUsername = regexp.MustCompile(`^([_][-a-z0-9._]+[_]|[a-z0-9][-a-z0-9._]*)$`).MatchString

 // EnsureSnapUserGroup uses the standard shadow utilities' 'useradd'
 // and 'groupadd' commands for creating non-login system users and
@@ -189,9 +192,14 @@ func sudoersFile(name string) string {
 	return filepath.Join(sudoersDotD, "create-user-"+strings.Replace(name, ".", "%2E", -1))
 }

+var hasAddUserExecutable = func() bool {
+	return ExecutableExists("adduser")
+}
+
 // AddUser uses the Debian/Ubuntu/derivative 'adduser' command for creating
 // regular login users on Ubuntu Core. 'adduser' is not portable cross-distro
 // but is convenient for creating regular login users.
+// if 'adduser' is not available, 'useradd' is used instead.
 //
 // The username created by this function will be checked against
 // IsValidUsername().
@@ -210,6 +218,19 @@ func AddUser(name string, opts *AddUserOptions) error {
 		"--gecos", opts.Gecos,
 		"--disabled-password",
 	}
+	if !hasAddUserExecutable() {
+		// No reason to use --badname for useradd, we are already a lot
+		// more strict than useradd, with our own regex
+		// "IsValidUsername". Users created by useradd have the password
+		// disabled by default.
+		cmdStr = []string{
+			"useradd",
+			"--comment", opts.Gecos,
+			"--create-home",
+			"--shell", "/bin/bash",
+		}
+	}
+
 	if opts.ExtraUsers {
 		cmdStr = append(cmdStr, "--extrausers")
 	}
@@ -217,7 +238,7 @@ func AddUser(name string, opts *AddUserOptions) error {

 	cmd := exec.Command(cmdStr[0], cmdStr[1:]...)
 	if output, err := cmd.CombinedOutput(); err != nil {
-		return fmt.Errorf("adduser failed with: %s", OutputErr(output, err))
+		return fmt.Errorf("%s failed with: %s", cmdStr[0], OutputErr(output, err))
 	}

 	if opts.Sudoer {
--- a/osutil/user_test.go
+++ b/osutil/user_test.go
@@ -40,6 +40,7 @@ type createUserSuite struct {
 	restorer func()

 	mockAddUser *testutil.MockCmd
+	mockUserAdd *testutil.MockCmd
 	mockUserMod *testutil.MockCmd
 	mockPasswd  *testutil.MockCmd
 }
@@ -60,6 +61,7 @@ func (s *createUserSuite) SetUpTest(c *check.C) {
 		}, nil
 	})
 	s.mockAddUser = testutil.MockCommand(c, "adduser", "")
+	s.mockUserAdd = testutil.MockCommand(c, "useradd", "")
 	s.mockUserMod = testutil.MockCommand(c, "usermod", "")
 	s.mockPasswd = testutil.MockCommand(c, "passwd", "")
 }
@@ -67,11 +69,15 @@ func (s *createUserSuite) SetUpTest(c *check.C) {
 func (s *createUserSuite) TearDownTest(c *check.C) {
 	s.restorer()
 	s.mockAddUser.Restore()
+	s.mockUserAdd.Restore()
 	s.mockUserMod.Restore()
 	s.mockPasswd.Restore()
 }

 func (s *createUserSuite) TestAddUserExtraUsersFalse(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
+
 	err := osutil.AddUser("lakatos", &osutil.AddUserOptions{
 		Gecos:      "my gecos",
 		ExtraUsers: false,
@@ -83,7 +89,25 @@ func (s *createUserSuite) TestAddUserExtraUsersFalse(c *check.C) {
 	})
 }

+func (s *createUserSuite) TestUserAddExtraUsersFalse(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return false })
+	defer r()
+
+	err := osutil.AddUser("lakatos", &osutil.AddUserOptions{
+		Gecos:      "my gecos",
+		ExtraUsers: false,
+	})
+	c.Assert(err, check.IsNil)
+
+	c.Check(s.mockUserAdd.Calls(), check.DeepEquals, [][]string{
+		{"useradd", "--comment", "my gecos", "--create-home", "--shell", "/bin/bash", "lakatos"},
+	})
+}
+
 func (s *createUserSuite) TestAddUserExtraUsersTrue(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
+
 	err := osutil.AddUser("lakatos", &osutil.AddUserOptions{
 		Gecos:      "my gecos",
 		ExtraUsers: true,
@@ -95,7 +119,24 @@ func (s *createUserSuite) TestAddUserExtraUsersTrue(c *check.C) {
 	})
 }

+func (s *createUserSuite) TestUserAddExtraUsersTrue(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return false })
+	defer r()
+
+	err := osutil.AddUser("lakatos", &osutil.AddUserOptions{
+		Gecos:      "my gecos",
+		ExtraUsers: true,
+	})
+	c.Assert(err, check.IsNil)
+
+	c.Check(s.mockUserAdd.Calls(), check.DeepEquals, [][]string{
+		{"useradd", "--comment", "my gecos", "--create-home", "--shell", "/bin/bash", "--extrausers", "lakatos"},
+	})
+}
+
 func (s *createUserSuite) TestAddSudoUser(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
 	mockSudoers := c.MkDir()
 	restorer := osutil.MockSudoersDotD(mockSudoers)
 	defer restorer()
@@ -123,6 +164,8 @@ karl.sagan ALL=(ALL) NOPASSWD:ALL
 }

 func (s *createUserSuite) TestAddUserSSHKeys(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
 	err := osutil.AddUser("karl.sagan", &osutil.AddUserOptions{
 		SSHKeys: []string{"ssh-key1", "ssh-key2"},
 	})
@@ -132,11 +175,16 @@ func (s *createUserSuite) TestAddUserSSHKeys(c *check.C) {
 }

 func (s *createUserSuite) TestAddUserInvalidUsername(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
 	err := osutil.AddUser("k!", nil)
 	c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid characters`)
 }

 func (s *createUserSuite) TestAddUserWithPassword(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
+
 	mockSudoers := c.MkDir()
 	restorer := osutil.MockSudoersDotD(mockSudoers)
 	defer restorer()
@@ -156,6 +204,9 @@ func (s *createUserSuite) TestAddUserWithPassword(c *check.C) {
 }

 func (s *createUserSuite) TestAddUserWithPasswordForceChange(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return false })
+	defer r()
+
 	mockSudoers := c.MkDir()
 	restorer := osutil.MockSudoersDotD(mockSudoers)
 	defer restorer()
@@ -167,8 +218,8 @@ func (s *createUserSuite) TestAddUserWithPasswordForceChange(c *check.C) {
 	})
 	c.Assert(err, check.IsNil)

-	c.Check(s.mockAddUser.Calls(), check.DeepEquals, [][]string{
-		{"adduser", "--force-badname", "--gecos", "my gecos", "--disabled-password", "karl.popper"},
+	c.Check(s.mockUserAdd.Calls(), check.DeepEquals, [][]string{
+		{"useradd", "--comment", "my gecos", "--create-home", "--shell", "/bin/bash", "karl.popper"},
 	})
 	c.Check(s.mockUserMod.Calls(), check.DeepEquals, [][]string{
 		{"usermod", "--password", "$6$salt$hash", "karl.popper"},
@@ -179,6 +230,8 @@ func (s *createUserSuite) TestAddUserWithPasswordForceChange(c *check.C) {
 }

 func (s *createUserSuite) TestAddUserPasswordForceChangeUnhappy(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
 	mockSudoers := c.MkDir()
 	restorer := osutil.MockSudoersDotD(mockSudoers)
 	defer restorer()
@@ -245,6 +298,9 @@ func (s *createUserSuite) TestUidGid(c *check.C) {
 }

 func (s *createUserSuite) TestAddUserUnhappy(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return true })
+	defer r()
+
 	mockAddUser := testutil.MockCommand(c, "adduser", "echo some error; exit 1")
 	defer mockAddUser.Restore()

@@ -253,14 +309,24 @@ func (s *createUserSuite) TestAddUserUnhappy(c *check.C) {

 }

+func (s *createUserSuite) TestUserAddUnhappy(c *check.C) {
+	r := osutil.MockhasAddUserExecutable(func() bool { return false })
+	defer r()
+
+	mockUserAdd := testutil.MockCommand(c, "useradd", "echo some error; exit 1")
+	defer mockUserAdd.Restore()
+	err := osutil.AddUser("lakatos", nil)
+	c.Assert(err, check.ErrorMatches, "useradd failed with: some error")
+}
+
 var usernameTestCases = map[string]bool{
 	"a":       true,
 	"a-b":     true,
-	"a+b":     true,
+	"a+b":     false,
 	"a.b":     true,
 	"a_b":     true,
 	"1":       true,
-	"1+":      true,
+	"1+":      false,
 	"1.":      true,
 	"1_":      true,
 	"-":       false,