From c534c450330759f6595c9601e3fe8b12e6987e69 Mon Sep 17 00:00:00 2001 From: Cody Robibero Date: Wed, 27 Oct 2021 19:20:14 -0600 Subject: [PATCH] Suggestions from review --- .../Controllers/ClientLogController.cs | 53 ++++++++++++++++--- .../ClientEvent/ClientEventLogger.cs | 6 +-- .../ClientEvent/IClientEventLogger.cs | 7 +-- .../ClientLog/ClientLogEvent.cs | 2 +- .../Configuration/ServerConfiguration.cs | 5 ++ 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/Jellyfin.Api/Controllers/ClientLogController.cs b/Jellyfin.Api/Controllers/ClientLogController.cs index 9fe3bf731c..aac3f6a735 100644 --- a/Jellyfin.Api/Controllers/ClientLogController.cs +++ b/Jellyfin.Api/Controllers/ClientLogController.cs @@ -1,7 +1,11 @@ -using System.Threading.Tasks; +using System.Net.Mime; +using System.Threading.Tasks; +using Jellyfin.Api.Attributes; using Jellyfin.Api.Constants; using Jellyfin.Api.Models.ClientLogDtos; using MediaBrowser.Controller.ClientEvent; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Net; using MediaBrowser.Model.ClientLog; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; @@ -15,15 +19,25 @@ namespace Jellyfin.Api.Controllers [Authorize(Policy = Policies.DefaultAuthorization)] public class ClientLogController : BaseJellyfinApiController { + private const int MaxDocumentSize = 1_000_000; private readonly IClientEventLogger _clientEventLogger; + private readonly IAuthorizationContext _authorizationContext; + private readonly IServerConfigurationManager _serverConfigurationManager; /// /// Initializes a new instance of the class. /// /// Instance of the interface. - public ClientLogController(IClientEventLogger clientEventLogger) + /// Instance of the interface. + /// Instance of the interface. + public ClientLogController( + IClientEventLogger clientEventLogger, + IAuthorizationContext authorizationContext, + IServerConfigurationManager serverConfigurationManager) { _clientEventLogger = clientEventLogger; + _authorizationContext = authorizationContext; + _serverConfigurationManager = serverConfigurationManager; } /// @@ -36,6 +50,11 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status204NoContent)] public ActionResult LogEvent([FromBody] ClientLogEventDto clientLogEventDto) { + if (!_serverConfigurationManager.Configuration.AllowClientLogUpload) + { + return Forbid(); + } + Log(clientLogEventDto); return NoContent(); } @@ -50,6 +69,11 @@ namespace Jellyfin.Api.Controllers [ProducesResponseType(StatusCodes.Status204NoContent)] public ActionResult LogEvents([FromBody] ClientLogEventDto[] clientLogEventDtos) { + if (!_serverConfigurationManager.Configuration.AllowClientLogUpload) + { + return Forbid(); + } + foreach (var dto in clientLogEventDtos) { Log(dto); @@ -59,15 +83,30 @@ namespace Jellyfin.Api.Controllers } /// - /// Upload a log file. + /// Upload a document. /// - /// The file. /// Submission status. - [HttpPost("File")] + [HttpPost("Document")] [ProducesResponseType(StatusCodes.Status204NoContent)] - public async Task LogFile(IFormFile file) + [AcceptsFile(MediaTypeNames.Text.Plain)] + [RequestSizeLimit(MaxDocumentSize)] + public async Task LogFile() { - await _clientEventLogger.WriteFileAsync(file.FileName, file.OpenReadStream()) + if (!_serverConfigurationManager.Configuration.AllowClientLogUpload) + { + return Forbid(); + } + + if (Request.ContentLength > MaxDocumentSize) + { + // Manually validate to return proper status code. + return StatusCode(StatusCodes.Status413PayloadTooLarge, $"Payload must be less than {MaxDocumentSize:N0} bytes"); + } + + var authorizationInfo = await _authorizationContext.GetAuthorizationInfo(Request) + .ConfigureAwait(false); + + await _clientEventLogger.WriteDocumentAsync(authorizationInfo, Request.Body) .ConfigureAwait(false); return NoContent(); } diff --git a/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs b/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs index bdc1a7eff1..61f7adff32 100644 --- a/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs +++ b/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Threading.Tasks; +using MediaBrowser.Controller.Net; using MediaBrowser.Model.ClientLog; using Microsoft.Extensions.Logging; @@ -43,10 +44,9 @@ namespace MediaBrowser.Controller.ClientEvent } /// - public async Task WriteFileAsync(string fileName, Stream fileContents) + public async Task WriteDocumentAsync(AuthorizationInfo authorizationInfo, Stream fileContents) { - // Force naming convention: upload_YYYYMMDD_$name - fileName = $"upload_{DateTime.UtcNow:yyyyMMdd}_{fileName}"; + var fileName = $"upload_{authorizationInfo.Client}_{authorizationInfo.Version}_{DateTime.UtcNow:yyyyMMddHHmmss}.log"; var logFilePath = Path.Combine(_applicationPaths.LogDirectoryPath, fileName); await using var fileStream = new FileStream(logFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); await fileContents.CopyToAsync(fileStream).ConfigureAwait(false); diff --git a/MediaBrowser.Controller/ClientEvent/IClientEventLogger.cs b/MediaBrowser.Controller/ClientEvent/IClientEventLogger.cs index 7cd71a60d6..ee8e5806b7 100644 --- a/MediaBrowser.Controller/ClientEvent/IClientEventLogger.cs +++ b/MediaBrowser.Controller/ClientEvent/IClientEventLogger.cs @@ -1,5 +1,6 @@ using System.IO; using System.Threading.Tasks; +using MediaBrowser.Controller.Net; using MediaBrowser.Model.ClientLog; namespace MediaBrowser.Controller.ClientEvent @@ -18,9 +19,9 @@ namespace MediaBrowser.Controller.ClientEvent /// /// Writes a file to the log directory. /// - /// The file name. - /// The file contents. + /// The current authorization info. + /// The file contents to write. /// A representing the asynchronous operation. - Task WriteFileAsync(string fileName, Stream fileContents); + Task WriteDocumentAsync(AuthorizationInfo authorizationInfo, Stream fileContents); } } diff --git a/MediaBrowser.Model/ClientLog/ClientLogEvent.cs b/MediaBrowser.Model/ClientLog/ClientLogEvent.cs index e4ee881454..21087b5647 100644 --- a/MediaBrowser.Model/ClientLog/ClientLogEvent.cs +++ b/MediaBrowser.Model/ClientLog/ClientLogEvent.cs @@ -72,4 +72,4 @@ namespace MediaBrowser.Model.ClientLog /// public string Message { get; } } -} \ No newline at end of file +} diff --git a/MediaBrowser.Model/Configuration/ServerConfiguration.cs b/MediaBrowser.Model/Configuration/ServerConfiguration.cs index d1e9996665..37dc49d7a8 100644 --- a/MediaBrowser.Model/Configuration/ServerConfiguration.cs +++ b/MediaBrowser.Model/Configuration/ServerConfiguration.cs @@ -459,5 +459,10 @@ namespace MediaBrowser.Model.Configuration /// Gets or sets a value indicating whether older plugins should automatically be deleted from the plugin folder. /// public bool RemoveOldPlugins { get; set; } + + /// + /// Gets or sets a value indicating whether clients should be allowed to upload logs. + /// + public bool AllowClientLogUpload { get; set; } } }