From d78a55adb4f66b8a82449216a11657da1388ab12 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sat, 8 Jun 2019 22:54:31 -0400 Subject: [PATCH 1/7] Implement InvalidAuthProvider Implements the InvalidAuthProvider, which acts as a fallback if a configured authentication provider, e.g. LDAP, is unavailable due to a load failure or removal. Until the user or the authentication plugin is corrected, this will cause users with the missing provider to be locked out, while throwing errors in the logs about the issue. Fixes #1445 part 2 --- .../Library/InvalidAuthProvider.cs | 46 +++++++++++++++++++ .../Library/UserManager.cs | 20 ++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 Emby.Server.Implementations/Library/InvalidAuthProvider.cs diff --git a/Emby.Server.Implementations/Library/InvalidAuthProvider.cs b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs new file mode 100644 index 0000000000..ee25695621 --- /dev/null +++ b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs @@ -0,0 +1,46 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; +using MediaBrowser.Controller.Authentication; +using MediaBrowser.Controller.Entities; + +namespace Emby.Server.Implementations.Library +{ + public class InvalidAuthProvider : IAuthenticationProvider + { + public string Name => "InvalidorMissingAuthenticationProvider"; + + public bool IsEnabled => true; + + public Task Authenticate(string username, string password) + { + throw new Exception("User Account cannot login with this provider. The Normal provider for this user cannot be found"); + } + + public Task HasPassword(User user) + { + return Task.FromResult(true); + } + + public Task ChangePassword(User user, string newPassword) + { + return Task.FromResult(true); + } + + public void ChangeEasyPassword(User user, string newPassword, string newPasswordHash) + { + // Nothing here + } + + public string GetPasswordHash(User user) + { + return ""; + } + + public string GetEasyPasswordHash(User user) + { + return ""; + } + } +} diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index a0b8d4ba42..ca43f7aaa1 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -79,6 +79,8 @@ namespace Emby.Server.Implementations.Library private IAuthenticationProvider[] _authenticationProviders; private DefaultAuthenticationProvider _defaultAuthenticationProvider; + private InvalidAuthProvider _invalidAuthProvider; + private IPasswordResetProvider[] _passwordResetProviders; private DefaultPasswordResetProvider _defaultPasswordResetProvider; @@ -141,6 +143,8 @@ namespace Emby.Server.Implementations.Library _defaultAuthenticationProvider = _authenticationProviders.OfType().First(); + _invalidAuthProvider = _authenticationProviders.OfType().First(); + _passwordResetProviders = passwordResetProviders.ToArray(); _defaultPasswordResetProvider = passwordResetProviders.OfType().First(); @@ -307,11 +311,14 @@ namespace Emby.Server.Implementations.Library user = Users .FirstOrDefault(i => string.Equals(username, i.Name, StringComparison.OrdinalIgnoreCase)); - var hasNewUserPolicy = authenticationProvider as IHasNewUserPolicy; - if (hasNewUserPolicy != null) + if (authenticationProvider.GetType() != typeof(InvalidAuthProvider)) { - var policy = hasNewUserPolicy.GetNewUserPolicy(); - UpdateUserPolicy(user, policy, true); + var hasNewUserPolicy = authenticationProvider as IHasNewUserPolicy; + if (hasNewUserPolicy != null) + { + var policy = hasNewUserPolicy.GetNewUserPolicy(); + UpdateUserPolicy(user, policy, true); + } } } } @@ -400,7 +407,10 @@ namespace Emby.Server.Implementations.Library if (providers.Length == 0) { - providers = new IAuthenticationProvider[] { _defaultAuthenticationProvider }; + // this function used to assign any user without an auth provider to the default. + // we're going to have it use a new function now. + _logger.LogWarning($"The user {user.Name} was found but no Authentication Provider with ID: {user.Policy.AuthenticationProviderId} was found. Assigning user to InvalidAuthProvider temporarily"); + providers = new IAuthenticationProvider[] { _invalidAuthProvider }; } return providers; From 74ef3898798033a7cad987c4a869e7e72f57b229 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 11:07:35 -0400 Subject: [PATCH 2/7] Add nicer log message and comment --- Emby.Server.Implementations/Library/UserManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index ca43f7aaa1..83584acf3d 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -407,9 +407,8 @@ namespace Emby.Server.Implementations.Library if (providers.Length == 0) { - // this function used to assign any user without an auth provider to the default. - // we're going to have it use a new function now. - _logger.LogWarning($"The user {user.Name} was found but no Authentication Provider with ID: {user.Policy.AuthenticationProviderId} was found. Assigning user to InvalidAuthProvider temporarily"); + // Assign the user to the InvalidAuthProvider since no configured auth provider was valid/found + _logger.LogWarning("User {0} was found with invalid/missing Authentication Provider {1}. Assigning user to InvalidAuthProvider until this is corrected", user.Name, user.Policy.AuthenticationProviderId); providers = new IAuthenticationProvider[] { _invalidAuthProvider }; } From b70083f3b370055b2942e450291ce42345732cb7 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 13:41:14 -0400 Subject: [PATCH 3/7] Apply suggestions from code review Co-Authored-By: Claus Vium Co-Authored-By: Bond-009 --- .../Library/InvalidAuthProvider.cs | 8 ++++---- Emby.Server.Implementations/Library/UserManager.cs | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Emby.Server.Implementations/Library/InvalidAuthProvider.cs b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs index ee25695621..133864708d 100644 --- a/Emby.Server.Implementations/Library/InvalidAuthProvider.cs +++ b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs @@ -9,7 +9,7 @@ namespace Emby.Server.Implementations.Library { public class InvalidAuthProvider : IAuthenticationProvider { - public string Name => "InvalidorMissingAuthenticationProvider"; + public string Name => "InvalidOrMissingAuthenticationProvider"; public bool IsEnabled => true; @@ -25,7 +25,7 @@ namespace Emby.Server.Implementations.Library public Task ChangePassword(User user, string newPassword) { - return Task.FromResult(true); + return Task.CompletedTask; } public void ChangeEasyPassword(User user, string newPassword, string newPasswordHash) @@ -35,12 +35,12 @@ namespace Emby.Server.Implementations.Library public string GetPasswordHash(User user) { - return ""; + return string.Empty; } public string GetEasyPasswordHash(User user) { - return ""; + return string.Empty; } } } diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 83584acf3d..04abfc3156 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -313,8 +313,7 @@ namespace Emby.Server.Implementations.Library if (authenticationProvider.GetType() != typeof(InvalidAuthProvider)) { - var hasNewUserPolicy = authenticationProvider as IHasNewUserPolicy; - if (hasNewUserPolicy != null) + if (authenticationProvider is IHasNewUserPolicy hasNewUserPolicy) { var policy = hasNewUserPolicy.GetNewUserPolicy(); UpdateUserPolicy(user, policy, true); @@ -408,7 +407,7 @@ namespace Emby.Server.Implementations.Library if (providers.Length == 0) { // Assign the user to the InvalidAuthProvider since no configured auth provider was valid/found - _logger.LogWarning("User {0} was found with invalid/missing Authentication Provider {1}. Assigning user to InvalidAuthProvider until this is corrected", user.Name, user.Policy.AuthenticationProviderId); + _logger.LogWarning("User {UserName} was found with invalid/missing Authentication Provider {AuthenticationProviderId}. Assigning user to InvalidAuthProvider until this is corrected", user.Name, user.Policy.AuthenticationProviderId); providers = new IAuthenticationProvider[] { _invalidAuthProvider }; } From 20e2cb2d8693e3f53749bcde5b5fd6367bd15007 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 13:45:51 -0400 Subject: [PATCH 4/7] Use SecurityException for auth failure --- Emby.Server.Implementations/Library/InvalidAuthProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/Library/InvalidAuthProvider.cs b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs index 133864708d..25d2331373 100644 --- a/Emby.Server.Implementations/Library/InvalidAuthProvider.cs +++ b/Emby.Server.Implementations/Library/InvalidAuthProvider.cs @@ -4,6 +4,7 @@ using System.Text; using System.Threading.Tasks; using MediaBrowser.Controller.Authentication; using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Net; namespace Emby.Server.Implementations.Library { @@ -15,7 +16,7 @@ namespace Emby.Server.Implementations.Library public Task Authenticate(string username, string password) { - throw new Exception("User Account cannot login with this provider. The Normal provider for this user cannot be found"); + throw new SecurityException("User Account cannot login with this provider. The Normal provider for this user cannot be found"); } public Task HasPassword(User user) From c230d49d7c37d4fbe77676b835c3afd6c8cb56e7 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 13:46:53 -0400 Subject: [PATCH 5/7] Don't set a default reset provider --- Emby.Server.Implementations/Library/UserManager.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 04abfc3156..4233ea8f44 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -425,11 +425,6 @@ namespace Emby.Server.Implementations.Library providers = providers.Where(i => string.Equals(passwordResetProviderId, GetPasswordResetProviderId(i), StringComparison.OrdinalIgnoreCase)).ToArray(); } - if (providers.Length == 0) - { - providers = new IPasswordResetProvider[] { _defaultPasswordResetProvider }; - } - return providers; } From 4b8f735cb89901bd1004d590f4f2820c23e2493c Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 13:57:49 -0400 Subject: [PATCH 6/7] Remove superfluous conditional This wasn't needed to prevent updating the policy on-disk from my tests and can be removed as suggested by @Bond-009 --- Emby.Server.Implementations/Library/UserManager.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 4233ea8f44..16becbd526 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -311,13 +311,10 @@ namespace Emby.Server.Implementations.Library user = Users .FirstOrDefault(i => string.Equals(username, i.Name, StringComparison.OrdinalIgnoreCase)); - if (authenticationProvider.GetType() != typeof(InvalidAuthProvider)) + if (authenticationProvider is IHasNewUserPolicy hasNewUserPolicy) { - if (authenticationProvider is IHasNewUserPolicy hasNewUserPolicy) - { - var policy = hasNewUserPolicy.GetNewUserPolicy(); - UpdateUserPolicy(user, policy, true); - } + var policy = hasNewUserPolicy.GetNewUserPolicy(); + UpdateUserPolicy(user, policy, true); } } } From 2946ae10092cddadade4c84cfa000129bf117e03 Mon Sep 17 00:00:00 2001 From: "Joshua M. Boniface" Date: Sun, 9 Jun 2019 15:27:38 -0400 Subject: [PATCH 7/7] Revert "Don't set a default reset provider" This reverts commit c230d49d7c37d4fbe77676b835c3afd6c8cb56e7. This reenables an edge case where an admin might want to reset, with the default auth provider, the password of an externally-provided user so they could "unlock" the account while it was failing. There might be minor security implications to this, but the malicious actor would need FS access to do it (as they would with any password resets) so it's probably best to keep it as-is. Removing this in the first place was due to a misunderstanding anyways so no harm. --- Emby.Server.Implementations/Library/UserManager.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 16becbd526..ff375e5900 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -422,6 +422,11 @@ namespace Emby.Server.Implementations.Library providers = providers.Where(i => string.Equals(passwordResetProviderId, GetPasswordResetProviderId(i), StringComparison.OrdinalIgnoreCase)).ToArray(); } + if (providers.Length == 0) + { + providers = new IPasswordResetProvider[] { _defaultPasswordResetProvider }; + } + return providers; }