Merge pull request #4330 from crobibero/api-key-auth

Fix ApiKey authentication

12 files changed, 124 insertions, 78 deletions

diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs
index 68d981ad1..7d53e886f 100644
--- a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs
+++ b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs
@@ -19,12 +19,12 @@ namespace Emby.Server.Implementations.HttpServer.Security
         public AuthorizationInfo Authenticate(HttpRequest request)
         {
             var auth = _authorizationContext.GetAuthorizationInfo(request);
-            if (auth?.User == null)
+            if (auth == null)
             {
-                return null;
+                throw new SecurityException("Unauthenticated request.");
             }
 
-            if (auth.User.HasPermission(PermissionKind.IsDisabled))
+            if (auth.User?.HasPermission(PermissionKind.IsDisabled) ?? false)
             {
                 throw new SecurityException("User account has been disabled.");
             }
diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthorizationContext.cs b/Emby.Server.Implementations/HttpServer/Security/AuthorizationContext.cs
index 8140fe81b..de7e7bf3b 100644
--- a/Emby.Server.Implementations/HttpServer/Security/AuthorizationContext.cs
+++ b/Emby.Server.Implementations/HttpServer/Security/AuthorizationContext.cs
@@ -111,81 +111,89 @@ namespace Emby.Server.Implementations.HttpServer.Security
                 Token = token
             };
 
-            AuthenticationInfo originalAuthenticationInfo = null;
-            if (!string.IsNullOrWhiteSpace(token))
+            if (string.IsNullOrWhiteSpace(token))
             {
-                var result = _authRepo.Get(new AuthenticationInfoQuery
-                {
-                    AccessToken = token
-                });
+                // Request doesn't contain a token.
+                return (null, null);
+            }
 
-                originalAuthenticationInfo = result.Items.Count > 0 ? result.Items[0] : null;
+            var result = _authRepo.Get(new AuthenticationInfoQuery
+            {
+                AccessToken = token
+            });
 
-                if (originalAuthenticationInfo != null)
-                {
-                    var updateToken = false;
+            var originalAuthenticationInfo = result.Items.Count > 0 ? result.Items[0] : null;
 
-                    // TODO: Remove these checks for IsNullOrWhiteSpace
-                    if (string.IsNullOrWhiteSpace(authInfo.Client))
-                    {
-                        authInfo.Client = originalAuthenticationInfo.AppName;
-                    }
+            if (originalAuthenticationInfo != null)
+            {
+                var updateToken = false;
 
-                    if (string.IsNullOrWhiteSpace(authInfo.DeviceId))
-                    {
-                        authInfo.DeviceId = originalAuthenticationInfo.DeviceId;
-                    }
+                // TODO: Remove these checks for IsNullOrWhiteSpace
+                if (string.IsNullOrWhiteSpace(authInfo.Client))
+                {
+                    authInfo.Client = originalAuthenticationInfo.AppName;
+                }
 
-                    // Temporary. TODO - allow clients to specify that the token has been shared with a casting device
-                    var allowTokenInfoUpdate = authInfo.Client == null || authInfo.Client.IndexOf("chromecast", StringComparison.OrdinalIgnoreCase) == -1;
+                if (string.IsNullOrWhiteSpace(authInfo.DeviceId))
+                {
+                    authInfo.DeviceId = originalAuthenticationInfo.DeviceId;
+                }
 
-                    if (string.IsNullOrWhiteSpace(authInfo.Device))
-                    {
-                        authInfo.Device = originalAuthenticationInfo.DeviceName;
-                    }
-                    else if (!string.Equals(authInfo.Device, originalAuthenticationInfo.DeviceName, StringComparison.OrdinalIgnoreCase))
-                    {
-                        if (allowTokenInfoUpdate)
-                        {
-                            updateToken = true;
-                            originalAuthenticationInfo.DeviceName = authInfo.Device;
-                        }
-                    }
+                // Temporary. TODO - allow clients to specify that the token has been shared with a casting device
+                var allowTokenInfoUpdate = authInfo.Client == null || authInfo.Client.IndexOf("chromecast", StringComparison.OrdinalIgnoreCase) == -1;
 
-                    if (string.IsNullOrWhiteSpace(authInfo.Version))
-                    {
-                        authInfo.Version = originalAuthenticationInfo.AppVersion;
-                    }
-                    else if (!string.Equals(authInfo.Version, originalAuthenticationInfo.AppVersion, StringComparison.OrdinalIgnoreCase))
+                if (string.IsNullOrWhiteSpace(authInfo.Device))
+                {
+                    authInfo.Device = originalAuthenticationInfo.DeviceName;
+                }
+                else if (!string.Equals(authInfo.Device, originalAuthenticationInfo.DeviceName, StringComparison.OrdinalIgnoreCase))
+                {
+                    if (allowTokenInfoUpdate)
                     {
-                        if (allowTokenInfoUpdate)
-                        {
-                            updateToken = true;
-                            originalAuthenticationInfo.AppVersion = authInfo.Version;
-                        }
+                        updateToken = true;
+                        originalAuthenticationInfo.DeviceName = authInfo.Device;
                     }
+                }
 
-                    if ((DateTime.UtcNow - originalAuthenticationInfo.DateLastActivity).TotalMinutes > 3)
+                if (string.IsNullOrWhiteSpace(authInfo.Version))
+                {
+                    authInfo.Version = originalAuthenticationInfo.AppVersion;
+                }
+                else if (!string.Equals(authInfo.Version, originalAuthenticationInfo.AppVersion, StringComparison.OrdinalIgnoreCase))
+                {
+                    if (allowTokenInfoUpdate)
                     {
-                        originalAuthenticationInfo.DateLastActivity = DateTime.UtcNow;
                         updateToken = true;
+                        originalAuthenticationInfo.AppVersion = authInfo.Version;
                     }
+                }
 
-                    if (!originalAuthenticationInfo.UserId.Equals(Guid.Empty))
-                    {
-                        authInfo.User = _userManager.GetUserById(originalAuthenticationInfo.UserId);
+                if ((DateTime.UtcNow - originalAuthenticationInfo.DateLastActivity).TotalMinutes > 3)
+                {
+                    originalAuthenticationInfo.DateLastActivity = DateTime.UtcNow;
+                    updateToken = true;
+                }
 
-                        if (authInfo.User != null && !string.Equals(authInfo.User.Username, originalAuthenticationInfo.UserName, StringComparison.OrdinalIgnoreCase))
-                        {
-                            originalAuthenticationInfo.UserName = authInfo.User.Username;
-                            updateToken = true;
-                        }
-                    }
+                if (!originalAuthenticationInfo.UserId.Equals(Guid.Empty))
+                {
+                    authInfo.User = _userManager.GetUserById(originalAuthenticationInfo.UserId);
 
-                    if (updateToken)
+                    if (authInfo.User != null && !string.Equals(authInfo.User.Username, originalAuthenticationInfo.UserName, StringComparison.OrdinalIgnoreCase))
                     {
-                        _authRepo.Update(originalAuthenticationInfo);
+                        originalAuthenticationInfo.UserName = authInfo.User.Username;
+                        updateToken = true;
                     }
+
+                    authInfo.IsApiKey = true;
+                }
+                else
+                {
+                    authInfo.IsApiKey = false;
+                }
+
+                if (updateToken)
+                {
+                    _authRepo.Update(originalAuthenticationInfo);
                 }
             }
 
diff --git a/Jellyfin.Api/Auth/BaseAuthorizationHandler.cs b/Jellyfin.Api/Auth/BaseAuthorizationHandler.cs
index d732b6bc6..7d68aecf9 100644
--- a/Jellyfin.Api/Auth/BaseAuthorizationHandler.cs
+++ b/Jellyfin.Api/Auth/BaseAuthorizationHandler.cs
@@ -50,6 +50,13 @@ namespace Jellyfin.Api.Auth
             bool localAccessOnly = false,
             bool requiredDownloadPermission = false)
         {
+            // ApiKey is currently global admin, always allow.
+            var isApiKey = ClaimHelpers.GetIsApiKey(claimsPrincipal);
+            if (isApiKey)
+            {
+                return true;
+            }
+
             // Ensure claim has userId.
             var userId = ClaimHelpers.GetUserId(claimsPrincipal);
             if (!userId.HasValue)
diff --git a/Jellyfin.Api/Auth/CustomAuthenticationHandler.cs b/Jellyfin.Api/Auth/CustomAuthenticationHandler.cs
index 733c6959e..e8cc38907 100644
--- a/Jellyfin.Api/Auth/CustomAuthenticationHandler.cs
+++ b/Jellyfin.Api/Auth/CustomAuthenticationHandler.cs
@@ -43,24 +43,23 @@ namespace Jellyfin.Api.Auth
             try
             {
                 var authorizationInfo = _authService.Authenticate(Request);
-                if (authorizationInfo == null)
+                var role = UserRoles.User;
+                if (authorizationInfo.IsApiKey || authorizationInfo.User.HasPermission(PermissionKind.IsAdministrator))
                 {
-                    return Task.FromResult(AuthenticateResult.NoResult());
-                    // TODO return when legacy API is removed.
-                    // Don't spam the log with "Invalid User"
-                    // return Task.FromResult(AuthenticateResult.Fail("Invalid user"));
+                    role = UserRoles.Administrator;
                 }
 
                 var claims = new[]
                 {
-                    new Claim(ClaimTypes.Name, authorizationInfo.User.Username),
-                    new Claim(ClaimTypes.Role, authorizationInfo.User.HasPermission(PermissionKind.IsAdministrator) ? UserRoles.Administrator : UserRoles.User),
+                    new Claim(ClaimTypes.Name, authorizationInfo.User?.Username ?? string.Empty),
+                    new Claim(ClaimTypes.Role, role),
                     new Claim(InternalClaimTypes.UserId, authorizationInfo.UserId.ToString("N", CultureInfo.InvariantCulture)),
                     new Claim(InternalClaimTypes.DeviceId, authorizationInfo.DeviceId),
                     new Claim(InternalClaimTypes.Device, authorizationInfo.Device),
                     new Claim(InternalClaimTypes.Client, authorizationInfo.Client),
                     new Claim(InternalClaimTypes.Version, authorizationInfo.Version),
                     new Claim(InternalClaimTypes.Token, authorizationInfo.Token),
+                    new Claim(InternalClaimTypes.IsApiKey, authorizationInfo.IsApiKey.ToString(CultureInfo.InvariantCulture))
                 };
 
                 var identity = new ClaimsIdentity(claims, Scheme.Name);
diff --git a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs
index b5913daab..be77b7a4e 100644
--- a/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs
+++ b/Jellyfin.Api/Auth/DefaultAuthorizationPolicy/DefaultAuthorizationHandler.cs
@@ -29,13 +29,15 @@ namespace Jellyfin.Api.Auth.DefaultAuthorizationPolicy
         protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DefaultAuthorizationRequirement requirement)
         {
             var validated = ValidateClaims(context.User);
-            if (!validated)
+            if (validated)
+            {
+                context.Succeed(requirement);
+            }
+            else
             {
                 context.Fail();
-                return Task.CompletedTask;
             }
 
-            context.Succeed(requirement);
             return Task.CompletedTask;
         }
     }
diff --git a/Jellyfin.Api/Auth/IgnoreParentalControlPolicy/IgnoreParentalControlHandler.cs b/Jellyfin.Api/Auth/IgnoreParentalControlPolicy/IgnoreParentalControlHandler.cs
index 5213bc4cb..a7623556a 100644
--- a/Jellyfin.Api/Auth/IgnoreParentalControlPolicy/IgnoreParentalControlHandler.cs
+++ b/Jellyfin.Api/Auth/IgnoreParentalControlPolicy/IgnoreParentalControlHandler.cs
@@ -29,13 +29,15 @@ namespace Jellyfin.Api.Auth.IgnoreParentalControlPolicy
         protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, IgnoreParentalControlRequirement requirement)
         {
             var validated = ValidateClaims(context.User, ignoreSchedule: true);
-            if (!validated)
+            if (validated)
+            {
+                context.Succeed(requirement);
+            }
+            else
             {
                 context.Fail();
-                return Task.CompletedTask;
             }
 
-            context.Succeed(requirement);
             return Task.CompletedTask;
         }
     }
diff --git a/Jellyfin.Api/Auth/LocalAccessPolicy/LocalAccessHandler.cs b/Jellyfin.Api/Auth/LocalAccessPolicy/LocalAccessHandler.cs
index af73352bc..d772ec554 100644
--- a/Jellyfin.Api/Auth/LocalAccessPolicy/LocalAccessHandler.cs
+++ b/Jellyfin.Api/Auth/LocalAccessPolicy/LocalAccessHandler.cs
@@ -29,13 +29,13 @@ namespace Jellyfin.Api.Auth.LocalAccessPolicy
         protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, LocalAccessRequirement requirement)
         {
             var validated = ValidateClaims(context.User, localAccessOnly: true);
-            if (!validated)
+            if (validated)
             {
-                context.Fail();
+                context.Succeed(requirement);
             }
             else
             {
-                context.Succeed(requirement);
+                context.Fail();
             }
 
             return Task.CompletedTask;
diff --git a/Jellyfin.Api/Constants/InternalClaimTypes.cs b/Jellyfin.Api/Constants/InternalClaimTypes.cs
index 4d7c7135d..8323312e5 100644
--- a/Jellyfin.Api/Constants/InternalClaimTypes.cs
+++ b/Jellyfin.Api/Constants/InternalClaimTypes.cs
@@ -34,5 +34,10 @@
         /// Token.
         /// </summary>
         public const string Token = "Jellyfin-Token";
+
+        /// <summary>
+        /// Is Api Key.
+        /// </summary>
+        public const string IsApiKey = "Jellyfin-IsApiKey";
     }
 }
diff --git a/Jellyfin.Api/Helpers/ClaimHelpers.cs b/Jellyfin.Api/Helpers/ClaimHelpers.cs
index df235ced2..29e6b4193 100644
--- a/Jellyfin.Api/Helpers/ClaimHelpers.cs
+++ b/Jellyfin.Api/Helpers/ClaimHelpers.cs
@@ -63,6 +63,19 @@ namespace Jellyfin.Api.Helpers
         public static string? GetToken(in ClaimsPrincipal user)
             => GetClaimValue(user, InternalClaimTypes.Token);
 
+        /// <summary>
+        /// Gets a flag specifying whether the request is using an api key.
+        /// </summary>
+        /// <param name="user">Current claims principal.</param>
+        /// <returns>The flag specifying whether the request is using an api key.</returns>
+        public static bool GetIsApiKey(in ClaimsPrincipal user)
+        {
+            var claimValue = GetClaimValue(user, InternalClaimTypes.IsApiKey);
+            return !string.IsNullOrEmpty(claimValue)
+                   && bool.TryParse(claimValue, out var parsedClaimValue)
+                   && parsedClaimValue;
+        }
+
         private static string? GetClaimValue(in ClaimsPrincipal user, string name)
         {
             return user?.Identities
diff --git a/MediaBrowser.Controller/Net/AuthorizationInfo.cs b/MediaBrowser.Controller/Net/AuthorizationInfo.cs
index 735c46ef8..5c642edff 100644
--- a/MediaBrowser.Controller/Net/AuthorizationInfo.cs
+++ b/MediaBrowser.Controller/Net/AuthorizationInfo.cs
@@ -1,10 +1,11 @@
-#pragma warning disable CS1591
-
 using System;
 using Jellyfin.Data.Entities;
 
 namespace MediaBrowser.Controller.Net
 {
+    /// <summary>
+    /// The request authorization info.
+    /// </summary>
     public class AuthorizationInfo
     {
         /// <summary>
@@ -43,6 +44,14 @@ namespace MediaBrowser.Controller.Net
         /// <value>The token.</value>
         public string Token { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether the authorization is from an api key.
+        /// </summary>
+        public bool IsApiKey { get; set; }
+
+        /// <summary>
+        /// Gets or sets the user making the request.
+        /// </summary>
         public User User { get; set; }
     }
 }
diff --git a/tests/Jellyfin.Api.Tests/Auth/CustomAuthenticationHandlerTests.cs b/tests/Jellyfin.Api.Tests/Auth/CustomAuthenticationHandlerTests.cs
index 4ea5094b6..33534abd2 100644
--- a/tests/Jellyfin.Api.Tests/Auth/CustomAuthenticationHandlerTests.cs
+++ b/tests/Jellyfin.Api.Tests/Auth/CustomAuthenticationHandlerTests.cs
@@ -128,6 +128,7 @@ namespace Jellyfin.Api.Tests.Auth
             var authorizationInfo = _fixture.Create<AuthorizationInfo>();
             authorizationInfo.User = _fixture.Create<User>();
             authorizationInfo.User.SetPermission(PermissionKind.IsAdministrator, isAdmin);
+            authorizationInfo.IsApiKey = false;
 
             _jellyfinAuthServiceMock.Setup(
                     a => a.Authenticate(
diff --git a/tests/Jellyfin.Api.Tests/TestHelpers.cs b/tests/Jellyfin.Api.Tests/TestHelpers.cs
index a4dd4e409..c4ce39885 100644
--- a/tests/Jellyfin.Api.Tests/TestHelpers.cs
+++ b/tests/Jellyfin.Api.Tests/TestHelpers.cs
@@ -45,7 +45,7 @@ namespace Jellyfin.Api.Tests
             {
                 new Claim(ClaimTypes.Role, role),
                 new Claim(ClaimTypes.Name, "jellyfin"),
-                new Claim(InternalClaimTypes.UserId, Guid.Empty.ToString("N", CultureInfo.InvariantCulture)),
+                new Claim(InternalClaimTypes.UserId, Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture)),
                 new Claim(InternalClaimTypes.DeviceId, Guid.Empty.ToString("N", CultureInfo.InvariantCulture)),
                 new Claim(InternalClaimTypes.Device, "test"),
                 new Claim(InternalClaimTypes.Client, "test"),