Skip to content

Commit 6d6b222

Browse files
authored
Merge pull request #3022 from TechnologyEnhancedLearning/Develop/Fix/TD-5108-Activity-Delegate-URL-manipulation
TD-5108-Activity Delegate - Export and Remove self-assessment URL can be still accessed via URL manipulation when admin category doesn't match
2 parents 4b0e1f9 + 8ecfa96 commit 6d6b222

File tree

3 files changed

+66
-15
lines changed

3 files changed

+66
-15
lines changed

DigitalLearningSolutions.Web/Controllers/TrackingSystem/Delegates/ActivityDelegatesController.cs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,14 @@ public IActionResult Index(
9595

9696
sortBy ??= DefaultSortByOptions.Name.PropertyName;
9797
sortDirection ??= GenericSortingHelper.Ascending;
98-
existingFilterString = FilteringHelper.GetFilterString(
99-
existingFilterString,
100-
newFilterToAdd,
101-
clearFilters,
102-
Request,
103-
filterCookieName,
104-
CourseDelegateAccountStatusFilterOptions.Active.FilterValue
105-
);
98+
existingFilterString = FilteringHelper.GetFilterString(
99+
existingFilterString,
100+
newFilterToAdd,
101+
clearFilters,
102+
Request,
103+
filterCookieName,
104+
CourseDelegateAccountStatusFilterOptions.Active.FilterValue
105+
);
106106

107107
if (isCourseDelegate)
108108
{
@@ -227,7 +227,7 @@ public IActionResult Index(
227227
page = 1; offSet = 0;
228228
(selfAssessmentDelegatesData, resultCount) = selfAssessmentService.GetSelfAssessmentDelegatesPerPage(searchString ?? string.Empty, offSet, itemsPerPage ?? 0, sortBy, sortDirection,
229229
selfAssessmentId, centreId, isDelegateActive, removed, submitted, signedOff, adminCategoryId);
230-
}
230+
}
231231

232232
var adminId = User.GetCustomClaimAsRequiredInt(CustomClaimTypes.UserAdminId);
233233

@@ -259,7 +259,7 @@ public IActionResult Index(
259259
: selfAssessmentService.GetSelfAssessmentNameById((int)selfAssessmentId);
260260
if (!string.IsNullOrEmpty(existingFilterString))
261261
{
262-
existingFilterString = FilteringHelper.GetValidFilters( existingFilterString, newFilterToAdd, availableFilters, Request, filterCookieName);
262+
existingFilterString = FilteringHelper.GetValidFilters(existingFilterString, newFilterToAdd, availableFilters, Request, filterCookieName);
263263
}
264264
if (isCourseDelegate)
265265
{
@@ -375,7 +375,9 @@ public IActionResult DownloadCurrent(
375375
fileName
376376
);
377377
}
378+
378379
[Route("DownloadActivityDelegates/{selfAssessmentId:int}")]
380+
[ServiceFilter(typeof(VerifyAdminUserCanAccessSelfAssessment))]
379381
public IActionResult DownloadActivityDelegates(
380382
int selfAssessmentId,
381383
string? searchString = null,
@@ -388,7 +390,6 @@ public IActionResult DownloadActivityDelegates(
388390
sortBy ??= DefaultSortByOptions.Name.PropertyName;
389391
sortDirection ??= GenericSortingHelper.Ascending;
390392

391-
392393
bool? isDelegateActive, isProgressLocked, removed, hasCompleted, submitted, signedOff;
393394
isDelegateActive = isProgressLocked = removed = hasCompleted = submitted = signedOff = null;
394395

@@ -456,6 +457,7 @@ public IActionResult DownloadActivityDelegates(
456457
fileName
457458
);
458459
}
460+
459461
[Route("TrackingSystem/Delegates/ActivityDelegates/{candidateAssessmentsId}/Remove")]
460462
[HttpGet]
461463
public IActionResult RemoveDelegateSelfAssessment(int candidateAssessmentsId)
@@ -466,12 +468,21 @@ public IActionResult RemoveDelegateSelfAssessment(int candidateAssessmentsId)
466468
return RedirectToAction("StatusCode", "LearningSolutions", new { code = 410 });
467469
}
468470
var selfAssessmentDelegate = selfAssessmentService.GetDelegateSelfAssessmentByCandidateAssessmentsId(candidateAssessmentsId);
469-
if (selfAssessmentDelegate == null)
471+
if (selfAssessmentDelegate != null)
472+
{
473+
var adminCategoryId = User.GetAdminCategoryId();
474+
var selfAssessmentCategoryId = selfAssessmentService.GetSelfAssessmentCategoryId(selfAssessmentDelegate.SelfAssessmentID);
475+
if (adminCategoryId > 0 && adminCategoryId != selfAssessmentCategoryId)
476+
{
477+
return RedirectToAction("StatusCode", "LearningSolutions", new { code = 403 });
478+
}
479+
var model = new DelegateSelfAssessmenteViewModel(selfAssessmentDelegate);
480+
return View(model);
481+
}
482+
else
470483
{
471484
return new NotFoundResult();
472485
}
473-
var model = new DelegateSelfAssessmenteViewModel(selfAssessmentDelegate);
474-
return View(model);
475486
}
476487

477488
[Route("TrackingSystem/Delegates/ActivityDelegates/{candidateAssessmentsId}/Remove")]
@@ -502,6 +513,7 @@ public IActionResult RemoveDelegateSelfAssessment(DelegateSelfAssessmenteViewMod
502513

503514
[HttpGet]
504515
[ServiceFilter(typeof(IsCentreAuthorizedSelfAssessment))]
516+
[ServiceFilter(typeof(VerifyAdminUserCanAccessSelfAssessment))]
505517
[Route("{selfAssessmentId:int}/EditCompleteByDate")]
506518
public IActionResult EditCompleteByDate(
507519
int delegateUserId,
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
namespace DigitalLearningSolutions.Web.ServiceFilter
2+
{
3+
using Microsoft.AspNetCore.Mvc;
4+
using Microsoft.AspNetCore.Mvc.Filters;
5+
using DigitalLearningSolutions.Web.Helpers;
6+
using DigitalLearningSolutions.Web.Services;
7+
using Microsoft.Extensions.Logging;
8+
9+
public class VerifyAdminUserCanAccessSelfAssessment : IActionFilter
10+
{
11+
private readonly ISelfAssessmentService selfAssessmentService;
12+
private readonly ILogger<VerifyAdminUserCanAccessSelfAssessment> logger;
13+
14+
public VerifyAdminUserCanAccessSelfAssessment(ISelfAssessmentService selfAssessmentService,
15+
ILogger<VerifyAdminUserCanAccessSelfAssessment> logger)
16+
{
17+
this.selfAssessmentService = selfAssessmentService;
18+
this.logger = logger;
19+
}
20+
21+
public void OnActionExecuted(ActionExecutedContext context) { }
22+
23+
public void OnActionExecuting(ActionExecutingContext context)
24+
{
25+
if (!(context.Controller is Controller controller))
26+
{
27+
return;
28+
}
29+
var adminCategoryId = controller.User.GetAdminCategoryId();
30+
var selfAssessmentId = int.Parse(context.ActionArguments["selfAssessmentId"].ToString()!);
31+
var selfAssessmentCategoryId = selfAssessmentService.GetSelfAssessmentCategoryId((selfAssessmentId));
32+
if (adminCategoryId > 0 && adminCategoryId != selfAssessmentCategoryId)
33+
{
34+
logger.LogWarning($"Attempt to access restricted self assessment {selfAssessmentId} by user {controller.User.GetUserIdKnownNotNull()}");
35+
context.Result = new RedirectToActionResult("StatusCode", "LearningSolutions", new { code = 403 });
36+
}
37+
}
38+
}
39+
}

DigitalLearningSolutions.Web/Startup.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ namespace DigitalLearningSolutions.Web
5151
using static DigitalLearningSolutions.Web.Services.ICentreApplicationsService;
5252
using static DigitalLearningSolutions.Web.Services.ICentreSelfAssessmentsService;
5353
using System;
54-
using IsolationLevel = System.Transactions.IsolationLevel;
5554
using Serilog;
5655

5756
public class Startup
@@ -581,6 +580,7 @@ private static void RegisterWebServiceFilters(IServiceCollection services)
581580
services.AddScoped<VerifyUserHasVerifiedPrimaryEmail>();
582581
services.AddScoped<VerifyAdminAndDelegateUserCentre>();
583582
services.AddScoped<IsCentreAuthorizedSelfAssessment>();
583+
services.AddScoped<VerifyAdminUserCanAccessSelfAssessment>();
584584
}
585585

586586
public void Configure(IApplicationBuilder app, IMigrationRunner migrationRunner, IFeatureManager featureManager)

0 commit comments

Comments
 (0)