Don't overwrite ancestor configurations for public access (#17797)

* Don't overwrite ancestor configurations for public access (#17709)

* Fix spacings

Author: kjac

src/Umbraco.Cms.Api.Management/Controllers/Document/GetPublicAccessDocumentController.cs

@@ -49,7 +49,7 @@ public async Task<IActionResult> GetPublicAccess(CancellationToken cancellationT
         }
 
         Attempt<PublicAccessEntry?, PublicAccessOperationStatus> accessAttempt =
-            await _publicAccessService.GetEntryByContentKeyAsync(id);
+            await _publicAccessService.GetEntryByContentKeyWithoutAncestorsAsync(id);
 
         if (accessAttempt.Success is false || accessAttempt.Result is null)
         {

src/Umbraco.Core/Services/IPublicAccessService.cs

@@ -85,8 +85,23 @@ public interface IPublicAccessService : IService
     /// </summary>
     /// <param name="key"></param>
     /// <returns>Returns null if no entry is found</returns>
+    /// <remarks>
+    /// This method supports inheritance by considering ancestor entries (if any),
+    /// if no entry is found for the specified content key.
+    /// </remarks>
     Task<Attempt<PublicAccessEntry?, PublicAccessOperationStatus>> GetEntryByContentKeyAsync(Guid key);
 
+    /// <summary>
+    ///     Gets the entry defined for the content item based on a content key, without taking ancestor entries into account.
+    /// </summary>
+    /// <param name="key"></param>
+    /// <returns>Returns null if no entry is found</returns>
+    /// <remarks>
+    /// This method does not support inheritance. Use <see cref="GetEntryByContentKeyAsync"/> to include ancestor entries (if any).
+    /// </remarks>
+    Task<Attempt<PublicAccessEntry?, PublicAccessOperationStatus>> GetEntryByContentKeyWithoutAncestorsAsync(Guid key)
+        => Task.FromResult(Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.EntryNotFound, null));
+
     /// <summary>
     ///     Deletes the entry and all associated rules for a given key.
     /// </summary>

src/Umbraco.Core/Services/PublicAccessService.cs

@@ -1,5 +1,7 @@
 using System.Globalization;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Entities;
@@ -16,19 +18,41 @@ internal class PublicAccessService : RepositoryService, IPublicAccessService
     private readonly IPublicAccessRepository _publicAccessRepository;
     private readonly IEntityService _entityService;
     private readonly IContentService _contentService;
+    private readonly IIdKeyMap _idKeyMap;
 
+    [Obsolete("Please use the constructor that accepts all parameter. Will be removed in V16.")]
     public PublicAccessService(
         ICoreScopeProvider provider,
         ILoggerFactory loggerFactory,
         IEventMessagesFactory eventMessagesFactory,
         IPublicAccessRepository publicAccessRepository,
         IEntityService entityService,
         IContentService contentService)
+        : this(
+            provider,
+            loggerFactory,
+            eventMessagesFactory,
+            publicAccessRepository,
+            entityService,
+            contentService,
+            StaticServiceProvider.Instance.GetRequiredService<IIdKeyMap>())
+    {
+    }
+
+    public PublicAccessService(
+        ICoreScopeProvider provider,
+        ILoggerFactory loggerFactory,
+        IEventMessagesFactory eventMessagesFactory,
+        IPublicAccessRepository publicAccessRepository,
+        IEntityService entityService,
+        IContentService contentService,
+        IIdKeyMap idKeyMap)
         : base(provider, loggerFactory, eventMessagesFactory)
     {
         _publicAccessRepository = publicAccessRepository;
         _entityService = entityService;
         _contentService = contentService;
+        _idKeyMap = idKeyMap;
     }
 
     /// <summary>
@@ -381,6 +405,23 @@ private Attempt<PublicAccessNodesValidationResult, PublicAccessOperationStatus>
         return Task.FromResult(Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.Success, entry));
     }
 
+    public async Task<Attempt<PublicAccessEntry?, PublicAccessOperationStatus>> GetEntryByContentKeyWithoutAncestorsAsync(Guid key)
+    {
+        Attempt<PublicAccessEntry?, PublicAccessOperationStatus> result = await GetEntryByContentKeyAsync(key);
+        if (result.Success is false || result.Result is null)
+        {
+            return result;
+        }
+
+        Attempt<Guid> idToKeyAttempt = _idKeyMap.GetKeyForId(result.Result.ProtectedNodeId, UmbracoObjectTypes.Document);
+        if (idToKeyAttempt.Success is false || idToKeyAttempt.Result != key)
+        {
+            return Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.EntryNotFound, null);
+        }
+
+        return result;
+    }
+
     public async Task<Attempt<PublicAccessOperationStatus>> DeleteAsync(Guid key)
     {
         using (ICoreScope scope = ScopeProvider.CreateCoreScope())

src/Umbraco.Web.UI.Client/src/packages/documents/documents/entity-actions/public-access/repository/public-access.server.data.ts

@@ -1,7 +1,7 @@
 import { DocumentService } from '@umbraco-cms/backoffice/external/backend-api';
 import type { PublicAccessRequestModel } from '@umbraco-cms/backoffice/external/backend-api';
 import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api';
-import { tryExecuteAndNotify } from '@umbraco-cms/backoffice/resources';
+import { tryExecute, tryExecuteAndNotify } from '@umbraco-cms/backoffice/resources';
 
 /**
  * A data source for the Document Public Access that fetches data from the server
@@ -41,7 +41,9 @@ export class UmbDocumentPublicAccessServerDataSource {
 	 */
 	async read(unique: string) {
 		if (!unique) throw new Error('unique is missing');
-		return tryExecuteAndNotify(this.#host, DocumentService.getDocumentByIdPublicAccess({ id: unique }));
+		// NOTE: The entity will not be present, when fetching Public Access for a descendant of a protected Document.
+		//       This is a perfectly valid scenario, which is handled in the view. In other words, just use tryExecute here.
+		return tryExecute(DocumentService.getDocumentByIdPublicAccess({ id: unique }));
 	}
 
 	/**