From a963bce9beb16f0d66ec0cef8d92f0d4f6536730 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Mon, 21 Aug 2023 19:09:32 +0200 Subject: [PATCH] Reduce log spam on failed logins Failed logins already get logged higher up the call chain --- .../Users/DefaultAuthenticationProvider.cs | 21 +++++++++++-------- .../Users/UserManager.cs | 2 +- .../Authentication/IAuthenticationProvider.cs | 8 +++---- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs b/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs index 72f3d6e8ec..cb2d09a670 100644 --- a/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs +++ b/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; using Jellyfin.Data.Entities; using MediaBrowser.Controller.Authentication; @@ -39,14 +40,18 @@ namespace Jellyfin.Server.Implementations.Users /// // This is the version that we need to use for local users. Because reasons. - public Task Authenticate(string username, string password, User resolvedUser) + public Task Authenticate(string username, string password, User? resolvedUser) { - if (resolvedUser is null) + [DoesNotReturn] + static void ThrowAuthenticationException() { - throw new AuthenticationException("Specified user does not exist."); + throw new AuthenticationException("Invalid username or password"); } - bool success = false; + if (resolvedUser is null) + { + ThrowAuthenticationException(); + } // As long as jellyfin supports password-less users, we need this little block here to accommodate if (!HasPassword(resolvedUser) && string.IsNullOrEmpty(password)) @@ -60,15 +65,13 @@ namespace Jellyfin.Server.Implementations.Users // Handle the case when the stored password is null, but the user tried to login with a password if (resolvedUser.Password is null) { - throw new AuthenticationException("Invalid username or password"); + ThrowAuthenticationException(); } PasswordHash readyHash = PasswordHash.Parse(resolvedUser.Password); - success = _cryptographyProvider.Verify(readyHash, password); - - if (!success) + if (!_cryptographyProvider.Verify(readyHash, password)) { - throw new AuthenticationException("Invalid username or password"); + ThrowAuthenticationException(); } // Migrate old hashes to the new default diff --git a/Jellyfin.Server.Implementations/Users/UserManager.cs b/Jellyfin.Server.Implementations/Users/UserManager.cs index ec0c64cd72..5010751ddb 100644 --- a/Jellyfin.Server.Implementations/Users/UserManager.cs +++ b/Jellyfin.Server.Implementations/Users/UserManager.cs @@ -833,7 +833,7 @@ namespace Jellyfin.Server.Implementations.Users } catch (AuthenticationException ex) { - _logger.LogError(ex, "Error authenticating with provider {Provider}", provider.Name); + _logger.LogDebug(ex, "Error authenticating with provider {Provider}", provider.Name); return (username, false); } diff --git a/MediaBrowser.Controller/Authentication/IAuthenticationProvider.cs b/MediaBrowser.Controller/Authentication/IAuthenticationProvider.cs index a56d3c8223..81b532fda8 100644 --- a/MediaBrowser.Controller/Authentication/IAuthenticationProvider.cs +++ b/MediaBrowser.Controller/Authentication/IAuthenticationProvider.cs @@ -1,5 +1,3 @@ -#nullable disable - #pragma warning disable CS1591 using System.Threading.Tasks; @@ -23,7 +21,7 @@ namespace MediaBrowser.Controller.Authentication public interface IRequiresResolvedUser { - Task Authenticate(string username, string password, User resolvedUser); + Task Authenticate(string username, string password, User? resolvedUser); } public interface IHasNewUserPolicy @@ -33,8 +31,8 @@ namespace MediaBrowser.Controller.Authentication public class ProviderAuthenticationResult { - public string Username { get; set; } + public required string Username { get; set; } - public string DisplayName { get; set; } + public string? DisplayName { get; set; } } }