Make public access endpoints work correctly (#16136)

Author: kjac

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

@@ -57,7 +57,7 @@ public async Task<IActionResult> Create(
         Attempt<PublicAccessEntry?, PublicAccessOperationStatus> saveAttempt = await _publicAccessService.CreateAsync(publicAccessEntrySlim);
 
         return saveAttempt.Success
-            ? CreatedAtId<GetPublicAccessDocumentController>(controller => nameof(controller.GetPublicAccess), saveAttempt.Result!.Key)
+            ? CreatedAtId<GetPublicAccessDocumentController>(controller => nameof(controller.GetPublicAccess), id)
             : PublicAccessOperationStatusResult(saveAttempt.Status);
     }
 }

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

@@ -162,7 +162,7 @@ protected IActionResult PublicAccessOperationStatusResult(PublicAccessOperationS
                 .WithTitle("Ambiguous Rule")
                 .WithDetail("The specified rule is ambiguous, because both member groups and member names were given.")
                 .Build()),
-            PublicAccessOperationStatus.EntryNotFound => BadRequest(problemDetailsBuilder
+            PublicAccessOperationStatus.EntryNotFound => NotFound(problemDetailsBuilder
                 .WithTitle("Entry not found")
                 .WithDetail("The specified entry was not found.")
                 .Build()),

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

@@ -3,7 +3,6 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Api.Management.Factories;
-using Umbraco.Cms.Api.Management.Security.Authorization.Content;
 using Umbraco.Cms.Api.Management.ViewModels.PublicAccess;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Actions;
@@ -35,6 +34,7 @@ public GetPublicAccessDocumentController(
 
     [MapToApiVersion("1.0")]
     [HttpGet("{id:guid}/public-access")]
+    [ProducesResponseType(typeof(PublicAccessResponseModel), StatusCodes.Status200OK)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
     public async Task<IActionResult> GetPublicAccess(CancellationToken cancellationToken, Guid id)
     {
@@ -51,16 +51,11 @@ public async Task<IActionResult> GetPublicAccess(CancellationToken cancellationT
         Attempt<PublicAccessEntry?, PublicAccessOperationStatus> accessAttempt =
             await _publicAccessService.GetEntryByContentKeyAsync(id);
 
-        if (accessAttempt.Success is false)
+        if (accessAttempt.Success is false || accessAttempt.Result is null)
         {
             return PublicAccessOperationStatusResult(accessAttempt.Status);
         }
 
-        if (accessAttempt.Result is null)
-        {
-            return Ok();
-        }
-
         Attempt<PublicAccessResponseModel?, PublicAccessOperationStatus> responseModelAttempt =
             _publicAccessPresentationFactory.CreatePublicAccessResponseModel(accessAttempt.Result);
 

src/Umbraco.Cms.Api.Management/Factories/IMemberPresentationFactory.cs

@@ -1,7 +1,5 @@
-using Umbraco.Cms.Api.Management.ViewModels.Content;
-using Umbraco.Cms.Api.Management.ViewModels.Member;
+using Umbraco.Cms.Api.Management.ViewModels.Member;
 using Umbraco.Cms.Api.Management.ViewModels.Member.Item;
-using Umbraco.Cms.Api.Management.ViewModels.MemberType;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Entities;
 using Umbraco.Cms.Core.Models.Membership;
@@ -16,7 +14,5 @@ public interface IMemberPresentationFactory
 
     MemberItemResponseModel CreateItemResponseModel(IMemberEntitySlim entity);
 
-    IEnumerable<VariantItemResponseModel> CreateVariantsItemResponseModels(IMemberEntitySlim entity);
-
-    MemberTypeReferenceResponseModel CreateMemberTypeReferenceResponseModel(IMemberEntitySlim entity);
+    MemberItemResponseModel CreateItemResponseModel(IMember entity);
 }

src/Umbraco.Cms.Api.Management/Factories/MemberPresentationFactory.cs

@@ -60,20 +60,21 @@ public async Task<IEnumerable<MemberResponseModel>> CreateMultipleAsync(IEnumera
     }
 
     public MemberItemResponseModel CreateItemResponseModel(IMemberEntitySlim entity)
-    {
-        var responseModel = new MemberItemResponseModel
+        => CreateItemResponseModel<IMemberEntitySlim>(entity);
+
+    public MemberItemResponseModel CreateItemResponseModel(IMember entity)
+        => CreateItemResponseModel<IMember>(entity);
+
+    private MemberItemResponseModel CreateItemResponseModel<T>(T entity)
+        where T : ITreeEntity
+        => new MemberItemResponseModel
         {
             Id = entity.Key,
+            MemberType = _umbracoMapper.Map<MemberTypeReferenceResponseModel>(entity)!,
+            Variants = CreateVariantsItemResponseModels(entity)
         };
 
-        responseModel.MemberType = _umbracoMapper.Map<MemberTypeReferenceResponseModel>(entity)!;
-
-        responseModel.Variants = CreateVariantsItemResponseModels(entity);
-
-        return responseModel;
-    }
-
-    public IEnumerable<VariantItemResponseModel> CreateVariantsItemResponseModels(IMemberEntitySlim entity)
+    private static IEnumerable<VariantItemResponseModel> CreateVariantsItemResponseModels(ITreeEntity entity)
         => new[]
         {
             new VariantItemResponseModel
@@ -83,9 +84,6 @@ public IEnumerable<VariantItemResponseModel> CreateVariantsItemResponseModels(IM
             }
         };
 
-    public MemberTypeReferenceResponseModel CreateMemberTypeReferenceResponseModel(IMemberEntitySlim entity)
-        => _umbracoMapper.Map<MemberTypeReferenceResponseModel>(entity)!;
-
     private async Task<MemberResponseModel> RemoveSensitiveDataAsync(IMember member, MemberResponseModel responseModel)
     {
         // these properties are considered sensitive; some of them are not nullable, so for

src/Umbraco.Cms.Api.Management/Factories/PublicAccessPresentationFactory.cs

@@ -20,17 +20,20 @@ public class PublicAccessPresentationFactory : IPublicAccessPresentationFactory
     private readonly IMemberService _memberService;
     private readonly IUmbracoMapper _mapper;
     private readonly IMemberRoleManager _memberRoleManager;
+    private readonly IMemberPresentationFactory _memberPresentationFactory;
 
     public PublicAccessPresentationFactory(
         IEntityService entityService,
         IMemberService memberService,
         IUmbracoMapper mapper,
-        IMemberRoleManager memberRoleManager)
+        IMemberRoleManager memberRoleManager,
+        IMemberPresentationFactory memberPresentationFactory)
     {
         _entityService = entityService;
         _memberService = memberService;
         _mapper = mapper;
         _memberRoleManager = memberRoleManager;
+        _memberPresentationFactory = memberPresentationFactory;
     }
 
     public Attempt<PublicAccessResponseModel?, PublicAccessOperationStatus> CreatePublicAccessResponseModel(PublicAccessEntry entry)
@@ -57,8 +60,8 @@ public PublicAccessPresentationFactory(
 
         MemberItemResponseModel[] members = usernames
             .Select(username => _memberService.GetByUsername(username))
-            .Select(_mapper.Map<MemberItemResponseModel>)
             .WhereNotNull()
+            .Select(_memberPresentationFactory.CreateItemResponseModel)
             .ToArray();
 
         var allGroups = _memberRoleManager.Roles.Where(x => x.Name != null).ToDictionary(x => x.Name!);
@@ -68,9 +71,12 @@ public PublicAccessPresentationFactory(
                 rule.RuleValue is not null && allGroups.TryGetValue(rule.RuleValue, out UmbracoIdentityRole? memberRole)
                     ? memberRole
                     : null)
-            .WhereNotNull();
+            .WhereNotNull()
+            .ToArray();
 
-        IEnumerable<IEntitySlim> groupsEntities = _entityService.GetAll(UmbracoObjectTypes.MemberGroup, identityRoles.Select(x => Convert.ToInt32(x.Id)).ToArray());
+        IEnumerable<IEntitySlim> groupsEntities = identityRoles.Any()
+            ? _entityService.GetAll(UmbracoObjectTypes.MemberGroup, identityRoles.Select(x => Convert.ToInt32(x.Id)).ToArray())
+            : Enumerable.Empty<IEntitySlim>();
         MemberGroupItemResponseModel[] memberGroups = groupsEntities.Select(x => _mapper.Map<MemberGroupItemResponseModel>(x)!).ToArray();
 
         var responseModel = new PublicAccessResponseModel

src/Umbraco.Cms.Api.Management/Mapping/MemberType/MemberTypeMapDefinition.cs

@@ -14,7 +14,7 @@ public void DefineMaps(IUmbracoMapper mapper)
         mapper.Define<IMemberType, MemberTypeResponseModel>((_, _) => new MemberTypeResponseModel(), Map);
         mapper.Define<IMemberType, MemberTypeReferenceResponseModel>((_, _) => new MemberTypeReferenceResponseModel(), Map);
         mapper.Define<IMemberEntitySlim, MemberTypeReferenceResponseModel>((_, _) => new MemberTypeReferenceResponseModel(), Map);
-        mapper.Define<IContentEntitySlim, MemberTypeReferenceResponseModel>((_, _) => new MemberTypeReferenceResponseModel(), Map);
+        mapper.Define<IMember, MemberTypeReferenceResponseModel>((_, _) => new MemberTypeReferenceResponseModel(), Map);
         mapper.Define<ISimpleContentType, MemberTypeReferenceResponseModel>((_, _) => new MemberTypeReferenceResponseModel(), Map);
     }
 
@@ -49,16 +49,18 @@ private void Map(IMemberType source, MemberTypeReferenceResponseModel target, Ma
         target.Icon = source.Icon ?? string.Empty;
     }
 
+    // Umbraco.Code.MapAll -Collection
     private void Map(IMemberEntitySlim source, MemberTypeReferenceResponseModel target, MapperContext context)
     {
         target.Id = source.ContentTypeKey;
         target.Icon = source.ContentTypeIcon ?? string.Empty;
     }
 
-    private void Map(IContentEntitySlim source, MemberTypeReferenceResponseModel target, MapperContext context)
+    // Umbraco.Code.MapAll -Collection
+    private void Map(IMember source, MemberTypeReferenceResponseModel target, MapperContext context)
     {
-        target.Id = source.ContentTypeKey;
-        target.Icon = source.ContentTypeIcon ?? string.Empty;
+        target.Id = source.ContentType.Key;
+        target.Icon = source.ContentType.Icon ?? string.Empty;
     }
 
     // Umbraco.Code.MapAll -Collection

src/Umbraco.Core/Services/PublicAccessService.cs

@@ -375,7 +375,7 @@ private Attempt<PublicAccessNodesValidationResult, PublicAccessOperationStatus>
 
         if (entry is null)
         {
-            return Task.FromResult(Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.Success, null));
+            return Task.FromResult(Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.EntryNotFound, null));
         }
 
         return Task.FromResult(Attempt.SucceedWithStatus<PublicAccessEntry?, PublicAccessOperationStatus>(PublicAccessOperationStatus.Success, entry));