From 86624e92d3539db92934f280c9efdbda1448486b Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Tue, 9 Jun 2020 15:18:26 -0500 Subject: [PATCH] Finish addressing review comments --- .../QuickConnect/ConfigurationExtension.cs | 22 ------ .../QuickConnectConfigurationFactory.cs | 27 ++++++++ .../QuickConnect/QuickConnectManager.cs | 67 ++++++++----------- 3 files changed, 56 insertions(+), 60 deletions(-) create mode 100644 Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs index 596ded8caa..2a19fc36c1 100644 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -17,26 +17,4 @@ namespace Emby.Server.Implementations.QuickConnect return manager.GetConfiguration("quickconnect"); } } - - /// - /// Configuration factory for quick connect. - /// - public class QuickConnectConfigurationFactory : IConfigurationFactory - { - /// - /// Returns the current quick connect configuration. - /// - /// Current quick connect configuration. - public IEnumerable GetConfigurations() - { - return new[] - { - new ConfigurationStore - { - Key = "quickconnect", - ConfigurationType = typeof(QuickConnectConfiguration) - } - }; - } - } } diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs new file mode 100644 index 0000000000..d7bc84c5e2 --- /dev/null +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs @@ -0,0 +1,27 @@ +using System.Collections.Generic; +using MediaBrowser.Common.Configuration; + +namespace Emby.Server.Implementations.QuickConnect +{ + /// + /// Configuration factory for quick connect. + /// + public class QuickConnectConfigurationFactory : IConfigurationFactory + { + /// + /// Returns the current quick connect configuration. + /// + /// Current quick connect configuration. + public IEnumerable GetConfigurations() + { + return new[] + { + new ConfigurationStore + { + Key = "quickconnect", + ConfigurationType = typeof(QuickConnectConfiguration) + } + }; + } + } +} diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 62b775fa60..adcc6f2cfc 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -1,20 +1,16 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Security.Cryptography; -using MediaBrowser.Common.Configuration; using MediaBrowser.Controller; using MediaBrowser.Controller.Configuration; -using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Controller.Security; -using MediaBrowser.Model.Globalization; using MediaBrowser.Model.QuickConnect; -using MediaBrowser.Model.Serialization; using MediaBrowser.Model.Services; -using MediaBrowser.Model.Tasks; using Microsoft.Extensions.Logging; namespace Emby.Server.Implementations.QuickConnect @@ -25,7 +21,7 @@ namespace Emby.Server.Implementations.QuickConnect public class QuickConnectManager : IQuickConnect { private readonly RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider(); - private Dictionary _currentRequests = new Dictionary(); + private readonly ConcurrentDictionary _currentRequests = new ConcurrentDictionary(); private readonly IServerConfigurationManager _config; private readonly ILogger _logger; @@ -39,44 +35,25 @@ namespace Emby.Server.Implementations.QuickConnect /// /// Configuration. /// Logger. - /// User manager. - /// Localization. - /// JSON serializer. /// Application host. /// Authentication context. /// Authentication repository. - /// Task scheduler. public QuickConnectManager( IServerConfigurationManager config, ILogger logger, - IUserManager userManager, - ILocalizationManager localization, - IJsonSerializer jsonSerializer, IServerApplicationHost appHost, IAuthorizationContext authContext, - IAuthenticationRepository authenticationRepository, - ITaskManager taskManager) + IAuthenticationRepository authenticationRepository) { _config = config; _logger = logger; - _userManager = userManager; - _localizationManager = localization; - _jsonSerializer = jsonSerializer; _appHost = appHost; _authContext = authContext; _authenticationRepository = authenticationRepository; - _taskManager = taskManager; ReloadConfiguration(); } - private void ReloadConfiguration() - { - var config = _config.GetQuickConnectConfiguration(); - - State = config.State; - } - /// public int CodeLength { get; set; } = 6; @@ -118,6 +95,7 @@ namespace Emby.Server.Implementations.QuickConnect { _logger.LogDebug("Changed quick connect state from {0} to {1}", State, newState); + ExpireRequests(true); State = newState; _config.SaveConfiguration("quickconnect", new QuickConnectConfiguration() @@ -167,12 +145,12 @@ namespace Emby.Server.Implementations.QuickConnect string lookup = _currentRequests.Where(x => x.Value.Secret == secret).Select(x => x.Value.Lookup).DefaultIfEmpty(string.Empty).First(); - if (!_currentRequests.ContainsKey(lookup)) + if (!_currentRequests.TryGetValue(lookup, out QuickConnectResult result)) { throw new KeyNotFoundException("Unable to find request with provided identifier"); } - return _currentRequests[lookup]; + return result; } /// @@ -215,13 +193,11 @@ namespace Emby.Server.Implementations.QuickConnect var auth = _authContext.GetAuthorizationInfo(request); - if (!_currentRequests.ContainsKey(lookup)) + if (!_currentRequests.TryGetValue(lookup, out QuickConnectResult result)) { throw new KeyNotFoundException("Unable to find request"); } - QuickConnectResult result = _currentRequests[lookup]; - if (result.Authenticated) { throw new InvalidOperationException("Request is already authorized"); @@ -268,6 +244,7 @@ namespace Emby.Server.Implementations.QuickConnect return tokens.Count(); } + /// /// Dispose. /// @@ -288,6 +265,7 @@ namespace Emby.Server.Implementations.QuickConnect _rng?.Dispose(); } } + private string GenerateSecureRandom(int length = 32) { var bytes = new byte[length]; @@ -296,12 +274,14 @@ namespace Emby.Server.Implementations.QuickConnect return string.Join(string.Empty, bytes.Select(x => x.ToString("x2", CultureInfo.InvariantCulture))); } - private void ExpireRequests() + /// + /// Expire quick connect requests that are over the time limit. If is true, all requests are unconditionally expired. + /// + /// If true, all requests will be expired. + private void ExpireRequests(bool expireAll = false) { - bool expireAll = false; - // Check if quick connect should be deactivated - if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active) + if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active && !expireAll) { _logger.LogDebug("Quick connect time expired, deactivating"); SetEnabled(QuickConnectState.Available); @@ -313,7 +293,7 @@ namespace Emby.Server.Implementations.QuickConnect var delete = new List(); var values = _currentRequests.Values.ToList(); - for (int i = 0; i < _currentRequests.Count; i++) + for (int i = 0; i < values.Count; i++) { var added = values[i].DateAdded ?? DateTime.UnixEpoch; if (DateTime.Now > added.AddMinutes(RequestExpiry) || expireAll) @@ -324,9 +304,20 @@ namespace Emby.Server.Implementations.QuickConnect foreach (var lookup in delete) { - _logger.LogDebug("Removing expired request {0}", lookup); - _currentRequests.Remove(lookup); + _logger.LogDebug("Removing expired request {lookup}", lookup); + + if (!_currentRequests.TryRemove(lookup, out _)) + { + _logger.LogWarning("Request {lookup} already expired", lookup); + } } } + + private void ReloadConfiguration() + { + var config = _config.GetQuickConnectConfiguration(); + + State = config.State; + } } }