From 07cc4be6a747cfea40ee7a540385d6d8f38de726 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Sun, 23 Feb 2020 12:11:43 +0100 Subject: Fix some warnings * Add analyzers to MediaBrowser.XbmcMetadata * Enable TreatWarningsAsErrors for MediaBrowser.XbmcMetadata * Add analyzers to MediaBrowser.WebDashboard * Enable TreatWarningsAsErrors for MediaBrowser.WebDashboard * Disable SA1600 in favor of CS1591 --- Emby.Server.Implementations/HttpServer/Security/AuthService.cs | 1 - 1 file changed, 1 deletion(-) (limited to 'Emby.Server.Implementations/HttpServer/Security/AuthService.cs') diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs index 03b5b748d..58421aaf1 100644 --- a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs +++ b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs @@ -1,5 +1,4 @@ #pragma warning disable CS1591 -#pragma warning disable SA1600 using System; using System.Linq; -- cgit v1.2.3 From 6d35dd6b326b98995e363c64083a2ca46b2582fd Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 13 Apr 2020 13:13:48 -0400 Subject: Clean up SecurityException - Remove unused SecurityExceptionType - Add missing constructor for InnerException - Add missing documentation --- .../HttpServer/Security/AuthService.cs | 30 ++++---------------- MediaBrowser.Api/UserService.cs | 2 +- MediaBrowser.Controller/Net/SecurityException.cs | 32 ++++++++++++++++------ 3 files changed, 31 insertions(+), 33 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/Security/AuthService.cs') diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs index 58421aaf1..1360a5e0c 100644 --- a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs +++ b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs @@ -108,18 +108,12 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (user.Policy.IsDisabled) { - throw new SecurityException("User account has been disabled.") - { - SecurityExceptionType = SecurityExceptionType.Unauthenticated - }; + throw new SecurityException("User account has been disabled."); } if (!user.Policy.EnableRemoteAccess && !_networkManager.IsInLocalNetwork(request.RemoteIp)) { - throw new SecurityException("User account has been disabled.") - { - SecurityExceptionType = SecurityExceptionType.Unauthenticated - }; + throw new SecurityException("User account has been disabled."); } if (!user.Policy.IsAdministrator @@ -128,10 +122,7 @@ namespace Emby.Server.Implementations.HttpServer.Security { request.Response.Headers.Add("X-Application-Error-Code", "ParentalControl"); - throw new SecurityException("This user account is not allowed access at this time.") - { - SecurityExceptionType = SecurityExceptionType.ParentalControl - }; + throw new SecurityException("This user account is not allowed access at this time."); } } @@ -190,10 +181,7 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (user == null || !user.Policy.IsAdministrator) { - throw new SecurityException("User does not have admin access.") - { - SecurityExceptionType = SecurityExceptionType.Unauthenticated - }; + throw new SecurityException("User does not have admin access."); } } @@ -201,10 +189,7 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (user == null || !user.Policy.EnableContentDeletion) { - throw new SecurityException("User does not have delete access.") - { - SecurityExceptionType = SecurityExceptionType.Unauthenticated - }; + throw new SecurityException("User does not have delete access."); } } @@ -212,10 +197,7 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (user == null || !user.Policy.EnableContentDownloading) { - throw new SecurityException("User does not have download access.") - { - SecurityExceptionType = SecurityExceptionType.Unauthenticated - }; + throw new SecurityException("User does not have download access."); } } } diff --git a/MediaBrowser.Api/UserService.cs b/MediaBrowser.Api/UserService.cs index 401514349..78fc6c694 100644 --- a/MediaBrowser.Api/UserService.cs +++ b/MediaBrowser.Api/UserService.cs @@ -426,7 +426,7 @@ namespace MediaBrowser.Api catch (SecurityException e) { // rethrow adding IP address to message - throw new SecurityException($"[{Request.RemoteIp}] {e.Message}"); + throw new SecurityException($"[{Request.RemoteIp}] {e.Message}", e); } } diff --git a/MediaBrowser.Controller/Net/SecurityException.cs b/MediaBrowser.Controller/Net/SecurityException.cs index 3ccecf0eb..a5b94ea5e 100644 --- a/MediaBrowser.Controller/Net/SecurityException.cs +++ b/MediaBrowser.Controller/Net/SecurityException.cs @@ -2,20 +2,36 @@ using System; namespace MediaBrowser.Controller.Net { + /// + /// The exception that is thrown when a user is authenticated, but not authorized to access a requested resource. + /// public class SecurityException : Exception { + /// + /// Initializes a new instance of the class. + /// + public SecurityException() + : base() + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The message that describes the error. public SecurityException(string message) : base(message) { - } - public SecurityExceptionType SecurityExceptionType { get; set; } - } - - public enum SecurityExceptionType - { - Unauthenticated = 0, - ParentalControl = 1 + /// + /// Initializes a new instance of the class. + /// + /// The message that describes the error + /// The exception that is the cause of the current exception, or a null reference if no inner exception is specified. + public SecurityException(string message, Exception innerException) + : base(message, innerException) + { + } } } -- cgit v1.2.3 From 53380689ad00f00efc0c1790f1d25d08c95d7f2d Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 13 Apr 2020 13:17:46 -0400 Subject: Return correct status codes for authentication and authorization errors - Use AuthenticatonException to return 401 - Use SecurityException to return 403 - Update existing throws to throw the correct exception for the circumstance --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 5 ++++- .../HttpServer/Security/AuthService.cs | 7 ++++--- Emby.Server.Implementations/Library/UserManager.cs | 11 ++++------- Emby.Server.Implementations/Session/SessionManager.cs | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/Security/AuthService.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 5ae65a4e3..f496ff1ba 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -14,6 +14,7 @@ using Emby.Server.Implementations.Services; using MediaBrowser.Common.Extensions; using MediaBrowser.Common.Net; using MediaBrowser.Controller; +using MediaBrowser.Controller.Authentication; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Net; using MediaBrowser.Model.Events; @@ -230,7 +231,8 @@ namespace Emby.Server.Implementations.HttpServer switch (ex) { case ArgumentException _: return 400; - case SecurityException _: return 401; + case AuthenticationException _: return 401; + case SecurityException _: return 403; case DirectoryNotFoundException _: case FileNotFoundException _: case ResourceNotFoundException _: return 404; @@ -550,6 +552,7 @@ namespace Emby.Server.Implementations.HttpServer || ex is IOException || ex is OperationCanceledException || ex is SecurityException + || ex is AuthenticationException || ex is FileNotFoundException; await ErrorHandler(ex, httpReq, !ignoreStackTrace, urlToLog).ConfigureAwait(false); } diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs index 1360a5e0c..256b24924 100644 --- a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs +++ b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs @@ -2,6 +2,7 @@ using System; using System.Linq; +using System.Security.Authentication; using Emby.Server.Implementations.SocketSharp; using MediaBrowser.Common.Net; using MediaBrowser.Controller.Configuration; @@ -68,7 +69,7 @@ namespace Emby.Server.Implementations.HttpServer.Security if (user == null && auth.UserId != Guid.Empty) { - throw new SecurityException("User with Id " + auth.UserId + " not found"); + throw new AuthenticationException("User with Id " + auth.UserId + " not found"); } if (user != null) @@ -212,14 +213,14 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (string.IsNullOrEmpty(token)) { - throw new SecurityException("Access token is required."); + throw new AuthenticationException("Access token is required."); } var info = GetTokenInfo(request); if (info == null) { - throw new SecurityException("Access token is invalid or expired."); + throw new AuthenticationException("Access token is invalid or expired."); } //if (!string.IsNullOrEmpty(info.UserId)) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 7b17cc913..f92cb6ae6 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -20,6 +20,7 @@ using MediaBrowser.Controller.Drawing; using MediaBrowser.Controller.Dto; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Net; using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Plugins; using MediaBrowser.Controller.Providers; @@ -324,21 +325,17 @@ namespace Emby.Server.Implementations.Library if (user.Policy.IsDisabled) { - throw new AuthenticationException( - string.Format( - CultureInfo.InvariantCulture, - "The {0} account is currently disabled. Please consult with your administrator.", - user.Name)); + throw new SecurityException($"The {user.Name} account is currently disabled. Please consult with your administrator."); } if (!user.Policy.EnableRemoteAccess && !_networkManager.IsInLocalNetwork(remoteEndPoint)) { - throw new AuthenticationException("Forbidden."); + throw new SecurityException("Forbidden."); } if (!user.IsParentalScheduleAllowed()) { - throw new AuthenticationException("User is not allowed access at this time."); + throw new SecurityException("User is not allowed access at this time."); } // Update LastActivityDate and LastLoginDate, then save diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index de768333d..c93c02c48 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1414,7 +1414,7 @@ namespace Emby.Server.Implementations.Session if (user == null) { AuthenticationFailed?.Invoke(this, new GenericEventArgs(request)); - throw new SecurityException("Invalid username or password entered."); + throw new AuthenticationException("Invalid username or password entered."); } if (!string.IsNullOrEmpty(request.DeviceId) -- cgit v1.2.3