From 442e7706880bba9a95404b4d04972674ad65d085 Mon Sep 17 00:00:00 2001 From: cvium Date: Wed, 17 Feb 2021 11:30:14 +0100 Subject: [PATCH] Validate the new username when renaming --- .../Properties/AssemblyInfo.cs | 23 +++++++++++++++ .../Users/UserManager.cs | 22 +++++++++------ ...llyfin.Server.Implementations.Tests.csproj | 1 + .../Users/UserManagerTests.cs | 28 +++++++++++++++++++ 4 files changed, 65 insertions(+), 9 deletions(-) create mode 100644 Jellyfin.Server.Implementations/Properties/AssemblyInfo.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs diff --git a/Jellyfin.Server.Implementations/Properties/AssemblyInfo.cs b/Jellyfin.Server.Implementations/Properties/AssemblyInfo.cs new file mode 100644 index 000000000..6528d4fdc --- /dev/null +++ b/Jellyfin.Server.Implementations/Properties/AssemblyInfo.cs @@ -0,0 +1,23 @@ +using System.Reflection; +using System.Resources; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +// General Information about an assembly is controlled through the following +// set of attributes. Change these attribute values to modify the information +// associated with an assembly. +[assembly: AssemblyTitle("Jellyfin.Server.Implementations")] +[assembly: AssemblyDescription("")] +[assembly: AssemblyConfiguration("")] +[assembly: AssemblyCompany("Jellyfin Project")] +[assembly: AssemblyProduct("Jellyfin Server")] +[assembly: AssemblyCopyright("Copyright © 2019 Jellyfin Contributors. Code released under the GNU General Public License")] +[assembly: AssemblyTrademark("")] +[assembly: AssemblyCulture("")] +[assembly: NeutralResourcesLanguage("en")] +[assembly: InternalsVisibleTo("Jellyfin.Server.Implementations.Tests")] + +// Setting ComVisible to false makes the types in this assembly not visible +// to COM components. If you need to access a type in this assembly from +// COM, set the ComVisible attribute to true on that type. +[assembly: ComVisible(false)] diff --git a/Jellyfin.Server.Implementations/Users/UserManager.cs b/Jellyfin.Server.Implementations/Users/UserManager.cs index a3a9e90d4..76d1389ca 100644 --- a/Jellyfin.Server.Implementations/Users/UserManager.cs +++ b/Jellyfin.Server.Implementations/Users/UserManager.cs @@ -137,10 +137,7 @@ namespace Jellyfin.Server.Implementations.Users throw new ArgumentNullException(nameof(user)); } - if (string.IsNullOrWhiteSpace(newName)) - { - throw new ArgumentException("Invalid username", nameof(newName)); - } + ThrowIfInvalidUsername(newName); if (user.Username.Equals(newName, StringComparison.Ordinal)) { @@ -201,10 +198,7 @@ namespace Jellyfin.Server.Implementations.Users /// public async Task CreateUserAsync(string name) { - if (!IsValidUsername(name)) - { - throw new ArgumentException("Usernames can contain unicode symbols, numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)"); - } + ThrowIfInvalidUsername(name); if (Users.Any(u => u.Username.Equals(name, StringComparison.OrdinalIgnoreCase))) { @@ -733,12 +727,22 @@ namespace Jellyfin.Server.Implementations.Users _users[user.Id] = user; } + internal static void ThrowIfInvalidUsername(string name) + { + if (!string.IsNullOrWhiteSpace(name) && IsValidUsername(name)) + { + return; + } + + throw new ArgumentException("Usernames can contain unicode symbols, numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)", nameof(name)); + } + private static bool IsValidUsername(string name) { // This is some regex that matches only on unicode "word" characters, as well as -, _ and @ // In theory this will cut out most if not all 'control' characters which should help minimize any weirdness // Usernames can contain letters (a-z + whatever else unicode is cool with), numbers (0-9), at-signs (@), dashes (-), underscores (_), apostrophes ('), periods (.) and spaces ( ) - return Regex.IsMatch(name, @"^[\w\ \-'._@]*$"); + return Regex.IsMatch(name, @"^[\w\ \-'._@]+$"); } private IAuthenticationProvider GetAuthenticationProvider(User user) diff --git a/tests/Jellyfin.Server.Implementations.Tests/Jellyfin.Server.Implementations.Tests.csproj b/tests/Jellyfin.Server.Implementations.Tests/Jellyfin.Server.Implementations.Tests.csproj index 174f29b09..c3b3155fe 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Jellyfin.Server.Implementations.Tests.csproj +++ b/tests/Jellyfin.Server.Implementations.Tests/Jellyfin.Server.Implementations.Tests.csproj @@ -39,6 +39,7 @@ + diff --git a/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs new file mode 100644 index 000000000..9bcd43bc0 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs @@ -0,0 +1,28 @@ +using System; +using Jellyfin.Server.Implementations.Users; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Users +{ + public class UserManagerTests + { + [Theory] + [InlineData("this_is_valid", true)] + [InlineData("this is also valid", true)] + [InlineData(" ", false)] + [InlineData("", false)] + [InlineData("0@_-' .", true)] + public void ThrowIfInvalidUsername_WhenInvalidUsername_ThrowsArgumentException(string username, bool isValid) + { + var ex = Record.Exception(() => UserManager.ThrowIfInvalidUsername(username)); + + var argumentExceptionNotThrown = ex is not ArgumentException; + if (ex != null) + { + Assert.Equal(typeof(ArgumentException), ex.GetType()); + } + + Assert.Equal(isValid, argumentExceptionNotThrown); + } + } +}