Require InductionReadWrite role for editing a person's induction (#1825)

DFE-Digital · Jan 22, 2025 · e09ea2a · e09ea2a
1 parent 830fcfa
commit e09ea2a
Show file tree

Hide file tree

Showing 10 changed files with 227 additions and 56 deletions.
diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Core/UserRoles.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Core/UserRoles.cs
@@ -20,13 +20,17 @@ public static class UserRoles
     [Display(Name = "DBS alerts - read & write")]
     public const string DbsAlertsReadWrite = "DbsAlertsReadWrite";
 
+    [Display(Name = "Induction - read & write")]
+    public const string InductionReadWrite = "InductionReadWrite";
+
     public static IReadOnlyCollection<string> All { get; } = new[]
     {
         Administrator,
         Helpdesk,
         AlertsReadWrite,
         DbsAlertsReadOnly,
         DbsAlertsReadWrite,
+        InductionReadWrite
     };
 
     public static string GetDisplayNameForRole(string role)

diff --git a/...ystem/src/TeachingRecordSystem.SupportUi/Infrastructure/Security/AuthorizationPolicies.cs b/...ystem/src/TeachingRecordSystem.SupportUi/Infrastructure/Security/AuthorizationPolicies.cs
@@ -11,4 +11,5 @@ public static class AuthorizationPolicies
     public const string NonDbsAlertFlag = "NonDbsAlertFlag";
     public const string NonDbsAlertRead = "NonDbsAlertRead";
     public const string NonDbsAlertWrite = "NonDbsAlertWrite";
+    public const string InductionReadWrite = "InductionReadWrite";
 }
diff --git a/...stem/src/TeachingRecordSystem.SupportUi/Infrastructure/Security/InductionAuthorization.cs b/...stem/src/TeachingRecordSystem.SupportUi/Infrastructure/Security/InductionAuthorization.cs
@@ -0,0 +1,13 @@
+using Microsoft.AspNetCore.Authorization;
+
+namespace TeachingRecordSystem.SupportUi.Infrastructure.Security;
+
+public static class InductionAuthorization
+{
+    public static AuthorizationBuilder AddInductionPolicies(this AuthorizationBuilder builder) => builder
+        .AddPolicy(
+            AuthorizationPolicies.InductionReadWrite,
+            policy => policy
+                .RequireAuthenticatedUser()
+                .RequireRole(UserRoles.InductionReadWrite, UserRoles.Administrator));
+}
diff --git a/...rc/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/EditInduction/Conventions.cs b/...rc/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/EditInduction/Conventions.cs
@@ -0,0 +1,24 @@
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Mvc.RazorPages;
+using TeachingRecordSystem.SupportUi.Infrastructure.Filters;
+using TeachingRecordSystem.SupportUi.Infrastructure.Security;
+
+namespace TeachingRecordSystem.SupportUi.Pages.Persons.PersonDetail.EditInduction;
+
+public class Conventions : IConfigureFolderConventions
+{
+    public void Configure(RazorPagesOptions options)
+    {
+        options.Conventions.AddFolderApplicationModelConvention(
+            this.GetFolderPathFromNamespace(),
+            model =>
+            {
+                model.EndpointMetadata.Add(new AuthorizeAttribute()
+                {
+                    Policy = AuthorizationPolicies.InductionReadWrite
+                });
+
+                model.Filters.Add(new CheckPersonExistsFilterFactory());
+            });
+    }
+}
diff --git a/...cordSystem/src/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/Induction.cshtml b/...cordSystem/src/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/Induction.cshtml
@@ -23,19 +23,25 @@
             <govuk-summary-list-row data-testid="induction-status">
                 <govuk-summary-list-row-key>Induction status</govuk-summary-list-row-key>
                 <govuk-summary-list-row-value>@Model.Status.GetTitle()</govuk-summary-list-row-value>
-                <govuk-summary-list-row-actions>
-                    <govuk-summary-list-row-action data-testid="change-induction-status" href="@LinkGenerator.InductionEditStatus(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change status">Change</govuk-summary-list-row-action>
-                </govuk-summary-list-row-actions>
+                @if (Model.CanWrite)
+                {
+                    <govuk-summary-list-row-actions>
+                        <govuk-summary-list-row-action data-testid="change-induction-status" href="@LinkGenerator.InductionEditStatus(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change status">Change</govuk-summary-list-row-action>
+                    </govuk-summary-list-row-actions>
+                }
             </govuk-summary-list-row>
 
             @if (Model.ExemptionReasonIds!.Length > 0)
             {
                 <govuk-summary-list-row data-testid="induction-exemption-reasons">
                     <govuk-summary-list-row-key>Exemption reason</govuk-summary-list-row-key>
                     <govuk-summary-list-row-value>@Model.ExemptionReasonsText</govuk-summary-list-row-value>
-                    <govuk-summary-list-row-actions>
-                        <govuk-summary-list-row-action data-testid="change-induction-exemption-reason" href="@LinkGenerator.InductionEditExemptionReason(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change exemption reason">Change</govuk-summary-list-row-action>
-                    </govuk-summary-list-row-actions>
+                    @if (Model.CanWrite)
+                    {
+                        <govuk-summary-list-row-actions>
+                            <govuk-summary-list-row-action data-testid="change-induction-exemption-reason" href="@LinkGenerator.InductionEditExemptionReason(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change exemption reason">Change</govuk-summary-list-row-action>
+                        </govuk-summary-list-row-actions>
+                    }
                 </govuk-summary-list-row>
             }
 
@@ -44,9 +50,12 @@
                 <govuk-summary-list-row data-testid="induction-start-date">
                     <govuk-summary-list-row-key>Induction start date</govuk-summary-list-row-key>
                     <govuk-summary-list-row-value>@Model.StartDate?.ToString(UiDefaults.DateOnlyDisplayFormat)</govuk-summary-list-row-value>
-                    <govuk-summary-list-row-actions>
-                        <govuk-summary-list-row-action data-testid="change-induction-start-date" href="@LinkGenerator.InductionEditStartDate(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change start date">Change</govuk-summary-list-row-action>
-                    </govuk-summary-list-row-actions>
+                    @if (Model.CanWrite)
+                    {
+                        <govuk-summary-list-row-actions>
+                            <govuk-summary-list-row-action data-testid="change-induction-start-date" href="@LinkGenerator.InductionEditStartDate(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change start date">Change</govuk-summary-list-row-action>
+                        </govuk-summary-list-row-actions>
+                    }
                 </govuk-summary-list-row>
             }
 
@@ -55,9 +64,12 @@
                 <govuk-summary-list-row data-testid="induction-completed-date">
                     <govuk-summary-list-row-key>Induction completed date</govuk-summary-list-row-key>
                     <govuk-summary-list-row-value>@Model.CompletedDate?.ToString(UiDefaults.DateOnlyDisplayFormat)</govuk-summary-list-row-value>
-                    <govuk-summary-list-row-actions>
-                        <govuk-summary-list-row-action data-testid="change-induction-completed-date" href="@LinkGenerator.InductionEditCompletedDate(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change completed date">Change</govuk-summary-list-row-action>
-                    </govuk-summary-list-row-actions>
+                    @if (Model.CanWrite)
+                    {
+                        <govuk-summary-list-row-actions>
+                            <govuk-summary-list-row-action data-testid="change-induction-completed-date" href="@LinkGenerator.InductionEditCompletedDate(person.PersonId, journeyInstanceId: null)" visually-hidden-text="change completed date">Change</govuk-summary-list-row-action>
+                        </govuk-summary-list-row-actions>
+                    }
                 </govuk-summary-list-row>
             }
         </govuk-summary-list>

diff --git a/...dSystem/src/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/Induction.cshtml.cs b/...dSystem/src/TeachingRecordSystem.SupportUi/Pages/Persons/PersonDetail/Induction.cshtml.cs
@@ -1,12 +1,19 @@
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.RazorPages;
 using TeachingRecordSystem.Core.DataStore.Postgres;
 using TeachingRecordSystem.Core.Dqt.Models;
 using TeachingRecordSystem.Core.Dqt.Queries;
+using TeachingRecordSystem.SupportUi.Infrastructure.Security;
 
 namespace TeachingRecordSystem.SupportUi.Pages.Persons.PersonDetail;
 
-public class InductionModel(TrsDbContext dbContext, ICrmQueryDispatcher crmQueryDispatcher, IClock clock, ReferenceDataCache referenceDataCache) : PageModel
+public class InductionModel(
+    TrsDbContext dbContext,
+    ICrmQueryDispatcher crmQueryDispatcher,
+    IClock clock,
+    ReferenceDataCache referenceDataCache,
+    IAuthorizationService authorizationService) : PageModel
 {
     private const string NoQualifiedTeacherStatusWarning = "This teacher has not been awarded QTS and is therefore ineligible for induction.";
     private const string InductionIsManagedByCpdWarning = "To change this teacher’s induction status to passed, failed, or in progress, use the Record inductions as an appropriate body service.";
@@ -50,6 +57,8 @@ public string? StatusWarningMessage
         }
     }
 
+    public bool CanWrite { get; set; }
+
     public async Task OnGetAsync()
     {
         var person = await dbContext.Persons
@@ -72,6 +81,9 @@ public async Task OnGetAsync()
         var exemptionReasons = allExemptionReasons.Where(r => ExemptionReasonIds.Contains(r.InductionExemptionReasonId))
             .ToArray();
         ExemptionReasonsText = string.Join(", ", exemptionReasons.Select(r => r.Name));
+
+        CanWrite = (await authorizationService.AuthorizeAsync(User, AuthorizationPolicies.InductionReadWrite))
+            .Succeeded;
     }
 
     private bool TeacherHoldsQualifiedTeacherStatusRule(DateTime? qtsDate)

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.SupportUi/Program.cs b/TeachingRecordSystem/src/TeachingRecordSystem.SupportUi/Program.cs
@@ -94,7 +94,8 @@
         policy => policy
             .RequireAuthenticatedUser()
             .RequireRole(UserRoles.Administrator))
-    .AddAlertPolicies();
+    .AddAlertPolicies()
+    .AddInductionPolicies();
 
 builder.Services
     .AddRazorPages()

diff --git a/...dSystem.SupportUi.Tests/PageTests/Persons/PersonDetail/EditInduction/ChangeReasonTests.cs b/...dSystem.SupportUi.Tests/PageTests/Persons/PersonDetail/EditInduction/ChangeReasonTests.cs
@@ -281,8 +281,8 @@ public async Task Post_SetValidFileUpload_CallsFileServiceUpload()
     }
 
     private Task<JourneyInstance<EditInductionState>> CreateJourneyInstanceAsync(Guid personId, EditInductionState? state = null) =>
-    CreateJourneyInstance(
-        JourneyNames.EditInduction,
-        state ?? new EditInductionState(),
-        new KeyValuePair<string, object>("personId", personId));
+        CreateJourneyInstance(
+            JourneyNames.EditInduction,
+            state ?? new EditInductionState(),
+            new KeyValuePair<string, object>("personId", personId));
 }
diff --git a/...ordSystem.SupportUi.Tests/PageTests/Persons/PersonDetail/EditInduction/CommonPageTests.cs b/...ordSystem.SupportUi.Tests/PageTests/Persons/PersonDetail/EditInduction/CommonPageTests.cs
@@ -369,6 +369,80 @@ public async Task FromCya_ToStartDate_Post_RedirectsToExpectedPage(InductionStat
         Assert.Contains(expectedUrl, location);
     }
 
+    [Theory]
+    [MemberData(nameof(GetPagesForUserWithoutInductionWriteRoleData))]
+    public async Task Get_UserDoesNotHavePermission_ReturnsForbidden(string page, string? role, InductionStatus inductionStatus)
+    {
+        // Arrange
+        SetCurrentUser(TestUsers.GetUser(role));
+
+        var person = await TestData.CreatePersonAsync(
+            p => p
+                .WithQts()
+                .WithInductionStatus(i => i
+                    .WithStatus(inductionStatus)));
+
+        var journeyInstance = await CreateJourneyInstanceAsync(
+            person.PersonId,
+            new EditInductionStateBuilder()
+                .WithInitialisedState(inductionStatus, InductionJourneyPage.Status)
+                .WithStartDate(Clock.Today.AddYears(-2))
+                .Create());
+
+        var request = new HttpRequestMessage(HttpMethod.Post,
+            $"/persons/{person.PersonId}/{page}?{journeyInstance.GetUniqueIdQueryParameter()}");
+
+        // Act
+        var response = await HttpClient.SendAsync(request);
+
+        // Assert
+        Assert.Equal(StatusCodes.Status403Forbidden, (int)response.StatusCode);
+    }
+
+    public static TheoryData<string, string?, InductionStatus> GetPagesForUserWithoutInductionWriteRoleData()
+    {
+        var pagesAndValidStatuses = new[]
+        {
+            ("edit-induction/status", InductionStatus.Exempt),
+            ("edit-induction/status", InductionStatus.InProgress),
+            ("edit-induction/status", InductionStatus.Failed),
+            ("edit-induction/status", InductionStatus.FailedInWales),
+            ("edit-induction/status", InductionStatus.Passed),
+            ("edit-induction/status", InductionStatus.RequiredToComplete),
+            ("edit-induction/exemption-reasons", InductionStatus.Exempt),
+            ("edit-induction/start-date", InductionStatus.InProgress),
+            ("edit-induction/start-date", InductionStatus.Failed),
+            ("edit-induction/start-date", InductionStatus.FailedInWales),
+            ("edit-induction/start-date", InductionStatus.Passed),
+            ("edit-induction/date-completed", InductionStatus.Failed),
+            ("edit-induction/date-completed", InductionStatus.FailedInWales),
+            ("edit-induction/date-completed", InductionStatus.Passed),
+            ("edit-induction/change-reason", InductionStatus.Exempt),
+            ("edit-induction/change-reason", InductionStatus.InProgress),
+            ("edit-induction/change-reason", InductionStatus.Failed),
+            ("edit-induction/change-reason", InductionStatus.FailedInWales),
+            ("edit-induction/change-reason", InductionStatus.Passed),
+            ("edit-induction/change-reason", InductionStatus.RequiredToComplete)
+        };
+
+        var rolesWithoutWritePermission = UserRoles.All
+            .Except([UserRoles.InductionReadWrite, UserRoles.Administrator])
+            .Append(null)
+            .ToArray();
+
+        var data = new TheoryData<string, string?, InductionStatus>();
+
+        foreach (var (page, status) in pagesAndValidStatuses)
+        {
+            foreach (var role in rolesWithoutWritePermission)
+            {
+                data.Add(page, role, status);
+            }
+        }
+
+        return data;
+    }
+
     private Task<JourneyInstance<EditInductionState>> CreateJourneyInstanceAsync(Guid personId, EditInductionState? state = null) =>
         CreateJourneyInstance(
             JourneyNames.EditInduction,