daemon,o/servicestate: fix selecting all users for non-root users

add unit test for this
canonical · Feb 21, 2024 · 1049780 · 1049780
1 parent 77437aa
commit 1049780
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 9 deletions.
diff --git a/daemon/api_apps_test.go b/daemon/api_apps_test.go
@@ -636,7 +636,7 @@ func (s *appsSuite) TestPostAppsFailedToGetUser(c *check.C) {
 	c.Check(rspe.Message, check.Matches, "cannot perform operation on services: failed")
 }
 
-func (s *appsSuite) TestPostAppsScopesUserAsRootNotAllowed(c *check.C) {
+func (s *appsSuite) TestPostAppsScopesSelfAsRootNotAllowed(c *check.C) {
 	inst := servicestate.Instruction{
 		Action: "start",
 		Names:  []string{"snap-a.svc1"},
@@ -653,10 +653,10 @@ func (s *appsSuite) TestPostAppsScopesUserAsRootNotAllowed(c *check.C) {
 
 	rspe := s.errorReq(c, req, s.authUser)
 	c.Check(rspe.Status, check.Equals, 400)
-	c.Check(rspe.Message, check.Matches, "cannot perform operation on services: cannot use \"self\" for root user")
+	c.Check(rspe.Message, check.Matches, `cannot use "self" for root user`)
 }
 
-func (s *appsSuite) TestPostAppsUsersAsRootHappy(c *check.C) {
+func (s *appsSuite) TestPostAppsAllUsersAsRootHappy(c *check.C) {
 	inst := servicestate.Instruction{
 		Action: "start",
 		Names:  []string{"snap-a.svc1"},

diff --git a/overlord/servicestate/servicestate.go b/overlord/servicestate/servicestate.go
@@ -215,13 +215,8 @@ func (i *Instruction) validateScope(u *user.User, apps []*snap.AppInfo) error {
 }
 
 func (i *Instruction) validateUsers(u *user.User, apps []*snap.AppInfo) error {
-	users, err := i.Users.UserList(u)
-	if err != nil {
-		return err
-	}
-
 	// Perform some additional user checks
-	if len(users) == 0 {
+	if i.Users.Selector == UserSelectionList && len(i.Users.Names) == 0 {
 		// It is an error for a non-root to not specify any users if we are targeting
 		// user daemons
 		if u.Uid != "0" && i.hasUserService(apps) {

diff --git a/overlord/servicestate/servicestate_test.go b/overlord/servicestate/servicestate_test.go
@@ -538,6 +538,15 @@ func (s *instructionSuite) TestValidateNoUsersForNonRootOnlySystemServicesHappy(
 	c.Check(inst.Validate(s.defaultUser, s.systemServices), IsNil)
 }
 
+func (s *instructionSuite) TestValidateAllUsersForNonRootHappy(c *C) {
+	// Provide scopes to avoid hitting any checks in validateScope
+	inst := &servicestate.Instruction{
+		Scope: servicestate.ScopeSelector{"system", "user"},
+		Users: servicestate.UserSelector{Selector: servicestate.UserSelectionAll},
+	}
+	c.Check(inst.Validate(s.defaultUser, s.mixServices), IsNil)
+}
+
 func (s *instructionSuite) TestValidateNoUsersForNonRootMixServicesFails(c *C) {
 	// Provide scopes to avoid hitting any checks in validateScope
 	inst := &servicestate.Instruction{Scope: servicestate.ScopeSelector{"system", "user"}}