Fix internal/policies/manager tests (#1158)

We were not overriding the polkit system reserved dir for the tests here, so it would cause a failure on launchpad builds (which work under a different user with different permissions).
ubuntu · Dec 9, 2024 · 99785ac · 99785ac
2 parents 63606d5 + 29a66cd
commit 99785ac
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 46 deletions.
diff --git a/internal/consts/consts.go b/internal/consts/consts.go
@@ -59,6 +59,8 @@ const (
 	DefaultSudoersDir = "/etc/sudoers.d"
 	// DefaultPolicyKitDir is the default directory for policykit configuration and rules.
 	DefaultPolicyKitDir = "/etc/polkit-1"
+	// DefaultPolicyKitSystemDir is the default directory for policykit reserved configuration.
+	DefaultPolicyKitSystemDir = "/usr/share/polkit-1"
 	// DefaultApparmorDir is the default directory for apparmor configuration.
 	DefaultApparmorDir = "/etc/apparmor.d/adsys"
 	// DefaultSystemUnitDir is the default directory for systemd unit files.

diff --git a/internal/policies/manager.go b/internal/policies/manager.go
@@ -93,20 +93,21 @@ type systemdCaller interface {
 }
 
 type options struct {
-	cacheDir       string
-	stateDir       string
-	dconfDir       string
-	sudoersDir     string
-	policyKitDir   string
-	runDir         string
-	shareDir       string
-	apparmorDir    string
-	apparmorFsDir  string
-	systemUnitDir  string
-	globalTrustDir string
-	proxyApplier   proxy.Caller
-	systemdCaller  systemdCaller
-	gdm            *gdm.Manager
+	cacheDir           string
+	stateDir           string
+	dconfDir           string
+	sudoersDir         string
+	policyKitDir       string
+	policyKitSystemDir string
+	runDir             string
+	shareDir           string
+	apparmorDir        string
+	apparmorFsDir      string
+	systemUnitDir      string
+	globalTrustDir     string
+	proxyApplier       proxy.Caller
+	systemdCaller      systemdCaller
+	gdm                *gdm.Manager
 
 	apparmorParserCmd []string
 	certAutoenrollCmd []string
@@ -155,6 +156,14 @@ func WithPolicyKitDir(p string) Option {
 	}
 }
 
+// WithPolicyKitSystemDir specifies a personalized policykit system reserved directory.
+func WithPolicyKitSystemDir(p string) Option {
+	return func(o *options) error {
+		o.policyKitSystemDir = p
+		return nil
+	}
+}
+
 // WithRunDir specifies a personalized run directory.
 func WithRunDir(p string) Option {
 	return func(o *options) error {
@@ -248,15 +257,16 @@ func NewManager(bus *dbus.Conn, hostname string, backend backends.Backend, opts
 
 	// defaults
 	args := options{
-		cacheDir:       consts.DefaultCacheDir,
-		stateDir:       consts.DefaultStateDir,
-		runDir:         consts.DefaultRunDir,
-		shareDir:       consts.DefaultShareDir,
-		apparmorDir:    consts.DefaultApparmorDir,
-		systemUnitDir:  consts.DefaultSystemUnitDir,
-		globalTrustDir: consts.DefaultGlobalTrustDir,
-		systemdCaller:  defaultSystemdCaller,
-		gdm:            nil,
+		cacheDir:           consts.DefaultCacheDir,
+		stateDir:           consts.DefaultStateDir,
+		runDir:             consts.DefaultRunDir,
+		shareDir:           consts.DefaultShareDir,
+		apparmorDir:        consts.DefaultApparmorDir,
+		systemUnitDir:      consts.DefaultSystemUnitDir,
+		globalTrustDir:     consts.DefaultGlobalTrustDir,
+		policyKitSystemDir: consts.DefaultPolicyKitSystemDir,
+		systemdCaller:      defaultSystemdCaller,
+		gdm:                nil,
 	}
 	// applied options (including dconf manager used by gdm)
 	for _, o := range opts {
@@ -271,7 +281,7 @@ func NewManager(bus *dbus.Conn, hostname string, backend backends.Backend, opts
 	}
 
 	// privilege manager
-	privilegeManager := privilege.NewWithDirs(args.sudoersDir, args.policyKitDir)
+	privilegeManager := privilege.NewWithDirs(args.sudoersDir, args.policyKitDir, args.policyKitSystemDir)
 
 	// scripts manager
 	scriptsManager, err := scripts.New(args.runDir, args.systemdCaller)

diff --git a/internal/policies/manager_test.go b/internal/policies/manager_test.go
@@ -78,6 +78,7 @@ func TestApplyPolicies(t *testing.T) {
 			runDir := filepath.Join(fakeRootDir, "run", "adsys")
 			dconfDir := filepath.Join(fakeRootDir, "etc", "dconf")
 			policyKitDir := filepath.Join(fakeRootDir, "etc", "polkit-1")
+			policyKitReservedDir := filepath.Join(fakeRootDir, "usr", "share", "polkit-1")
 			sudoersDir := filepath.Join(fakeRootDir, "etc", "sudoers.d")
 			apparmorDir := filepath.Join(fakeRootDir, "etc", "apparmor.d", "adsys")
 			systemUnitDir := filepath.Join(fakeRootDir, "etc", "systemd", "system")
@@ -108,6 +109,7 @@ func TestApplyPolicies(t *testing.T) {
 				policies.WithShareDir(shareDir),
 				policies.WithDconfDir(dconfDir),
 				policies.WithPolicyKitDir(policyKitDir),
+				policies.WithPolicyKitSystemDir(policyKitReservedDir),
 				policies.WithSudoersDir(sudoersDir),
 				policies.WithApparmorDir(apparmorDir),
 				policies.WithApparmorFsDir(filepath.Dir(loadedPoliciesFile)),

diff --git a/internal/policies/privilege/export_test.go b/internal/policies/privilege/export_test.go
diff --git a/internal/policies/privilege/privilege.go b/internal/policies/privilege/privilege.go
@@ -55,8 +55,6 @@ const (
 
 	adsysOldPolkitName  = "99-adsys-privilege-enforcement"
 	adsysBasePolkitName = "00-adsys-privilege-enforcement"
-
-	polkitSystemReservedPath = "/usr/share/polkit-1"
 )
 
 // Templates to generate the polkit configuration files.
@@ -74,6 +72,13 @@ type option struct {
 // Option is a functional option for the manager.
 type Option func(*option)
 
+// WithPolicyKitSystemDir sets the directory where the default policykit files are stored.
+func WithPolicyKitSystemDir(dir string) func(*option) {
+	return func(o *option) {
+		o.policyKitSystemDir = dir
+	}
+}
+
 // Manager prevents running multiple privilege update process in parallel while parsing policy in ApplyPolicy.
 type Manager struct {
 	sudoersDir   string
@@ -84,18 +89,11 @@ type Manager struct {
 }
 
 // NewWithDirs creates a manager with a specific root directory.
-func NewWithDirs(sudoersDir, policyKitDir string, opts ...Option) *Manager {
-	o := &option{
-		policyKitSystemDir: polkitSystemReservedPath,
-	}
-	for _, opt := range opts {
-		opt(o)
-	}
-
+func NewWithDirs(sudoersDir, policyKitDir, policyKitSystemDir string) *Manager {
 	return &Manager{
 		sudoersDir:         sudoersDir,
 		policyKitDir:       policyKitDir,
-		policyKitSystemDir: o.policyKitSystemDir,
+		policyKitSystemDir: policyKitSystemDir,
 	}
 }
 

diff --git a/internal/policies/privilege/privilege_test.go b/internal/policies/privilege/privilege_test.go
@@ -123,7 +123,7 @@ func TestApplyPolicy(t *testing.T) {
 				require.NoError(t, os.MkdirAll(filepath.Join(tmpRootDir, tc.destIsDir), 0750), "Setup: can't create fake unwritable file")
 			}
 
-			m := privilege.NewWithDirs(sudoersDir, policyKitDir, privilege.WithPolicyKitSystemDir(tc.polkitSystemReservedPath))
+			m := privilege.NewWithDirs(sudoersDir, policyKitDir, tc.polkitSystemReservedPath)
 			err = m.ApplyPolicy(context.Background(), "ubuntu", !tc.notComputer, tc.entries)
 			if tc.wantErr {
 				require.NotNil(t, err, "ApplyPolicy should have failed but didn't")

diff --git a/...estApplyPolicies/golden/succeed/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules b/...estApplyPolicies/golden/succeed/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules
@@ -3,5 +3,5 @@
 # Any changes will be overwritten.
 
 polkit.addAdminRule(function(action, subject){
-	return ["unix-group:sudo","unix-group:admin","unix-user:alice@domain","unix-user:bob@domain2","unix-group:mygroup@domain","unix-user:cosmic carole@domain"];
+	return ["unix-user:alice@domain","unix-user:bob@domain2","unix-group:mygroup@domain","unix-user:cosmic carole@domain"];
 });
diff --git a/..._online_status_returns_an_error/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules b/..._online_status_returns_an_error/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules
@@ -3,5 +3,5 @@
 # Any changes will be overwritten.
 
 polkit.addAdminRule(function(action, subject){
-	return ["unix-group:sudo","unix-group:admin","unix-user:alice@domain","unix-user:bob@domain2","unix-group:mygroup@domain","unix-user:cosmic carole@domain"];
+	return ["unix-user:alice@domain","unix-user:bob@domain2","unix-group:mygroup@domain","unix-user:cosmic carole@domain"];
 });