From 5d760b7ee806d3fb00ac5aa7d0981362526f1d11 Mon Sep 17 00:00:00 2001
From: Davide Polonio <poloniodavide@gmail.com>
Date: Sun, 1 Mar 2020 21:38:34 +0100
Subject: [PATCH] Fix emby/user/public API leaking private data

This commit fixes the emby/user/public API that was returning more data
than necessary. Now only the following information are returned:
- the account name
- the primary image tag
- the field hasPassword
- the field hasConfiguredPassword, useful for the first wizard only
(see
https://github.com/jellyfin/jellyfin/issues/880#issuecomment-465370051)
- the primary image aspect ratio

A new DTO class, PrivateUserDTO has been created, and the route has been
modified in order to return that data object.
---
 .../Library/UserManager.cs                    | 25 ++++++++++
 MediaBrowser.Api/UserService.cs               | 36 +++++++++-----
 .../Library/IUserManager.cs                   |  8 ++++
 MediaBrowser.Model/Dto/PublicUserDto.cs       | 48 +++++++++++++++++++
 4 files changed, 106 insertions(+), 11 deletions(-)
 create mode 100644 MediaBrowser.Model/Dto/PublicUserDto.cs

diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs
index 6e203f894f..8941767b41 100644
--- a/Emby.Server.Implementations/Library/UserManager.cs
+++ b/Emby.Server.Implementations/Library/UserManager.cs
@@ -613,6 +613,31 @@ namespace Emby.Server.Implementations.Library
             return dto;
         }
 
+        public PublicUserDto GetPublicUserDto(User user, string remoteEndPoint = null)
+        {
+            if (user == null)
+            {
+                throw new ArgumentNullException(nameof(user));
+            }
+
+            bool hasConfiguredPassword = GetAuthenticationProvider(user).HasPassword(user);
+            bool hasConfiguredEasyPassword = !string.IsNullOrEmpty(GetAuthenticationProvider(user).GetEasyPasswordHash(user));
+
+            bool hasPassword = user.Configuration.EnableLocalPassword &&
+                !string.IsNullOrEmpty(remoteEndPoint) &&
+                _networkManager.IsInLocalNetwork(remoteEndPoint) ? hasConfiguredEasyPassword : hasConfiguredPassword;
+
+
+            PublicUserDto dto = new PublicUserDto
+            {
+                Name = user.Name,
+                HasPassword = hasPassword,
+                HasConfiguredPassword = hasConfiguredPassword,
+            };
+
+            return dto;
+        }
+
         public UserDto GetOfflineUserDto(User user)
         {
             var dto = GetUserDto(user);
diff --git a/MediaBrowser.Api/UserService.cs b/MediaBrowser.Api/UserService.cs
index 4015143497..b4ab8c9740 100644
--- a/MediaBrowser.Api/UserService.cs
+++ b/MediaBrowser.Api/UserService.cs
@@ -35,7 +35,7 @@ namespace MediaBrowser.Api
     }
 
     [Route("/Users/Public", "GET", Summary = "Gets a list of publicly visible users for display on a login screen.")]
-    public class GetPublicUsers : IReturn<UserDto[]>
+    public class GetPublicUsers : IReturn<PublicUserDto[]>
     {
     }
 
@@ -266,22 +266,36 @@ namespace MediaBrowser.Api
             _authContext = authContext;
         }
 
+        /// <summary>
+        /// Gets the public available Users information
+        /// </summary>
+        /// <param name="request">The request.</param>
+        /// <returns>System.Object.</returns>
         public object Get(GetPublicUsers request)
         {
-            // If the startup wizard hasn't been completed then just return all users
-            if (!ServerConfigurationManager.Configuration.IsStartupWizardCompleted)
+            var users = _userManager
+                .Users
+                .Where(item => item.Policy.IsDisabled == false)
+                .Where(item => item.Policy.IsHidden == false);
+
+            var deviceId = _authContext.GetAuthorizationInfo(Request).DeviceId;
+
+            if (!string.IsNullOrWhiteSpace(deviceId))
             {
-                return Get(new GetUsers
-                {
-                    IsDisabled = false
-                });
+                users = users.Where(i => _deviceManager.CanAccessDevice(i, deviceId));
             }
 
-            return Get(new GetUsers
+            if (!_networkManager.IsInLocalNetwork(Request.RemoteIp))
             {
-                IsHidden = false,
-                IsDisabled = false
-            }, true, true);
+                users = users.Where(i => i.Policy.EnableRemoteAccess);
+            }
+
+            var result = users
+                .OrderBy(u => u.Name)
+                .Select(i => _userManager.GetPublicUserDto(i, Request.RemoteIp))
+                .ToArray();
+
+            return ToOptimizedResult(result);
         }
 
         /// <summary>
diff --git a/MediaBrowser.Controller/Library/IUserManager.cs b/MediaBrowser.Controller/Library/IUserManager.cs
index be7b4ce59d..ec6cb35eb9 100644
--- a/MediaBrowser.Controller/Library/IUserManager.cs
+++ b/MediaBrowser.Controller/Library/IUserManager.cs
@@ -143,6 +143,14 @@ namespace MediaBrowser.Controller.Library
         /// <returns>UserDto.</returns>
         UserDto GetUserDto(User user, string remoteEndPoint = null);
 
+        /// <summary>
+        /// Gets the user public dto.
+        /// </summary>
+        /// <param name="user">Ther user.</param>\
+        /// <param name="remoteEndPoint">The remote end point.</param>
+        /// <returns>A public UserDto, aka a UserDto stripped of personal data.</returns>
+        PublicUserDto GetPublicUserDto(User user, string remoteEndPoint = null);
+
         /// <summary>
         /// Authenticates the user.
         /// </summary>
diff --git a/MediaBrowser.Model/Dto/PublicUserDto.cs b/MediaBrowser.Model/Dto/PublicUserDto.cs
new file mode 100644
index 0000000000..bf529a2d0b
--- /dev/null
+++ b/MediaBrowser.Model/Dto/PublicUserDto.cs
@@ -0,0 +1,48 @@
+using System;
+using MediaBrowser.Model.Configuration;
+using MediaBrowser.Model.Users;
+
+namespace MediaBrowser.Model.Dto
+{
+    /// <summary>
+    /// Class PublicUserDto. Its goal is to show only public information about a user
+    /// </summary>
+    public class PublicUserDto : IItemDto
+    {
+        /// <summary>
+        /// Gets or sets the name.
+        /// </summary>
+        /// <value>The name.</value>
+        public string Name { get; set; }
+
+        /// <summary>
+        /// Gets or sets the primary image tag.
+        /// </summary>
+        /// <value>The primary image tag.</value>
+        public string PrimaryImageTag { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether this instance has password.
+        /// </summary>
+        /// <value><c>true</c> if this instance has password; otherwise, <c>false</c>.</value>
+        public bool HasPassword { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether this instance has configured password.
+        /// </summary>
+        /// <value><c>true</c> if this instance has configured password; otherwise, <c>false</c>.</value>
+        public bool HasConfiguredPassword { get; set; }
+
+        /// <summary>
+        /// Gets or sets the primary image aspect ratio.
+        /// </summary>
+        /// <value>The primary image aspect ratio.</value>
+        public double? PrimaryImageAspectRatio { get; set; }
+
+        /// <inheritdoc />
+        public override string ToString()
+        {
+            return Name ?? base.ToString();
+        }
+    }
+}
\ No newline at end of file