From 3c6204ca71f9a42949c7afb8337bf5053e749496 Mon Sep 17 00:00:00 2001 From: Kevin Joy Date: Wed, 15 Nov 2023 12:25:21 +0000 Subject: [PATCH] Add roles authentication to v2 endpoints --- .../Infrastructure/Security/ApiClient.cs | 1 + .../Security/ApiKeyAuthenticationHandler.cs | 8 +++- .../Security/AuthorizationPolicies.cs | 4 ++ .../ConfigurationApiClientRepository.cs | 4 +- .../Infrastructure/Security/RoleNames.cs | 9 +++++ .../src/TeachingRecordSystem.Api/Program.cs | 24 +++++++++++ .../NpqQualificationsController.cs | 3 ++ .../V2/Controllers/TeachersController.cs | 6 +++ .../V2/Controllers/UnlockTeacherController.cs | 3 ++ .../ApiTestBase.cs | 5 ++- .../Security/TestAuthentication.cs | 11 ++++- .../V2/Operations/GetTeacherTests.cs | 40 +++++++++++++++++++ 12 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/RoleNames.cs diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiClient.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiClient.cs index 41feec5f74..8187680bb4 100644 --- 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 ApiKey { get; set; } + public required List Roles { get; set; } } diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs index d47cf5c8cf..5d53f0f194 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs +++ b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ApiKeyAuthenticationHandler.cs @@ -46,7 +46,7 @@ protected override Task 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); @@ -54,13 +54,17 @@ protected override Task HandleAuthenticateAsync() return Task.FromResult(AuthenticateResult.Success(ticket)); } - public static ClaimsPrincipal CreatePrincipal(string clientId) + public static ClaimsPrincipal CreatePrincipal(string clientId, IEnumerable 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/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/AuthorizationPolicies.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/AuthorizationPolicies.cs index f0223c56bc..05518dd07e 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/AuthorizationPolicies.cs +++ b/TeachingRecordSystem/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 GetTeacher = nameof(GetTeacher); + public const string UpdateTeacher = nameof(UpdateTeacher); + public const string UpdateNpq = nameof(UpdateNpq); + public const string UnlockTeacher = nameof(UnlockTeacher); } diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ConfigurationApiClientRepository.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ConfigurationApiClientRepository.cs index 2785673b0e..4eec35720c 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Infrastructure/Security/ConfigurationApiClientRepository.cs +++ b/TeachingRecordSystem/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() - + ApiKey = new List(), + Roles = new List() }; 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 new file mode 100644 index 0000000000..ad5d5b0733 --- /dev/null +++ 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 UnlockTeacher = "UnlockTeacher"; +} diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/Program.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/Program.cs index dfcf87d0c1..31a67aac63 100644 --- 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.GetTeacher, + policy => policy + .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme) + .RequireRole(new[] { RoleNames.GetPerson, RoleNames.UpdatePerson })); + + options.AddPolicy( + AuthorizationPolicies.UpdateTeacher, + 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.UnlockTeacher, + policy => policy + .AddAuthenticationSchemes(ApiKeyAuthenticationHandler.AuthenticationScheme) + .RequireRole(new[] { RoleNames.UnlockTeacher })); }); services diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/NpqQualificationsController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/NpqQualificationsController.cs index 9595443013..f4ec6e65d1 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/NpqQualificationsController.cs +++ b/TeachingRecordSystem/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 index 54722cac08..9fd1ccc6cf 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TeachersController.cs +++ b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/TeachersController.cs @@ -1,8 +1,10 @@ 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; @@ -25,6 +27,7 @@ public TeachersController(IMediator mediator) summary: "Find teachers", description: "Returns teachers matching the specified criteria")] [ProducesResponseType(typeof(FindTeachersResponse), StatusCodes.Status200OK)] + [Authorize(Policy= AuthorizationPolicies.GetTeacher)] public async Task FindTeachers(FindTeachersRequest request) { var response = await _mediator.Send(request); @@ -37,12 +40,14 @@ public async Task FindTeachers(FindTeachersRequest request) summary: "Get teacher", description: "Gets an individual teacher by their TRN")] [ProducesResponseType(typeof(GetTeacherResponse), StatusCodes.Status200OK)] + [Authorize(Policy = AuthorizationPolicies.GetTeacher)] public async Task GetTeacher([FromRoute] GetTeacherRequest request) { var response = await _mediator.Send(request); return response != null ? Ok(response) : NotFound(); } + [HttpPatch("update/{trn}")] [OpenApiOperation( operationId: "UpdateTeacher", @@ -52,6 +57,7 @@ public async Task GetTeacher([FromRoute] GetTeacherRequest reques [MapError(10001, statusCode: StatusCodes.Status404NotFound)] [MapError(10002, statusCode: StatusCodes.Status409Conflict)] [RedactQueryParam("birthdate")] + [Authorize(Policy = AuthorizationPolicies.UpdateTeacher)] public async Task Update([FromBody] UpdateTeacherRequest request) { await _mediator.Send(request); diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs index 991c6f5861..92369537cf 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs +++ b/TeachingRecordSystem/src/TeachingRecordSystem.Api/V2/Controllers/UnlockTeacherController.cs @@ -1,6 +1,8 @@ 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; @@ -8,6 +10,7 @@ namespace TeachingRecordSystem.Api.V2.Controllers; [ApiController] [Route("unlock-teacher")] +[Authorize(Policy = AuthorizationPolicies.UnlockTeacher)] 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 index 92c348f633..fe6bfc9666 100644 --- a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/ApiTestBase.cs +++ b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/ApiTestBase.cs @@ -21,7 +21,7 @@ protected ApiTestBase(ApiFixture apiFixture) { ApiFixture = apiFixture; _testServices = TestScopedServices.Reset(); - SetCurrentApiClient("tests"); + SetCurrentApiClient(Array.Empty()); { HttpClientWithApiKey = apiFixture.CreateClient(); @@ -80,10 +80,11 @@ public HttpClient GetHttpClientWithIdentityAccessToken(string trn, string scope return httpClient; } - protected void SetCurrentApiClient(string clientId) + protected void SetCurrentApiClient(IEnumerable roles, string clientId="tests") { var currentUserProvider = ApiFixture.Services.GetRequiredService(); currentUserProvider.CurrentApiClientId = clientId; + currentUserProvider.Roles = roles.ToArray(); } public virtual async Task WithDbContext(Func> action) diff --git a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs index b1c0d9f826..92e4740b83 100644 --- a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs +++ b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/Infrastructure/Security/TestAuthentication.cs @@ -30,10 +30,11 @@ protected override Task 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,6 +52,7 @@ public class TestAuthenticationOptions : AuthenticationSchemeOptions { } public class CurrentApiClientProvider { private readonly AsyncLocal _currentApiClientId = new(); + private readonly AsyncLocal _roles = new(); [DisallowNull] public string? CurrentApiClientId @@ -58,4 +60,11 @@ 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/GetTeacherTests.cs b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/GetTeacherTests.cs index 86e78fa4f3..09315d5cb8 100644 --- a/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/GetTeacherTests.cs +++ b/TeachingRecordSystem/tests/TeachingRecordSystem.Api.Tests/V2/Operations/GetTeacherTests.cs @@ -1,7 +1,10 @@ #nullable disable +using System.Reflection; using Microsoft.Xrm.Sdk; +using TeachingRecordSystem.Api.Infrastructure.Security; using TeachingRecordSystem.Api.Properties; using TeachingRecordSystem.Api.V2.ApiModels; +using Xunit.Sdk; namespace TeachingRecordSystem.Api.Tests.V2.Operations; @@ -10,6 +13,27 @@ public class GetTeacherTests : ApiTestBase public GetTeacherTests(ApiFixture apiFixture) : base(apiFixture) { + SetCurrentApiClient(new[] { RoleNames.GetPerson }); + } + + [Theory, RoleNamesData(new[] { RoleNames.GetPerson, RoleNames.UpdatePerson })] + public async Task GetTeacher_ClientDoesNotHavePermission_ReturnsForbidden(string[] roles) + { + // Arrange + SetCurrentApiClient(roles); + var trn = "1234567"; + + DataverseAdapterMock + .Setup(mock => mock.GetTeacherByTrn(trn, It.IsAny(), true)) + .ReturnsAsync((Contact)null); + + var request = new HttpRequestMessage(HttpMethod.Get, $"/v2/teachers/{trn}"); + + // Act + var response = await HttpClientWithApiKey.SendAsync(request); + + // Assert + Assert.Equal(StatusCodes.Status403Forbidden, (int)response.StatusCode); } [Theory] @@ -338,3 +362,19 @@ await AssertEx.JsonResponseEquals( }); } } + +public class RoleNamesData : DataAttribute +{ + private string[] RolesToExclude { get; } + + public RoleNamesData(string[] except = null) + { + RolesToExclude = except; + } + public override IEnumerable GetData(MethodInfo testMethod) + { + var allRoles = new object[] { RoleNames.UpdateNpq, RoleNames.UpdatePerson, RoleNames.GetPerson, RoleNames.UnlockTeacher}; + var excluded = allRoles.Except(RolesToExclude); + return new[] { new object[] { excluded } }; + } +}