dataretention: remove ignore_user_permissions

Required permissions are now automatically granted as for any other
filesystem action

Fixes #1564

Signed-off-by: Nicola Murino <[email protected]>

internal/common/dataretention.go

@@ -201,10 +201,8 @@ func (c *RetentionCheck) Validate() error {
 }
 
 func (c *RetentionCheck) updateUserPermissions() {
-	for _, folder := range c.Folders {
-		if folder.IgnoreUserPermissions {
-			c.conn.User.Permissions[folder.Path] = []string{dataprovider.PermAny}
-		}
+	for k := range c.conn.User.Permissions {
+		c.conn.User.Permissions[k] = []string{dataprovider.PermAny}
 	}
 }
 
@@ -229,16 +227,6 @@ func (c *RetentionCheck) removeFile(virtualPath string, info os.FileInfo) error
 	return c.conn.RemoveFile(fs, fsPath, virtualPath, info)
 }
 
-func (c *RetentionCheck) hasCleanupPerms(folderPath string) bool {
-	if !c.conn.User.HasPerm(dataprovider.PermListItems, folderPath) {
-		return false
-	}
-	if !c.conn.User.HasAnyPerm([]string{dataprovider.PermDelete, dataprovider.PermDeleteFiles}, folderPath) {
-		return false
-	}
-	return true
-}
-
 func (c *RetentionCheck) cleanupFolder(folderPath string, recursion int) error {
 	startTime := time.Now()
 	result := folderRetentionCheckResult{
@@ -255,13 +243,6 @@ func (c *RetentionCheck) cleanupFolder(folderPath string, recursion int) error {
 		return util.ErrRecursionTooDeep
 	}
 	recursion++
-	if !c.hasCleanupPerms(folderPath) {
-		result.Elapsed = time.Since(startTime)
-		result.Info = "data retention check skipped: no permissions"
-		c.conn.Log(logger.LevelInfo, "user %q does not have permissions to check retention on %q, retention check skipped",
-			c.conn.User.Username, folderPath)
-		return nil
-	}
 
 	folderRetention, err := c.getFolderRetention(folderPath)
 	if err != nil {
@@ -277,8 +258,8 @@ func (c *RetentionCheck) cleanupFolder(folderPath string, recursion int) error {
 		c.conn.Log(logger.LevelDebug, "retention check skipped for folder %q, retention is set to 0", folderPath)
 		return nil
 	}
-	c.conn.Log(logger.LevelDebug, "start retention check for folder %q, retention: %v hours, delete empty dirs? %v, ignore user perms? %v",
-		folderPath, folderRetention.Retention, folderRetention.DeleteEmptyDirs, folderRetention.IgnoreUserPermissions)
+	c.conn.Log(logger.LevelDebug, "start retention check for folder %q, retention: %v hours, delete empty dirs? %v",
+		folderPath, folderRetention.Retention, folderRetention.DeleteEmptyDirs)
 	lister, err := c.conn.ListDir(folderPath)
 	if err != nil {
 		result.Elapsed = time.Since(startTime)

internal/common/dataretention_test.go

@@ -252,19 +252,16 @@ func TestRetentionPermissionsAndGetFolder(t *testing.T) {
 	check := RetentionCheck{
 		Folders: []dataprovider.FolderRetention{
 			{
-				Path:                  "/dir2",
-				Retention:             24 * 7,
-				IgnoreUserPermissions: true,
+				Path:      "/dir2",
+				Retention: 24 * 7,
 			},
 			{
-				Path:                  "/dir3",
-				Retention:             24 * 7,
-				IgnoreUserPermissions: false,
+				Path:      "/dir3",
+				Retention: 24 * 7,
 			},
 			{
-				Path:                  "/dir2/sub1/sub",
-				Retention:             24,
-				IgnoreUserPermissions: true,
+				Path:      "/dir2/sub1/sub",
+				Retention: 24,
 			},
 		},
 	}
@@ -273,15 +270,11 @@ func TestRetentionPermissionsAndGetFolder(t *testing.T) {
 	conn.SetProtocol(ProtocolDataRetention)
 	conn.ID = fmt.Sprintf("data_retention_%v", user.Username)
 	check.conn = conn
-	assert.False(t, check.hasCleanupPerms(check.Folders[2].Path))
 	check.updateUserPermissions()
-	assert.True(t, check.hasCleanupPerms(check.Folders[2].Path))
-	assert.Equal(t, []string{dataprovider.PermListItems, dataprovider.PermDelete}, conn.User.Permissions["/"])
-	assert.Equal(t, []string{dataprovider.PermListItems}, conn.User.Permissions["/dir1"])
-	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/dir2"])
-	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/dir2/sub1/sub"])
-	assert.Equal(t, []string{dataprovider.PermCreateDirs}, conn.User.Permissions["/dir2/sub1"])
-	assert.Equal(t, []string{dataprovider.PermDelete}, conn.User.Permissions["/dir2/sub2"])
+	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/"])
+	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/dir1"])
+	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/dir2/sub1"])
+	assert.Equal(t, []string{dataprovider.PermAny}, conn.User.Permissions["/dir2/sub2"])
 
 	_, err := check.getFolderRetention("/")
 	assert.Error(t, err)

internal/common/protocol_test.go

@@ -6087,10 +6087,9 @@ func TestEventActionsRetentionReports(t *testing.T) {
 			RetentionConfig: dataprovider.EventActionDataRetentionConfig{
 				Folders: []dataprovider.FolderRetention{
 					{
-						Path:                  testDir,
-						Retention:             1,
-						DeleteEmptyDirs:       true,
-						IgnoreUserPermissions: true,
+						Path:            testDir,
+						Retention:       1,
+						DeleteEmptyDirs: true,
 					},
 				},
 			},
@@ -7690,6 +7689,9 @@ func TestGetQuotaError(t *testing.T) {
 
 func TestRetentionAPI(t *testing.T) {
 	u := getTestUser()
+	u.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload,
+		dataprovider.PermOverwrite, dataprovider.PermDownload, dataprovider.PermCreateDirs,
+		dataprovider.PermChtimes}
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
 	uploadPath := path.Join(testDir, testFileName)
@@ -7765,7 +7767,7 @@ func TestRetentionAPI(t *testing.T) {
 		assert.NoError(t, err)
 	}
 
-	// remove delete permissions to the user
+	// remove delete permissions to the user, it will be automatically granted
 	user.Permissions["/"+testDir] = []string{dataprovider.PermListItems, dataprovider.PermUpload,
 		dataprovider.PermCreateDirs, dataprovider.PermChtimes}
 	user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
@@ -7796,9 +7798,8 @@ func TestRetentionAPI(t *testing.T) {
 				DeleteEmptyDirs: true,
 			},
 			{
-				Path:                  path.Dir(innerUploadFilePath),
-				Retention:             0,
-				IgnoreUserPermissions: true,
+				Path:      path.Dir(innerUploadFilePath),
+				Retention: 0,
 			},
 		}
 		_, err = httpdtest.StartRetentionCheck(user.Username, folderRetention, http.StatusAccepted)
@@ -7808,19 +7809,6 @@ func TestRetentionAPI(t *testing.T) {
 			return len(common.RetentionChecks.Get("")) == 0
 		}, 1000*time.Millisecond, 50*time.Millisecond)
 
-		_, err = client.Stat(uploadPath)
-		assert.NoError(t, err)
-		_, err = client.Stat(innerUploadFilePath)
-		assert.NoError(t, err)
-
-		folderRetention[1].IgnoreUserPermissions = true
-		_, err = httpdtest.StartRetentionCheck(user.Username, folderRetention, http.StatusAccepted)
-		assert.NoError(t, err)
-
-		assert.Eventually(t, func() bool {
-			return len(common.RetentionChecks.Get("")) == 0
-		}, 1000*time.Millisecond, 50*time.Millisecond)
-
 		_, err = client.Stat(uploadPath)
 		assert.ErrorIs(t, err, os.ErrNotExist)
 		_, err = client.Stat(innerUploadFilePath)
@@ -7829,10 +7817,9 @@ func TestRetentionAPI(t *testing.T) {
 		folderRetention = []dataprovider.FolderRetention{
 
 			{
-				Path:                  "/" + testDir,
-				Retention:             24,
-				DeleteEmptyDirs:       true,
-				IgnoreUserPermissions: true,
+				Path:            "/" + testDir,
+				Retention:       24,
+				DeleteEmptyDirs: true,
 			},
 		}
 
@@ -7864,10 +7851,9 @@ func TestRetentionAPI(t *testing.T) {
 		folderRetention := []dataprovider.FolderRetention{
 
 			{
-				Path:                  "/adir",
-				Retention:             24,
-				DeleteEmptyDirs:       true,
-				IgnoreUserPermissions: true,
+				Path:            "/adir",
+				Retention:       24,
+				DeleteEmptyDirs: true,
 			},
 		}
 

internal/dataprovider/eventrule.go

@@ -581,10 +581,6 @@ type FolderRetention struct {
 	// DeleteEmptyDirs defines if empty directories will be deleted.
 	// The user need the delete permission
 	DeleteEmptyDirs bool `json:"delete_empty_dirs,omitempty"`
-	// IgnoreUserPermissions defines whether to delete files even if the user does not have the delete permission.
-	// The default is "false" which means that files will be skipped if the user does not have the permission
-	// to delete them. This applies to sub directories too.
-	IgnoreUserPermissions bool `json:"ignore_user_permissions,omitempty"`
 }
 
 // Validate returns an error if the configuration is not valid
@@ -996,10 +992,9 @@ func (o *BaseEventActionOptions) getACopy() BaseEventActionOptions {
 	folders := make([]FolderRetention, 0, len(o.RetentionConfig.Folders))
 	for _, folder := range o.RetentionConfig.Folders {
 		folders = append(folders, FolderRetention{
-			Path:                  folder.Path,
-			Retention:             folder.Retention,
-			DeleteEmptyDirs:       folder.DeleteEmptyDirs,
-			IgnoreUserPermissions: folder.IgnoreUserPermissions,
+			Path:            folder.Path,
+			Retention:       folder.Retention,
+			DeleteEmptyDirs: folder.DeleteEmptyDirs,
 		})
 	}
 	httpParts := make([]HTTPPart, 0, len(o.HTTPConfig.Parts))

internal/httpd/httpd_test.go

@@ -23338,11 +23338,10 @@ func TestWebEventAction(t *testing.T) {
 	assert.Contains(t, rr.Body.String(), util.I18nError500Message)
 	form.Set("data_retention[10][folder_retention_val]", "24")
 	form.Set("data_retention[10][folder_retention_options][]", "1")
-	form.Add("data_retention[10][folder_retention_options][]", "2")
 	form.Set("data_retention[11][folder_retention_path]", "../p2")
 	form.Set("data_retention[11][folder_retention_val]", "48")
 	form.Set("data_retention[11][folder_retention_options][]", "1")
-	form.Set("data_retention[12][folder_retention_options][]", "2") // ignored
+	form.Set("data_retention[13][folder_retention_options][]", "1") // ignored
 	req, err = http.NewRequest(http.MethodPost, path.Join(webAdminEventActionPath, action.Name),
 		bytes.NewBuffer([]byte(form.Encode())))
 	assert.NoError(t, err)
@@ -23360,11 +23359,9 @@ func TestWebEventAction(t *testing.T) {
 			case "/p1":
 				assert.Equal(t, 24, folder.Retention)
 				assert.True(t, folder.DeleteEmptyDirs)
-				assert.True(t, folder.IgnoreUserPermissions)
 			case "/p2":
 				assert.Equal(t, 48, folder.Retention)
 				assert.True(t, folder.DeleteEmptyDirs)
-				assert.False(t, folder.IgnoreUserPermissions)
 			default:
 				t.Errorf("unexpected folder path %v", folder.Path)
 			}

internal/httpd/webadmin.go

@@ -2220,10 +2220,9 @@ func getFoldersRetentionFromPostFields(r *http.Request) ([]dataprovider.FolderRe
 			}
 			opts := r.Form["folder_retention_options"+strconv.Itoa(idx)]
 			res = append(res, dataprovider.FolderRetention{
-				Path:                  p,
-				Retention:             retention,
-				DeleteEmptyDirs:       util.Contains(opts, "1"),
-				IgnoreUserPermissions: util.Contains(opts, "2"),
+				Path:            p,
+				Retention:       retention,
+				DeleteEmptyDirs: util.Contains(opts, "1"),
 			})
 		}
 	}

internal/httpdtest/httpdtest.go

@@ -2813,9 +2813,6 @@ func compareEventActionDataRetentionFields(expected, actual dataprovider.EventAc
 				if f1.DeleteEmptyDirs != f2.DeleteEmptyDirs {
 					return fmt.Errorf("delete_empty_dirs mismatch for folder %s", f1.Path)
 				}
-				if f1.IgnoreUserPermissions != f2.IgnoreUserPermissions {
-					return fmt.Errorf("ignore_user_permissions mismatch for folder %s", f1.Path)
-				}
 				break
 			}
 		}

static/locales/en/translation.json

@@ -983,7 +983,6 @@
         "data_retention": "Data retention",
         "data_retention_help": "Set the data retention, as hours, per path. Retention applies recursively. Setting 0 as retention means excluding the specified path. \"Ignore user permissions\" defines whether to delete files even if the user does not have the \"delete\" permission, by default files will be skipped if the user does not have the \"delete\" permission",
         "delete_empty_dirs": "Delete empty dirs",
-        "ignore_user_perms": "Ignore user permissions",
         "fs_action": "Filesystem action",
         "paths_src_dst_help": "Paths as seen by SFTPGo users. Placeholders are supported. The required permissions are granted automatically",
         "source_path": "Source",

static/locales/it/translation.json

@@ -983,7 +983,6 @@
         "data_retention": "Conservazione dati",
         "data_retention_help": "Imposta la conservazione dei dati, in ore, per percorso. La conservazione si applica in modo ricorsivo. Impostare 0 come conservazione significa escludere il percorso specificato. \"Ignora permessi utente\" definisce se eliminare i file anche se l'utente non dispone dell'autorizzazione \"delete\", per impostazione predefinita i file verranno ignorati se l'utente non dispone dell'autorizzazione \"delete\"",
         "delete_empty_dirs": "Cancella cartelle vuote",
-        "ignore_user_perms": "Ignora permessi utente",
         "fs_action": "Azione del filesystem",
         "paths_src_dst_help": "Percorsi visti dagli utenti SFTPGo. I segnaposto sono supportati. Le autorizzazioni richieste vengono concesse automaticamente",
         "source_path": "Origine",

templates/webadmin/eventaction.html

@@ -566,10 +566,9 @@ explicit grant from the SFTPGo Team ([email protected]).
                                                 <input data-i18n="[placeholder]general.hours" type="text" class="form-control" name="folder_retention_val" value="{{$val.Retention}}" />
                                             </div>
                                             <div class="col-md-4 mt-3 mt-md-8">
-                                                <select name="folder_retention_options" data-i18n="[data-placeholder]general.folder_placeholder" class="form-select select-repetear" data-allow-clear="true" data-close-on-select="false" data-hide-search="true" multiple>
+                                                <select name="folder_retention_options" data-i18n="[data-placeholder]general.options" class="form-select select-repetear" data-allow-clear="true" data-close-on-select="false" data-hide-search="true" multiple>
                                                     <option value=""></option>
                                                     <option value="1" data-i18n="actions.delete_empty_dirs" {{if $val.DeleteEmptyDirs}}selected{{end}}>Delete empty dirs</option>
-                                                    <option value="2" data-i18n="actions.ignore_user_perms" {{if $val.IgnoreUserPermissions}}selected{{end}}>Ignore user permissions</option>
                                                 </select>
                                             </div>
                                             <div class="col-md-1 mt-3 mt-md-8">
@@ -597,10 +596,9 @@ explicit grant from the SFTPGo Team ([email protected]).
                                             <input data-i18n="[placeholder]general.hours" type="text" class="form-control" name="folder_retention_val" value="" />
                                         </div>
                                         <div class="col-md-4 mt-3 mt-md-8">
-                                            <select name="folder_retention_options" data-i18n="[data-placeholder]general.folder_placeholder" class="form-select select-repetear" data-allow-clear="true" data-close-on-select="false" data-hide-search="true" multiple>
+                                            <select name="folder_retention_options" data-i18n="[data-placeholder]general.options" class="form-select select-repetear" data-allow-clear="true" data-close-on-select="false" data-hide-search="true" multiple>
                                                 <option value=""></option>
                                                 <option value="1" data-i18n="actions.delete_empty_dirs">Delete empty dirs</option>
-                                                <option value="2" data-i18n="actions.ignore_user_perms">Ignore user permissions</option>
                                             </select>
                                         </div>
                                         <div class="col-md-1 mt-3 mt-md-8">