fixup! Support for Polkit 124

internal/policies/privilege/privilege.go

@@ -50,15 +50,23 @@ import (
 	Both are installed under respective /etc directories.
 */
 
-const adsysBaseSudoersName = "99-adsys-privilege-enforcement"
-
 const (
+	adsysBaseSudoersName = "99-adsys-privilege-enforcement"
+
 	adsysOldPolkitName  = "99-adsys-privilege-enforcement"
 	adsysBasePolkitName = "00-adsys-privilege-enforcement"
 
 	polkitSystemReservedPath = "/usr/share/polkit-1"
 )
 
+// Templates to generate the polkit configuration files.
+const (
+	policyKitConfTemplate  = "%s[Configuration]\nAdminIdentities=%s"
+	policyKitRulesTemplate = `%spolkit.addAdminRule(function(action, subject){
+	return [%s];
+});`
+)
+
 type option struct {
 	policyKitSystemDir string
 }
@@ -299,6 +307,8 @@ func splitAndNormalizeUsersAndGroups(ctx context.Context, v string) []string {
 	return elems
 }
 
+// getSystemPolkitAdminIdentities parses the system polkit configuration (based on its version) to get the
+// list of admin identities.
 func (m *Manager) getSystemPolkitAdminIdentities(ctx context.Context, policyKitDir string, oldPolkit bool) (adminIdentities string, err error) {
 	if oldPolkit {
 		return polkitAdminIdentitiesFromConf(ctx, policyKitDir)
@@ -309,74 +319,91 @@ func (m *Manager) getSystemPolkitAdminIdentities(ctx context.Context, policyKitD
 	})
 }
 
+// polkitAdminIdentitiesFromRules parses the polkit rules files to get the list of admin identities.
+//
+// Since polkit >= 124 now only cares about the first valid return, this function will sort the files from all the
+// specified directories (priority: lesser ascii value, higher priority), parse them and identify the first valid return.
 func polkitAdminIdentitiesFromRules(ctx context.Context, rulesDirPaths []string) (adminIdentities string, err error) {
-	currentRulesFile := "99"
-	for _, rulesDirPath := range rulesDirPaths {
-		ruleFiles, err := filepath.Glob(filepath.Join(rulesDirPath, "rules.d", "*.rules"))
+	// Compile the regex needed to parse the polkit admin rules.
+	// Matches: polkit.addAdminRule(function(action, subject){(.*)});
+	adminRulesRegex, err := regexp.Compile(`polkit\.addAdminRule\s*\(\s*function\s*\(\s*action\s*\,\s*subject\s*\)\s*{\s*[^\}]*}\s*\)\s*\;`)
+	if err != nil {
+		return "", err
+	}
+	// Matches for: { return [(.*)] }
+	returnRegex, err := regexp.Compile(`\{\s*return\s*\[(\s*([^\]]*))*\s*\]\s*;\s*\}`)
+	if err != nil {
+		return "", err
+	}
+	// Matches for: "someuser" or 'someuser'
+	userRegex, err := regexp.Compile(`(["']+([^,]*)["']+)`)
+	if err != nil {
+		return "", err
+	}
+
+	var ruleFiles []string
+	for _, path := range rulesDirPaths {
+		files, err := filepath.Glob(filepath.Join(path, "rules.d", "*.rules"))
 		if err != nil {
 			return "", err
 		}
+		ruleFiles = append(ruleFiles, files...)
+	}
 
-		slices.Sort(ruleFiles)
-	ruleFilesRange:
-		for _, path := range ruleFiles {
-			if filepath.Base(path) >= currentRulesFile {
-				break
-			}
-
-			if filepath.Base(path) == adsysBasePolkitName+".rules" {
-				continue
-			}
+	// Sort the files respecting the priority that Polkit assigns to them.
+	slices.SortFunc(ruleFiles, func(i, j string) int {
+		// If the files have different name, we return the one with the lowest ascii value.
+		if order := strings.Compare(filepath.Base(i), filepath.Base(j)); order != 0 {
+			return order
+		}
 
-			pathStat, err := os.Stat(path)
-			if err != nil {
-				return "", err
-			}
-			if pathStat.IsDir() {
-				log.Warning(ctx, gotext.Get("%s is a directory. Ignoring.", path))
-				continue
+		// If the files have the same name, we respect the directory priority in rulesDirPaths (lesser index, higher prio).
+		var idxI, idxJ int
+		for idx, dir := range rulesDirPaths {
+			if strings.Contains(i, dir) {
+				idxI = idx
 			}
-
-			b, err := os.ReadFile(path)
-			if err != nil {
-				return "", err
+			if strings.Contains(j, dir) {
+				idxJ = idx
 			}
-			rules := string(b)
+		}
+		return idxI - idxJ
+	})
 
-			// Check if the file contains the rule we are looking for
-			if !strings.Contains(rules, "polkit.addAdminRule") {
-				continue
-			}
+	for _, path := range ruleFiles {
+		if filepath.Base(path) == adsysBasePolkitName+".rules" {
+			continue
+		}
 
-			// Extract the return value of the polkit.addAdminRule function
-			// Matches: polkit.addAdminRule(function(action, subject){(.*)});
-			adminRulesRegex, err := regexp.Compile(`polkit\.addAdminRule\s*\(\s*function\s*\(\s*action\s*\,\s*subject\s*\)\s*{\s*[^\}]*}\s*\)\s*\;`)
-			if err != nil {
-				return "", err
-			}
-			// Matches for: { return [(.*)] }
-			returnRegex, err := regexp.Compile(`\{\s*return\s*\[(\s*([^\]]*))*\s*\]\s*;\s*\}`)
-			if err != nil {
-				return "", err
-			}
-			// Matches for: "someuser" or 'someuser'
-			userRegex, err := regexp.Compile(`(["']+([^,]*)["']+)`)
-			if err != nil {
+		b, err := os.ReadFile(path)
+		if err != nil {
+			pathErr := &os.PathError{}
+			if errors.As(err, &pathErr) && pathErr.Op == "open" {
+				// This means that we couldn't open the file for reading, likely due to permission errors.
+				// If so, we can not ensure that we will match the expected admin identities from the system
+				// and we should return an error.
 				return "", err
 			}
+			// If we get an error when reading the file, it's likely due to it being a directory.
+			// This case we can ignore and continue to the next file.
+			log.Debugf(ctx, gotext.Get("Ignoring %s: %v", path, err))
+			continue
+		}
+		rules := string(b)
 
-			for _, adminRule := range adminRulesRegex.FindAllString(rules, -1) {
-				returnStmt := returnRegex.FindString(adminRule)
-				if returnStmt == "" {
-					continue
-				}
-
-				adminIdentities = strings.Join(userRegex.FindAllString(returnStmt, -1), ",")
-				currentRulesFile = filepath.Base(path)
+		// Check if the file contains the rule we are looking for
+		if !strings.Contains(rules, "polkit.addAdminRule") {
+			continue
+		}
 
-				// Since we only care about the first returned value, we can skip the other files in the directory.
-				break ruleFilesRange
+		for _, adminRule := range adminRulesRegex.FindAllString(rules, -1) {
+			returnStmt := returnRegex.FindString(adminRule)
+			if returnStmt == "" {
+				continue
 			}
+
+			log.Debugf(ctx, gotext.Get("Using polkit admin identities from %q", path))
+			return strings.Join(userRegex.FindAllString(returnStmt, -1), ","), nil
 		}
 	}
 
@@ -436,19 +463,17 @@ func isOldPolkit(policyKitDir, policyKitReservedDir string) bool {
 		return false
 	}
 
+	// If the directory only contains the adsys generated file, we can assume that the version is >= 124
 	if nEntries == 1 && dirEntries[0].Name() == adsysOldPolkitName+".conf" {
 		return false
 	}
 
+	// If the old directory isn't empty and there's no new configuration file, we can assume that the version is < 124.
 	if _, err := os.Stat(filepath.Join(policyKitReservedDir, "rules.d/49-ubuntu-admin.rules")); err != nil {
 		return true
 	}
+
+	// If the new configuration file exists but the old directory is not empty, it likely means that the user
+	// installed the compatibility package (polkitd-pkla), but polkit version is still >= 124.
 	return false
 }
-
-const (
-	policyKitConfTemplate  = "%s[Configuration]\nAdminIdentities=%s"
-	policyKitRulesTemplate = `%spolkit.addAdminRule(function(action, subject){
-	return [%s];
-});`
-)