From 36f3e933a23d802d154c16fd304a82c3fe3f453d Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Wed, 15 Apr 2020 14:28:42 -0500 Subject: [PATCH 01/20] Add quick connect --- CONTRIBUTORS.md | 1 + .../ApplicationHost.cs | 3 + .../QuickConnect/QuickConnectManager.cs | 262 ++++++++++++++++++ .../Session/SessionManager.cs | 18 ++ .../QuickConnect/QuickConnectService.cs | 145 ++++++++++ MediaBrowser.Api/UserService.cs | 34 +++ .../QuickConnect/IQuickConnect.cs | 91 ++++++ .../Session/ISessionManager.cs | 2 + .../QuickConnect/QuickConnectResult.cs | 50 ++++ .../QuickConnect/QuickConnectResultDto.cs | 53 ++++ .../QuickConnect/QuickConnectState.cs | 23 ++ 11 files changed, 682 insertions(+) create mode 100644 Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs create mode 100644 MediaBrowser.Api/QuickConnect/QuickConnectService.cs create mode 100644 MediaBrowser.Controller/QuickConnect/IQuickConnect.cs create mode 100644 MediaBrowser.Model/QuickConnect/QuickConnectResult.cs create mode 100644 MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs create mode 100644 MediaBrowser.Model/QuickConnect/QuickConnectState.cs diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index ce956176e8..edd33a2fb3 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -15,6 +15,7 @@ - [bugfixin](https://github.com/bugfixin) - [chaosinnovator](https://github.com/chaosinnovator) - [ckcr4lyf](https://github.com/ckcr4lyf) + - [ConfusedPolarBear](https://github.com/ConfusedPolarBear) - [crankdoofus](https://github.com/crankdoofus) - [crobibero](https://github.com/crobibero) - [cromefire](https://github.com/cromefire) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 81a80ddb26..de044a4aa2 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -74,6 +74,7 @@ using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Playlists; using MediaBrowser.Controller.Plugins; using MediaBrowser.Controller.Providers; +using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Controller.Resolvers; using MediaBrowser.Controller.Security; using MediaBrowser.Controller.Session; @@ -857,6 +858,8 @@ namespace Emby.Server.Implementations serviceCollection.AddSingleton(typeof(IAttachmentExtractor), typeof(MediaBrowser.MediaEncoding.Attachments.AttachmentExtractor)); + serviceCollection.AddSingleton(typeof(IQuickConnect), typeof(QuickConnect.QuickConnectManager)); + _displayPreferencesRepository.Initialize(); var userDataRepo = new SqliteUserDataRepository(LoggerFactory.CreateLogger(), ApplicationPaths); diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs new file mode 100644 index 0000000000..30418097ca --- /dev/null +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -0,0 +1,262 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Security.Cryptography; +using MediaBrowser.Controller; +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 Microsoft.Extensions.Logging; + +namespace Emby.Server.Implementations.QuickConnect +{ + /// + /// Quick connect implementation. + /// + public class QuickConnectManager : IQuickConnect + { + private readonly RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider(); + private Dictionary _currentRequests = new Dictionary(); + + private ILogger _logger; + private IUserManager _userManager; + private ILocalizationManager _localizationManager; + private IJsonSerializer _jsonSerializer; + private IAuthenticationRepository _authenticationRepository; + private IAuthorizationContext _authContext; + private IServerApplicationHost _appHost; + + /// + /// Initializes a new instance of the class. + /// Should only be called at server startup when a singleton is created. + /// + /// Logger. + /// User manager. + /// Localization. + /// JSON serializer. + /// Application host. + /// Authentication context. + /// Authentication repository. + public QuickConnectManager( + ILoggerFactory loggerFactory, + IUserManager userManager, + ILocalizationManager localization, + IJsonSerializer jsonSerializer, + IServerApplicationHost appHost, + IAuthorizationContext authContext, + IAuthenticationRepository authenticationRepository) + { + _logger = loggerFactory.CreateLogger(nameof(QuickConnectManager)); + _userManager = userManager; + _localizationManager = localization; + _jsonSerializer = jsonSerializer; + _appHost = appHost; + _authContext = authContext; + _authenticationRepository = authenticationRepository; + } + + /// + public int CodeLength { get; set; } = 6; + + /// + public string TokenNamePrefix { get; set; } = "QuickConnect-"; + + /// + public QuickConnectState State { get; private set; } = QuickConnectState.Unavailable; + + /// + public int RequestExpiry { get; set; } = 30; + + /// + public void AssertActive() + { + if (State != QuickConnectState.Active) + { + throw new InvalidOperationException("Quick connect is not active on this server"); + } + } + + /// + public void SetEnabled(QuickConnectState newState) + { + _logger.LogDebug("Changed quick connect state from {0} to {1}", State, newState); + + State = newState; + } + + /// + public QuickConnectResult TryConnect(string friendlyName) + { + if (State != QuickConnectState.Active) + { + _logger.LogDebug("Refusing quick connect initiation request, current state is {0}", State); + + return new QuickConnectResult() + { + Error = "Quick connect is not active on this server" + }; + } + + _logger.LogDebug("Got new quick connect request from {friendlyName}", friendlyName); + + var lookup = GenerateSecureRandom(); + var result = new QuickConnectResult() + { + Lookup = lookup, + Secret = GenerateSecureRandom(), + FriendlyName = friendlyName, + DateAdded = DateTime.Now, + Code = GenerateCode() + }; + + _currentRequests[lookup] = result; + return result; + } + + /// + public QuickConnectResult CheckRequestStatus(string secret) + { + AssertActive(); + ExpireRequests(); + + string lookup = _currentRequests.Where(x => x.Value.Secret == secret).Select(x => x.Value.Lookup).DefaultIfEmpty(string.Empty).First(); + + _logger.LogDebug("Transformed private identifier {0} into public lookup {1}", secret, lookup); + + if (!_currentRequests.ContainsKey(lookup)) + { + throw new KeyNotFoundException("Unable to find request with provided identifier"); + } + + return _currentRequests[lookup]; + } + + /// + public List GetCurrentRequests() + { + return GetCurrentRequestsInternal().Select(x => (QuickConnectResultDto)x).ToList(); + } + + /// + public List GetCurrentRequestsInternal() + { + AssertActive(); + ExpireRequests(); + return _currentRequests.Values.ToList(); + } + + /// + public string GenerateCode() + { + // TODO: output may be biased + + int min = (int)Math.Pow(10, CodeLength - 1); + int max = (int)Math.Pow(10, CodeLength); + + uint scale = uint.MaxValue; + while (scale == uint.MaxValue) + { + byte[] raw = new byte[4]; + _rng.GetBytes(raw); + scale = BitConverter.ToUInt32(raw, 0); + } + + int code = (int)(min + (max - min) * (scale / (double)uint.MaxValue)); + return code.ToString(CultureInfo.InvariantCulture); + } + + /// + public bool AuthorizeRequest(IRequest request, string lookup) + { + AssertActive(); + + var auth = _authContext.GetAuthorizationInfo(request); + + ExpireRequests(); + + if (!_currentRequests.ContainsKey(lookup)) + { + throw new KeyNotFoundException("Unable to find request"); + } + + QuickConnectResult result = _currentRequests[lookup]; + + if (result.Authenticated) + { + throw new InvalidOperationException("Request is already authorized"); + } + + result.Authentication = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); + + // Advance the time on the request so it expires sooner as the client will pick up the changes in a few seconds + result.DateAdded = result.DateAdded.Subtract(new TimeSpan(0, RequestExpiry - 1, 0)); + + _authenticationRepository.Create(new AuthenticationInfo + { + AppName = TokenNamePrefix + result.FriendlyName, + AccessToken = result.Authentication, + DateCreated = DateTime.UtcNow, + DeviceId = _appHost.SystemId, + DeviceName = _appHost.FriendlyName, + AppVersion = _appHost.ApplicationVersionString, + UserId = auth.UserId + }); + + return true; + } + + /// + public int DeleteAllDevices(Guid user) + { + var raw = _authenticationRepository.Get(new AuthenticationInfoQuery() + { + DeviceId = _appHost.SystemId, + UserId = user + }); + + var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenNamePrefix, StringComparison.CurrentCulture)); + + foreach (var token in tokens) + { + _authenticationRepository.Delete(token); + _logger.LogDebug("Deleted token {0}", token.AccessToken); + } + + return tokens.Count(); + } + + private string GenerateSecureRandom(int length = 32) + { + var bytes = new byte[length]; + _rng.GetBytes(bytes); + + return string.Join(string.Empty, bytes.Select(x => x.ToString("x2", CultureInfo.InvariantCulture))); + } + + private void ExpireRequests() + { + var delete = new List(); + var values = _currentRequests.Values.ToList(); + + for (int i = 0; i < _currentRequests.Count; i++) + { + if (DateTime.Now > values[i].DateAdded.AddMinutes(RequestExpiry)) + { + delete.Add(values[i].Lookup); + } + } + + foreach (var lookup in delete) + { + _logger.LogDebug("Removing expired request {0}", lookup); + _currentRequests.Remove(lookup); + } + } + } +} diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index de768333d8..2c8b2f29d7 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1386,6 +1386,24 @@ namespace Emby.Server.Implementations.Session return AuthenticateNewSessionInternal(request, false); } + public Task AuthenticateQuickConnect(AuthenticationRequest request, string token) + { + var result = _authRepo.Get(new AuthenticationInfoQuery() + { + AccessToken = token, + DeviceId = _appHost.SystemId, + Limit = 1 + }); + + if(result.TotalRecordCount < 1) + { + throw new SecurityException("Unknown quick connect token"); + } + + request.UserId = result.Items[0].UserId; + return AuthenticateNewSessionInternal(request, false); + } + private async Task AuthenticateNewSessionInternal(AuthenticationRequest request, bool enforcePassword) { CheckDisposed(); diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs new file mode 100644 index 0000000000..889a788391 --- /dev/null +++ b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs @@ -0,0 +1,145 @@ +using System; +using System.Collections.Generic; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Net; +using MediaBrowser.Controller.QuickConnect; +using MediaBrowser.Model.QuickConnect; +using MediaBrowser.Model.Services; +using Microsoft.Extensions.Logging; + +namespace MediaBrowser.Api.QuickConnect +{ + [Route("/QuickConnect/Initiate", "GET", Summary = "Requests a new quick connect code")] + public class Initiate : IReturn + { + [ApiMember(Name = "FriendlyName", Description = "Device friendly name", IsRequired = false, DataType = "string", ParameterType = "query", Verb = "GET")] + public string FriendlyName { get; set; } + } + + [Route("/QuickConnect/Connect", "GET", Summary = "Attempts to retrieve authentication information")] + public class Connect : IReturn + { + [ApiMember(Name = "Secret", Description = "Quick connect secret", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] + public string Secret { get; set; } + } + + [Route("/QuickConnect/List", "GET", Summary = "Lists all quick connect requests")] + [Authenticated] + public class QuickConnectList : IReturn> + { + } + + [Route("/QuickConnect/Authorize", "POST", Summary = "Authorizes a pending quick connect request")] + [Authenticated] + public class Authorize : IReturn + { + [ApiMember(Name = "Lookup", Description = "Quick connect public lookup", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] + public string Lookup { get; set; } + } + + [Route("/QuickConnect/Deauthorize", "POST", Summary = "Deletes all quick connect authorization tokens for the current user")] + [Authenticated] + public class Deauthorize : IReturn + { + [ApiMember(Name = "UserId", Description = "User Id", IsRequired = false, DataType = "string", ParameterType = "query", Verb = "GET")] + public Guid UserId { get; set; } + } + + [Route("/QuickConnect/Status", "GET", Summary = "Gets the current quick connect state")] + public class QuickConnectStatus : IReturn + { + + } + + [Route("/QuickConnect/Available", "POST", Summary = "Enables or disables quick connect")] + [Authenticated(Roles = "Admin")] + public class Available : IReturn + { + [ApiMember(Name = "Status", Description = "New quick connect status", IsRequired = false, DataType = "QuickConnectState", ParameterType = "query", Verb = "GET")] + public QuickConnectState Status { get; set; } + } + + [Route("/QuickConnect/Activate", "POST", Summary = "Temporarily activates quick connect for the time period defined in the server configuration")] + [Authenticated] + public class Activate : IReturn + { + } + + public class QuickConnectService : BaseApiService + { + private IQuickConnect _quickConnect; + private IUserManager _userManager; + private IAuthorizationContext _authContext; + + public QuickConnectService( + ILogger logger, + IServerConfigurationManager serverConfigurationManager, + IHttpResultFactory httpResultFactory, + IUserManager userManager, + IAuthorizationContext authContext, + IQuickConnect quickConnect) + : base(logger, serverConfigurationManager, httpResultFactory) + { + _userManager = userManager; + _quickConnect = quickConnect; + _authContext = authContext; + } + + public object Get(Initiate request) + { + return _quickConnect.TryConnect(request.FriendlyName); + } + + public object Get(Connect request) + { + return _quickConnect.CheckRequestStatus(request.Secret); + } + + public object Get(QuickConnectList request) + { + return _quickConnect.GetCurrentRequests(); + } + + public object Get(QuickConnectStatus request) + { + return _quickConnect.State; + } + + public object Post(Deauthorize request) + { + AssertCanUpdateUser(_authContext, _userManager, request.UserId, true); + + return _quickConnect.DeleteAllDevices(request.UserId); + } + + public object Post(Authorize request) + { + bool result = _quickConnect.AuthorizeRequest(Request, request.Lookup); + + Logger.LogInformation("Result of authorizing quick connect {0}: {1}", request.Lookup[..10], result); + + return result; + } + + public object Post(Activate request) + { + if (_quickConnect.State == QuickConnectState.Available) + { + _quickConnect.SetEnabled(QuickConnectState.Active); + + string name = _authContext.GetAuthorizationInfo(Request).User.Name; + Logger.LogInformation("{name} enabled quick connect", name); + } + + return _quickConnect.State; + } + + public object Post(Available request) + { + _quickConnect.SetEnabled(request.Status); + + return _quickConnect.State; + } + } +} diff --git a/MediaBrowser.Api/UserService.cs b/MediaBrowser.Api/UserService.cs index 4015143497..ebcacd2a3d 100644 --- a/MediaBrowser.Api/UserService.cs +++ b/MediaBrowser.Api/UserService.cs @@ -117,6 +117,17 @@ namespace MediaBrowser.Api public string Pw { get; set; } } + [Route("/Users/AuthenticateWithQuickConnect", "POST", Summary = "Authenticates a user")] + public class AuthenticateUserQuickConnect : IReturn + { + /// + /// Gets or sets the token. + /// + /// The token + [ApiMember(Name = "Token", IsRequired = true, DataType = "string", ParameterType = "body", Verb = "POST")] + public string Token { get; set; } + } + /// /// Class UpdateUserPassword /// @@ -430,6 +441,29 @@ namespace MediaBrowser.Api } } + public async Task Post(AuthenticateUserQuickConnect request) + { + var auth = _authContext.GetAuthorizationInfo(Request); + + try + { + var result = await _sessionMananger.AuthenticateQuickConnect(new AuthenticationRequest + { + App = auth.Client, + AppVersion = auth.Version, + DeviceId = auth.DeviceId, + DeviceName = auth.Device + }, request.Token).ConfigureAwait(false); + + return ToOptimizedResult(result); + } + catch (SecurityException e) + { + // rethrow adding IP address to message + throw new SecurityException($"[{Request.RemoteIp}] {e.Message}"); + } + } + /// /// Posts the specified request. /// diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs new file mode 100644 index 0000000000..e4a790ffe7 --- /dev/null +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -0,0 +1,91 @@ +using System; +using System.Collections.Generic; +using MediaBrowser.Model.QuickConnect; +using MediaBrowser.Model.Services; + +namespace MediaBrowser.Controller.QuickConnect +{ + /// + /// Quick connect standard interface. + /// + public interface IQuickConnect + { + /// + /// Gets or sets the length of user facing codes. + /// + public int CodeLength { get; set; } + + /// + /// Gets or sets the string to prefix internal access tokens with. + /// + public string TokenNamePrefix { get; set; } + + /// + /// Gets the current state of quick connect. + /// + public QuickConnectState State { get; } + + /// + /// Gets or sets the time (in minutes) before a pending request will expire. + /// + public int RequestExpiry { get; set; } + + /// + /// Assert that quick connect is currently active and throws an exception if it is not. + /// + void AssertActive(); + + /// + /// Changes the status of quick connect. + /// + /// New state to change to + void SetEnabled(QuickConnectState newState); + + /// + /// Initiates a new quick connect request. + /// + /// Friendly device name to display in the request UI. + /// A quick connect result with tokens to proceed or a descriptive error message otherwise. + QuickConnectResult TryConnect(string friendlyName); + + /// + /// Checks the status of an individual request. + /// + /// Unique secret identifier of the request. + /// Quick connect result. + QuickConnectResult CheckRequestStatus(string secret); + + /// + /// Returns all current quick connect requests as DTOs. Does not include sensitive information. + /// + /// List of all quick connect results. + List GetCurrentRequests(); + + /// + /// Returns all current quick connect requests (including sensitive information). + /// + /// List of all quick connect results. + List GetCurrentRequestsInternal(); + + /// + /// Authorizes a quick connect request to connect as the calling user. + /// + /// HTTP request object. + /// Public request lookup value. + /// A boolean indicating if the authorization completed successfully. + bool AuthorizeRequest(IRequest request, string lookup); + + /// + /// Deletes all quick connect access tokens for the provided user. + /// + /// Guid of the user to delete tokens for. + /// A count of the deleted tokens. + int DeleteAllDevices(Guid user); + + /// + /// Generates a short code to display to the user to uniquely identify this request. + /// + /// A short, unique alphanumeric string. + string GenerateCode(); + } +} diff --git a/MediaBrowser.Controller/Session/ISessionManager.cs b/MediaBrowser.Controller/Session/ISessionManager.cs index 771027103b..74ffd5a181 100644 --- a/MediaBrowser.Controller/Session/ISessionManager.cs +++ b/MediaBrowser.Controller/Session/ISessionManager.cs @@ -246,6 +246,8 @@ namespace MediaBrowser.Controller.Session /// Task{SessionInfo}. Task AuthenticateNewSession(AuthenticationRequest request); + public Task AuthenticateQuickConnect(AuthenticationRequest request, string token); + /// /// Creates the new session. /// diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs new file mode 100644 index 0000000000..bc3fd00466 --- /dev/null +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs @@ -0,0 +1,50 @@ +using System; + +namespace MediaBrowser.Model.QuickConnect +{ + /// + /// Stores the result of an incoming quick connect request. + /// + public class QuickConnectResult + { + /// + /// Gets a value indicating whether this request is authorized. + /// + public bool Authenticated => !string.IsNullOrEmpty(Authentication); + + /// + /// Gets or sets the secret value used to uniquely identify this request. Can be used to retrieve authentication information. + /// + public string Secret { get; set; } + + /// + /// Gets or sets the public value used to uniquely identify this request. Can only be used to authorize the request. + /// + public string Lookup { get; set; } + + /// + /// Gets or sets the user facing code used so the user can quickly differentiate this request from others. + /// + public string Code { get; set; } + + /// + /// Gets or sets the device friendly name. + /// + public string FriendlyName { get; set; } + + /// + /// Gets or sets the private access token. + /// + public string Authentication { get; set; } + + /// + /// Gets or sets an error message. + /// + public string Error { get; set; } + + /// + /// Gets or sets the DateTime that this request was created. + /// + public DateTime DateAdded { get; set; } + } +} diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs new file mode 100644 index 0000000000..671b7cc943 --- /dev/null +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs @@ -0,0 +1,53 @@ +using System; + +namespace MediaBrowser.Model.QuickConnect +{ + /// + /// Stores the non-sensitive results of an incoming quick connect request. + /// + public class QuickConnectResultDto + { + /// + /// Gets a value indicating whether this request is authorized. + /// + public bool Authenticated { get; private set; } + + /// + /// Gets the user facing code used so the user can quickly differentiate this request from others. + /// + public string Code { get; private set; } + + /// + /// Gets the public value used to uniquely identify this request. Can only be used to authorize the request. + /// + public string Lookup { get; private set; } + + /// + /// Gets the device friendly name. + /// + public string FriendlyName { get; private set; } + + /// + /// Gets the DateTime that this request was created. + /// + public DateTime DateAdded { get; private set; } + + /// + /// Cast an internal quick connect result to a DTO by removing all sensitive properties. + /// + /// QuickConnectResult object to cast + public static implicit operator QuickConnectResultDto(QuickConnectResult result) + { + QuickConnectResultDto resultDto = new QuickConnectResultDto + { + Authenticated = result.Authenticated, + Code = result.Code, + FriendlyName = result.FriendlyName, + DateAdded = result.DateAdded, + Lookup = result.Lookup + }; + + return resultDto; + } + } +} diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectState.cs b/MediaBrowser.Model/QuickConnect/QuickConnectState.cs new file mode 100644 index 0000000000..9f250519b1 --- /dev/null +++ b/MediaBrowser.Model/QuickConnect/QuickConnectState.cs @@ -0,0 +1,23 @@ +namespace MediaBrowser.Model.QuickConnect +{ + /// + /// Quick connect state. + /// + public enum QuickConnectState + { + /// + /// This feature has not been opted into and is unavailable until the server administrator chooses to opt-in. + /// + Unavailable, + + /// + /// The feature is enabled for use on the server but is not currently accepting connection requests. + /// + Available, + + /// + /// The feature is actively accepting connection requests. + /// + Active + } +} From 387a07c6dd4792ea1e77d333e178f9b4e9c56678 Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Sun, 19 Apr 2020 01:33:09 -0500 Subject: [PATCH 02/20] Add persistent setting configuration and temporary activation --- .../QuickConnect/ConfigurationExtension.cs | 30 ++++++++ .../QuickConnect/QuickConnectConfiguration.cs | 13 ++++ .../QuickConnect/QuickConnectManager.cs | 72 ++++++++++++++++--- .../QuickConnect/QuickConnectService.cs | 40 +++++++++-- .../QuickConnect/IQuickConnect.cs | 8 ++- 5 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs create mode 100644 Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs new file mode 100644 index 0000000000..0e35ba80ab --- /dev/null +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -0,0 +1,30 @@ +#pragma warning disable CS1591 + +using System.Collections.Generic; +using MediaBrowser.Common.Configuration; + +namespace Emby.Server.Implementations.QuickConnect +{ + public static class ConfigurationExtension + { + public static QuickConnectConfiguration GetQuickConnectConfiguration(this IConfigurationManager manager) + { + return manager.GetConfiguration("quickconnect"); + } + } + + public class QuickConnectConfigurationFactory : IConfigurationFactory + { + public IEnumerable GetConfigurations() + { + return new ConfigurationStore[] + { + new ConfigurationStore + { + Key = "quickconnect", + ConfigurationType = typeof(QuickConnectConfiguration) + } + }; + } + } +} diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs new file mode 100644 index 0000000000..befc463796 --- /dev/null +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs @@ -0,0 +1,13 @@ +using MediaBrowser.Model.QuickConnect; + +namespace Emby.Server.Implementations.QuickConnect +{ + public class QuickConnectConfiguration + { + public QuickConnectConfiguration() + { + } + + public QuickConnectState State { get; set; } + } +} diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 30418097ca..671ddc2b96 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -3,7 +3,9 @@ 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; @@ -12,6 +14,7 @@ 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 @@ -24,6 +27,7 @@ namespace Emby.Server.Implementations.QuickConnect private readonly RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider(); private Dictionary _currentRequests = new Dictionary(); + private IServerConfigurationManager _config; private ILogger _logger; private IUserManager _userManager; private ILocalizationManager _localizationManager; @@ -31,11 +35,13 @@ namespace Emby.Server.Implementations.QuickConnect private IAuthenticationRepository _authenticationRepository; private IAuthorizationContext _authContext; private IServerApplicationHost _appHost; + private ITaskManager _taskManager; /// /// Initializes a new instance of the class. /// Should only be called at server startup when a singleton is created. /// + /// Configuration. /// Logger. /// User manager. /// Localization. @@ -43,15 +49,19 @@ namespace Emby.Server.Implementations.QuickConnect /// Application host. /// Authentication context. /// Authentication repository. + /// Task scheduler. public QuickConnectManager( + IServerConfigurationManager config, ILoggerFactory loggerFactory, IUserManager userManager, ILocalizationManager localization, IJsonSerializer jsonSerializer, IServerApplicationHost appHost, IAuthorizationContext authContext, - IAuthenticationRepository authenticationRepository) + IAuthenticationRepository authenticationRepository, + ITaskManager taskManager) { + _config = config; _logger = loggerFactory.CreateLogger(nameof(QuickConnectManager)); _userManager = userManager; _localizationManager = localization; @@ -59,6 +69,16 @@ namespace Emby.Server.Implementations.QuickConnect _appHost = appHost; _authContext = authContext; _authenticationRepository = authenticationRepository; + _taskManager = taskManager; + + ReloadConfiguration(); + } + + private void ReloadConfiguration() + { + var config = _config.GetQuickConnectConfiguration(); + + State = config.State; } /// @@ -73,6 +93,10 @@ namespace Emby.Server.Implementations.QuickConnect /// public int RequestExpiry { get; set; } = 30; + private bool TemporaryActivation { get; set; } = false; + + private DateTime DateActivated { get; set; } + /// public void AssertActive() { @@ -82,17 +106,37 @@ namespace Emby.Server.Implementations.QuickConnect } } + /// + public QuickConnectResult Activate() + { + // This should not call SetEnabled since that would persist the "temporary" activation to the configuration file + State = QuickConnectState.Active; + DateActivated = DateTime.Now; + TemporaryActivation = true; + + return new QuickConnectResult(); + } + /// public void SetEnabled(QuickConnectState newState) { _logger.LogDebug("Changed quick connect state from {0} to {1}", State, newState); State = newState; + + _config.SaveConfiguration("quickconnect", new QuickConnectConfiguration() + { + State = State + }); + + _logger.LogDebug("Configuration saved"); } /// public QuickConnectResult TryConnect(string friendlyName) { + ExpireRequests(true); + if (State != QuickConnectState.Active) { _logger.LogDebug("Refusing quick connect initiation request, current state is {0}", State); @@ -122,13 +166,11 @@ namespace Emby.Server.Implementations.QuickConnect /// public QuickConnectResult CheckRequestStatus(string secret) { - AssertActive(); ExpireRequests(); + AssertActive(); string lookup = _currentRequests.Where(x => x.Value.Secret == secret).Select(x => x.Value.Lookup).DefaultIfEmpty(string.Empty).First(); - _logger.LogDebug("Transformed private identifier {0} into public lookup {1}", secret, lookup); - if (!_currentRequests.ContainsKey(lookup)) { throw new KeyNotFoundException("Unable to find request with provided identifier"); @@ -146,8 +188,8 @@ namespace Emby.Server.Implementations.QuickConnect /// public List GetCurrentRequestsInternal() { - AssertActive(); ExpireRequests(); + AssertActive(); return _currentRequests.Values.ToList(); } @@ -174,12 +216,11 @@ namespace Emby.Server.Implementations.QuickConnect /// public bool AuthorizeRequest(IRequest request, string lookup) { + ExpireRequests(); AssertActive(); var auth = _authContext.GetAuthorizationInfo(request); - ExpireRequests(); - if (!_currentRequests.ContainsKey(lookup)) { throw new KeyNotFoundException("Unable to find request"); @@ -208,6 +249,8 @@ namespace Emby.Server.Implementations.QuickConnect UserId = auth.UserId }); + _logger.LogInformation("Allowing device {0} to login as user {1} with quick connect code {2}", result.FriendlyName, auth.User.Name, result.Code); + return true; } @@ -239,8 +282,21 @@ namespace Emby.Server.Implementations.QuickConnect return string.Join(string.Empty, bytes.Select(x => x.ToString("x2", CultureInfo.InvariantCulture))); } - private void ExpireRequests() + private void ExpireRequests(bool onlyCheckTime = false) { + // check if quick connect should be deactivated + if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active) + { + _logger.LogDebug("Quick connect time expired, deactivating"); + SetEnabled(QuickConnectState.Available); + } + + if (onlyCheckTime) + { + return; + } + + // expire stale connection requests var delete = new List(); var values = _currentRequests.Values.ToList(); diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs index 889a788391..60d6ac4147 100644 --- a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs +++ b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs @@ -98,6 +98,11 @@ namespace MediaBrowser.Api.QuickConnect public object Get(QuickConnectList request) { + if(_quickConnect.State != QuickConnectState.Active) + { + return Array.Empty(); + } + return _quickConnect.GetCurrentRequests(); } @@ -124,15 +129,40 @@ namespace MediaBrowser.Api.QuickConnect public object Post(Activate request) { - if (_quickConnect.State == QuickConnectState.Available) + string name = _authContext.GetAuthorizationInfo(Request).User.Name; + + if(_quickConnect.State == QuickConnectState.Unavailable) + { + return new QuickConnectResult() + { + Error = "Quick connect is not enabled on this server" + }; + } + + else if(_quickConnect.State == QuickConnectState.Available) { - _quickConnect.SetEnabled(QuickConnectState.Active); + var result = _quickConnect.Activate(); - string name = _authContext.GetAuthorizationInfo(Request).User.Name; - Logger.LogInformation("{name} enabled quick connect", name); + if (string.IsNullOrEmpty(result.Error)) + { + Logger.LogInformation("{name} temporarily activated quick connect", name); + } + + return result; } - return _quickConnect.State; + else if(_quickConnect.State == QuickConnectState.Active) + { + return new QuickConnectResult() + { + Error = "" + }; + } + + return new QuickConnectResult() + { + Error = "Unknown current state" + }; } public object Post(Available request) diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index e4a790ffe7..d44765e112 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -35,10 +35,16 @@ namespace MediaBrowser.Controller.QuickConnect /// void AssertActive(); + /// + /// Temporarily activates quick connect for a short amount of time. + /// + /// A quick connect result object indicating success. + QuickConnectResult Activate(); + /// /// Changes the status of quick connect. /// - /// New state to change to + /// New state to change to. void SetEnabled(QuickConnectState newState); /// From 33390153fdfec33e4149c12dd3a876248f4e08cc Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Thu, 23 Apr 2020 23:44:15 -0500 Subject: [PATCH 03/20] Minor fix --- .../QuickConnect/QuickConnectManager.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 671ddc2b96..e24dc3a675 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -135,7 +135,7 @@ namespace Emby.Server.Implementations.QuickConnect /// public QuickConnectResult TryConnect(string friendlyName) { - ExpireRequests(true); + ExpireRequests(); if (State != QuickConnectState.Active) { @@ -282,18 +282,17 @@ namespace Emby.Server.Implementations.QuickConnect return string.Join(string.Empty, bytes.Select(x => x.ToString("x2", CultureInfo.InvariantCulture))); } - private void ExpireRequests(bool onlyCheckTime = false) + private void ExpireRequests() { + bool expireAll = false; + // check if quick connect should be deactivated if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active) { _logger.LogDebug("Quick connect time expired, deactivating"); SetEnabled(QuickConnectState.Available); - } - - if (onlyCheckTime) - { - return; + expireAll = true; + TemporaryActivation = false; } // expire stale connection requests @@ -302,7 +301,7 @@ namespace Emby.Server.Implementations.QuickConnect for (int i = 0; i < _currentRequests.Count; i++) { - if (DateTime.Now > values[i].DateAdded.AddMinutes(RequestExpiry)) + if (DateTime.Now > values[i].DateAdded.AddMinutes(RequestExpiry) || expireAll) { delete.Add(values[i].Lookup); } From 70e50dfa90575cc5e906be1509d3ed363eb1ada4 Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Fri, 24 Apr 2020 18:51:19 -0500 Subject: [PATCH 04/20] Apply suggestions from code review --- .../QuickConnect/ConfigurationExtension.cs | 2 -- .../QuickConnect/QuickConnectManager.cs | 8 +++----- MediaBrowser.Model/QuickConnect/QuickConnectState.cs | 6 +++--- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs index 0e35ba80ab..458bb7614d 100644 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -1,5 +1,3 @@ -#pragma warning disable CS1591 - using System.Collections.Generic; using MediaBrowser.Common.Configuration; diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index e24dc3a675..b8b51adb6e 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -42,7 +42,7 @@ namespace Emby.Server.Implementations.QuickConnect /// Should only be called at server startup when a singleton is created. /// /// Configuration. - /// Logger. + /// Logger. /// User manager. /// Localization. /// JSON serializer. @@ -52,7 +52,7 @@ namespace Emby.Server.Implementations.QuickConnect /// Task scheduler. public QuickConnectManager( IServerConfigurationManager config, - ILoggerFactory loggerFactory, + ILogger logger, IUserManager userManager, ILocalizationManager localization, IJsonSerializer jsonSerializer, @@ -62,7 +62,7 @@ namespace Emby.Server.Implementations.QuickConnect ITaskManager taskManager) { _config = config; - _logger = loggerFactory.CreateLogger(nameof(QuickConnectManager)); + _logger = logger; _userManager = userManager; _localizationManager = localization; _jsonSerializer = jsonSerializer; @@ -196,8 +196,6 @@ namespace Emby.Server.Implementations.QuickConnect /// public string GenerateCode() { - // TODO: output may be biased - int min = (int)Math.Pow(10, CodeLength - 1); int max = (int)Math.Pow(10, CodeLength); diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectState.cs b/MediaBrowser.Model/QuickConnect/QuickConnectState.cs index 9f250519b1..f1074f25f2 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectState.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectState.cs @@ -8,16 +8,16 @@ namespace MediaBrowser.Model.QuickConnect /// /// This feature has not been opted into and is unavailable until the server administrator chooses to opt-in. /// - Unavailable, + Unavailable = 0, /// /// The feature is enabled for use on the server but is not currently accepting connection requests. /// - Available, + Available = 1, /// /// The feature is actively accepting connection requests. /// - Active + Active = 2 } } From 0d6a63bf84d7ad971128c6ba6cad77e76e023536 Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Mon, 8 Jun 2020 15:48:18 -0500 Subject: [PATCH 05/20] Make all properties nullable --- .../QuickConnect/ConfigurationExtension.cs | 2 ++ .../QuickConnect/QuickConnectConfiguration.cs | 2 ++ .../QuickConnect/QuickConnectManager.cs | 10 ++++++---- .../QuickConnect/QuickConnectResult.cs | 14 +++++++------- .../QuickConnect/QuickConnectResultDto.cs | 8 ++++---- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs index 458bb7614d..0e35ba80ab 100644 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -1,3 +1,5 @@ +#pragma warning disable CS1591 + using System.Collections.Generic; using MediaBrowser.Common.Configuration; diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs index befc463796..11e558bae1 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs @@ -1,3 +1,5 @@ +#pragma warning disable CS1591 + using MediaBrowser.Model.QuickConnect; namespace Emby.Server.Implementations.QuickConnect diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index b8b51adb6e..929e021a3d 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -234,7 +234,8 @@ namespace Emby.Server.Implementations.QuickConnect result.Authentication = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); // Advance the time on the request so it expires sooner as the client will pick up the changes in a few seconds - result.DateAdded = result.DateAdded.Subtract(new TimeSpan(0, RequestExpiry - 1, 0)); + var added = result.DateAdded ?? DateTime.Now.Subtract(new TimeSpan(0, RequestExpiry, 0)); + result.DateAdded = added.Subtract(new TimeSpan(0, RequestExpiry - 1, 0)); _authenticationRepository.Create(new AuthenticationInfo { @@ -284,7 +285,7 @@ namespace Emby.Server.Implementations.QuickConnect { bool expireAll = false; - // check if quick connect should be deactivated + // Check if quick connect should be deactivated if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active) { _logger.LogDebug("Quick connect time expired, deactivating"); @@ -293,13 +294,14 @@ namespace Emby.Server.Implementations.QuickConnect TemporaryActivation = false; } - // expire stale connection requests + // Expire stale connection requests var delete = new List(); var values = _currentRequests.Values.ToList(); for (int i = 0; i < _currentRequests.Count; i++) { - if (DateTime.Now > values[i].DateAdded.AddMinutes(RequestExpiry) || expireAll) + var added = values[i].DateAdded ?? DateTime.UnixEpoch; + if (DateTime.Now > added.AddMinutes(RequestExpiry) || expireAll) { delete.Add(values[i].Lookup); } diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs index bc3fd00466..32d7f6aba6 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs @@ -15,36 +15,36 @@ namespace MediaBrowser.Model.QuickConnect /// /// Gets or sets the secret value used to uniquely identify this request. Can be used to retrieve authentication information. /// - public string Secret { get; set; } + public string? Secret { get; set; } /// /// Gets or sets the public value used to uniquely identify this request. Can only be used to authorize the request. /// - public string Lookup { get; set; } + public string? Lookup { get; set; } /// /// Gets or sets the user facing code used so the user can quickly differentiate this request from others. /// - public string Code { get; set; } + public string? Code { get; set; } /// /// Gets or sets the device friendly name. /// - public string FriendlyName { get; set; } + public string? FriendlyName { get; set; } /// /// Gets or sets the private access token. /// - public string Authentication { get; set; } + public string? Authentication { get; set; } /// /// Gets or sets an error message. /// - public string Error { get; set; } + public string? Error { get; set; } /// /// Gets or sets the DateTime that this request was created. /// - public DateTime DateAdded { get; set; } + public DateTime? DateAdded { get; set; } } } diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs index 671b7cc943..19acc7cd88 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs @@ -15,22 +15,22 @@ namespace MediaBrowser.Model.QuickConnect /// /// Gets the user facing code used so the user can quickly differentiate this request from others. /// - public string Code { get; private set; } + public string? Code { get; private set; } /// /// Gets the public value used to uniquely identify this request. Can only be used to authorize the request. /// - public string Lookup { get; private set; } + public string? Lookup { get; private set; } /// /// Gets the device friendly name. /// - public string FriendlyName { get; private set; } + public string? FriendlyName { get; private set; } /// /// Gets the DateTime that this request was created. /// - public DateTime DateAdded { get; private set; } + public DateTime? DateAdded { get; private set; } /// /// Cast an internal quick connect result to a DTO by removing all sensitive properties. From 001c78573eb132dadad1fcd8162d2966fbf0d402 Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Mon, 8 Jun 2020 17:14:20 -0500 Subject: [PATCH 06/20] Add XML documentation --- .../QuickConnect/ConfigurationExtension.cs | 17 +++++++++++++++-- .../QuickConnect/QuickConnectConfiguration.cs | 11 +++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs index 0e35ba80ab..349010039b 100644 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -1,20 +1,33 @@ -#pragma warning disable CS1591 - using System.Collections.Generic; using MediaBrowser.Common.Configuration; namespace Emby.Server.Implementations.QuickConnect { + /// + /// Configuration extension to support persistent quick connect configuration + /// public static class ConfigurationExtension { + /// + /// Return the current quick connect configuration + /// + /// Configuration manager + /// public static QuickConnectConfiguration GetQuickConnectConfiguration(this IConfigurationManager manager) { return manager.GetConfiguration("quickconnect"); } } + /// + /// Configuration factory for quick connect + /// public class QuickConnectConfigurationFactory : IConfigurationFactory { + /// + /// Returns the current quick connect configuration + /// + /// public IEnumerable GetConfigurations() { return new ConfigurationStore[] diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs index 11e558bae1..e1881f2783 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs @@ -1,15 +1,22 @@ -#pragma warning disable CS1591 - using MediaBrowser.Model.QuickConnect; namespace Emby.Server.Implementations.QuickConnect { + /// + /// Persistent quick connect configuration + /// public class QuickConnectConfiguration { + /// + /// Quick connect configuration object + /// public QuickConnectConfiguration() { } + /// + /// Persistent quick connect availability state + /// public QuickConnectState State { get; set; } } } From 7d9b5524031fe6b5c23b4282cb1f9ec850b114fe Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Tue, 9 Jun 2020 13:28:40 -0500 Subject: [PATCH 07/20] Apply suggestions from code review Co-authored-by: Cody Robibero --- .../ApplicationHost.cs | 1 - .../QuickConnect/ConfigurationExtension.cs | 17 +++++---- .../QuickConnect/QuickConnectConfiguration.cs | 11 ++---- .../QuickConnect/QuickConnectManager.cs | 35 +++++++++++++------ .../Session/SessionManager.cs | 2 +- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 0a349bb331..51e63ecfc2 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -646,7 +646,6 @@ namespace Emby.Server.Implementations serviceCollection.AddSingleton(); } - /// /// Create services registered with the service container that need to be initialized at application startup. /// diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs index 349010039b..596ded8caa 100644 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs @@ -1,18 +1,17 @@ -using System.Collections.Generic; using MediaBrowser.Common.Configuration; namespace Emby.Server.Implementations.QuickConnect { /// - /// Configuration extension to support persistent quick connect configuration + /// Configuration extension to support persistent quick connect configuration. /// public static class ConfigurationExtension { /// - /// Return the current quick connect configuration + /// Return the current quick connect configuration. /// - /// Configuration manager - /// + /// Configuration manager. + /// Current quick connect configuration. public static QuickConnectConfiguration GetQuickConnectConfiguration(this IConfigurationManager manager) { return manager.GetConfiguration("quickconnect"); @@ -20,17 +19,17 @@ namespace Emby.Server.Implementations.QuickConnect } /// - /// Configuration factory for quick connect + /// Configuration factory for quick connect. /// public class QuickConnectConfigurationFactory : IConfigurationFactory { /// - /// Returns the current quick connect configuration + /// Returns the current quick connect configuration. /// - /// + /// Current quick connect configuration. public IEnumerable GetConfigurations() { - return new ConfigurationStore[] + return new[] { new ConfigurationStore { diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs index e1881f2783..2302ddbc3f 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs @@ -3,19 +3,12 @@ using MediaBrowser.Model.QuickConnect; namespace Emby.Server.Implementations.QuickConnect { /// - /// Persistent quick connect configuration + /// Persistent quick connect configuration. /// public class QuickConnectConfiguration { /// - /// Quick connect configuration object - /// - public QuickConnectConfiguration() - { - } - - /// - /// Persistent quick connect availability state + /// Gets or sets persistent quick connect availability state. /// public QuickConnectState State { get; set; } } diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 929e021a3d..62b775fa60 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -27,15 +27,11 @@ namespace Emby.Server.Implementations.QuickConnect private readonly RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider(); private Dictionary _currentRequests = new Dictionary(); - private IServerConfigurationManager _config; - private ILogger _logger; - private IUserManager _userManager; - private ILocalizationManager _localizationManager; - private IJsonSerializer _jsonSerializer; - private IAuthenticationRepository _authenticationRepository; - private IAuthorizationContext _authContext; - private IServerApplicationHost _appHost; - private ITaskManager _taskManager; + private readonly IServerConfigurationManager _config; + private readonly ILogger _logger; + private readonly IAuthenticationRepository _authenticationRepository; + private readonly IAuthorizationContext _authContext; + private readonly IServerApplicationHost _appHost; /// /// Initializes a new instance of the class. @@ -207,7 +203,7 @@ namespace Emby.Server.Implementations.QuickConnect scale = BitConverter.ToUInt32(raw, 0); } - int code = (int)(min + (max - min) * (scale / (double)uint.MaxValue)); + int code = (int)(min + ((max - min) * (scale / (double)uint.MaxValue))); return code.ToString(CultureInfo.InvariantCulture); } @@ -272,7 +268,26 @@ namespace Emby.Server.Implementations.QuickConnect return tokens.Count(); } + /// + /// Dispose. + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + /// + /// Dispose. + /// + /// Dispose unmanaged resources. + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _rng?.Dispose(); + } + } private string GenerateSecureRandom(int length = 32) { var bytes = new byte[length]; diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index d7054e0b11..188b366aae 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1413,7 +1413,7 @@ namespace Emby.Server.Implementations.Session Limit = 1 }); - if(result.TotalRecordCount < 1) + if (result.TotalRecordCount < 1) { throw new SecurityException("Unknown quick connect token"); } 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 08/20] 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; + } } } From 82887ec7105e38070d91f5d29ce73637fcfe3b1d Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Tue, 9 Jun 2020 18:40:35 -0500 Subject: [PATCH 09/20] Add IDisposable --- Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index adcc6f2cfc..7a584c7cd0 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -18,7 +18,7 @@ namespace Emby.Server.Implementations.QuickConnect /// /// Quick connect implementation. /// - public class QuickConnectManager : IQuickConnect + public class QuickConnectManager : IQuickConnect, IDisposable { private readonly RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider(); private readonly ConcurrentDictionary _currentRequests = new ConcurrentDictionary(); From 4be476ec5312387f87134915d0fd132b2ad5fa3f Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Thu, 18 Jun 2020 01:29:47 -0500 Subject: [PATCH 10/20] Move all settings into the main server configuration Decreased the timeout from 30 minutes to 5. Public lookup values have been replaced with the short code. --- .../QuickConnect/ConfigurationExtension.cs | 20 ------ .../QuickConnect/QuickConnectConfiguration.cs | 15 ----- .../QuickConnectConfigurationFactory.cs | 27 -------- .../QuickConnect/QuickConnectManager.cs | 66 +++++++++---------- .../QuickConnect/IQuickConnect.cs | 8 +-- .../Configuration/ServerConfiguration.cs | 6 ++ .../QuickConnect/QuickConnectResult.cs | 5 -- .../QuickConnect/QuickConnectResultDto.cs | 14 +--- 8 files changed, 41 insertions(+), 120 deletions(-) delete mode 100644 Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs delete mode 100644 Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs delete mode 100644 Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs diff --git a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs b/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs deleted file mode 100644 index 2a19fc36c1..0000000000 --- a/Emby.Server.Implementations/QuickConnect/ConfigurationExtension.cs +++ /dev/null @@ -1,20 +0,0 @@ -using MediaBrowser.Common.Configuration; - -namespace Emby.Server.Implementations.QuickConnect -{ - /// - /// Configuration extension to support persistent quick connect configuration. - /// - public static class ConfigurationExtension - { - /// - /// Return the current quick connect configuration. - /// - /// Configuration manager. - /// Current quick connect configuration. - public static QuickConnectConfiguration GetQuickConnectConfiguration(this IConfigurationManager manager) - { - return manager.GetConfiguration("quickconnect"); - } - } -} diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs deleted file mode 100644 index 2302ddbc3f..0000000000 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectConfiguration.cs +++ /dev/null @@ -1,15 +0,0 @@ -using MediaBrowser.Model.QuickConnect; - -namespace Emby.Server.Implementations.QuickConnect -{ - /// - /// Persistent quick connect configuration. - /// - public class QuickConnectConfiguration - { - /// - /// Gets or sets persistent quick connect availability state. - /// - public QuickConnectState State { get; set; } - } -} diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs deleted file mode 100644 index d7bc84c5e2..0000000000 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectConfigurationFactory.cs +++ /dev/null @@ -1,27 +0,0 @@ -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 7a584c7cd0..8d704f32b7 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -11,7 +11,9 @@ using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Controller.Security; using MediaBrowser.Model.QuickConnect; using MediaBrowser.Model.Services; +using MediaBrowser.Common; using Microsoft.Extensions.Logging; +using MediaBrowser.Common.Extensions; namespace Emby.Server.Implementations.QuickConnect { @@ -64,9 +66,7 @@ namespace Emby.Server.Implementations.QuickConnect public QuickConnectState State { get; private set; } = QuickConnectState.Unavailable; /// - public int RequestExpiry { get; set; } = 30; - - private bool TemporaryActivation { get; set; } = false; + public int Timeout { get; set; } = 5; private DateTime DateActivated { get; set; } @@ -82,10 +82,9 @@ namespace Emby.Server.Implementations.QuickConnect /// public QuickConnectResult Activate() { - // This should not call SetEnabled since that would persist the "temporary" activation to the configuration file - State = QuickConnectState.Active; + SetEnabled(QuickConnectState.Active); + DateActivated = DateTime.Now; - TemporaryActivation = true; return new QuickConnectResult(); } @@ -96,12 +95,10 @@ 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() - { - State = State - }); + State = newState; + _config.Configuration.QuickConnectAvailable = newState == QuickConnectState.Available || newState == QuickConnectState.Active; + _config.SaveConfiguration(); _logger.LogDebug("Configuration saved"); } @@ -123,17 +120,16 @@ namespace Emby.Server.Implementations.QuickConnect _logger.LogDebug("Got new quick connect request from {friendlyName}", friendlyName); - var lookup = GenerateSecureRandom(); + var code = GenerateCode(); var result = new QuickConnectResult() { - Lookup = lookup, Secret = GenerateSecureRandom(), FriendlyName = friendlyName, DateAdded = DateTime.Now, - Code = GenerateCode() + Code = code }; - _currentRequests[lookup] = result; + _currentRequests[code] = result; return result; } @@ -143,17 +139,16 @@ namespace Emby.Server.Implementations.QuickConnect ExpireRequests(); AssertActive(); - string lookup = _currentRequests.Where(x => x.Value.Secret == secret).Select(x => x.Value.Lookup).DefaultIfEmpty(string.Empty).First(); + string code = _currentRequests.Where(x => x.Value.Secret == secret).Select(x => x.Value.Code).DefaultIfEmpty(string.Empty).First(); - if (!_currentRequests.TryGetValue(lookup, out QuickConnectResult result)) + if (!_currentRequests.TryGetValue(code, out QuickConnectResult result)) { - throw new KeyNotFoundException("Unable to find request with provided identifier"); + throw new ResourceNotFoundException("Unable to find request with provided secret"); } return result; } - /// public List GetCurrentRequests() { return GetCurrentRequestsInternal().Select(x => (QuickConnectResultDto)x).ToList(); @@ -186,16 +181,16 @@ namespace Emby.Server.Implementations.QuickConnect } /// - public bool AuthorizeRequest(IRequest request, string lookup) + public bool AuthorizeRequest(IRequest request, string code) { ExpireRequests(); AssertActive(); var auth = _authContext.GetAuthorizationInfo(request); - if (!_currentRequests.TryGetValue(lookup, out QuickConnectResult result)) + if (!_currentRequests.TryGetValue(code, out QuickConnectResult result)) { - throw new KeyNotFoundException("Unable to find request"); + throw new ResourceNotFoundException("Unable to find request"); } if (result.Authenticated) @@ -205,9 +200,9 @@ namespace Emby.Server.Implementations.QuickConnect result.Authentication = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); - // Advance the time on the request so it expires sooner as the client will pick up the changes in a few seconds - var added = result.DateAdded ?? DateTime.Now.Subtract(new TimeSpan(0, RequestExpiry, 0)); - result.DateAdded = added.Subtract(new TimeSpan(0, RequestExpiry - 1, 0)); + // Change the time on the request so it expires one minute into the future. It can't expire immediately as otherwise some clients wouldn't ever see that they have been authenticated. + var added = result.DateAdded ?? DateTime.Now.Subtract(new TimeSpan(0, Timeout, 0)); + result.DateAdded = added.Subtract(new TimeSpan(0, Timeout - 1, 0)); _authenticationRepository.Create(new AuthenticationInfo { @@ -271,7 +266,7 @@ namespace Emby.Server.Implementations.QuickConnect var bytes = new byte[length]; _rng.GetBytes(bytes); - return string.Join(string.Empty, bytes.Select(x => x.ToString("x2", CultureInfo.InvariantCulture))); + return Hex.Encode(bytes); } /// @@ -281,12 +276,11 @@ namespace Emby.Server.Implementations.QuickConnect private void ExpireRequests(bool expireAll = false) { // Check if quick connect should be deactivated - if (TemporaryActivation && DateTime.Now > DateActivated.AddMinutes(10) && State == QuickConnectState.Active && !expireAll) + if (State == QuickConnectState.Active && DateTime.Now > DateActivated.AddMinutes(Timeout) && !expireAll) { _logger.LogDebug("Quick connect time expired, deactivating"); SetEnabled(QuickConnectState.Available); expireAll = true; - TemporaryActivation = false; } // Expire stale connection requests @@ -296,28 +290,28 @@ namespace Emby.Server.Implementations.QuickConnect for (int i = 0; i < values.Count; i++) { var added = values[i].DateAdded ?? DateTime.UnixEpoch; - if (DateTime.Now > added.AddMinutes(RequestExpiry) || expireAll) + if (DateTime.Now > added.AddMinutes(Timeout) || expireAll) { - delete.Add(values[i].Lookup); + delete.Add(values[i].Code); } } - foreach (var lookup in delete) + foreach (var code in delete) { - _logger.LogDebug("Removing expired request {lookup}", lookup); + _logger.LogDebug("Removing expired request {code}", code); - if (!_currentRequests.TryRemove(lookup, out _)) + if (!_currentRequests.TryRemove(code, out _)) { - _logger.LogWarning("Request {lookup} already expired", lookup); + _logger.LogWarning("Request {code} already expired", code); } } } private void ReloadConfiguration() { - var config = _config.GetQuickConnectConfiguration(); + var available = _config.Configuration.QuickConnectAvailable; - State = config.State; + State = available ? QuickConnectState.Available : QuickConnectState.Unavailable; } } } diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index d44765e112..d31d0e5097 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -26,9 +26,9 @@ namespace MediaBrowser.Controller.QuickConnect public QuickConnectState State { get; } /// - /// Gets or sets the time (in minutes) before a pending request will expire. + /// Gets or sets the time (in minutes) before quick connect will automatically deactivate. /// - public int RequestExpiry { get; set; } + public int Timeout { get; set; } /// /// Assert that quick connect is currently active and throws an exception if it is not. @@ -77,9 +77,9 @@ namespace MediaBrowser.Controller.QuickConnect /// Authorizes a quick connect request to connect as the calling user. /// /// HTTP request object. - /// Public request lookup value. + /// Identifying code for the request.. /// A boolean indicating if the authorization completed successfully. - bool AuthorizeRequest(IRequest request, string lookup); + bool AuthorizeRequest(IRequest request, string code); /// /// Deletes all quick connect access tokens for the provided user. diff --git a/MediaBrowser.Model/Configuration/ServerConfiguration.cs b/MediaBrowser.Model/Configuration/ServerConfiguration.cs index afbe02dd36..76b2906069 100644 --- a/MediaBrowser.Model/Configuration/ServerConfiguration.cs +++ b/MediaBrowser.Model/Configuration/ServerConfiguration.cs @@ -76,6 +76,11 @@ namespace MediaBrowser.Model.Configuration /// true if this instance is port authorized; otherwise, false. public bool IsPortAuthorized { get; set; } + /// + /// Gets or sets if quick connect is available for use on this server. + /// + public bool QuickConnectAvailable { get; set; } + public bool AutoRunWebApp { get; set; } public bool EnableRemoteAccess { get; set; } @@ -281,6 +286,7 @@ namespace MediaBrowser.Model.Configuration AutoRunWebApp = true; EnableRemoteAccess = true; + QuickConnectAvailable = false; EnableUPnP = false; MinResumePct = 5; diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs index 32d7f6aba6..a10d60d57e 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs @@ -17,11 +17,6 @@ namespace MediaBrowser.Model.QuickConnect /// public string? Secret { get; set; } - /// - /// Gets or sets the public value used to uniquely identify this request. Can only be used to authorize the request. - /// - public string? Lookup { get; set; } - /// /// Gets or sets the user facing code used so the user can quickly differentiate this request from others. /// diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs index 19acc7cd88..26084caf1e 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs @@ -17,25 +17,15 @@ namespace MediaBrowser.Model.QuickConnect /// public string? Code { get; private set; } - /// - /// Gets the public value used to uniquely identify this request. Can only be used to authorize the request. - /// - public string? Lookup { get; private set; } - /// /// Gets the device friendly name. /// public string? FriendlyName { get; private set; } - /// - /// Gets the DateTime that this request was created. - /// - public DateTime? DateAdded { get; private set; } - /// /// Cast an internal quick connect result to a DTO by removing all sensitive properties. /// - /// QuickConnectResult object to cast + /// QuickConnectResult object to cast. public static implicit operator QuickConnectResultDto(QuickConnectResult result) { QuickConnectResultDto resultDto = new QuickConnectResultDto @@ -43,8 +33,6 @@ namespace MediaBrowser.Model.QuickConnect Authenticated = result.Authenticated, Code = result.Code, FriendlyName = result.FriendlyName, - DateAdded = result.DateAdded, - Lookup = result.Lookup }; return resultDto; From 329980c727cf03587ff5f4011a3af3ef2fa5e4f1 Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Thu, 18 Jun 2020 01:58:58 -0500 Subject: [PATCH 11/20] API cleanup --- .../QuickConnect/QuickConnectManager.cs | 35 ++-------- .../QuickConnect/QuickConnectService.cs | 67 ++++--------------- .../QuickConnect/IQuickConnect.cs | 23 +++---- .../QuickConnect/QuickConnectResultDto.cs | 41 ------------ 4 files changed, 27 insertions(+), 139 deletions(-) delete mode 100644 MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 8d704f32b7..263556e9d7 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -75,18 +75,15 @@ namespace Emby.Server.Implementations.QuickConnect { if (State != QuickConnectState.Active) { - throw new InvalidOperationException("Quick connect is not active on this server"); + throw new ArgumentException("Quick connect is not active on this server"); } } /// - public QuickConnectResult Activate() + public void Activate() { - SetEnabled(QuickConnectState.Active); - DateActivated = DateTime.Now; - - return new QuickConnectResult(); + SetEnabled(QuickConnectState.Active); } /// @@ -149,19 +146,6 @@ namespace Emby.Server.Implementations.QuickConnect return result; } - public List GetCurrentRequests() - { - return GetCurrentRequestsInternal().Select(x => (QuickConnectResultDto)x).ToList(); - } - - /// - public List GetCurrentRequestsInternal() - { - ExpireRequests(); - AssertActive(); - return _currentRequests.Values.ToList(); - } - /// public string GenerateCode() { @@ -215,7 +199,7 @@ namespace Emby.Server.Implementations.QuickConnect UserId = auth.UserId }); - _logger.LogInformation("Allowing device {0} to login as user {1} with quick connect code {2}", result.FriendlyName, auth.User.Name, result.Code); + _logger.LogInformation("Allowing device {0} to login as user {1} with quick connect code {2}", result.FriendlyName, auth.User.Username, result.Code); return true; } @@ -269,11 +253,8 @@ namespace Emby.Server.Implementations.QuickConnect return Hex.Encode(bytes); } - /// - /// 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) + /// + public void ExpireRequests(bool expireAll = false) { // Check if quick connect should be deactivated if (State == QuickConnectState.Active && DateTime.Now > DateActivated.AddMinutes(Timeout) && !expireAll) @@ -309,9 +290,7 @@ namespace Emby.Server.Implementations.QuickConnect private void ReloadConfiguration() { - var available = _config.Configuration.QuickConnectAvailable; - - State = available ? QuickConnectState.Available : QuickConnectState.Unavailable; + State = _config.Configuration.QuickConnectAvailable ? QuickConnectState.Available : QuickConnectState.Unavailable; } } } diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs index 60d6ac4147..9047a1e957 100644 --- a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs +++ b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Net; @@ -24,18 +23,12 @@ namespace MediaBrowser.Api.QuickConnect public string Secret { get; set; } } - [Route("/QuickConnect/List", "GET", Summary = "Lists all quick connect requests")] - [Authenticated] - public class QuickConnectList : IReturn> - { - } - [Route("/QuickConnect/Authorize", "POST", Summary = "Authorizes a pending quick connect request")] [Authenticated] - public class Authorize : IReturn + public class Authorize : IReturn { - [ApiMember(Name = "Lookup", Description = "Quick connect public lookup", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] - public string Lookup { get; set; } + [ApiMember(Name = "Code", Description = "Quick connect identifying code", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] + public string Code { get; set; } } [Route("/QuickConnect/Deauthorize", "POST", Summary = "Deletes all quick connect authorization tokens for the current user")] @@ -62,8 +55,9 @@ namespace MediaBrowser.Api.QuickConnect [Route("/QuickConnect/Activate", "POST", Summary = "Temporarily activates quick connect for the time period defined in the server configuration")] [Authenticated] - public class Activate : IReturn + public class Activate : IReturn { + } public class QuickConnectService : BaseApiService @@ -96,18 +90,9 @@ namespace MediaBrowser.Api.QuickConnect return _quickConnect.CheckRequestStatus(request.Secret); } - public object Get(QuickConnectList request) - { - if(_quickConnect.State != QuickConnectState.Active) - { - return Array.Empty(); - } - - return _quickConnect.GetCurrentRequests(); - } - public object Get(QuickConnectStatus request) { + _quickConnect.ExpireRequests(); return _quickConnect.State; } @@ -120,55 +105,27 @@ namespace MediaBrowser.Api.QuickConnect public object Post(Authorize request) { - bool result = _quickConnect.AuthorizeRequest(Request, request.Lookup); - - Logger.LogInformation("Result of authorizing quick connect {0}: {1}", request.Lookup[..10], result); - - return result; + return _quickConnect.AuthorizeRequest(Request, request.Code); } public object Post(Activate request) { - string name = _authContext.GetAuthorizationInfo(Request).User.Name; - if(_quickConnect.State == QuickConnectState.Unavailable) { - return new QuickConnectResult() - { - Error = "Quick connect is not enabled on this server" - }; + return false; } - else if(_quickConnect.State == QuickConnectState.Available) - { - var result = _quickConnect.Activate(); - - if (string.IsNullOrEmpty(result.Error)) - { - Logger.LogInformation("{name} temporarily activated quick connect", name); - } + string name = _authContext.GetAuthorizationInfo(Request).User.Username; - return result; - } + Logger.LogInformation("{name} temporarily activated quick connect", name); + _quickConnect.Activate(); - else if(_quickConnect.State == QuickConnectState.Active) - { - return new QuickConnectResult() - { - Error = "" - }; - } - - return new QuickConnectResult() - { - Error = "Unknown current state" - }; + return true; } public object Post(Available request) { _quickConnect.SetEnabled(request.Status); - return _quickConnect.State; } } diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index d31d0e5097..10ec9e6cb5 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -38,8 +38,7 @@ namespace MediaBrowser.Controller.QuickConnect /// /// Temporarily activates quick connect for a short amount of time. /// - /// A quick connect result object indicating success. - QuickConnectResult Activate(); + void Activate(); /// /// Changes the status of quick connect. @@ -61,26 +60,20 @@ namespace MediaBrowser.Controller.QuickConnect /// Quick connect result. QuickConnectResult CheckRequestStatus(string secret); - /// - /// Returns all current quick connect requests as DTOs. Does not include sensitive information. - /// - /// List of all quick connect results. - List GetCurrentRequests(); - - /// - /// Returns all current quick connect requests (including sensitive information). - /// - /// List of all quick connect results. - List GetCurrentRequestsInternal(); - /// /// Authorizes a quick connect request to connect as the calling user. /// /// HTTP request object. - /// Identifying code for the request.. + /// Identifying code for the request. /// A boolean indicating if the authorization completed successfully. bool AuthorizeRequest(IRequest request, string code); + /// + /// 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. + public void ExpireRequests(bool expireAll = false); + /// /// Deletes all quick connect access tokens for the provided user. /// diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs deleted file mode 100644 index 26084caf1e..0000000000 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResultDto.cs +++ /dev/null @@ -1,41 +0,0 @@ -using System; - -namespace MediaBrowser.Model.QuickConnect -{ - /// - /// Stores the non-sensitive results of an incoming quick connect request. - /// - public class QuickConnectResultDto - { - /// - /// Gets a value indicating whether this request is authorized. - /// - public bool Authenticated { get; private set; } - - /// - /// Gets the user facing code used so the user can quickly differentiate this request from others. - /// - public string? Code { get; private set; } - - /// - /// Gets the device friendly name. - /// - public string? FriendlyName { get; private set; } - - /// - /// Cast an internal quick connect result to a DTO by removing all sensitive properties. - /// - /// QuickConnectResult object to cast. - public static implicit operator QuickConnectResultDto(QuickConnectResult result) - { - QuickConnectResultDto resultDto = new QuickConnectResultDto - { - Authenticated = result.Authenticated, - Code = result.Code, - FriendlyName = result.FriendlyName, - }; - - return resultDto; - } - } -} From 0945659cb572be510c7bdd30315f23b0e3c9a8f3 Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Sun, 26 Jul 2020 18:14:35 -0500 Subject: [PATCH 12/20] Apply suggestions from code review --- .../QuickConnect/QuickConnectManager.cs | 25 ++++++++----------- .../QuickConnect/QuickConnectService.cs | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 263556e9d7..a69ea2267b 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -149,15 +149,16 @@ namespace Emby.Server.Implementations.QuickConnect /// public string GenerateCode() { + Span raw = stackalloc byte[4]; + int min = (int)Math.Pow(10, CodeLength - 1); int max = (int)Math.Pow(10, CodeLength); uint scale = uint.MaxValue; while (scale == uint.MaxValue) { - byte[] raw = new byte[4]; _rng.GetBytes(raw); - scale = BitConverter.ToUInt32(raw, 0); + scale = BitConverter.ToUInt32(raw); } int code = (int)(min + ((max - min) * (scale / (double)uint.MaxValue))); @@ -247,7 +248,7 @@ namespace Emby.Server.Implementations.QuickConnect private string GenerateSecureRandom(int length = 32) { - var bytes = new byte[length]; + Span bytes = stackalloc byte[length]; _rng.GetBytes(bytes); return Hex.Encode(bytes); @@ -265,7 +266,7 @@ namespace Emby.Server.Implementations.QuickConnect } // Expire stale connection requests - var delete = new List(); + var code = string.Empty; var values = _currentRequests.Values.ToList(); for (int i = 0; i < values.Count; i++) @@ -273,17 +274,13 @@ namespace Emby.Server.Implementations.QuickConnect var added = values[i].DateAdded ?? DateTime.UnixEpoch; if (DateTime.Now > added.AddMinutes(Timeout) || expireAll) { - delete.Add(values[i].Code); - } - } + code = values[i].Code; + _logger.LogDebug("Removing expired request {code}", code); - foreach (var code in delete) - { - _logger.LogDebug("Removing expired request {code}", code); - - if (!_currentRequests.TryRemove(code, out _)) - { - _logger.LogWarning("Request {code} already expired", code); + if (!_currentRequests.TryRemove(code, out _)) + { + _logger.LogWarning("Request {code} already expired", code); + } } } } diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs index 9047a1e957..6298f66e59 100644 --- a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs +++ b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs @@ -110,7 +110,7 @@ namespace MediaBrowser.Api.QuickConnect public object Post(Activate request) { - if(_quickConnect.State == QuickConnectState.Unavailable) + if (_quickConnect.State == QuickConnectState.Unavailable) { return false; } From 3c91aa0c3d4af3c3d11b4c732ea14c7e641ba662 Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Sun, 26 Jul 2020 23:13:14 -0500 Subject: [PATCH 13/20] Code cleanup --- .../QuickConnect/QuickConnectManager.cs | 25 +++++++++---------- .../QuickConnect/QuickConnectService.cs | 2 +- .../QuickConnect/IQuickConnect.cs | 6 ++--- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index a69ea2267b..23e94afd72 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -14,6 +14,7 @@ using MediaBrowser.Model.Services; using MediaBrowser.Common; using Microsoft.Extensions.Logging; using MediaBrowser.Common.Extensions; +using MediaBrowser.Controller.Authentication; namespace Emby.Server.Implementations.QuickConnect { @@ -83,13 +84,13 @@ namespace Emby.Server.Implementations.QuickConnect public void Activate() { DateActivated = DateTime.Now; - SetEnabled(QuickConnectState.Active); + SetState(QuickConnectState.Active); } /// - public void SetEnabled(QuickConnectState newState) + public void SetState(QuickConnectState newState) { - _logger.LogDebug("Changed quick connect state from {0} to {1}", State, newState); + _logger.LogDebug("Changed quick connect state from {State} to {newState}", State, newState); ExpireRequests(true); @@ -107,12 +108,8 @@ namespace Emby.Server.Implementations.QuickConnect if (State != QuickConnectState.Active) { - _logger.LogDebug("Refusing quick connect initiation request, current state is {0}", State); - - return new QuickConnectResult() - { - Error = "Quick connect is not active on this server" - }; + _logger.LogDebug("Refusing quick connect initiation request, current state is {State}", State); + throw new AuthenticationException("Quick connect is not active on this server"); } _logger.LogDebug("Got new quick connect request from {friendlyName}", friendlyName); @@ -200,7 +197,7 @@ namespace Emby.Server.Implementations.QuickConnect UserId = auth.UserId }); - _logger.LogInformation("Allowing device {0} to login as user {1} with quick connect code {2}", result.FriendlyName, auth.User.Username, result.Code); + _logger.LogInformation("Allowing device {FriendlyName} to login as user {Username} with quick connect code {Code}", result.FriendlyName, auth.User.Username, result.Code); return true; } @@ -216,13 +213,15 @@ namespace Emby.Server.Implementations.QuickConnect var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenNamePrefix, StringComparison.CurrentCulture)); + var removed = 0; foreach (var token in tokens) { _authenticationRepository.Delete(token); - _logger.LogDebug("Deleted token {0}", token.AccessToken); + _logger.LogDebug("Deleted token {AccessToken}", token.AccessToken); + removed++; } - return tokens.Count(); + return removed; } /// @@ -261,7 +260,7 @@ namespace Emby.Server.Implementations.QuickConnect if (State == QuickConnectState.Active && DateTime.Now > DateActivated.AddMinutes(Timeout) && !expireAll) { _logger.LogDebug("Quick connect time expired, deactivating"); - SetEnabled(QuickConnectState.Available); + SetState(QuickConnectState.Available); expireAll = true; } diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs index 6298f66e59..7093be9908 100644 --- a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs +++ b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs @@ -125,7 +125,7 @@ namespace MediaBrowser.Api.QuickConnect public object Post(Available request) { - _quickConnect.SetEnabled(request.Status); + _quickConnect.SetState(request.Status); return _quickConnect.State; } } diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index 10ec9e6cb5..5518e0385d 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -41,16 +41,16 @@ namespace MediaBrowser.Controller.QuickConnect void Activate(); /// - /// Changes the status of quick connect. + /// Changes the state of quick connect. /// /// New state to change to. - void SetEnabled(QuickConnectState newState); + void SetState(QuickConnectState newState); /// /// Initiates a new quick connect request. /// /// Friendly device name to display in the request UI. - /// A quick connect result with tokens to proceed or a descriptive error message otherwise. + /// A quick connect result with tokens to proceed or throws an exception if not active. QuickConnectResult TryConnect(string friendlyName); /// From 31d3b1b83aa356221e8af2f316b58584579207fe Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Mon, 27 Jul 2020 15:40:14 -0500 Subject: [PATCH 14/20] Cleanup interfaces --- MediaBrowser.Controller/QuickConnect/IQuickConnect.cs | 10 +++++----- MediaBrowser.Controller/Session/ISessionManager.cs | 7 ++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index 5518e0385d..993637c8aa 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -13,22 +13,22 @@ namespace MediaBrowser.Controller.QuickConnect /// /// Gets or sets the length of user facing codes. /// - public int CodeLength { get; set; } + int CodeLength { get; set; } /// /// Gets or sets the string to prefix internal access tokens with. /// - public string TokenNamePrefix { get; set; } + string TokenNamePrefix { get; set; } /// /// Gets the current state of quick connect. /// - public QuickConnectState State { get; } + QuickConnectState State { get; } /// /// Gets or sets the time (in minutes) before quick connect will automatically deactivate. /// - public int Timeout { get; set; } + int Timeout { get; set; } /// /// Assert that quick connect is currently active and throws an exception if it is not. @@ -72,7 +72,7 @@ namespace MediaBrowser.Controller.QuickConnect /// 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. - public void ExpireRequests(bool expireAll = false); + void ExpireRequests(bool expireAll = false); /// /// Deletes all quick connect access tokens for the provided user. diff --git a/MediaBrowser.Controller/Session/ISessionManager.cs b/MediaBrowser.Controller/Session/ISessionManager.cs index 23230e41e5..ffa19fb690 100644 --- a/MediaBrowser.Controller/Session/ISessionManager.cs +++ b/MediaBrowser.Controller/Session/ISessionManager.cs @@ -264,7 +264,12 @@ namespace MediaBrowser.Controller.Session /// Task{SessionInfo}. Task AuthenticateNewSession(AuthenticationRequest request); - public Task AuthenticateQuickConnect(AuthenticationRequest request, string token); + /// + /// Authenticates a new session with quick connect. + /// + /// The request. + /// Task{SessionInfo}. + Task AuthenticateQuickConnect(AuthenticationRequest request, string token); /// /// Creates the new session. From 035d29fb357006c29ffb40e0a53c1e999237cdd1 Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Thu, 13 Aug 2020 15:35:04 -0500 Subject: [PATCH 15/20] Migrate to new API standard --- .../QuickConnect/QuickConnectManager.cs | 5 +- .../Controllers/QuickConnectController.cs | 160 ++++++++++++++++++ Jellyfin.Api/Controllers/UserController.cs | 41 +++++ .../Models/UserDtos/QuickConnectDto.cs | 13 ++ .../QuickConnect/QuickConnectService.cs | 132 --------------- .../QuickConnect/IQuickConnect.cs | 5 +- .../Session/ISessionManager.cs | 1 + 7 files changed, 219 insertions(+), 138 deletions(-) create mode 100644 Jellyfin.Api/Controllers/QuickConnectController.cs create mode 100644 Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs delete mode 100644 MediaBrowser.Api/QuickConnect/QuickConnectService.cs diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 23e94afd72..949c3b5058 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Security.Cryptography; @@ -10,7 +9,7 @@ using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Controller.Security; using MediaBrowser.Model.QuickConnect; -using MediaBrowser.Model.Services; +using Microsoft.AspNetCore.Http; using MediaBrowser.Common; using Microsoft.Extensions.Logging; using MediaBrowser.Common.Extensions; @@ -163,7 +162,7 @@ namespace Emby.Server.Implementations.QuickConnect } /// - public bool AuthorizeRequest(IRequest request, string code) + public bool AuthorizeRequest(HttpRequest request, string code) { ExpireRequests(); AssertActive(); diff --git a/Jellyfin.Api/Controllers/QuickConnectController.cs b/Jellyfin.Api/Controllers/QuickConnectController.cs new file mode 100644 index 0000000000..d45ea058d2 --- /dev/null +++ b/Jellyfin.Api/Controllers/QuickConnectController.cs @@ -0,0 +1,160 @@ +using System.ComponentModel.DataAnnotations; +using Jellyfin.Api.Constants; +using MediaBrowser.Common.Extensions; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Net; +using MediaBrowser.Controller.QuickConnect; +using MediaBrowser.Model.QuickConnect; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; + +namespace Jellyfin.Api.Controllers +{ + /// + /// Quick connect controller. + /// + public class QuickConnectController : BaseJellyfinApiController + { + private readonly IQuickConnect _quickConnect; + private readonly IUserManager _userManager; + private readonly IAuthorizationContext _authContext; + + /// + /// Initializes a new instance of the class. + /// + /// Instance of the interface. + /// Instance of the interface. + /// Instance of the interface. + public QuickConnectController( + IQuickConnect quickConnect, + IUserManager userManager, + IAuthorizationContext authContext) + { + _quickConnect = quickConnect; + _userManager = userManager; + _authContext = authContext; + } + + /// + /// Gets the current quick connect state. + /// + /// Quick connect state returned. + /// The current . + [HttpGet("Status")] + [ProducesResponseType(StatusCodes.Status200OK)] + public ActionResult GetStatus() + { + _quickConnect.ExpireRequests(); + return Ok(_quickConnect.State); + } + + /// + /// Initiate a new quick connect request. + /// + /// Device friendly name. + /// Quick connect request successfully created. + /// Quick connect is not active on this server. + /// A with a secret and code for future use or an error message. + [HttpGet("Initiate")] + [ProducesResponseType(StatusCodes.Status200OK)] + public ActionResult Initiate([FromQuery] string? friendlyName) + { + return Ok(_quickConnect.TryConnect(friendlyName)); + } + + /// + /// Attempts to retrieve authentication information. + /// + /// Secret previously returned from the Initiate endpoint. + /// Quick connect result returned. + /// Unknown quick connect secret. + /// An updated . + [HttpGet("Connect")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + public ActionResult Connect([FromQuery] string? secret) + { + try + { + var result = _quickConnect.CheckRequestStatus(secret); + return Ok(result); + } + catch (ResourceNotFoundException) + { + return NotFound("Unknown secret"); + } + } + + /// + /// Temporarily activates quick connect for five minutes. + /// + /// Quick connect has been temporarily activated. + /// Quick connect is unavailable on this server. + /// An on success. + [HttpPost("Activate")] + [Authorize(Policy = Policies.DefaultAuthorization)] + [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + public ActionResult Activate() + { + if (_quickConnect.State == QuickConnectState.Unavailable) + { + return Forbid("Quick connect is unavailable"); + } + + _quickConnect.Activate(); + return NoContent(); + } + + /// + /// Enables or disables quick connect. + /// + /// New . + /// Quick connect state set successfully. + /// An on success. + [HttpPost("Available")] + [Authorize(Policy = Policies.RequiresElevation)] + [ProducesResponseType(StatusCodes.Status204NoContent)] + public ActionResult Available([FromQuery] QuickConnectState? status) + { + _quickConnect.SetState(status ?? QuickConnectState.Available); + return NoContent(); + } + + /// + /// Authorizes a pending quick connect request. + /// + /// Quick connect code to authorize. + /// Quick connect result authorized successfully. + /// Missing quick connect code. + /// Boolean indicating if the authorization was successful. + [HttpPost("Authorize")] + [Authorize(Policy = Policies.DefaultAuthorization)] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public ActionResult Authorize([FromQuery, Required] string? code) + { + if (code == null) + { + return BadRequest("Missing code"); + } + + return Ok(_quickConnect.AuthorizeRequest(Request, code)); + } + + /// + /// Deauthorize all quick connect devices for the current user. + /// + /// All quick connect devices were deleted. + /// The number of devices that were deleted. + [HttpPost("Deauthorize")] + [Authorize(Policy = Policies.DefaultAuthorization)] + [ProducesResponseType(StatusCodes.Status200OK)] + public ActionResult Deauthorize() + { + var userId = _authContext.GetAuthorizationInfo(Request).UserId; + return _quickConnect.DeleteAllDevices(userId); + } + } +} diff --git a/Jellyfin.Api/Controllers/UserController.cs b/Jellyfin.Api/Controllers/UserController.cs index 2723125224..131fffb7ac 100644 --- a/Jellyfin.Api/Controllers/UserController.cs +++ b/Jellyfin.Api/Controllers/UserController.cs @@ -216,6 +216,47 @@ namespace Jellyfin.Api.Controllers } } + /// + /// Authenticates a user with quick connect. + /// + /// The request. + /// User authenticated. + /// Missing token. + /// A containing an with information about the new session. + [HttpPost("AuthenticateWithQuickConnect")] + [ProducesResponseType(StatusCodes.Status200OK)] + public async Task> AuthenticateWithQuickConnect([FromBody, Required] QuickConnectDto request) + { + if (request.Token == null) + { + return BadRequest("Access token is required."); + } + + var auth = _authContext.GetAuthorizationInfo(Request); + + try + { + var authRequest = new AuthenticationRequest + { + App = auth.Client, + AppVersion = auth.Version, + DeviceId = auth.DeviceId, + DeviceName = auth.Device, + }; + + var result = await _sessionManager.AuthenticateQuickConnect( + authRequest, + request.Token).ConfigureAwait(false); + + return result; + } + catch (SecurityException e) + { + // rethrow adding IP address to message + throw new SecurityException($"[{HttpContext.Connection.RemoteIpAddress}] {e.Message}", e); + } + } + /// /// Updates a user's password. /// diff --git a/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs new file mode 100644 index 0000000000..8f53d5f37f --- /dev/null +++ b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs @@ -0,0 +1,13 @@ +namespace Jellyfin.Api.Models.UserDtos +{ + /// + /// The quick connect request body. + /// + public class QuickConnectDto + { + /// + /// Gets or sets the quick connect token. + /// + public string? Token { get; set; } + } +} diff --git a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs b/MediaBrowser.Api/QuickConnect/QuickConnectService.cs deleted file mode 100644 index 7093be9908..0000000000 --- a/MediaBrowser.Api/QuickConnect/QuickConnectService.cs +++ /dev/null @@ -1,132 +0,0 @@ -using System; -using MediaBrowser.Controller.Configuration; -using MediaBrowser.Controller.Library; -using MediaBrowser.Controller.Net; -using MediaBrowser.Controller.QuickConnect; -using MediaBrowser.Model.QuickConnect; -using MediaBrowser.Model.Services; -using Microsoft.Extensions.Logging; - -namespace MediaBrowser.Api.QuickConnect -{ - [Route("/QuickConnect/Initiate", "GET", Summary = "Requests a new quick connect code")] - public class Initiate : IReturn - { - [ApiMember(Name = "FriendlyName", Description = "Device friendly name", IsRequired = false, DataType = "string", ParameterType = "query", Verb = "GET")] - public string FriendlyName { get; set; } - } - - [Route("/QuickConnect/Connect", "GET", Summary = "Attempts to retrieve authentication information")] - public class Connect : IReturn - { - [ApiMember(Name = "Secret", Description = "Quick connect secret", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] - public string Secret { get; set; } - } - - [Route("/QuickConnect/Authorize", "POST", Summary = "Authorizes a pending quick connect request")] - [Authenticated] - public class Authorize : IReturn - { - [ApiMember(Name = "Code", Description = "Quick connect identifying code", IsRequired = true, DataType = "string", ParameterType = "query", Verb = "GET")] - public string Code { get; set; } - } - - [Route("/QuickConnect/Deauthorize", "POST", Summary = "Deletes all quick connect authorization tokens for the current user")] - [Authenticated] - public class Deauthorize : IReturn - { - [ApiMember(Name = "UserId", Description = "User Id", IsRequired = false, DataType = "string", ParameterType = "query", Verb = "GET")] - public Guid UserId { get; set; } - } - - [Route("/QuickConnect/Status", "GET", Summary = "Gets the current quick connect state")] - public class QuickConnectStatus : IReturn - { - - } - - [Route("/QuickConnect/Available", "POST", Summary = "Enables or disables quick connect")] - [Authenticated(Roles = "Admin")] - public class Available : IReturn - { - [ApiMember(Name = "Status", Description = "New quick connect status", IsRequired = false, DataType = "QuickConnectState", ParameterType = "query", Verb = "GET")] - public QuickConnectState Status { get; set; } - } - - [Route("/QuickConnect/Activate", "POST", Summary = "Temporarily activates quick connect for the time period defined in the server configuration")] - [Authenticated] - public class Activate : IReturn - { - - } - - public class QuickConnectService : BaseApiService - { - private IQuickConnect _quickConnect; - private IUserManager _userManager; - private IAuthorizationContext _authContext; - - public QuickConnectService( - ILogger logger, - IServerConfigurationManager serverConfigurationManager, - IHttpResultFactory httpResultFactory, - IUserManager userManager, - IAuthorizationContext authContext, - IQuickConnect quickConnect) - : base(logger, serverConfigurationManager, httpResultFactory) - { - _userManager = userManager; - _quickConnect = quickConnect; - _authContext = authContext; - } - - public object Get(Initiate request) - { - return _quickConnect.TryConnect(request.FriendlyName); - } - - public object Get(Connect request) - { - return _quickConnect.CheckRequestStatus(request.Secret); - } - - public object Get(QuickConnectStatus request) - { - _quickConnect.ExpireRequests(); - return _quickConnect.State; - } - - public object Post(Deauthorize request) - { - AssertCanUpdateUser(_authContext, _userManager, request.UserId, true); - - return _quickConnect.DeleteAllDevices(request.UserId); - } - - public object Post(Authorize request) - { - return _quickConnect.AuthorizeRequest(Request, request.Code); - } - - public object Post(Activate request) - { - if (_quickConnect.State == QuickConnectState.Unavailable) - { - return false; - } - - string name = _authContext.GetAuthorizationInfo(Request).User.Username; - - Logger.LogInformation("{name} temporarily activated quick connect", name); - _quickConnect.Activate(); - - return true; - } - - public object Post(Available request) - { - _quickConnect.SetState(request.Status); - return _quickConnect.State; - } - } -} diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index 993637c8aa..fd7e973f67 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -1,7 +1,6 @@ using System; -using System.Collections.Generic; using MediaBrowser.Model.QuickConnect; -using MediaBrowser.Model.Services; +using Microsoft.AspNetCore.Http; namespace MediaBrowser.Controller.QuickConnect { @@ -66,7 +65,7 @@ namespace MediaBrowser.Controller.QuickConnect /// HTTP request object. /// Identifying code for the request. /// A boolean indicating if the authorization completed successfully. - bool AuthorizeRequest(IRequest request, string code); + bool AuthorizeRequest(HttpRequest request, string code); /// /// Expire quick connect requests that are over the time limit. If is true, all requests are unconditionally expired. diff --git a/MediaBrowser.Controller/Session/ISessionManager.cs b/MediaBrowser.Controller/Session/ISessionManager.cs index ffa19fb690..d44787b885 100644 --- a/MediaBrowser.Controller/Session/ISessionManager.cs +++ b/MediaBrowser.Controller/Session/ISessionManager.cs @@ -268,6 +268,7 @@ namespace MediaBrowser.Controller.Session /// Authenticates a new session with quick connect. /// /// The request. + /// Quick connect access token. /// Task{SessionInfo}. Task AuthenticateQuickConnect(AuthenticationRequest request, string token); From eaa57115347f6f70d478f2ca39601d2e70efbdaf Mon Sep 17 00:00:00 2001 From: ConfusedPolarBear <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Sun, 16 Aug 2020 17:21:08 -0500 Subject: [PATCH 16/20] Apply suggestions from code review Co-authored-by: Cody Robibero --- .../Controllers/QuickConnectController.cs | 15 +++++---------- Jellyfin.Api/Controllers/UserController.cs | 5 ----- Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs | 1 + 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/Jellyfin.Api/Controllers/QuickConnectController.cs b/Jellyfin.Api/Controllers/QuickConnectController.cs index d45ea058d2..fd54535953 100644 --- a/Jellyfin.Api/Controllers/QuickConnectController.cs +++ b/Jellyfin.Api/Controllers/QuickConnectController.cs @@ -46,7 +46,7 @@ namespace Jellyfin.Api.Controllers public ActionResult GetStatus() { _quickConnect.ExpireRequests(); - return Ok(_quickConnect.State); + return _quickConnect.State; } /// @@ -60,7 +60,7 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status200OK)] public ActionResult Initiate([FromQuery] string? friendlyName) { - return Ok(_quickConnect.TryConnect(friendlyName)); + return _quickConnect.TryConnect(friendlyName); } /// @@ -78,7 +78,7 @@ namespace Jellyfin.Api.Controllers try { var result = _quickConnect.CheckRequestStatus(secret); - return Ok(result); + return result; } catch (ResourceNotFoundException) { @@ -135,12 +135,7 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status400BadRequest)] public ActionResult Authorize([FromQuery, Required] string? code) { - if (code == null) - { - return BadRequest("Missing code"); - } - - return Ok(_quickConnect.AuthorizeRequest(Request, code)); + return _quickConnect.AuthorizeRequest(Request, code); } /// @@ -153,7 +148,7 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status200OK)] public ActionResult Deauthorize() { - var userId = _authContext.GetAuthorizationInfo(Request).UserId; + var userId = ClaimHelpers.GetUserId(request.HttpContext.User); return _quickConnect.DeleteAllDevices(userId); } } diff --git a/Jellyfin.Api/Controllers/UserController.cs b/Jellyfin.Api/Controllers/UserController.cs index 131fffb7ac..355816bd32 100644 --- a/Jellyfin.Api/Controllers/UserController.cs +++ b/Jellyfin.Api/Controllers/UserController.cs @@ -227,11 +227,6 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status200OK)] public async Task> AuthenticateWithQuickConnect([FromBody, Required] QuickConnectDto request) { - if (request.Token == null) - { - return BadRequest("Access token is required."); - } - var auth = _authContext.GetAuthorizationInfo(Request); try diff --git a/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs index 8f53d5f37f..ac09497328 100644 --- a/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs +++ b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs @@ -8,6 +8,7 @@ /// /// Gets or sets the quick connect token. /// + [Required] public string? Token { get; set; } } } From c49a357f85edbabab11b61b9d4a2938bdb8f3df9 Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Sun, 16 Aug 2020 17:45:53 -0500 Subject: [PATCH 17/20] Fix compile errors --- Jellyfin.Api/Controllers/QuickConnectController.cs | 10 ++++++++-- Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Jellyfin.Api/Controllers/QuickConnectController.cs b/Jellyfin.Api/Controllers/QuickConnectController.cs index fd54535953..1625bcffe7 100644 --- a/Jellyfin.Api/Controllers/QuickConnectController.cs +++ b/Jellyfin.Api/Controllers/QuickConnectController.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Jellyfin.Api.Constants; +using Jellyfin.Api.Helpers; using MediaBrowser.Common.Extensions; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Net; @@ -148,8 +149,13 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status200OK)] public ActionResult Deauthorize() { - var userId = ClaimHelpers.GetUserId(request.HttpContext.User); - return _quickConnect.DeleteAllDevices(userId); + var userId = ClaimHelpers.GetUserId(Request.HttpContext.User); + if (!userId.HasValue) + { + return 0; + } + + return _quickConnect.DeleteAllDevices(userId.Value); } } } diff --git a/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs index ac09497328..c3a2d5cec2 100644 --- a/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs +++ b/Jellyfin.Api/Models/UserDtos/QuickConnectDto.cs @@ -1,4 +1,6 @@ -namespace Jellyfin.Api.Models.UserDtos +using System.ComponentModel.DataAnnotations; + +namespace Jellyfin.Api.Models.UserDtos { /// /// The quick connect request body. From 5f1a86324170387f12602d77dad7249faf30548f Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Mon, 17 Aug 2020 16:36:45 -0500 Subject: [PATCH 18/20] Apply suggestions from code review --- .../QuickConnect/QuickConnectManager.cs | 38 ++++++++----------- .../Session/SessionManager.cs | 2 +- .../Controllers/QuickConnectController.cs | 34 ++++++++--------- Jellyfin.Api/Controllers/UserController.cs | 4 +- .../QuickConnect/IQuickConnect.cs | 12 +++--- .../QuickConnect/QuickConnectResult.cs | 5 --- 6 files changed, 40 insertions(+), 55 deletions(-) diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 949c3b5058..52e934229a 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -3,17 +3,16 @@ using System.Collections.Concurrent; using System.Globalization; using System.Linq; using System.Security.Cryptography; +using MediaBrowser.Common; +using MediaBrowser.Common.Extensions; using MediaBrowser.Controller; +using MediaBrowser.Controller.Authentication; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Controller.Security; using MediaBrowser.Model.QuickConnect; -using Microsoft.AspNetCore.Http; -using MediaBrowser.Common; using Microsoft.Extensions.Logging; -using MediaBrowser.Common.Extensions; -using MediaBrowser.Controller.Authentication; namespace Emby.Server.Implementations.QuickConnect { @@ -60,7 +59,7 @@ namespace Emby.Server.Implementations.QuickConnect public int CodeLength { get; set; } = 6; /// - public string TokenNamePrefix { get; set; } = "QuickConnect-"; + public string TokenName { get; set; } = "QuickConnect"; /// public QuickConnectState State { get; private set; } = QuickConnectState.Unavailable; @@ -82,7 +81,7 @@ namespace Emby.Server.Implementations.QuickConnect /// public void Activate() { - DateActivated = DateTime.Now; + DateActivated = DateTime.UtcNow; SetState(QuickConnectState.Active); } @@ -101,7 +100,7 @@ namespace Emby.Server.Implementations.QuickConnect } /// - public QuickConnectResult TryConnect(string friendlyName) + public QuickConnectResult TryConnect() { ExpireRequests(); @@ -111,14 +110,11 @@ namespace Emby.Server.Implementations.QuickConnect throw new AuthenticationException("Quick connect is not active on this server"); } - _logger.LogDebug("Got new quick connect request from {friendlyName}", friendlyName); - var code = GenerateCode(); var result = new QuickConnectResult() { Secret = GenerateSecureRandom(), - FriendlyName = friendlyName, - DateAdded = DateTime.Now, + DateAdded = DateTime.UtcNow, Code = code }; @@ -162,13 +158,11 @@ namespace Emby.Server.Implementations.QuickConnect } /// - public bool AuthorizeRequest(HttpRequest request, string code) + public bool AuthorizeRequest(Guid userId, string code) { ExpireRequests(); AssertActive(); - var auth = _authContext.GetAuthorizationInfo(request); - if (!_currentRequests.TryGetValue(code, out QuickConnectResult result)) { throw new ResourceNotFoundException("Unable to find request"); @@ -182,21 +176,21 @@ namespace Emby.Server.Implementations.QuickConnect result.Authentication = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); // Change the time on the request so it expires one minute into the future. It can't expire immediately as otherwise some clients wouldn't ever see that they have been authenticated. - var added = result.DateAdded ?? DateTime.Now.Subtract(new TimeSpan(0, Timeout, 0)); - result.DateAdded = added.Subtract(new TimeSpan(0, Timeout - 1, 0)); + var added = result.DateAdded ?? DateTime.UtcNow.Subtract(TimeSpan.FromMinutes(Timeout)); + result.DateAdded = added.Subtract(TimeSpan.FromMinutes(Timeout - 1)); _authenticationRepository.Create(new AuthenticationInfo { - AppName = TokenNamePrefix + result.FriendlyName, + AppName = TokenName, AccessToken = result.Authentication, DateCreated = DateTime.UtcNow, DeviceId = _appHost.SystemId, DeviceName = _appHost.FriendlyName, AppVersion = _appHost.ApplicationVersionString, - UserId = auth.UserId + UserId = userId }); - _logger.LogInformation("Allowing device {FriendlyName} to login as user {Username} with quick connect code {Code}", result.FriendlyName, auth.User.Username, result.Code); + _logger.LogDebug("Authorizing device with code {Code} to login as user {userId}", code, userId); return true; } @@ -210,7 +204,7 @@ namespace Emby.Server.Implementations.QuickConnect UserId = user }); - var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenNamePrefix, StringComparison.CurrentCulture)); + var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenName, StringComparison.CurrentCulture)); var removed = 0; foreach (var token in tokens) @@ -256,7 +250,7 @@ namespace Emby.Server.Implementations.QuickConnect public void ExpireRequests(bool expireAll = false) { // Check if quick connect should be deactivated - if (State == QuickConnectState.Active && DateTime.Now > DateActivated.AddMinutes(Timeout) && !expireAll) + if (State == QuickConnectState.Active && DateTime.UtcNow > DateActivated.AddMinutes(Timeout) && !expireAll) { _logger.LogDebug("Quick connect time expired, deactivating"); SetState(QuickConnectState.Available); @@ -270,7 +264,7 @@ namespace Emby.Server.Implementations.QuickConnect for (int i = 0; i < values.Count; i++) { var added = values[i].DateAdded ?? DateTime.UnixEpoch; - if (DateTime.Now > added.AddMinutes(Timeout) || expireAll) + if (DateTime.UtcNow > added.AddMinutes(Timeout) || expireAll) { code = values[i].Code; _logger.LogDebug("Removing expired request {code}", code); diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index 8a8223ee7f..fbe8e065c0 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1433,7 +1433,7 @@ namespace Emby.Server.Implementations.Session Limit = 1 }); - if (result.TotalRecordCount < 1) + if (result.TotalRecordCount == 0) { throw new SecurityException("Unknown quick connect token"); } diff --git a/Jellyfin.Api/Controllers/QuickConnectController.cs b/Jellyfin.Api/Controllers/QuickConnectController.cs index 1625bcffe7..b1ee2ff53b 100644 --- a/Jellyfin.Api/Controllers/QuickConnectController.cs +++ b/Jellyfin.Api/Controllers/QuickConnectController.cs @@ -1,8 +1,8 @@ +using System; using System.ComponentModel.DataAnnotations; using Jellyfin.Api.Constants; using Jellyfin.Api.Helpers; using MediaBrowser.Common.Extensions; -using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Model.QuickConnect; @@ -18,22 +18,18 @@ namespace Jellyfin.Api.Controllers public class QuickConnectController : BaseJellyfinApiController { private readonly IQuickConnect _quickConnect; - private readonly IUserManager _userManager; private readonly IAuthorizationContext _authContext; /// /// Initializes a new instance of the class. /// /// Instance of the interface. - /// Instance of the interface. /// Instance of the interface. public QuickConnectController( IQuickConnect quickConnect, - IUserManager userManager, IAuthorizationContext authContext) { _quickConnect = quickConnect; - _userManager = userManager; _authContext = authContext; } @@ -53,15 +49,14 @@ namespace Jellyfin.Api.Controllers /// /// Initiate a new quick connect request. /// - /// Device friendly name. /// Quick connect request successfully created. /// Quick connect is not active on this server. /// A with a secret and code for future use or an error message. [HttpGet("Initiate")] [ProducesResponseType(StatusCodes.Status200OK)] - public ActionResult Initiate([FromQuery] string? friendlyName) + public ActionResult Initiate() { - return _quickConnect.TryConnect(friendlyName); + return _quickConnect.TryConnect(); } /// @@ -74,12 +69,11 @@ namespace Jellyfin.Api.Controllers [HttpGet("Connect")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status404NotFound)] - public ActionResult Connect([FromQuery] string? secret) + public ActionResult Connect([FromQuery, Required] string secret) { try { - var result = _quickConnect.CheckRequestStatus(secret); - return result; + return _quickConnect.CheckRequestStatus(secret); } catch (ResourceNotFoundException) { @@ -117,9 +111,9 @@ namespace Jellyfin.Api.Controllers [HttpPost("Available")] [Authorize(Policy = Policies.RequiresElevation)] [ProducesResponseType(StatusCodes.Status204NoContent)] - public ActionResult Available([FromQuery] QuickConnectState? status) + public ActionResult Available([FromQuery] QuickConnectState status = QuickConnectState.Available) { - _quickConnect.SetState(status ?? QuickConnectState.Available); + _quickConnect.SetState(status); return NoContent(); } @@ -127,16 +121,22 @@ namespace Jellyfin.Api.Controllers /// Authorizes a pending quick connect request. /// /// Quick connect code to authorize. + /// User id. /// Quick connect result authorized successfully. - /// Missing quick connect code. + /// User is not allowed to authorize quick connect requests. /// Boolean indicating if the authorization was successful. [HttpPost("Authorize")] [Authorize(Policy = Policies.DefaultAuthorization)] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public ActionResult Authorize([FromQuery, Required] string? code) + [ProducesResponseType(StatusCodes.Status403Forbidden)] + public ActionResult Authorize([FromQuery, Required] string code, [FromQuery, Required] Guid userId) { - return _quickConnect.AuthorizeRequest(Request, code); + if (!RequestHelpers.AssertCanUpdateUser(_authContext, HttpContext.Request, userId, true)) + { + return Forbid("User is not allowed to authorize quick connect requests."); + } + + return _quickConnect.AuthorizeRequest(userId, code); } /// diff --git a/Jellyfin.Api/Controllers/UserController.cs b/Jellyfin.Api/Controllers/UserController.cs index 355816bd32..d67f82219a 100644 --- a/Jellyfin.Api/Controllers/UserController.cs +++ b/Jellyfin.Api/Controllers/UserController.cs @@ -239,11 +239,9 @@ namespace Jellyfin.Api.Controllers DeviceName = auth.Device, }; - var result = await _sessionManager.AuthenticateQuickConnect( + return await _sessionManager.AuthenticateQuickConnect( authRequest, request.Token).ConfigureAwait(false); - - return result; } catch (SecurityException e) { diff --git a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs index fd7e973f67..959a2d7712 100644 --- a/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs +++ b/MediaBrowser.Controller/QuickConnect/IQuickConnect.cs @@ -1,6 +1,5 @@ using System; using MediaBrowser.Model.QuickConnect; -using Microsoft.AspNetCore.Http; namespace MediaBrowser.Controller.QuickConnect { @@ -15,9 +14,9 @@ namespace MediaBrowser.Controller.QuickConnect int CodeLength { get; set; } /// - /// Gets or sets the string to prefix internal access tokens with. + /// Gets or sets the name of internal access tokens. /// - string TokenNamePrefix { get; set; } + string TokenName { get; set; } /// /// Gets the current state of quick connect. @@ -48,9 +47,8 @@ namespace MediaBrowser.Controller.QuickConnect /// /// Initiates a new quick connect request. /// - /// Friendly device name to display in the request UI. /// A quick connect result with tokens to proceed or throws an exception if not active. - QuickConnectResult TryConnect(string friendlyName); + QuickConnectResult TryConnect(); /// /// Checks the status of an individual request. @@ -62,10 +60,10 @@ namespace MediaBrowser.Controller.QuickConnect /// /// Authorizes a quick connect request to connect as the calling user. /// - /// HTTP request object. + /// User id. /// Identifying code for the request. /// A boolean indicating if the authorization completed successfully. - bool AuthorizeRequest(HttpRequest request, string code); + bool AuthorizeRequest(Guid userId, string code); /// /// Expire quick connect requests that are over the time limit. If is true, all requests are unconditionally expired. diff --git a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs index a10d60d57e..0fa40b6a72 100644 --- a/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs +++ b/MediaBrowser.Model/QuickConnect/QuickConnectResult.cs @@ -22,11 +22,6 @@ namespace MediaBrowser.Model.QuickConnect /// public string? Code { get; set; } - /// - /// Gets or sets the device friendly name. - /// - public string? FriendlyName { get; set; } - /// /// Gets or sets the private access token. /// From 1ff4f8e6c64b453eb9096b8da09f4041dbd463fc Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Mon, 17 Aug 2020 18:48:58 -0500 Subject: [PATCH 19/20] Get userId from claim --- .../Controllers/QuickConnectController.cs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/Jellyfin.Api/Controllers/QuickConnectController.cs b/Jellyfin.Api/Controllers/QuickConnectController.cs index b1ee2ff53b..73da2f906a 100644 --- a/Jellyfin.Api/Controllers/QuickConnectController.cs +++ b/Jellyfin.Api/Controllers/QuickConnectController.cs @@ -1,9 +1,7 @@ -using System; using System.ComponentModel.DataAnnotations; using Jellyfin.Api.Constants; using Jellyfin.Api.Helpers; using MediaBrowser.Common.Extensions; -using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using MediaBrowser.Model.QuickConnect; using Microsoft.AspNetCore.Authorization; @@ -18,19 +16,14 @@ namespace Jellyfin.Api.Controllers public class QuickConnectController : BaseJellyfinApiController { private readonly IQuickConnect _quickConnect; - private readonly IAuthorizationContext _authContext; /// /// Initializes a new instance of the class. /// /// Instance of the interface. - /// Instance of the interface. - public QuickConnectController( - IQuickConnect quickConnect, - IAuthorizationContext authContext) + public QuickConnectController(IQuickConnect quickConnect) { _quickConnect = quickConnect; - _authContext = authContext; } /// @@ -121,22 +114,22 @@ namespace Jellyfin.Api.Controllers /// Authorizes a pending quick connect request. /// /// Quick connect code to authorize. - /// User id. /// Quick connect result authorized successfully. - /// User is not allowed to authorize quick connect requests. + /// Unknown user id. /// Boolean indicating if the authorization was successful. [HttpPost("Authorize")] [Authorize(Policy = Policies.DefaultAuthorization)] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status403Forbidden)] - public ActionResult Authorize([FromQuery, Required] string code, [FromQuery, Required] Guid userId) + public ActionResult Authorize([FromQuery, Required] string code) { - if (!RequestHelpers.AssertCanUpdateUser(_authContext, HttpContext.Request, userId, true)) + var userId = ClaimHelpers.GetUserId(Request.HttpContext.User); + if (!userId.HasValue) { - return Forbid("User is not allowed to authorize quick connect requests."); + return Forbid("Unknown user id"); } - return _quickConnect.AuthorizeRequest(userId, code); + return _quickConnect.AuthorizeRequest(userId.Value, code); } /// From df0d197dc8509b38c6b4e5bd9ec442810bc0c647 Mon Sep 17 00:00:00 2001 From: Matt Montgomery <33811686+ConfusedPolarBear@users.noreply.github.com> Date: Wed, 26 Aug 2020 15:24:24 -0500 Subject: [PATCH 20/20] Apply suggestions from code review --- Emby.Server.Implementations/ApplicationHost.cs | 2 +- Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 2f578b3e33..257e78b9b4 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -37,6 +37,7 @@ using Emby.Server.Implementations.LiveTv; using Emby.Server.Implementations.Localization; using Emby.Server.Implementations.Net; using Emby.Server.Implementations.Playlists; +using Emby.Server.Implementations.QuickConnect; using Emby.Server.Implementations.ScheduledTasks; using Emby.Server.Implementations.Security; using Emby.Server.Implementations.Serialization; @@ -102,7 +103,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Prometheus.DotNetRuntime; using OperatingSystem = MediaBrowser.Common.System.OperatingSystem; -using Emby.Server.Implementations.QuickConnect; namespace Emby.Server.Implementations { diff --git a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs index 52e934229a..140a675414 100644 --- a/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs +++ b/Emby.Server.Implementations/QuickConnect/QuickConnectManager.cs @@ -204,7 +204,7 @@ namespace Emby.Server.Implementations.QuickConnect UserId = user }); - var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenName, StringComparison.CurrentCulture)); + var tokens = raw.Items.Where(x => x.AppName.StartsWith(TokenName, StringComparison.Ordinal)); var removed = 0; foreach (var token in tokens)