Skip to content

Commit

Permalink
Update existing test with newer format
Browse files Browse the repository at this point in the history
  • Loading branch information
denisonbarbosa committed Nov 27, 2024
1 parent d2dec50 commit f9d7370
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 25 deletions.
39 changes: 17 additions & 22 deletions internal/policies/privilege/internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package privilege

import (
"context"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/ubuntu/adsys/internal/testutils"
)

func TestSplitAndNormalizeUsersAndGroups(t *testing.T) {
Expand Down Expand Up @@ -62,43 +64,36 @@ func TestSplitAndNormalizeUsersAndGroups(t *testing.T) {
}
}

func TestGetSystemPolkitAdminIdentities(t *testing.T) {
func TestPolkitAdminIdentitiesFromConf(t *testing.T) {
t.Parallel()

tests := map[string]struct {
policyKitDir string

want string
wantErr bool
emptyReturn bool
}{
"Fetch previous admin identities": {policyKitDir: "testdata/existing-previous-local-admins-one/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities from highest ascii file": {policyKitDir: "testdata/existing-previous-local-admins-multi/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities ignoring adsys": {policyKitDir: "testdata/existing-previous-local-admins-with-adsys-file/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities": {policyKitDir: "old-polkit/polkit-1"},
"Fetch previous admin identities from highest ascii file": {policyKitDir: "old-polkit-multiple-files/polkit-1"},
"Fetch previous admin identities ignoring adsys": {policyKitDir: "old-polkit/polkit-1"},

// Edge cases
"No previous admin identities but regular directory structure": {policyKitDir: "testdata/existing-other-files/polkit-1",
want: ""},
"Returns an empty string if directory does not exists": {policyKitDir: "testdata/doesnotexists",
want: ""},
"Directory instead of a conf file is ignored": {policyKitDir: "testdata/incorrect-policikit-conf-is-dir/polkit-1",
want: ""},
"No previous admin identities but regular directory structure": {policyKitDir: "existing-other-files/polkit-1", emptyReturn: true},
"Returns an empty string if directory does not exists": {policyKitDir: "doesnotexists", emptyReturn: true},
"Directory instead of a conf file is ignored": {policyKitDir: "incorrect-policikit-conf-is-dir/polkit-1", emptyReturn: true},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()

got, err := getSystemPolkitAdminIdentities(context.Background(), tc.policyKitDir)
if tc.wantErr {
require.NotNil(t, err, "getSystemPolkitAdminIdentities should have failed but didn't")
got, err := polkitAdminIdentitiesFromConf(context.Background(), filepath.Join("testdata", tc.policyKitDir))
require.NoError(t, err, "polkitAdminIdentitiesFromConf failed but shouldn't have")

if tc.emptyReturn {
require.Empty(t, got)
return
}
require.NoError(t, err, "ApplyPolicy failed but shouldn't have")

assert.Equal(t, tc.want, got, "getSystemPolkitAdminIdentities returned expected value")
want := testutils.LoadWithUpdateFromGolden(t, got)
require.Equal(t, want, got, "polkitAdminIdentitiesFromConf did not return expected value")
})
}
}
Expand Down
45 changes: 42 additions & 3 deletions internal/policies/privilege/privilege_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func TestApplyPolicy(t *testing.T) {

defaultLocalAdminDisabledRule := []entry.Entry{{Key: "allow-local-admins", Disabled: true}}

defaultPolkitReservedPath := createPolkitReservedPath(t)

tests := map[string]struct {
notComputer bool
entries []entry.Entry
Expand All @@ -26,6 +28,8 @@ func TestApplyPolicy(t *testing.T) {
makeReadOnly string
destIsDir string

polkitSystemReservedPath string

wantErr bool
}{
// local admin cases
Expand Down Expand Up @@ -75,16 +79,20 @@ func TestApplyPolicy(t *testing.T) {
"No rules still overwrite those files": {existingSudoersDir: "existing-files", existingPolkitDir: "existing-files"},
"Don't overwrite other existing files": {existingSudoersDir: "existing-other-files", existingPolkitDir: "existing-other-files", entries: defaultLocalAdminDisabledRule},

// Migration
"Create on new polkit version and remove old file": {existingPolkitDir: "existing-old-adsys-conf", entries: []entry.Entry{{Key: "client-admins", Value: "[email protected]"}}},
"Assume old polkit if cant read system reserved path": {existingPolkitDir: "old-polkit", entries: []entry.Entry{{Key: "client-admins", Value: "[email protected]"}}, polkitSystemReservedPath: "doesnotexist"},

// Not a computer, don’t do anything (even not create new files)
"Not a computer": {notComputer: true, existingSudoersDir: "existing-other-files", existingPolkitDir: "existing-other-files"},

// Error cases
"Error on writing to sudoers file": {makeReadOnly: "sudoers.d/", existingSudoersDir: "existing-files", existingPolkitDir: "existing-files", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error on writing to polkit subdirectory creation": {makeReadOnly: "polkit-1/", existingSudoersDir: "existing-files", existingPolkitDir: "only-base-polkit-dir", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error on writing to polkit conf file": {makeReadOnly: "polkit-1/localauthority.conf.d", existingSudoersDir: "existing-files", existingPolkitDir: "existing-files", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error on writing to polkit conf file": {makeReadOnly: "polkit-1/rules.d", existingSudoersDir: "existing-files", existingPolkitDir: "existing-files", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error on creating sudoers and polkit base directory": {makeReadOnly: ".", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error if can’t rename to destination for sudoers file": {destIsDir: "sudoers.d/99-adsys-privilege-enforcement", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error if can’t rename to destination for polkit conf file": {destIsDir: "polkit-1/localauthority.conf.d/99-adsys-privilege-enforcement.conf", entries: defaultLocalAdminDisabledRule, wantErr: true},
"Error if can’t rename to destination for polkit conf file": {destIsDir: "polkit-1/rules.d/00-adsys-privilege-enforcement.rules", entries: defaultLocalAdminDisabledRule, wantErr: true},
}

for name, tc := range tests {
Expand All @@ -95,6 +103,10 @@ func TestApplyPolicy(t *testing.T) {
sudoersDir := filepath.Join(tempEtc, "sudoers.d")
policyKitDir := filepath.Join(tempEtc, "polkit-1")

if tc.polkitSystemReservedPath == "" {
tc.polkitSystemReservedPath = defaultPolkitReservedPath
}

if tc.existingSudoersDir != "" {
require.NoError(t,
shutil.CopyTree(
Expand All @@ -119,7 +131,7 @@ func TestApplyPolicy(t *testing.T) {
require.NoError(t, os.MkdirAll(filepath.Join(tempEtc, tc.destIsDir), 0750), "Setup: can't create fake unwritable file")
}

m := privilege.NewWithDirs(sudoersDir, policyKitDir)
m := privilege.NewWithDirs(sudoersDir, policyKitDir, privilege.WithPolicyKitSystemDir(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")
Expand All @@ -131,3 +143,30 @@ func TestApplyPolicy(t *testing.T) {
})
}
}

// Creates a temporary directory with the default rules file for Polkit.
//
// This is needed to avoid the huge absolute paths that come from using t.TempDir() and can
// eventually go over the maximum limit for paths.
func createPolkitReservedPath(t *testing.T) string {
t.Helper()

tmp, err := os.MkdirTemp("", "polkit-test")
require.NoError(t, err, "Setup: Failed to create tempdir for tests")
t.Cleanup(func() { _ = os.RemoveAll(tmp) })

rulesDir := filepath.Join(tmp, "rules.d")
err = os.Mkdir(rulesDir, 0750)
require.NoError(t, err, "Setup: Failed to create rules directory")

content := `
polkit.addAdminRule(function(action, subject) {
return ["unix-group:sudo", "unix-group:admin"];
});
`

err = os.WriteFile(filepath.Join(rulesDir, "49-ubuntu-admin.rules"), []byte(content), 0600)
require.NoError(t, err, "Setup: Failed to write admin rules")

return tmp
}

0 comments on commit f9d7370

Please sign in to comment.