From c792420c15cb91da47f2bfdad2c242678eb30955 Mon Sep 17 00:00:00 2001 From: Leon Segal Date: Thu, 6 Mar 2025 12:12:58 +0100 Subject: [PATCH] #192 use attribute-based authorization on controllers --- .../IWopiAuthorizationRequirement.cs | 41 ++++++ .../IWopiSecurityHandler.cs | 5 +- .../WopiAuthorizationRequirement.cs | 18 --- .../WopiCheckFileInfo.cs | 1 + .../Controllers/ContainersController.cs | 5 +- .../Controllers/FilesController.cs | 38 ++---- .../Authentication/AccessTokenHandler.cs | 6 + .../Authorization/WopiAuthorizationHandler.cs | 21 +++- .../Authorization/WopiAuthorizeAttribute.cs | 42 +++++++ src/WopiHost.Core/Security/FileResource.cs | 16 --- src/WopiHost.Core/Security/WopiOperations.cs | 29 ----- .../WopiFileSystemProvider.cs | 5 +- .../WopiSecurityHandler.cs | 2 +- src/WopiHost/Startup.cs | 119 ------------------ .../Controllers/FilesControllerTests.cs | 30 ----- .../Authentication/AccessTokenHandlerTests.cs | 21 ++++ .../WopiAuthorizationHandlerTests.cs | 95 ++++++++++++++ .../WopiHost.Core.Tests.csproj | 1 + 18 files changed, 238 insertions(+), 257 deletions(-) create mode 100644 src/WopiHost.Abstractions/IWopiAuthorizationRequirement.cs delete mode 100644 src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs create mode 100644 src/WopiHost.Core/Security/Authorization/WopiAuthorizeAttribute.cs delete mode 100644 src/WopiHost.Core/Security/FileResource.cs delete mode 100644 src/WopiHost.Core/Security/WopiOperations.cs delete mode 100644 src/WopiHost/Startup.cs create mode 100644 test/WopiHost.Core.Tests/Security/Authorization/WopiAuthorizationHandlerTests.cs diff --git a/src/WopiHost.Abstractions/IWopiAuthorizationRequirement.cs b/src/WopiHost.Abstractions/IWopiAuthorizationRequirement.cs new file mode 100644 index 00000000..cafe80af --- /dev/null +++ b/src/WopiHost.Abstractions/IWopiAuthorizationRequirement.cs @@ -0,0 +1,41 @@ +namespace WopiHost.Abstractions; + +/// +/// The Wopi resource type +/// +public enum WopiResourceType +{ + /// + /// + /// + File, + /// + /// + /// + Container +}; + + +/// +/// Represents an authorization requirement for a given combination of resource, user, and action. +/// +/// +/// Creates an instance of initialized +/// +public interface IWopiAuthorizationRequirement +{ + /// + /// Gets a permissions required for a given combination of resource, user, and action. + /// + Permission Permission { get; } + + /// + /// Gets the type of the resource. + /// + WopiResourceType ResourceType { get; } + + /// + /// Gets or sets the identifier of the resource. + /// + string? ResourceId { get; set; } +} diff --git a/src/WopiHost.Abstractions/IWopiSecurityHandler.cs b/src/WopiHost.Abstractions/IWopiSecurityHandler.cs index be49ac6d..1f5b7cd2 100644 --- a/src/WopiHost.Abstractions/IWopiSecurityHandler.cs +++ b/src/WopiHost.Abstractions/IWopiSecurityHandler.cs @@ -29,11 +29,10 @@ public interface IWopiSecurityHandler /// Verifies whether the given principal is authorized to perform a given operation on the given resource. /// /// User principal object - /// Identifier of a resource - /// Type of operation to be performed + /// Type of operation to be performed /// Cancellation token /// TRUE if the given principal is authorized to perform a given operation on the given resource. - Task IsAuthorized(ClaimsPrincipal principal, string resourceId, WopiAuthorizationRequirement operation, CancellationToken cancellationToken = default); + Task IsAuthorized(ClaimsPrincipal principal, IWopiAuthorizationRequirement requirement, CancellationToken cancellationToken = default); /// /// Returns a string representation of a diff --git a/src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs b/src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs deleted file mode 100644 index 7288318d..00000000 --- a/src/WopiHost.Abstractions/WopiAuthorizationRequirement.cs +++ /dev/null @@ -1,18 +0,0 @@ -using Microsoft.AspNetCore.Authorization; - -namespace WopiHost.Abstractions; - -/// -/// Represents an authorization requirement for a given combination of resource, user, and action. -/// -/// -/// Creates an instance of initialized with . -/// -/// Permissions required for a given combination of resource, user, and action. -public class WopiAuthorizationRequirement(Permission permission) : IAuthorizationRequirement -{ - /// - /// Gets a permissions required for a given combination of resource, user, and action. - /// - public Permission Permission { get; } = permission; -} diff --git a/src/WopiHost.Abstractions/WopiCheckFileInfo.cs b/src/WopiHost.Abstractions/WopiCheckFileInfo.cs index 05b120c6..bd2a0213 100644 --- a/src/WopiHost.Abstractions/WopiCheckFileInfo.cs +++ b/src/WopiHost.Abstractions/WopiCheckFileInfo.cs @@ -457,6 +457,7 @@ public class WopiCheckFileInfo : IWopiHostCapabilities /// A string value indicating the domain that the host page is sending and receiving PostMessages to and from. /// Microsoft 365 for the web only sends outgoing PostMessages to this domain, and only listens to PostMessages from this domain. /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? PostMessageOrigin { get; set; } /// diff --git a/src/WopiHost.Core/Controllers/ContainersController.cs b/src/WopiHost.Core/Controllers/ContainersController.cs index 32dc8c2e..f89f2d77 100644 --- a/src/WopiHost.Core/Controllers/ContainersController.cs +++ b/src/WopiHost.Core/Controllers/ContainersController.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Options; using WopiHost.Abstractions; using WopiHost.Core.Models; +using WopiHost.Core.Security.Authorization; namespace WopiHost.Core.Controllers; @@ -18,7 +19,7 @@ namespace WopiHost.Core.Controllers; /// WOPI Host configuration [Route("wopi/[controller]")] public class ContainersController( - IWopiStorageProvider storageProvider, + IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptions wopiHostOptions) : WopiControllerBase(storageProvider, securityHandler, wopiHostOptions) { @@ -31,6 +32,7 @@ public class ContainersController( /// [HttpGet("{id}")] [Produces(MediaTypeNames.Application.Json)] + [WopiAuthorize(Permission.Read, WopiResourceType.Container)] public CheckContainerInfo GetCheckContainerInfo(string id) { var container = StorageProvider.GetWopiContainer(id); @@ -48,6 +50,7 @@ public CheckContainerInfo GetCheckContainerInfo(string id) /// Container identifier. /// [HttpGet("{id}/children")] + [WopiAuthorize(Permission.Read, WopiResourceType.Container)] [Produces(MediaTypeNames.Application.Json)] public Container EnumerateChildren(string id) { diff --git a/src/WopiHost.Core/Controllers/FilesController.cs b/src/WopiHost.Core/Controllers/FilesController.cs index 40e43ff5..ce3f1026 100644 --- a/src/WopiHost.Core/Controllers/FilesController.cs +++ b/src/WopiHost.Core/Controllers/FilesController.cs @@ -1,7 +1,6 @@ using System.Net.Mime; using System.Security.Claims; using System.Text.Json; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; @@ -11,7 +10,7 @@ using WopiHost.Core.Infrastructure; using WopiHost.Core.Models; using WopiHost.Core.Results; -using WopiHost.Core.Security; +using WopiHost.Core.Security.Authorization; namespace WopiHost.Core.Controllers; @@ -27,7 +26,6 @@ public class FilesController : WopiControllerBase /// private readonly ICobaltProcessor? cobaltProcessor; private readonly IWopiLockProvider? lockProvider; - private readonly IAuthorizationService authorizationService; private WopiHostCapabilities HostCapabilities => new() { SupportsCobalt = cobaltProcessor is not null, @@ -45,18 +43,15 @@ public class FilesController : WopiControllerBase /// Storage provider instance for retrieving files and folders. /// Security handler instance for performing security-related operations. /// WOPI Host configuration - /// An instance of authorization service capable of resource-based authorization. /// An instance of the lock provider. /// An instance of a MS-FSSHTTP processor. public FilesController( IWopiStorageProvider storageProvider, IWopiSecurityHandler securityHandler, IOptions wopiHostOptions, - IAuthorizationService authorizationService, IWopiLockProvider? lockProvider = null, ICobaltProcessor? cobaltProcessor = null) : base(storageProvider, securityHandler, wopiHostOptions) { - this.authorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); this.lockProvider = lockProvider; this.cobaltProcessor = cobaltProcessor; } @@ -70,13 +65,9 @@ public FilesController( /// Cancellation token /// [HttpGet("{id}")] + [WopiAuthorize(Permission.Read, WopiResourceType.File)] public async Task GetCheckFileInfo(string id, CancellationToken cancellationToken = default) { - if (!(await authorizationService.AuthorizeAsync(User, new FileResource(id), WopiOperations.Read)).Succeeded) - { - return Unauthorized(); - } - // Get file var file = StorageProvider.GetWopiFile(id); if (file is null) @@ -107,17 +98,12 @@ public async Task GetCheckFileInfo(string id, CancellationToken c /// Cancellation token /// FileStreamResult [HttpGet("{id}/contents")] + [WopiAuthorize(Permission.Read, WopiResourceType.File)] public async Task GetFile( string id, [FromHeader(Name = WopiHeaders.MAX_EXPECTED_SIZE)] int? maximumExpectedSize = null, CancellationToken cancellationToken = default) { - // Check permissions - if (!(await authorizationService.AuthorizeAsync(User, new FileResource(id), WopiOperations.Read)).Succeeded) - { - return Unauthorized(); - } - // Get file var file = StorageProvider.GetWopiFile(id); if (file is null) @@ -157,20 +143,13 @@ public async Task GetFile( /// Returns if succeeded. [HttpPut("{id}/contents")] [HttpPost("{id}/contents")] + [WopiAuthorize(Permission.Update, WopiResourceType.File)] public async Task PutFile( string id, [FromHeader(Name = WopiHeaders.LOCK)] string? newLockIdentifier = null, CancellationToken cancellationToken = default) { - // Check permissions - var authorizationResult = await authorizationService.AuthorizeAsync(User, new FileResource(id), WopiOperations.Update); - - if (!authorizationResult.Succeeded) - { - return Unauthorized(); - } - - // https://learn.microsoft.com/microsoft-365/cloud-storage-partner-program/online/scenarios/createnew + // https://learn.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/online/scenarios/createnew var file = StorageProvider.GetWopiFile(id); // If ... missing altogether, the host should respond with a 409 Conflict if (file is null) @@ -217,6 +196,7 @@ await HttpContext.Request.Body.CopyToAsync( /// File identifier. /// Returns if succeeded. [HttpPost("{id}"), WopiOverrideHeader([WopiFileOperations.PutRelativeFile])] + [WopiAuthorize(Permission.Update, WopiResourceType.File)] public Task PutRelativeFile(string id) => throw new NotImplementedException($"{nameof(PutRelativeFile)} is not implemented yet."); /// @@ -228,16 +208,12 @@ await HttpContext.Request.Body.CopyToAsync( /// File identifier. /// [HttpPost("{id}"), WopiOverrideHeader([WopiFileOperations.Cobalt])] + [WopiAuthorize(Permission.Update, WopiResourceType.File)] public async Task ProcessCobalt( string id, [FromHeader(Name = WopiHeaders.CORRELATION_ID)] string? correlationId = null) { ArgumentNullException.ThrowIfNull(cobaltProcessor); - // Check permissions - if (!(await authorizationService.AuthorizeAsync(User, new FileResource(id), WopiOperations.Update)).Succeeded) - { - return Unauthorized(); - } var file = StorageProvider.GetWopiFile(id); diff --git a/src/WopiHost.Core/Security/Authentication/AccessTokenHandler.cs b/src/WopiHost.Core/Security/Authentication/AccessTokenHandler.cs index 5878d0ec..deb44f03 100644 --- a/src/WopiHost.Core/Security/Authentication/AccessTokenHandler.cs +++ b/src/WopiHost.Core/Security/Authentication/AccessTokenHandler.cs @@ -33,6 +33,11 @@ protected override async Task HandleAuthenticateAsync() { //TODO: implement access_token_ttl https://learn.microsoft.com/openspecs/office_protocols/ms-wopi/adb48ba9-118a-43b6-82d7-9a508aad1583 + if (Context.Request.Path.Value?.StartsWith("/wopi", StringComparison.OrdinalIgnoreCase) != true) + { + return AuthenticateResult.NoResult(); + } + var token = Context.Request.Query[AccessTokenDefaults.ACCESS_TOKEN_QUERY_NAME].ToString(); if (Context.Request.Path.Value == "/wopibootstrapper") @@ -43,6 +48,7 @@ protected override async Task HandleAuthenticateAsync() await securityHandler.GenerateAccessToken("Anonymous", Convert.ToBase64String(Encoding.UTF8.GetBytes(".\\")))); } + if (!string.IsNullOrEmpty(token)) { var principal = await securityHandler.GetPrincipal(token); diff --git a/src/WopiHost.Core/Security/Authorization/WopiAuthorizationHandler.cs b/src/WopiHost.Core/Security/Authorization/WopiAuthorizationHandler.cs index a09770c4..8f5b2035 100644 --- a/src/WopiHost.Core/Security/Authorization/WopiAuthorizationHandler.cs +++ b/src/WopiHost.Core/Security/Authorization/WopiAuthorizationHandler.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; using WopiHost.Abstractions; namespace WopiHost.Core.Security.Authorization; @@ -10,18 +11,26 @@ namespace WopiHost.Core.Security.Authorization; /// Creates an instance of . /// /// AuthNZ handler. -public class WopiAuthorizationHandler(IWopiSecurityHandler securityHandler) - : AuthorizationHandler +public class WopiAuthorizationHandler(IWopiSecurityHandler securityHandler) + : AuthorizationHandler { /// /// Performs resource-based authorization check. /// /// Context of the - /// Security requirement to be fulfilled (e.g. a permission). - /// Resource to check the security authorization requirement against. - protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, WopiAuthorizationRequirement requirement, FileResource resource) + /// Security requirement to be fulfilled (a permission on which resource). + /// httpContext resource + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, WopiAuthorizeAttribute requirement, HttpContext resource) { - if (await securityHandler.IsAuthorized(context.User, resource.FileId, requirement)) + // try to retrieve resource identifier from the route + if (resource.Request.RouteValues.TryGetValue("id", out var fileIdRaw) && + fileIdRaw is not null) + { + requirement.ResourceId = fileIdRaw.ToString(); + } + + // check if the user is authorized + if (await securityHandler.IsAuthorized(context.User, requirement)) { context.Succeed(requirement); } diff --git a/src/WopiHost.Core/Security/Authorization/WopiAuthorizeAttribute.cs b/src/WopiHost.Core/Security/Authorization/WopiAuthorizeAttribute.cs new file mode 100644 index 00000000..e083bbc7 --- /dev/null +++ b/src/WopiHost.Core/Security/Authorization/WopiAuthorizeAttribute.cs @@ -0,0 +1,42 @@ +using Microsoft.AspNetCore.Authorization; +using WopiHost.Abstractions; + +namespace WopiHost.Core.Security.Authorization; + +/// +/// Represents an authorization requirement for a given combination of resource, user, and action. +/// +/// +/// Creates an instance of initialized with . +/// +/// Permissions required for a given combination of resource, user, and action. +/// type of the resource. +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)] +public class WopiAuthorizeAttribute( + Permission permission, + WopiResourceType resourceType) : AuthorizeAttribute, IWopiAuthorizationRequirement, IAuthorizationRequirement, IAuthorizationRequirementData +{ + /// + /// Gets a permissions required for a given combination of resource, user, and action. + /// + public Permission Permission { get; } = permission; + + /// + /// Gets the type of the resource. + /// + public WopiResourceType ResourceType { get; } = resourceType; + + /// + /// Gets or sets the identifier of the resource. + /// + public string? ResourceId { get; set; } + + /// + /// Gets the requirements. + /// + /// + public IEnumerable GetRequirements() + { + yield return this; + } +} \ No newline at end of file diff --git a/src/WopiHost.Core/Security/FileResource.cs b/src/WopiHost.Core/Security/FileResource.cs deleted file mode 100644 index 14969032..00000000 --- a/src/WopiHost.Core/Security/FileResource.cs +++ /dev/null @@ -1,16 +0,0 @@ -namespace WopiHost.Core.Security; - -/// -/// Represents a resource for a resource-based authroization. -/// -/// -/// Creates an object representing a resource for a resource-based authroization. -/// -/// Identifier of a resource. -public class FileResource(string fileId) -{ - /// - /// Identifier of a resource. - /// - public string FileId { get; } = fileId; -} diff --git a/src/WopiHost.Core/Security/WopiOperations.cs b/src/WopiHost.Core/Security/WopiOperations.cs deleted file mode 100644 index 40f933d2..00000000 --- a/src/WopiHost.Core/Security/WopiOperations.cs +++ /dev/null @@ -1,29 +0,0 @@ -using WopiHost.Abstractions; - -namespace WopiHost.Core.Security; - -/// -/// Operations to be used with resource-based authorization. -/// -public static class WopiOperations -{ - /// - /// Given operation requires the user to have a create file permission. - /// - public static readonly WopiAuthorizationRequirement Create = new(Permission.Create); - - /// - /// Given operation requires the user to have a read file permission. - /// - public static readonly WopiAuthorizationRequirement Read = new(Permission.Read); - - /// - /// Given operation requires the user to have a update file permission. - /// - public static readonly WopiAuthorizationRequirement Update = new(Permission.Update); - - /// - /// Given operation requires the user to have a delete file permission. - /// - public static readonly WopiAuthorizationRequirement Delete = new(Permission.Delete); -} \ No newline at end of file diff --git a/src/WopiHost.FileSystemProvider/WopiFileSystemProvider.cs b/src/WopiHost.FileSystemProvider/WopiFileSystemProvider.cs index f0020ceb..36867e35 100644 --- a/src/WopiHost.FileSystemProvider/WopiFileSystemProvider.cs +++ b/src/WopiHost.FileSystemProvider/WopiFileSystemProvider.cs @@ -1,5 +1,4 @@ -using System.Security.Claims; -using System.Text; +using System.Text; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; using WopiHost.Abstractions; @@ -42,7 +41,7 @@ public WopiFileSystemProvider(IHostEnvironment env, IConfiguration configuration FileSystemProviderOptions = configuration.GetSection(WopiConfigurationSections.STORAGE_OPTIONS)? .Get() ?? throw new ArgumentNullException(nameof(configuration)); } - + /// public IWopiFile GetWopiFile(string identifier) { diff --git a/src/WopiHost.FileSystemProvider/WopiSecurityHandler.cs b/src/WopiHost.FileSystemProvider/WopiSecurityHandler.cs index 6cc05abf..e35a7cc1 100644 --- a/src/WopiHost.FileSystemProvider/WopiSecurityHandler.cs +++ b/src/WopiHost.FileSystemProvider/WopiSecurityHandler.cs @@ -104,7 +104,7 @@ public Task GenerateAccessToken(string userId, string resourceId, } /// - public Task IsAuthorized(ClaimsPrincipal principal, string resourceId, WopiAuthorizationRequirement operation, CancellationToken cancellationToken = default) + public Task IsAuthorized(ClaimsPrincipal principal, IWopiAuthorizationRequirement requirement, CancellationToken cancellationToken = default) { return Task.FromResult(principal.Identity?.IsAuthenticated == true); } diff --git a/src/WopiHost/Startup.cs b/src/WopiHost/Startup.cs deleted file mode 100644 index 3a4374a4..00000000 --- a/src/WopiHost/Startup.cs +++ /dev/null @@ -1,119 +0,0 @@ -using WopiHost.Abstractions; -using WopiHost.Core.Models; -using Serilog; -using WopiHost.Core.Extensions; -using WopiHost.Core.Infrastructure; - -namespace WopiHost; - -public class Startup(IConfiguration configuration) -{ - /// - /// Sets up the DI container. - /// - public void ConfigureServices(IServiceCollection services) - { - services.AddLogging(loggingBuilder => - { - loggingBuilder.AddConsole(); //Configuration.GetSection("Logging") - loggingBuilder.AddDebug(); - }); - - // Configuration - // this makes sure that the configuration exists and is valid - var wopiHostOptionsSection = configuration.GetRequiredSection(WopiConfigurationSections.WOPI_ROOT); - services - .AddOptionsWithValidateOnStart() - .BindConfiguration(wopiHostOptionsSection.Path) - .ValidateDataAnnotations(); - - var wopiHostOptions = wopiHostOptionsSection.Get(); - // Add file provider - services.AddStorageProvider(wopiHostOptions.StorageProviderAssemblyName); - // Add lock provider - services.AddLockProvider(wopiHostOptions.LockProviderAssemblyName); - // Add Cobalt support - if (wopiHostOptions.UseCobalt) - { - // Add cobalt - services.AddCobalt(); - } - - services.AddControllers(); - - // Add WOPI - services.AddWopi(o => - { - o.OnCheckFileInfo = GetWopiCheckFileInfo; - }); - } - - /// - /// Configure is called after ConfigureServices is called. - /// - public static void Configure(IApplicationBuilder app, IWebHostEnvironment env) - { - if (env.IsDevelopment()) - { - app.UseDeveloperExceptionPage(); - } - - app.UseSerilogRequestLogging(options => - { - options.EnrichDiagnosticContext = LogHelper.EnrichWithWopiDiagnostics; - options.MessageTemplate = "HTTP {RequestMethod} {RequestPath} with [WOPI CorrelationID: {" + nameof(WopiHeaders.CORRELATION_ID) + "}, WOPI SessionID: {" + nameof(WopiHeaders.SESSION_ID) + "}] responded {StatusCode} in {Elapsed:0.0000} ms"; - }); - - app.UseRouting(); - - // Automatically authenticate - app.UseAuthentication(); - app.UseAuthorization(); - - app.UseEndpoints(endpoints => - { - endpoints.MapControllers(); - endpoints.MapGet("/", () => "This is just a WOPI server. You need a WOPI client to access it...").ShortCircuit(404); - }); - } - - /// - /// Custom handling of CheckFileInfo results for WOPI-Validator - /// - /// - /// - private static Task GetWopiCheckFileInfo(WopiCheckFileInfoContext context) - { - var wopiCheckFileInfo = context.CheckFileInfo; - wopiCheckFileInfo.AllowAdditionalMicrosoftServices = true; - wopiCheckFileInfo.AllowErrorReportPrompt = true; - - // ##183 required for WOPI-Validator - if (wopiCheckFileInfo.BaseFileName == "test.wopitest") - { - wopiCheckFileInfo.CloseUrl = new("https://example.com/close"); - wopiCheckFileInfo.DownloadUrl = new("https://example.com/download"); - wopiCheckFileInfo.FileSharingUrl = new("https://example.com/share"); - wopiCheckFileInfo.FileUrl = new("https://example.com/file"); - wopiCheckFileInfo.FileVersionUrl = new("https://example.com/version"); - wopiCheckFileInfo.HostEditUrl = new("https://example.com/edit"); - wopiCheckFileInfo.HostEmbeddedViewUrl = new("https://example.com/embedded"); - wopiCheckFileInfo.HostEmbeddedEditUrl = new("https://example.com/embeddededit"); - wopiCheckFileInfo.HostRestUrl = new("https://example.com/rest"); - wopiCheckFileInfo.HostViewUrl = new("https://example.com/view"); - wopiCheckFileInfo.SignInUrl = new("https://example.com/signin"); - wopiCheckFileInfo.SignoutUrl = new("https://example.com/signout"); - - wopiCheckFileInfo.ClientUrl = new("https://example.com/client"); - wopiCheckFileInfo.FileEmbedCommandUrl = new("https://example.com/embed"); - - // https://learn.microsoft.com/microsoft-365/cloud-storage-partner-program/rest/files/checkfileinfo/checkfileinfo-other#breadcrumb-properties - wopiCheckFileInfo.BreadcrumbBrandName = "WopiHost"; - wopiCheckFileInfo.BreadcrumbBrandUrl = new("https://example.com"); - wopiCheckFileInfo.BreadcrumbDocName = "test"; - wopiCheckFileInfo.BreadcrumbFolderName = "root"; - wopiCheckFileInfo.BreadcrumbFolderUrl = new("https://example.com/folder"); - } - return Task.FromResult(wopiCheckFileInfo); - } -} diff --git a/test/WopiHost.Core.Tests/Controllers/FilesControllerTests.cs b/test/WopiHost.Core.Tests/Controllers/FilesControllerTests.cs index 3133fbab..6ea2f518 100644 --- a/test/WopiHost.Core.Tests/Controllers/FilesControllerTests.cs +++ b/test/WopiHost.Core.Tests/Controllers/FilesControllerTests.cs @@ -2,7 +2,6 @@ using System.Net.Mime; using System.Security.Claims; using System.Text.Json; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; @@ -20,7 +19,6 @@ public class FilesControllerTests private readonly Mock storageProviderMock; private readonly Mock securityHandlerMock; private readonly Mock> wopiHostOptionsMock; - private readonly Mock authorizationServiceMock; private readonly Mock lockProviderMock; private FilesController controller; @@ -37,14 +35,12 @@ public FilesControllerTests() LockProviderAssemblyName = "test", OnCheckFileInfo = o => Task.FromResult(o.CheckFileInfo) }); - authorizationServiceMock = new Mock(); lockProviderMock = new Mock(); controller = new FilesController( storageProviderMock.Object, securityHandlerMock.Object, wopiHostOptionsMock.Object, - authorizationServiceMock.Object, lockProviderMock.Object) { ControllerContext = new ControllerContext @@ -54,30 +50,11 @@ public FilesControllerTests() }; } - [Fact] - public async Task GetCheckFileInfo_Unauthorized_ReturnsUnauthorized() - { - // Arrange - var fileId = "testFileId"; - authorizationServiceMock - .Setup(a => a.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny>())) - .ReturnsAsync(AuthorizationResult.Failed()); - - // Act - var result = await controller.GetCheckFileInfo(fileId); - - // Assert - Assert.IsType(result); - } - [Fact] public async Task GetCheckFileInfo_FileNotFound_ReturnsNotFound() { // Arrange var fileId = "testFileId"; - authorizationServiceMock - .Setup(a => a.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny>())) - .ReturnsAsync(AuthorizationResult.Success()); storageProviderMock.Setup(s => s.GetWopiFile(fileId)).Returns(null!); // Act @@ -101,9 +78,6 @@ public async Task GetCheckFileInfo_Success_ReturnsFileInfoForAnonymous() fileMock.SetupGet(f => f.Length).Returns(1024); fileMock.Setup(f => f.GetReadStream(It.IsAny())).ReturnsAsync(new System.IO.MemoryStream()); - authorizationServiceMock - .Setup(a => a.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny>())) - .ReturnsAsync(AuthorizationResult.Success()); storageProviderMock .Setup(s => s.GetWopiFile(fileId)) .Returns(fileMock.Object); @@ -145,9 +119,6 @@ public async Task GetCheckFileInfo_Success_ReturnsFileInfoWithAuthenticatedUser( fileMock.SetupGet(f => f.Length).Returns(1024); fileMock.Setup(f => f.GetReadStream(It.IsAny())).ReturnsAsync(new System.IO.MemoryStream()); - authorizationServiceMock - .Setup(a => a.AuthorizeAsync(It.IsAny(), It.IsAny(), It.IsAny>())) - .ReturnsAsync(AuthorizationResult.Success()); storageProviderMock .Setup(s => s.GetWopiFile(fileId)) .Returns(fileMock.Object); @@ -193,7 +164,6 @@ public void ProcessLock_LockingNotSupported_ReturnsLockMismatchResult() storageProviderMock.Object, securityHandlerMock.Object, wopiHostOptionsMock.Object, - authorizationServiceMock.Object, null, null) { diff --git a/test/WopiHost.Core.Tests/Security/Authentication/AccessTokenHandlerTests.cs b/test/WopiHost.Core.Tests/Security/Authentication/AccessTokenHandlerTests.cs index 59022821..464b4e09 100644 --- a/test/WopiHost.Core.Tests/Security/Authentication/AccessTokenHandlerTests.cs +++ b/test/WopiHost.Core.Tests/Security/Authentication/AccessTokenHandlerTests.cs @@ -43,6 +43,26 @@ public AccessTokenHandlerTests() ); } + [Fact] + public async Task HandleAuthenticateAsync_ShouldReturnNoResult_WhenNotWopiPath() + { + // Arrange + var context = new DefaultHttpContext() + { + Request = + { + Path = "/whatever", + } + }; + await _accessTokenHandler.InitializeAsync(scheme, context); + + // Act + var result = await _accessTokenHandler.AuthenticateAsync(); + + // Assert + Assert.True(result.None); + } + [Fact] public async Task HandleAuthenticateAsync_ShouldReturnSuccess_WhenTokenIsValid() { @@ -56,6 +76,7 @@ public async Task HandleAuthenticateAsync_ShouldReturnSuccess_WhenTokenIsValid() { Request = { + Path = "/wopi", Query = new QueryCollection(new Dictionary { { AccessTokenDefaults.ACCESS_TOKEN_QUERY_NAME, token } diff --git a/test/WopiHost.Core.Tests/Security/Authorization/WopiAuthorizationHandlerTests.cs b/test/WopiHost.Core.Tests/Security/Authorization/WopiAuthorizationHandlerTests.cs new file mode 100644 index 00000000..1497e5c6 --- /dev/null +++ b/test/WopiHost.Core.Tests/Security/Authorization/WopiAuthorizationHandlerTests.cs @@ -0,0 +1,95 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Moq; +using WopiHost.Abstractions; +using WopiHost.Core.Security.Authorization; + +namespace WopiHost.Core.Tests.Security.Authorization; + +public class WopiAuthorizationHandlerTests +{ + private readonly Mock _mockSecurityHandler; + private readonly WopiAuthorizationHandler _handler; + private readonly WopiAuthorizeAttribute _requirement; + + public WopiAuthorizationHandlerTests() + { + _mockSecurityHandler = new Mock(); + _handler = new WopiAuthorizationHandler(_mockSecurityHandler.Object); + _requirement = new WopiAuthorizeAttribute(Permission.Read, WopiResourceType.File); + } + + [Fact] + public async Task HandleRequirementAsync_ShouldSucceed_WhenUserIsAuthorized() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.Request.RouteValues["id"] = "fileId"; + var context = new AuthorizationHandlerContext([_requirement], new ClaimsPrincipal(), httpContext); + _mockSecurityHandler + .Setup(s => s.IsAuthorized(It.IsAny(), _requirement, It.IsAny())) + .ReturnsAsync(true); + + // Act + await _handler.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Fact] + public async Task HandleRequirementAsync_ShouldFail_WhenUserIsNotAuthorized() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.Request.RouteValues["id"] = "fileId"; + var context = new AuthorizationHandlerContext([_requirement], new ClaimsPrincipal(), httpContext); + _mockSecurityHandler + .Setup(s => s.IsAuthorized(It.IsAny(), _requirement, It.IsAny())) + .ReturnsAsync(false); + + // Act + await _handler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } + + [Fact] + public async Task HandleRequirementAsync_ShouldSetResourceId_WhenRouteValueExists() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.Request.RouteValues["id"] = "fileId"; + var context = new AuthorizationHandlerContext([_requirement], new ClaimsPrincipal(), httpContext); + _mockSecurityHandler + .Setup(s => s.IsAuthorized(It.IsAny(), _requirement, It.IsAny())) + .Callback((p, r, c) => Assert.Equal("fileId", r.ResourceId)) + .ReturnsAsync(true); + + // Act + await _handler.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Fact] + public async Task HandleRequirementAsync_ShouldNotSetResourceId_WhenRouteValueDoesNotExist() + { + // Arrange + var httpContext = new DefaultHttpContext(); + var context = new AuthorizationHandlerContext([_requirement], new ClaimsPrincipal(), httpContext); + _mockSecurityHandler + .Setup(s => s.IsAuthorized(It.IsAny(), _requirement, It.IsAny())) + .Callback((p, r, c) => Assert.Null(r.ResourceId)) + .ReturnsAsync(false); + + // Act + await _handler.HandleAsync(context); + + // Assert + Assert.False(context.HasSucceeded); + } +} diff --git a/test/WopiHost.Core.Tests/WopiHost.Core.Tests.csproj b/test/WopiHost.Core.Tests/WopiHost.Core.Tests.csproj index 12e0ef18..869a7661 100644 --- a/test/WopiHost.Core.Tests/WopiHost.Core.Tests.csproj +++ b/test/WopiHost.Core.Tests/WopiHost.Core.Tests.csproj @@ -9,6 +9,7 @@ +