Skip to content

Commit 2c12b56

Browse files
FrostyApeOneFrostyApeOne
FrostyApeOne
authored and
FrostyApeOne
committed
Application listing permission issues resolved
1 parent 857c0cd commit 2c12b56

6 files changed

Lines changed: 216 additions & 5 deletions

File tree

‎src/GovUK.Dfe.FlexForms.Api/Security/Handlers/TemplatePermissionHandler.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ protected override Task HandleRequirementAsync(
1717
return Task.CompletedTask;
1818
}
1919

20+
// Caseworker-style roles have Application:Any:Read but not Template:*:Read.
21+
// They still need custom status labels (and other template reads) on /applications.
22+
if (requirement.Action.Equals("Read", StringComparison.OrdinalIgnoreCase)
23+
&& PermissionClaimEvaluator.CanReadAllApplications(context.User))
24+
{
25+
context.Succeed(requirement);
26+
return Task.CompletedTask;
27+
}
28+
2029
var templateId = accessor.HttpContext?.Request.RouteValues["templateId"]?.ToString();
2130
if (!string.IsNullOrWhiteSpace(templateId)
2231
&& PermissionClaimEvaluator.CanManageTemplate(context.User, templateId))

‎src/GovUK.Dfe.FlexForms.Application/Applications/Queries/GetApplicationsByTemplateQueryHandler.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ public sealed class GetApplicationsByTemplateQueryHandler(
3838
IApplicationRepository applicationRepository,
3939
ICacheService<IRedisCacheType> cacheService,
4040
ITenantContextAccessor tenantContextAccessor,
41-
ITenantTemplateResolver tenantTemplateResolver)
41+
ITenantTemplateResolver tenantTemplateResolver,
42+
IPermissionCheckerService permissionCheckerService)
4243
: IRequestHandler<GetApplicationsByTemplateQuery, Result<PagedResult<ApplicationDto>>>
4344
{
4445
public async Task<Result<PagedResult<ApplicationDto>>> Handle(
@@ -61,7 +62,7 @@ public async Task<Result<PagedResult<ApplicationDto>>> Handle(
6162
var templateId = new TemplateId(request.TemplateId);
6263
var searchKey = request.Search?.ToCacheKeySuffix() ?? "";
6364
var baseCacheKey =
64-
$"Applications_ByTemplate_{request.TemplateId}_{searchKey}_p{request.PageNumber}_ps{request.PageSize}_{CacheKeyHelper.GenerateHashedCacheKey(principalId)}";
65+
$"Applications_ByTemplate_{request.TemplateId}_{searchKey}_p{request.PageNumber}_ps{request.PageSize}_{CacheKeyHelper.GenerateHashedCacheKey(principalId)}_claimList";
6566
var cacheKey = TenantCacheKeyHelper.CreateTenantScopedKey(tenantContextAccessor, baseCacheKey);
6667
var methodName = nameof(GetApplicationsByTemplateQueryHandler);
6768

@@ -80,7 +81,15 @@ public async Task<Result<PagedResult<ApplicationDto>>> Handle(
8081
return Result<PagedResult<ApplicationDto>>.Forbid(
8182
"Template does not belong to the current tenant");
8283

83-
if (!ApplicationAccessResolver.CanListAllApplicationsForTemplate(userWithAuthorization, templateId))
84+
// Custom-role grants live on RolePermissions and are issued as JWT claims
85+
// (Application:Any:Read). User.Permissions only has per-user overrides, so
86+
// honour the claim as well as the DB resolver used for Admin / user-level Any.
87+
var canListAll = permissionCheckerService.CanReadAllApplications()
88+
|| ApplicationAccessResolver.CanListAllApplicationsForTemplate(
89+
userWithAuthorization,
90+
templateId);
91+
92+
if (!canListAll)
8493
return Result<PagedResult<ApplicationDto>>.Forbid(
8594
"User does not have permission to list all applications for this template");
8695

‎src/Tests/GovUK.Dfe.FlexForms.Api.Tests/Security/Handlers/ApplicationListPermissionHandlerTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public async Task Handle_ShouldSucceed_WhenUserHasTemplateReadPermission()
5050
}
5151

5252
[Fact]
53-
public async Task Handle_ShouldFail_WhenUserHasOnlyApplicationAnyReadWildcard()
53+
public async Task Handle_ShouldSucceed_WhenUserHasApplicationAnyReadWildcard()
5454
{
5555
var requirement = new ApplicationListPermissionRequirement("Read");
5656
var claims = new[] { new Claim("permission", "Application:Any:Read") };
@@ -60,6 +60,6 @@ public async Task Handle_ShouldFail_WhenUserHasOnlyApplicationAnyReadWildcard()
6060

6161
await handler.HandleAsync(context);
6262

63-
Assert.False(context.HasSucceeded);
63+
Assert.True(context.HasSucceeded);
6464
}
6565
}

‎src/Tests/GovUK.Dfe.FlexForms.Api.Tests/Security/Handlers/TemplatePermissionHandlerTests.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,40 @@ public async Task Handle_ShouldFail_WhenClaimMissing()
4343

4444
Assert.False(context.HasSucceeded);
4545
}
46+
47+
[Fact]
48+
public async Task Handle_ShouldSucceed_WhenUserHasApplicationAnyReadClaim()
49+
{
50+
var requirement = new TemplatePermissionRequirement("Read");
51+
var httpContext = new DefaultHttpContext();
52+
httpContext.Request.RouteValues["templateId"] = "t1";
53+
var accessor = Substitute.For<IHttpContextAccessor>();
54+
accessor.HttpContext.Returns(httpContext);
55+
var claims = new[] { new Claim("permission", "Application:Any:Read") };
56+
var user = new ClaimsPrincipal(new ClaimsIdentity(claims));
57+
var context = new AuthorizationHandlerContext([requirement], user, null);
58+
var handler = new TemplatePermissionHandler(accessor);
59+
60+
await handler.HandleAsync(context);
61+
62+
Assert.True(context.HasSucceeded);
63+
}
64+
65+
[Fact]
66+
public async Task Handle_ShouldNotGrantWrite_WhenUserHasApplicationAnyReadClaim()
67+
{
68+
var requirement = new TemplatePermissionRequirement("Write");
69+
var httpContext = new DefaultHttpContext();
70+
httpContext.Request.RouteValues["templateId"] = "t1";
71+
var accessor = Substitute.For<IHttpContextAccessor>();
72+
accessor.HttpContext.Returns(httpContext);
73+
var claims = new[] { new Claim("permission", "Application:Any:Read") };
74+
var user = new ClaimsPrincipal(new ClaimsIdentity(claims));
75+
var context = new AuthorizationHandlerContext([requirement], user, null);
76+
var handler = new TemplatePermissionHandler(accessor);
77+
78+
await handler.HandleAsync(context);
79+
80+
Assert.False(context.HasSucceeded);
81+
}
4682
}

‎src/Tests/GovUK.Dfe.FlexForms.Application.Tests/Helpers/ApplicationListingTestHelper.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
using GovUK.Dfe.FlexForms.Application.Services;
33
using GovUK.Dfe.FlexForms.Domain.Entities;
44
using GovUK.Dfe.FlexForms.Domain.Interfaces.Repositories;
5+
using GovUK.Dfe.FlexForms.Domain.Services;
56
using GovUK.Dfe.FlexForms.Domain.Tenancy;
67
using GovUK.Dfe.FlexForms.Domain.ValueObjects;
78
using GovUK.Dfe.CoreLibs.Caching.Interfaces;
89
using GovUK.Dfe.CoreLibs.Contracts.ExternalApplications.Models.Response;
10+
using Microsoft.AspNetCore.Http;
911
using Microsoft.Extensions.Logging;
1012
using NSubstitute;
1113
using ApplicationId = GovUK.Dfe.FlexForms.Domain.ValueObjects.ApplicationId;
@@ -149,4 +151,27 @@ internal static GetApplicationsForUserByExternalProviderIdQueryHandler CreateGet
149151
tenantContextAccessor,
150152
accessibleTemplateService);
151153
}
154+
155+
internal static GetApplicationsByTemplateQueryHandler CreateGetApplicationsByTemplateQueryHandler(
156+
IHttpContextAccessor httpContextAccessor,
157+
IEaRepository<User> userRepo,
158+
IEaRepository<Domain.Entities.Application> appRepo,
159+
ITenantContextAccessor tenantContextAccessor,
160+
ITenantTemplateResolver tenantTemplateResolver,
161+
IPermissionCheckerService permissionCheckerService,
162+
ICacheService<IRedisCacheType>? cache = null)
163+
{
164+
cache ??= Substitute.For<ICacheService<IRedisCacheType>>();
165+
ConfigurePassthroughCache(cache, nameof(GetApplicationsByTemplateQueryHandler));
166+
167+
return new GetApplicationsByTemplateQueryHandler(
168+
httpContextAccessor,
169+
userRepo,
170+
appRepo,
171+
CreateApplicationRepository(),
172+
cache,
173+
tenantContextAccessor,
174+
tenantTemplateResolver,
175+
permissionCheckerService);
176+
}
152177
}

‎src/Tests/GovUK.Dfe.FlexForms.Application.Tests/QueryHandlers/Applications/GetApplicationsByTemplateQueryHandlerTests.cs‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
using System.Security.Claims;
2+
using GovUK.Dfe.CoreLibs.Contracts.ExternalApplications.Enums;
3+
using GovUK.Dfe.FlexForms.Application.Applications.Queries;
4+
using GovUK.Dfe.FlexForms.Application.Tests.Helpers;
5+
using GovUK.Dfe.FlexForms.Domain.Common;
6+
using GovUK.Dfe.FlexForms.Domain.Entities;
7+
using GovUK.Dfe.FlexForms.Domain.Interfaces.Repositories;
8+
using GovUK.Dfe.FlexForms.Domain.Services;
9+
using GovUK.Dfe.FlexForms.Domain.Tenancy;
10+
using GovUK.Dfe.FlexForms.Domain.ValueObjects;
11+
using Microsoft.AspNetCore.Http;
12+
using MockQueryable;
13+
using NSubstitute;
14+
using ApplicationId = GovUK.Dfe.FlexForms.Domain.ValueObjects.ApplicationId;
15+
16+
namespace GovUK.Dfe.FlexForms.Application.Tests.QueryHandlers.Applications;
17+
18+
public class GetApplicationsByTemplateQueryHandlerTests
19+
{
20+
[Fact]
21+
public async Task Handle_ShouldReturnApplications_WhenCallerHasApplicationAnyReadClaim()
22+
{
23+
var email = "caseworker@example.com";
24+
var templateId = new TemplateId(Guid.NewGuid());
25+
var user = CreateStandardUser(email);
26+
var application = CreateApplication(user, templateId);
27+
28+
var httpContext = new DefaultHttpContext
29+
{
30+
User = new ClaimsPrincipal(new ClaimsIdentity(
31+
[new Claim(ClaimTypes.Email, email), new Claim("permission", "Application:Any:Read")],
32+
"TestAuth"))
33+
};
34+
var httpContextAccessor = Substitute.For<IHttpContextAccessor>();
35+
httpContextAccessor.HttpContext.Returns(httpContext);
36+
37+
var userRepo = Substitute.For<IEaRepository<User>>();
38+
userRepo.Query().Returns(new List<User> { user }.AsQueryable().BuildMock());
39+
40+
var appRepo = Substitute.For<IEaRepository<Domain.Entities.Application>>();
41+
appRepo.Query().Returns(new List<Domain.Entities.Application> { application }.AsQueryable().BuildMock());
42+
43+
var permissionChecker = Substitute.For<IPermissionCheckerService>();
44+
permissionChecker.CanReadAllApplications().Returns(true);
45+
46+
var handler = ApplicationListingTestHelper.CreateGetApplicationsByTemplateQueryHandler(
47+
httpContextAccessor,
48+
userRepo,
49+
appRepo,
50+
Substitute.For<ITenantContextAccessor>(),
51+
ApplicationListingTestHelper.CreateTemplateResolver(templateId),
52+
permissionChecker);
53+
54+
var result = await handler.Handle(
55+
new GetApplicationsByTemplateQuery(templateId.Value),
56+
CancellationToken.None);
57+
58+
Assert.True(result.IsSuccess);
59+
Assert.Single(result.Value!.Items);
60+
Assert.Equal(application.Id!.Value, result.Value.Items.First().ApplicationId);
61+
}
62+
63+
[Fact]
64+
public async Task Handle_ShouldForbid_WhenCallerCannotListAllApplications()
65+
{
66+
var email = "user@example.com";
67+
var templateId = new TemplateId(Guid.NewGuid());
68+
var user = CreateStandardUser(email);
69+
70+
var httpContext = new DefaultHttpContext
71+
{
72+
User = new ClaimsPrincipal(new ClaimsIdentity(
73+
[new Claim(ClaimTypes.Email, email)],
74+
"TestAuth"))
75+
};
76+
var httpContextAccessor = Substitute.For<IHttpContextAccessor>();
77+
httpContextAccessor.HttpContext.Returns(httpContext);
78+
79+
var userRepo = Substitute.For<IEaRepository<User>>();
80+
userRepo.Query().Returns(new List<User> { user }.AsQueryable().BuildMock());
81+
82+
var permissionChecker = Substitute.For<IPermissionCheckerService>();
83+
permissionChecker.CanReadAllApplications().Returns(false);
84+
85+
var handler = ApplicationListingTestHelper.CreateGetApplicationsByTemplateQueryHandler(
86+
httpContextAccessor,
87+
userRepo,
88+
Substitute.For<IEaRepository<Domain.Entities.Application>>(),
89+
Substitute.For<ITenantContextAccessor>(),
90+
ApplicationListingTestHelper.CreateTemplateResolver(templateId),
91+
permissionChecker);
92+
93+
var result = await handler.Handle(
94+
new GetApplicationsByTemplateQuery(templateId.Value),
95+
CancellationToken.None);
96+
97+
Assert.False(result.IsSuccess);
98+
Assert.Equal(DomainErrorCode.Forbidden, result.ErrorCode);
99+
}
100+
101+
private static User CreateStandardUser(string email)
102+
{
103+
var user = new User(
104+
new UserId(Guid.NewGuid()),
105+
new RoleId(RoleConstants.UserRoleId),
106+
"Caseworker",
107+
email,
108+
DateTime.UtcNow,
109+
null,
110+
null,
111+
null);
112+
113+
user.GetType().GetProperty(nameof(User.Role))!.SetValue(
114+
user,
115+
new Role(new RoleId(RoleConstants.UserRoleId), RoleNames.User));
116+
117+
return user;
118+
}
119+
120+
private static Domain.Entities.Application CreateApplication(User user, TemplateId templateId)
121+
{
122+
var application = new Domain.Entities.Application(
123+
new ApplicationId(Guid.NewGuid()),
124+
"REF-1",
125+
new TemplateVersionId(Guid.NewGuid()),
126+
DateTime.UtcNow,
127+
user.Id!);
128+
129+
ApplicationListingTestHelper.AttachTemplateVersion(application, templateId, user.Id!);
130+
return application;
131+
}
132+
}

0 commit comments

Comments
 (0)