New: Removed chown and simplified chmod options for linux/osx

Closes #3760
Closes #3752

frontend/src/Settings/MediaManagement/MediaManagement.js

@@ -371,7 +371,7 @@ class MediaManagement extends Component {
                         <FormInputGroup
                           type={inputTypes.CHECK}
                           name="setPermissionsLinux"
-                          helpText="Should chmod/chown be run when files are imported/renamed?"
+                          helpText="Should chmod be run when files are imported/renamed?"
                           helpTextWarning="If you're unsure what these settings do, do not alter them."
                           onChange={onInputChange}
                           {...settings.setPermissionsLinux}
@@ -387,59 +387,14 @@ class MediaManagement extends Component {
                         <FormInputGroup
                           type={inputTypes.TEXT}
                           name="fileChmod"
-                          helpText="Octal, applied to media files when imported/renamed by Sonarr"
+                          helpTexts={[
+                            'Octal, applied to media files when imported/renamed by Sonarr',
+                            'The same mode is applied to series/season folders with the execute bit added, e.g., 0644 becomes 0755'
+                          ]}
                           onChange={onInputChange}
                           {...settings.fileChmod}
                         />
                       </FormGroup>
-
-                      <FormGroup
-                        advancedSettings={advancedSettings}
-                        isAdvanced={true}
-                      >
-                        <FormLabel>Folder chmod mode</FormLabel>
-
-                        <FormInputGroup
-                          type={inputTypes.TEXT}
-                          name="folderChmod"
-                          helpText="Octal, applied to series/season folders created by Sonarr"
-                          values={fileDateOptions}
-                          onChange={onInputChange}
-                          {...settings.folderChmod}
-                        />
-                      </FormGroup>
-
-                      <FormGroup
-                        advancedSettings={advancedSettings}
-                        isAdvanced={true}
-                      >
-                        <FormLabel>chown User</FormLabel>
-
-                        <FormInputGroup
-                          type={inputTypes.TEXT}
-                          name="chownUser"
-                          helpText="Username or uid. Use uid for remote file systems."
-                          values={fileDateOptions}
-                          onChange={onInputChange}
-                          {...settings.chownUser}
-                        />
-                      </FormGroup>
-
-                      <FormGroup
-                        advancedSettings={advancedSettings}
-                        isAdvanced={true}
-                      >
-                        <FormLabel>chown Group</FormLabel>
-
-                        <FormInputGroup
-                          type={inputTypes.TEXT}
-                          name="chownGroup"
-                          helpText="Group name or gid. Use gid for remote file systems."
-                          values={fileDateOptions}
-                          onChange={onInputChange}
-                          {...settings.chownGroup}
-                        />
-                      </FormGroup>
                     </FieldSet>
                 }
               </Form>

src/NzbDrone.Api/Config/MediaManagementConfigModule.cs

@@ -1,16 +1,17 @@
 using FluentValidation;
+using NzbDrone.Common.EnvironmentInfo;
 using NzbDrone.Core.Configuration;
+using NzbDrone.Core.Validation;
 using NzbDrone.Core.Validation.Paths;
 
 namespace NzbDrone.Api.Config
 {
     public class MediaManagementConfigModule : NzbDroneConfigModule<MediaManagementConfigResource>
     {
-        public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator)
+        public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator, FileChmodValidator fileChmodValidator)
             : base(configService)
         {
-            SharedValidator.RuleFor(c => c.FileChmod).NotEmpty();
+            SharedValidator.RuleFor(c => c.FileChmod).SetValidator(fileChmodValidator).When(c => !string.IsNullOrEmpty(c.FileChmod) && PlatformInfo.IsMono);
-            SharedValidator.RuleFor(c => c.FolderChmod).NotEmpty();
             SharedValidator.RuleFor(c => c.RecycleBin).IsValidPath().SetValidator(pathExistsValidator).When(c => !string.IsNullOrWhiteSpace(c.RecycleBin));
         }
 

src/NzbDrone.Api/Config/MediaManagementConfigResource.cs

@@ -16,9 +16,6 @@ namespace NzbDrone.Api.Config
 
         public bool SetPermissionsLinux { get; set; }
         public string FileChmod { get; set; }
-        public string FolderChmod { get; set; }
-        public string ChownUser { get; set; }
-        public string ChownGroup { get; set; }
 
         public bool SkipFreeSpaceCheckWhenImporting { get; set; }
         public bool CopyUsingHardlinks { get; set; }
@@ -42,9 +39,6 @@ namespace NzbDrone.Api.Config
 
                 SetPermissionsLinux = model.SetPermissionsLinux,
                 FileChmod = model.FileChmod,
-                FolderChmod = model.FolderChmod,
-                ChownUser = model.ChownUser,
-                ChownGroup = model.ChownGroup,
 
                 SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting,
                 CopyUsingHardlinks = model.CopyUsingHardlinks,

src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs

@@ -1037,7 +1037,7 @@ namespace NzbDrone.Common.Test.DiskTests
                .Returns(new List<FileInfo>());
 
             Mocker.GetMock<IDiskProvider>()
-                .Setup(v => v.CopyPermissions(It.IsAny<string>(), It.IsAny<string>(), false));
+                .Setup(v => v.CopyPermissions(It.IsAny<string>(), It.IsAny<string>()));
         }
 
         private void WithRealDiskProvider()
@@ -1094,7 +1094,7 @@ namespace NzbDrone.Common.Test.DiskTests
                 .Returns<string>(s => new FileStream(s, FileMode.Open, FileAccess.Read));
 
             Mocker.GetMock<IDiskProvider>()
-                .Setup(v => v.CopyPermissions(It.IsAny<string>(), It.IsAny<string>(), false));
+                .Setup(v => v.CopyPermissions(It.IsAny<string>(), It.IsAny<string>()));
         }
 
         private void WithMockMount(string root)

src/NzbDrone.Common/Disk/DiskProviderBase.cs

@@ -31,8 +31,8 @@ namespace NzbDrone.Common.Disk
 
         public abstract long? GetAvailableSpace(string path);
         public abstract void InheritFolderPermissions(string filename);
-        public abstract void SetPermissions(string path, string mask, string user, string group);
+        public abstract void SetPermissions(string path, string mask);
-        public abstract void CopyPermissions(string sourcePath, string targetPath, bool includeOwner);
+        public abstract void CopyPermissions(string sourcePath, string targetPath);
         public abstract long? GetTotalSize(string path);
 
         public DateTime FolderGetCreationTime(string path)
@@ -515,5 +515,10 @@ namespace NzbDrone.Common.Disk
                 stream.CopyTo(fileStream);
             }
         }
+
+        public virtual bool IsValidFilePermissionMask(string mask)
+        {
+            throw new NotSupportedException();
+        }
     }
 }

src/NzbDrone.Common/Disk/IDiskProvider.cs

@@ -10,8 +10,8 @@ namespace NzbDrone.Common.Disk
     {
         long? GetAvailableSpace(string path);
         void InheritFolderPermissions(string filename);
-        void SetPermissions(string path, string mask, string user, string group);
+        void SetPermissions(string path, string mask);
-        void CopyPermissions(string sourcePath, string targetPath, bool includeOwner = false);
+        void CopyPermissions(string sourcePath, string targetPath);
         long? GetTotalSize(string path);
         DateTime FolderGetCreationTime(string path);
         DateTime FolderGetLastWrite(string path);
@@ -52,5 +52,6 @@ namespace NzbDrone.Common.Disk
         List<FileInfo> GetFileInfos(string path);
         void RemoveEmptySubfolders(string path);
         void SaveStream(Stream stream, string path);
+        bool IsValidFilePermissionMask(string mask);
     }
 }

src/NzbDrone.Core/Configuration/ConfigService.cs

@@ -259,27 +259,6 @@ namespace NzbDrone.Core.Configuration
             set { SetValue("FileChmod", value); }
         }
 
-        public string FolderChmod
-        {
-            get { return GetValue("FolderChmod", "0755"); }
-
-            set { SetValue("FolderChmod", value); }
-        }
-
-        public string ChownUser
-        {
-            get { return GetValue("ChownUser", ""); }
-
-            set { SetValue("ChownUser", value); }
-        }
-
-        public string ChownGroup
-        {
-            get { return GetValue("ChownGroup", ""); }
-
-            set { SetValue("ChownGroup", value); }
-        }
-
         public int FirstDayOfWeek
         {
             get { return GetValueInt("FirstDayOfWeek", (int)CultureInfo.CurrentCulture.DateTimeFormat.FirstDayOfWeek); }

src/NzbDrone.Core/Configuration/IConfigService.cs

@@ -44,9 +44,6 @@ namespace NzbDrone.Core.Configuration
         //Permissions (Media Management)
         bool SetPermissionsLinux { get; set; }
         string FileChmod { get; set; }
-        string FolderChmod { get; set; }
-        string ChownUser { get; set; }
-        string ChownGroup { get; set; }
 
         //Indexers
         int Retention { get; set; }

src/NzbDrone.Core/Datastore/Migration/140_remove_chown_and_folderchmod_config.cs

@@ -0,0 +1,15 @@
+using System.Data;
+using FluentMigrator;
+using NzbDrone.Core.Datastore.Migration.Framework;
+
+namespace NzbDrone.Core.Datastore.Migration
+{
+    [Migration(140)]
+    public class remove_chown_and_folderchmod_config : NzbDroneMigrationBase
+    {
+        protected override void MainDbUpgrade()
+        {
+            Execute.Sql("DELETE FROM config WHERE Key IN ('folderchmod', 'chownuser', 'chowngroup')");
+        }
+    }
+}

src/NzbDrone.Core/MediaFiles/DiskScanService.cs

@@ -186,8 +186,8 @@ namespace NzbDrone.Core.MediaFiles
 
             try
             {
-                var permissions = _configService.FolderChmod;
+                var permissions = _configService.FileChmod;
-                _diskProvider.SetPermissions(path, permissions, _configService.ChownUser, _configService.ChownGroup);
+                _diskProvider.SetPermissions(path, permissions);
             }
 
             catch (Exception ex)

src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs

@@ -63,7 +63,7 @@ namespace NzbDrone.Core.MediaFiles
         {
             if (OsInfo.IsNotWindows)
             {
-                SetMonoPermissions(path, _configService.FolderChmod);
+                SetMonoPermissions(path, _configService.FileChmod);
             }
         }
 
@@ -85,7 +85,7 @@ namespace NzbDrone.Core.MediaFiles
 
             try
             {
-                _diskProvider.SetPermissions(path, permissions, _configService.ChownUser, _configService.ChownGroup);
+                _diskProvider.SetPermissions(path, permissions);
             }
 
             catch (Exception ex)

src/NzbDrone.Core/Validation/FileChmodValidator.cs

@@ -0,0 +1,23 @@
+using FluentValidation.Validators;
+using NzbDrone.Common.Disk;
+
+namespace NzbDrone.Core.Validation
+{
+    public class FileChmodValidator : PropertyValidator
+    {
+        private readonly IDiskProvider _diskProvider;
+
+        public FileChmodValidator(IDiskProvider diskProvider)
+            : base("Must contain a valid Unix permissions octal")
+        {
+            _diskProvider = diskProvider;
+        }
+
+        protected override bool IsValid(PropertyValidatorContext context)
+        {
+            if (context.PropertyValue == null) return false;
+
+            return _diskProvider.IsValidFilePermissionMask(context.PropertyValue.ToString());
+        }
+    }
+}

src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs

@@ -151,11 +151,95 @@ namespace NzbDrone.Mono.Test.DiskProviderTests
             Syscall.stat(dst, out var dstStat);
             dstStat.st_mode.Should().Be(origStat.st_mode);
 
-            Subject.CopyPermissions(src, dst, false);
+            Subject.CopyPermissions(src, dst);
 
             // Verify CopyPermissions
             Syscall.stat(dst, out dstStat);
             dstStat.st_mode.Should().Be(srcStat.st_mode);
         }
+
+        [Test]
+        public void should_set_file_permissions()
+        {
+            var tempFile = GetTempFilePath();
+
+            File.WriteAllText(tempFile, "File1");
+            SetWritePermissions(tempFile, false);
+
+            // Verify test setup
+            Syscall.stat(tempFile, out var fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0444");
+
+            Subject.SetPermissions(tempFile, "644");
+            Syscall.stat(tempFile, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644");
+
+            Subject.SetPermissions(tempFile, "0644");
+            Syscall.stat(tempFile, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644");
+
+            Subject.SetPermissions(tempFile, "1664");
+            Syscall.stat(tempFile, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1664");
+        }
+
+        [Test]
+        public void should_set_folder_permissions()
+        {
+            var tempPath = GetTempFilePath();
+
+            Directory.CreateDirectory(tempPath);
+            SetWritePermissions(tempPath, false);
+
+            // Verify test setup
+            Syscall.stat(tempPath, out var fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0555");
+
+            Subject.SetPermissions(tempPath, "644");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755");
+
+            Subject.SetPermissions(tempPath, "0644");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755");
+
+            Subject.SetPermissions(tempPath, "1664");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1775");
+
+            Subject.SetPermissions(tempPath, "775");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775");
+
+            Subject.SetPermissions(tempPath, "640");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750");
+
+            Subject.SetPermissions(tempPath, "0041");
+            Syscall.stat(tempPath, out fileStat);
+            NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051");
+        }
+
+        [Test]
+        public void IsValidFilePermissionMask_should_return_correct()
+        {
+            // Files may not be executable
+            Subject.IsValidFilePermissionMask("0777").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0544").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0454").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0445").Should().BeFalse();
+
+            // No special bits should be set
+            Subject.IsValidFilePermissionMask("1644").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("2644").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("4644").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("7644").Should().BeFalse();
+
+            // Files should be readable and writeable by owner
+            Subject.IsValidFilePermissionMask("0400").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0000").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0200").Should().BeFalse();
+            Subject.IsValidFilePermissionMask("0600").Should().BeTrue();
+        }
     }
 }

src/NzbDrone.Mono/Disk/DiskProvider.cs

@@ -72,13 +72,62 @@ namespace NzbDrone.Mono.Disk
             }
         }
 
-        public override void SetPermissions(string path, string mask, string user, string group)
+        public override void SetPermissions(string path, string mask)
         {
-            SetPermissions(path, mask);
+            Logger.Debug("Setting permissions: {0} on {1}", mask, path);
-            SetOwner(path, user, group);
+
+            var permissions = NativeConvert.FromOctalPermissionString(mask);
+
+            if (Directory.Exists(path))
+            {
+                permissions = GetFolderPermissions(permissions);
             }
 
-        public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner)
+            if (Syscall.chmod(path, permissions) < 0)
+            {
+                var error = Stdlib.GetLastError();
+
+                throw new LinuxPermissionsException("Error setting permissions: " + error);
+            }
+        }
+
+        private static FilePermissions GetFolderPermissions(FilePermissions permissions)
+        {
+            permissions |= (FilePermissions) ((int) (permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IRGRP | FilePermissions.S_IROTH)) >> 2);
+
+            return permissions;
+        }
+
+        public override bool IsValidFilePermissionMask(string mask)
+        {
+            try
+            {
+                var permissions = NativeConvert.FromOctalPermissionString(mask);
+
+                if ((permissions & (FilePermissions.S_ISUID | FilePermissions.S_ISGID | FilePermissions.S_ISVTX)) != 0)
+                {
+                    return false;
+                }
+
+                if ((permissions & (FilePermissions.S_IXUSR | FilePermissions.S_IXGRP | FilePermissions.S_IXOTH)) != 0)
+                {
+                    return false;
+                }
+
+                if ((permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) != (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR))
+                {
+                    return false;
+                }
+
+                return true;
+            }
+            catch (FormatException)
+            {
+                return false;
+            }
+        }
+
+        public override void CopyPermissions(string sourcePath, string targetPath)
         {
             try
             {
@@ -89,11 +138,6 @@ namespace NzbDrone.Mono.Disk
                 {
                     Syscall.chmod(targetPath, srcStat.st_mode);
                 }
-
-                if (includeOwner && (srcStat.st_uid != tgtStat.st_uid || srcStat.st_gid != tgtStat.st_gid))
-                {
-                    Syscall.chown(targetPath, srcStat.st_uid, srcStat.st_gid);
-                }
             }
             catch (Exception ex)
             {
@@ -361,39 +405,6 @@ namespace NzbDrone.Mono.Disk
             }
         }
 
-        private void SetPermissions(string path, string mask)
-        {
-            Logger.Debug("Setting permissions: {0} on {1}", mask, path);
-
-            var filePermissions = NativeConvert.FromOctalPermissionString(mask);
-
-            if (Syscall.chmod(path, filePermissions) < 0)
-            {
-                var error = Stdlib.GetLastError();
-
-                throw new LinuxPermissionsException("Error setting file permissions: " + error);
-            }
-        }
-
-        private void SetOwner(string path, string user, string group)
-        {
-            if (string.IsNullOrWhiteSpace(user) && string.IsNullOrWhiteSpace(group))
-            {
-                Logger.Debug("User and Group for chown not configured, skipping chown.");
-                return;
-            }
-
-            var userId = GetUserId(user);
-            var groupId = GetGroupId(group);
-
-            if (Syscall.chown(path, userId, groupId) < 0)
-            {
-                var error = Stdlib.GetLastError();
-
-                throw new LinuxPermissionsException("Error setting file owner and/or group: " + error);
-            }
-        }
-
         private uint GetUserId(string user)
         {
             if (user.IsNullOrWhiteSpace())

src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs

@@ -129,7 +129,7 @@ namespace NzbDrone.Update.UpdateEngine
                         {
                             // Old MacOS App stores Sonarr binaries in MacOS together with shell script
                             // Make shim executable
-                            _diskProvider.SetPermissions(shimPath, "0755", null, null);
+                            _diskProvider.SetPermissions(shimPath, "0755");
                         }
                     }
                 }

src/NzbDrone.Windows/Disk/DiskProvider.cs

@@ -44,12 +44,12 @@ namespace NzbDrone.Windows.Disk
             File.SetAccessControl(filename, fs);
         }
 
-        public override void SetPermissions(string path, string mask, string user, string group)
+        public override void SetPermissions(string path, string mask)
         {
 
         }
 
-        public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner)
+        public override void CopyPermissions(string sourcePath, string targetPath)
         {
 
         }

src/Sonarr.Api.V3/Config/MediaManagementConfigModule.cs

@@ -1,17 +1,18 @@
 using FluentValidation;
+using NzbDrone.Common.EnvironmentInfo;
 using NzbDrone.Core.Configuration;
+using NzbDrone.Core.Validation;
 using NzbDrone.Core.Validation.Paths;
 
 namespace Sonarr.Api.V3.Config
 {
     public class MediaManagementConfigModule : SonarrConfigModule<MediaManagementConfigResource>
     {
-        public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator)
+        public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator, FileChmodValidator fileChmodValidator)
             : base(configService)
         {
             SharedValidator.RuleFor(c => c.RecycleBinCleanupDays).GreaterThanOrEqualTo(0);
-            SharedValidator.RuleFor(c => c.FileChmod).NotEmpty();
+            SharedValidator.RuleFor(c => c.FileChmod).SetValidator(fileChmodValidator).When(c => !string.IsNullOrEmpty(c.FileChmod) && PlatformInfo.IsMono);
-            SharedValidator.RuleFor(c => c.FolderChmod).NotEmpty();
             SharedValidator.RuleFor(c => c.RecycleBin).IsValidPath().SetValidator(pathExistsValidator).When(c => !string.IsNullOrWhiteSpace(c.RecycleBin));
             SharedValidator.RuleFor(c => c.MinimumFreeSpaceWhenImporting).GreaterThanOrEqualTo(100);
         }

src/Sonarr.Api.V3/Config/MediaManagementConfigResource.cs

@@ -19,9 +19,6 @@ namespace Sonarr.Api.V3.Config
 
         public bool SetPermissionsLinux { get; set; }
         public string FileChmod { get; set; }
-        public string FolderChmod { get; set; }
-        public string ChownUser { get; set; }
-        public string ChownGroup { get; set; }
 
         public EpisodeTitleRequiredType EpisodeTitleRequired { get; set; }
         public bool SkipFreeSpaceCheckWhenImporting { get; set; }
@@ -49,9 +46,6 @@ namespace Sonarr.Api.V3.Config
 
                 SetPermissionsLinux = model.SetPermissionsLinux,
                 FileChmod = model.FileChmod,
-                FolderChmod = model.FolderChmod,
-                ChownUser = model.ChownUser,
-                ChownGroup = model.ChownGroup,
 
                 EpisodeTitleRequired = model.EpisodeTitleRequired,
                 SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting,