Merge pull request #5495 from BaronGreenback/RemoteAccessFix

(cherry picked from commit a890a8509228f6a357a6986549afc492da74b146)
Signed-off-by: Joshua M. Boniface <joshua@boniface.me>

Jellyfin.Networking/Manager/NetworkManager.cs

@@ -587,6 +587,29 @@ namespace Jellyfin.Networking.Manager
             return false;
         }
 
+        /// <inheritdoc/>
+        public bool HasRemoteAccess(IPAddress remoteIp)
+        {
+            var config = _configurationManager.GetNetworkConfiguration();
+            if (config.EnableRemoteAccess)
+            {
+                // Comma separated list of IP addresses or IP/netmask entries for networks that will be allowed to connect remotely.
+                // If left blank, all remote addresses will be allowed.
+                if (RemoteAddressFilter.Count > 0 && !IsInLocalNetwork(remoteIp))
+                {
+                    // remoteAddressFilter is a whitelist or blacklist.
+                    return RemoteAddressFilter.ContainsAddress(remoteIp) == !config.IsRemoteIPFilterBlacklist;
+                }
+            }
+            else if (!IsInLocalNetwork(remoteIp))
+            {
+                // Remote not enabled. So everyone should be LAN.
+                return false;
+            }
+
+            return true;
+        }
+
         /// <summary>
         /// Reloads all settings and re-initialises the instance.
         /// </summary>

Jellyfin.Server/Middleware/IpBasedAccessValidationMiddleware.cs

@@ -29,9 +29,8 @@ namespace Jellyfin.Server.Middleware
         /// </summary>
         /// <param name="httpContext">The current HTTP context.</param>
         /// <param name="networkManager">The network manager.</param>
-        /// <param name="serverConfigurationManager">The server configuration manager.</param>
         /// <returns>The async task.</returns>
-        public async Task Invoke(HttpContext httpContext, INetworkManager networkManager, IServerConfigurationManager serverConfigurationManager)
+        public async Task Invoke(HttpContext httpContext, INetworkManager networkManager)
         {
             if (httpContext.IsLocal())
             {
@@ -42,32 +41,8 @@ namespace Jellyfin.Server.Middleware
 
             var remoteIp = httpContext.Connection.RemoteIpAddress ?? IPAddress.Loopback;
 
-            if (serverConfigurationManager.GetNetworkConfiguration().EnableRemoteAccess)
+            if (!networkManager.HasRemoteAccess(remoteIp))
             {
-                // Comma separated list of IP addresses or IP/netmask entries for networks that will be allowed to connect remotely.
-                // If left blank, all remote addresses will be allowed.
-                var remoteAddressFilter = networkManager.RemoteAddressFilter;
-
-                if (remoteAddressFilter.Count > 0 && !networkManager.IsInLocalNetwork(remoteIp))
-                {
-                    // remoteAddressFilter is a whitelist or blacklist.
-                    bool isListed = remoteAddressFilter.ContainsAddress(remoteIp);
-                    if (!serverConfigurationManager.GetNetworkConfiguration().IsRemoteIPFilterBlacklist)
-                    {
-                        // Black list, so flip over.
-                        isListed = !isListed;
-                    }
-
-                    if (!isListed)
-                    {
-                        // If your name isn't on the list, you arn't coming in.
-                        return;
-                    }
-                }
-            }
-            else if (!networkManager.IsInLocalNetwork(remoteIp))
-            {
-                // Remote not enabled. So everyone should be LAN.
                 return;
             }
 

MediaBrowser.Common/Net/INetworkManager.cs

@@ -229,5 +229,12 @@ namespace MediaBrowser.Common.Net
         /// <param name="filter">Optional filter for the list.</param>
         /// <returns>Returns a filtered list of LAN addresses.</returns>
         Collection<IPObject> GetFilteredLANSubnets(Collection<IPObject>? filter = null);
+
+        /// <summary>
+        /// Checks to see if <paramref name="remoteIp"/> has access.
+        /// </summary>
+        /// <param name="remoteIp">IP Address of client.</param>
+        /// <returns><b>True</b> if has access, otherwise <b>false</b>.</returns>
+        bool HasRemoteAccess(IPAddress remoteIp);
     }
 }

tests/Jellyfin.Networking.Tests/NetworkTesting/NetworkParseTests.cs

@@ -201,29 +201,29 @@ namespace Jellyfin.Networking.Tests
             using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger<NetworkManager>());
 
             // Test included.
-            Collection<IPObject> nc = nm.CreateIPCollection(settings.Split(","), false); 
+            Collection<IPObject> nc = nm.CreateIPCollection(settings.Split(','), false);
             Assert.Equal(nc.AsString(), result1);
 
             // Test excluded.
-            nc = nm.CreateIPCollection(settings.Split(","), true);
+            nc = nm.CreateIPCollection(settings.Split(','), true);
             Assert.Equal(nc.AsString(), result3);
 
             conf.EnableIPV6 = false;
             nm.UpdateSettings(conf);
             
             // Test IP4 included.
-            nc = nm.CreateIPCollection(settings.Split(","), false);
+            nc = nm.CreateIPCollection(settings.Split(','), false);
             Assert.Equal(nc.AsString(), result2);
 
             // Test IP4 excluded.
-            nc = nm.CreateIPCollection(settings.Split(","), true);
+            nc = nm.CreateIPCollection(settings.Split(','), true);
             Assert.Equal(nc.AsString(), result4);
 
             conf.EnableIPV6 = true;
             nm.UpdateSettings(conf);
 
             // Test network addresses of collection.
-            nc = nm.CreateIPCollection(settings.Split(","), false);
+            nc = nm.CreateIPCollection(settings.Split(','), false);
             nc = nc.AsNetworks();
             Assert.Equal(nc.AsString(), result5);
         }
@@ -263,8 +263,8 @@ namespace Jellyfin.Networking.Tests
 
             using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger<NetworkManager>());
 
-            Collection<IPObject> nc1 = nm.CreateIPCollection(settings.Split(","), false);
-            Collection<IPObject> nc2 = nm.CreateIPCollection(compare.Split(","), false);
+            Collection<IPObject> nc1 = nm.CreateIPCollection(settings.Split(','), false);
+            Collection<IPObject> nc2 = nm.CreateIPCollection(compare.Split(','), false);
 
             Assert.Equal(nc1.Union(nc2).AsString(), result);
         }
@@ -373,10 +373,10 @@ namespace Jellyfin.Networking.Tests
             using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger<NetworkManager>());
 
             // Test included, IP6.
-            Collection<IPObject> ncSource = nm.CreateIPCollection(source.Split(","));
-            Collection<IPObject> ncDest = nm.CreateIPCollection(dest.Split(","));
+            Collection<IPObject> ncSource = nm.CreateIPCollection(source.Split(','));
+            Collection<IPObject> ncDest = nm.CreateIPCollection(dest.Split(','));
             Collection<IPObject> ncResult = ncSource.Union(ncDest);
-            Collection<IPObject> resultCollection = nm.CreateIPCollection(result.Split(","));
+            Collection<IPObject> resultCollection = nm.CreateIPCollection(result.Split(','));
             Assert.True(ncResult.Compare(resultCollection));
         }
 
@@ -517,5 +517,45 @@ namespace Jellyfin.Networking.Tests
 
             Assert.Equal(intf, result);
         }
+
+        [Theory]
+        [InlineData("185.10.10.10,200.200.200.200", "79.2.3.4", true)]
+        [InlineData("185.10.10.10", "185.10.10.10", false)]
+        [InlineData("", "100.100.100.100", false)]
+
+        public void HasRemoteAccess_GivenWhitelist_AllowsOnlyIpsInWhitelist(string addresses, string remoteIp, bool denied)
+        {
+            // Comma separated list of IP addresses or IP/netmask entries for networks that will be allowed to connect remotely.
+            // If left blank, all remote addresses will be allowed.
+            var conf = new NetworkConfiguration()
+            {
+                EnableIPV4 = true,
+                RemoteIPFilter = addresses.Split(','),
+                IsRemoteIPFilterBlacklist = false
+            };
+            using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger<NetworkManager>());
+
+            Assert.NotEqual(nm.HasRemoteAccess(IPAddress.Parse(remoteIp)), denied);
+        }
+
+        [Theory]
+        [InlineData("185.10.10.10", "79.2.3.4", false)]
+        [InlineData("185.10.10.10", "185.10.10.10", true)]
+        [InlineData("", "100.100.100.100", false)]
+        public void HasRemoteAccess_GivenBlacklist_BlacklistTheIps(string addresses, string remoteIp, bool denied)
+        {
+            // Comma separated list of IP addresses or IP/netmask entries for networks that will be allowed to connect remotely.
+            // If left blank, all remote addresses will be allowed.
+            var conf = new NetworkConfiguration()
+            {
+                EnableIPV4 = true,
+                RemoteIPFilter = addresses.Split(','),
+                IsRemoteIPFilterBlacklist = true
+            };
+
+            using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger<NetworkManager>());
+
+            Assert.NotEqual(nm.HasRemoteAccess(IPAddress.Parse(remoteIp)), denied);
+        }
     }
 }