diff --git a/internal/common/dataretention.go b/internal/common/dataretention.go index f2f6c21a..4ecc8c90 100644 --- a/internal/common/dataretention.go +++ b/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) diff --git a/internal/common/dataretention_test.go b/internal/common/dataretention_test.go index adc59314..b0c17e3b 100644 --- a/internal/common/dataretention_test.go +++ b/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) diff --git a/internal/common/protocol_test.go b/internal/common/protocol_test.go index 9171f95c..e8f590b2 100644 --- a/internal/common/protocol_test.go +++ b/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, }, } diff --git a/internal/dataprovider/eventrule.go b/internal/dataprovider/eventrule.go index a46d9c6f..2780860c 100644 --- a/internal/dataprovider/eventrule.go +++ b/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)) diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index a7a19086..adf923cd 100644 --- a/internal/httpd/httpd_test.go +++ b/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) } diff --git a/internal/httpd/webadmin.go b/internal/httpd/webadmin.go index 1188018d..84abbbf8 100644 --- a/internal/httpd/webadmin.go +++ b/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"), }) } } diff --git a/internal/httpdtest/httpdtest.go b/internal/httpdtest/httpdtest.go index d495a2d0..382e2ca0 100644 --- a/internal/httpdtest/httpdtest.go +++ b/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 } } diff --git a/static/locales/en/translation.json b/static/locales/en/translation.json index 8fd39e89..b4145112 100644 --- a/static/locales/en/translation.json +++ b/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", diff --git a/static/locales/it/translation.json b/static/locales/it/translation.json index 447994ee..ef9ddf34 100644 --- a/static/locales/it/translation.json +++ b/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", diff --git a/templates/webadmin/eventaction.html b/templates/webadmin/eventaction.html index 1cad592f..d7e6d098 100644 --- a/templates/webadmin/eventaction.html +++ b/templates/webadmin/eventaction.html @@ -566,10 +566,9 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
- -
@@ -597,10 +596,9 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
- -