Add roles authentication to v2 endpoints

DFE-Digital · Nov 21, 2023 · f6d31ab · f6d31ab
1 parent 6508c7e
commit f6d31ab
Show file tree

Hide file tree

Showing 22 changed files with 338 additions and 7 deletions.
diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiClient.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiClient.cs
@@ -4,4 +4,5 @@ public class ApiClient
 {
     public required string ClientId { get; set; }
     public required List<string> ApiKey { get; set; }
+    public required List<string> Roles { get; set; }
 }
diff --git a/...ystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs b/...ystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs
@@ -46,21 +46,25 @@ protected override Task<AuthenticateResult> HandleAuthenticateAsync()
             return Task.FromResult(AuthenticateResult.Fail($"No client found with specified API key."));
         }
 
-        var principal = CreatePrincipal(client.ClientId);
+        var principal = CreatePrincipal(client.ClientId, client.Roles!);
         var ticket = new AuthenticationTicket(principal, Scheme.Name);
 
         LogContext.PushProperty("ClientId", client.ClientId);
 
         return Task.FromResult(AuthenticateResult.Success(ticket));
     }
 
-    public static ClaimsPrincipal CreatePrincipal(string clientId)
+    public static ClaimsPrincipal CreatePrincipal(string clientId, IEnumerable<string> roles)
     {
         var identity = new ClaimsIdentity(new[]
         {
             new Claim(ClaimTypes.Name, clientId)
         });
 
+        foreach (var role in roles)
+        {
+            identity.AddClaim(new Claim(ClaimTypes.Role, role));
+        }
         return new ClaimsPrincipal(identity);
     }
 }
diff --git a/...ecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/AuthorizationPolicies.cs b/...ecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/AuthorizationPolicies.cs
@@ -5,4 +5,8 @@ public static class AuthorizationPolicies
     public const string ApiKey = nameof(ApiKey);
     public const string IdentityUserWithTrn = nameof(IdentityUserWithTrn);
     public const string Hangfire = nameof(Hangfire);
+    public const string GetPerson = nameof(GetPerson);
+    public const string UpdatePerson = nameof(UpdatePerson);
+    public const string UpdateNpq = nameof(UpdateNpq);
+    public const string UnlockPerson = nameof(UnlockPerson);
 }
diff --git a/.../src/TeachingRecordSystem.Api/Infrastructure/Security/ConfigurationApiClientRepository.cs b/.../src/TeachingRecordSystem.Api/Infrastructure/Security/ConfigurationApiClientRepository.cs
@@ -24,8 +24,8 @@ private static ApiClient[] GetClientsFromConfiguration(IConfiguration configurat
                 var client = new ApiClient()
                 {
                     ClientId = clientId,
-                    ApiKey = new List<string>()
-
+                    ApiKey = new List<string>(),
+                    Roles = new List<string>()
                 };
                 kvp.Bind(client);
                 if (!client.ApiKey.Any() && !string.IsNullOrEmpty(apiKey))

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/RoleNames.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/RoleNames.cs
@@ -0,0 +1,9 @@
+namespace TeachingRecordSystem.Api.Infrastructure.Security;
+
+public class RoleNames
+{
+    public const string GetPerson = "GetPerson";
+    public const string UpdatePerson = "UpdatePerson";
+    public const string UpdateNpq = "UpdateNpq";
+    public const string UnlockPerson = "UnlockPerson";
+}
diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Program.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Program.cs
@@ -137,6 +137,30 @@ public static void Main(string[] args)
                     .AddAuthenticationSchemes(BasicAuthenticationDefaults.AuthenticationScheme)
                     .RequireAuthenticatedUser()
                 );
+
+            options.AddPolicy(
+                AuthorizationPolicies.GetPerson,
+                policy => policy
+                    .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme)
+                    .RequireRole(new[] { RoleNames.GetPerson, RoleNames.UpdatePerson }));
+
+            options.AddPolicy(
+                AuthorizationPolicies.UpdatePerson,
+                policy => policy
+                    .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme)
+                    .RequireRole(new[] { RoleNames.UpdatePerson }));
+
+            options.AddPolicy(
+                AuthorizationPolicies.UpdateNpq,
+                policy => policy
+                    .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme)
+                    .RequireRole(new[] { RoleNames.UpdateNpq }));
+
+            options.AddPolicy(
+                AuthorizationPolicies.UnlockPerson,
+                policy => policy
+                    .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme)
+                    .RequireRole(new[] { RoleNames.UnlockPerson }));
         });
 
         services

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/IttOutcomeController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/IttOutcomeController.cs
@@ -1,15 +1,18 @@
 using MediatR;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using NSwag.Annotations;
 using TeachingRecordSystem.Api.Infrastructure.Filters;
 using TeachingRecordSystem.Api.Infrastructure.Logging;
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.V2.Requests;
 using TeachingRecordSystem.Api.V2.Responses;
 
 namespace TeachingRecordSystem.Api.V2.Controllers;
 
 [ApiController]
 [Route("teachers/{trn}/itt-outcome")]
+[Authorize(Policy = AuthorizationPolicies.UpdatePerson)]
 public class IttOutcomeController : ControllerBase
 {
     private readonly IMediator _mediator;

diff --git a/...ngRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/NpqQualificationsController.cs b/...ngRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/NpqQualificationsController.cs
@@ -1,13 +1,16 @@
 using MediatR;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using NSwag.Annotations;
 using TeachingRecordSystem.Api.Infrastructure.Filters;
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.V2.Requests;
 
 namespace TeachingRecordSystem.Api.V2.Controllers;
 
 [ApiController]
 [Route("npq-qualifications")]
+[Authorize(Policy = AuthorizationPolicies.UpdateNpq)]
 public class NpqQualificationsController : ControllerBase
 {
     private readonly IMediator _mediator;

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TeachersController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TeachersController.cs
@@ -1,15 +1,18 @@
 using MediatR;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using NSwag.Annotations;
 using TeachingRecordSystem.Api.Infrastructure.Filters;
 using TeachingRecordSystem.Api.Infrastructure.Logging;
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.V2.Requests;
 using TeachingRecordSystem.Api.V2.Responses;
 
 namespace TeachingRecordSystem.Api.V2.Controllers;
 
 [ApiController]
 [Route("teachers")]
+[Authorize(Policy = AuthorizationPolicies.GetPerson)]
 public class TeachersController : Controller
 {
     private readonly IMediator _mediator;
@@ -43,6 +46,7 @@ public async Task<IActionResult> GetTeacher([FromRoute] GetTeacherRequest reques
         return response != null ? Ok(response) : NotFound();
     }
 
+
     [HttpPatch("update/{trn}")]
     [OpenApiOperation(
         operationId: "UpdateTeacher",
@@ -52,6 +56,7 @@ public async Task<IActionResult> GetTeacher([FromRoute] GetTeacherRequest reques
     [MapError(10001, statusCode: StatusCodes.Status404NotFound)]
     [MapError(10002, statusCode: StatusCodes.Status409Conflict)]
     [RedactQueryParam("birthdate")]
+    [Authorize(Policy = AuthorizationPolicies.UpdatePerson)]
     public async Task<IActionResult> Update([FromBody] UpdateTeacherRequest request)
     {
         await _mediator.Send(request);

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TrnRequestsController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TrnRequestsController.cs
@@ -1,13 +1,16 @@
 using MediatR;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using NSwag.Annotations;
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.V2.Requests;
 using TeachingRecordSystem.Api.V2.Responses;
 
 namespace TeachingRecordSystem.Api.V2.Controllers;
 
 [ApiController]
 [Route("trn-requests")]
+[Authorize(Policy = AuthorizationPolicies.UpdatePerson)]
 public class TrnRequestsController : ControllerBase
 {
     private readonly IMediator _mediator;
@@ -38,6 +41,7 @@ public async Task<IActionResult> GetTrnRequest(GetTrnRequest request)
     [ProducesResponseType(typeof(TrnRequestInfo), StatusCodes.Status200OK)]
     [ProducesResponseType(typeof(TrnRequestInfo), StatusCodes.Status201Created)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
+
     public async Task<IActionResult> GetOrCreateTrnRequest([FromBody] GetOrCreateTrnRequest request)
     {
         var response = await _mediator.Send(request);

diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs
@@ -1,13 +1,16 @@
 using MediatR;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using NSwag.Annotations;
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.V2.Requests;
 using TeachingRecordSystem.Api.V2.Responses;
 
 namespace TeachingRecordSystem.Api.V2.Controllers;
 
 [ApiController]
 [Route("unlock-teacher")]
+[Authorize(Policy = AuthorizationPolicies.UnlockPerson)]
 public class UnlockTeacherController : ControllerBase
 {
     private readonly IMediator _mediator;

diff --git a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/ApiTestBase.cs b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/ApiTestBase.cs
@@ -20,7 +20,7 @@ protected ApiTestBase(ApiFixture apiFixture)
     {
         ApiFixture = apiFixture;
         _testServices = TestScopedServices.Reset();
-        SetCurrentApiClient("tests");
+        SetCurrentApiClient(Array.Empty<string>());
 
         {
             HttpClientWithApiKey = apiFixture.CreateClient();
@@ -77,10 +77,11 @@ public HttpClient GetHttpClientWithIdentityAccessToken(string trn, string scope
         return httpClient;
     }
 
-    protected void SetCurrentApiClient(string clientId)
+    protected void SetCurrentApiClient(IEnumerable<string> roles, string clientId = "tests")
     {
         var currentUserProvider = ApiFixture.Services.GetRequiredService<CurrentApiClientProvider>();
         currentUserProvider.CurrentApiClientId = clientId;
+        currentUserProvider.Roles = roles.ToArray();
     }
 
     public virtual async Task<T> WithDbContext<T>(Func<TrsDbContext, Task<T>> action)

diff --git a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Attributes/RoleNamesData.cs b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Attributes/RoleNamesData.cs
@@ -0,0 +1,21 @@
+using System.Reflection;
+using TeachingRecordSystem.Api.Infrastructure.Security;
+using Xunit.Sdk;
+
+namespace TeachingRecordSystem.Api.Tests.Attributes;
+
+public class RoleNamesData : DataAttribute
+{
+    private string[] RolesToExclude { get; }
+
+    public RoleNamesData(params string[] except)
+    {
+        RolesToExclude = except;
+    }
+    public override IEnumerable<object[]> GetData(MethodInfo testMethod)
+    {
+        var allRoles = new object[] { RoleNames.UpdateNpq, RoleNames.UpdatePerson, RoleNames.GetPerson, RoleNames.UnlockPerson };
+        var excluded = allRoles.Except(RolesToExclude);
+        return new[] { new object[] { excluded } };
+    }
+}
diff --git a/...System/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs b/...System/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs
@@ -30,10 +30,11 @@ protected override Task<AuthenticateResult> HandleAuthenticateAsync()
         }
 
         var currentApiClientId = _currentApiClientProvider.CurrentApiClientId;
+        var currentRoles = _currentApiClientProvider.Roles!;
 
         if (currentApiClientId is not null)
         {
-            var principal = ApiKeyAuthenticationHandler.CreatePrincipal(currentApiClientId);
+            var principal = ApiKeyAuthenticationHandler.CreatePrincipal(currentApiClientId, currentRoles);
 
             var ticket = new AuthenticationTicket(principal, Scheme.Name);
 
@@ -51,11 +52,19 @@ public class TestAuthenticationOptions : AuthenticationSchemeOptions { }
 public class CurrentApiClientProvider
 {
     private readonly AsyncLocal<string> _currentApiClientId = new();
+    private readonly AsyncLocal<string[]> _roles = new();
 
     [DisallowNull]
     public string? CurrentApiClientId
     {
         get => _currentApiClientId.Value;
         set => _currentApiClientId.Value = value;
     }
+
+    [DisallowNull]
+    public string[]? Roles
+    {
+        get => _roles.Value;
+        set => _roles.Value = value;
+    }
 }
diff --git a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/FindTeachersTests.cs b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/FindTeachersTests.cs
@@ -1,4 +1,6 @@
 #nullable disable
+using TeachingRecordSystem.Api.Infrastructure.Security;
+using TeachingRecordSystem.Api.Tests.Attributes;
 using TeachingRecordSystem.Api.V2.Responses;
 
 namespace TeachingRecordSystem.Api.Tests.V2.Operations;
@@ -7,8 +9,28 @@ public class FindTeachersTests : ApiTestBase
 {
     public FindTeachersTests(ApiFixture apiFixture) : base(apiFixture)
     {
+        SetCurrentApiClient(new[] { RoleNames.GetPerson });
     }
 
+    [Theory, RoleNamesData(new[] { RoleNames.GetPerson, RoleNames.UpdatePerson })]
+    public async Task FindTeachers_ClientDoesNotHavePermission_ReturnsForbidden(string[] roles)
+    {
+        // Arrange
+        SetCurrentApiClient(roles);
+        DataverseAdapterMock
+            .Setup(mock => mock.FindTeachers(It.IsAny<FindTeachersQuery>()))
+            .ReturnsAsync(Array.Empty<Contact>());
+
+        var request = new HttpRequestMessage(HttpMethod.Get, $"v2/teachers/find");
+
+        // Act
+        var response = await HttpClientWithApiKey.SendAsync(request);
+
+        // Assert
+        Assert.Equal(StatusCodes.Status403Forbidden, (int)response.StatusCode);
+    }
+
+
     [Fact]
     public async Task Given_no_results_returns_ok()
     {

diff --git a/...rdSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/GetOrCreateTrnRequestTests.cs b/...rdSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/GetOrCreateTrnRequestTests.cs
@@ -1,5 +1,7 @@
 #nullable disable
+using TeachingRecordSystem.Api.Infrastructure.Security;
 using TeachingRecordSystem.Api.Properties;
+using TeachingRecordSystem.Api.Tests.Attributes;
 using TeachingRecordSystem.Api.V2.ApiModels;
 using TeachingRecordSystem.Api.V2.Requests;
 using TeachingRecordSystem.Core.DataStore.Postgres.Models;
@@ -11,6 +13,48 @@ public class GetOrCreateTrnRequestTests : ApiTestBase
 {
     public GetOrCreateTrnRequestTests(ApiFixture apiFixture) : base(apiFixture)
     {
+        SetCurrentApiClient(new[] { RoleNames.UpdatePerson });
+    }
+
+    [Theory, RoleNamesData(new[] { RoleNames.UpdatePerson })]
+    public async Task GetOrCreateTrn_ClientDoesNotHavePermission_ReturnsForbidden(string[] roles)
+    {
+        // Arrange
+        SetCurrentApiClient(roles);
+        var requestId = Guid.NewGuid().ToString();
+        var teacherId = Guid.NewGuid();
+        var trn = "1234567";
+
+        DataverseAdapterMock
+            .Setup(mock => mock.GetTeacher(teacherId, /* resolveMerges: */ It.IsAny<string[]>(), true))
+            .ReturnsAsync(new Contact()
+            {
+                Id = teacherId,
+                dfeta_TRN = trn
+            });
+
+        await WithDbContext(async dbContext =>
+        {
+            dbContext.Add(new TrnRequest()
+            {
+                ClientId = ClientId,
+                RequestId = requestId,
+                TeacherId = teacherId
+            });
+
+            await dbContext.SaveChangesAsync();
+        });
+        var slugId = Guid.NewGuid().ToString();
+        var request = CreateRequest(req =>
+        {
+            req.SlugId = slugId;
+        });
+
+        // Act
+        var response = await HttpClientWithApiKey.PutAsync($"v2/trn-requests/{requestId}", request);
+
+        // Assert
+        Assert.Equal(StatusCodes.Status403Forbidden, (int)response.StatusCode);
     }
 
     [Fact]