From 29a66cda3a41dc4c1fa2ef9e8f3d21f431da5610 Mon Sep 17 00:00:00 2001 From: denisonbarbosa Date: Fri, 6 Dec 2024 10:15:08 -0400 Subject: [PATCH] Fix internal/policies/manager tests 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). --- internal/consts/consts.go | 2 + internal/policies/manager.go | 58 +++++++++++-------- internal/policies/manager_test.go | 2 + internal/policies/privilege/export_test.go | 8 --- internal/policies/privilege/privilege.go | 20 +++---- internal/policies/privilege/privilege_test.go | 2 +- .../00-adsys-privilege-enforcement.rules | 2 +- .../00-adsys-privilege-enforcement.rules | 2 +- 8 files changed, 50 insertions(+), 46 deletions(-) delete mode 100644 internal/policies/privilege/export_test.go diff --git a/internal/consts/consts.go b/internal/consts/consts.go index 39c2c1c49..c9efec585 100644 --- 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 index c331e73f5..6030acef3 100644 --- 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 index 5fb019831..dadf9a768 100644 --- 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 deleted file mode 100644 index 7c06b0c2e..000000000 --- a/internal/policies/privilege/export_test.go +++ /dev/null @@ -1,8 +0,0 @@ -package privilege - -// WithPolicyKitSystemDir sets the directory where the default policykit files are stored. -func WithPolicyKitSystemDir(dir string) func(*option) { - return func(o *option) { - o.policyKitSystemDir = dir - } -} diff --git a/internal/policies/privilege/privilege.go b/internal/policies/privilege/privilege.go index 4261ac312..c0bb3c3cc 100644 --- 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 index b83a97d8a..3a7da19dc 100644 --- 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/internal/policies/testdata/TestApplyPolicies/golden/succeed/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules b/internal/policies/testdata/TestApplyPolicies/golden/succeed/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules index cbf3b6829..a9589a631 100644 --- a/internal/policies/testdata/TestApplyPolicies/golden/succeed/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules +++ b/internal/policies/testdata/TestApplyPolicies/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/internal/policies/testdata/TestApplyPolicies/golden/succeed_if_checking_for_backend_online_status_returns_an_error/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules b/internal/policies/testdata/TestApplyPolicies/golden/succeed_if_checking_for_backend_online_status_returns_an_error/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules index cbf3b6829..a9589a631 100644 --- a/internal/policies/testdata/TestApplyPolicies/golden/succeed_if_checking_for_backend_online_status_returns_an_error/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules +++ b/internal/policies/testdata/TestApplyPolicies/golden/succeed_if_checking_for_backend_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"]; });