Skip to content

Commit

Permalink
Merge branch 'develop' into test
Browse files Browse the repository at this point in the history
  • Loading branch information
Paahn committed Feb 28, 2024
2 parents 8bf9735 + 3952066 commit 6e89bb5
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 70 deletions.
19 changes: 18 additions & 1 deletion backend/webapi.tests/Features/Credentials/BCProvider.Create.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace PidpTests.Features.Credentials;
using Pidp.Infrastructure.Auth;
using Pidp.Infrastructure.HttpClients.BCProvider;
using Pidp.Infrastructure.HttpClients.Plr;
using Pidp.Infrastructure.HttpClients.Keycloak;
using Pidp.Models;
using Pidp.Models.Lookups;
using PidpTests.TestingExtensions;
Expand All @@ -21,6 +22,7 @@ public async void CreateBCProvider_VaryingLicenceStatus_ProviderCreatedMatchingF
{
var expectedPassword = "p4ssw@rD";
var expectedHpdid = "AnHpdid1123123";
var expectedNewUserId = Guid.NewGuid();
var party = this.TestDb.HasAParty(party =>
{
party.FirstName = "partyfirst";
Expand Down Expand Up @@ -48,11 +50,18 @@ public async void CreateBCProvider_VaryingLicenceStatus_ProviderCreatedMatchingF
.Returns(new User { UserPrincipalName = "aname" });
var plrClient = A.Fake<IPlrClient>()
.ReturningAStandingsDigest(plrStanding);
var handler = this.MockDependenciesFor<CommandHandler>(bcProviderClient, plrClient);
UserRepresentation? capturedNewKeycloakUser = null;
var keycloakClient = A.Fake<IKeycloakAdministrationClient>();
A.CallTo(() => keycloakClient.CreateUser(A<UserRepresentation>._))
.Invokes(i => capturedNewKeycloakUser = i.GetArgument<UserRepresentation>(0))
.Returns(expectedNewUserId);

var handler = this.MockDependenciesFor<CommandHandler>(bcProviderClient, plrClient, keycloakClient);

var result = await handler.HandleAsync(new Command { PartyId = party.Id, Password = expectedPassword });

Assert.True(result.IsSuccess);

Assert.NotNull(capturedNewUser);
Assert.Equal(party.Cpn, capturedNewUser.Cpn);
Assert.Equal(party.FirstName, capturedNewUser.FirstName);
Expand All @@ -63,6 +72,14 @@ public async void CreateBCProvider_VaryingLicenceStatus_ProviderCreatedMatchingF
Assert.Equal(expectedRnp, capturedNewUser.IsRnp);
Assert.Equal(party.Email, capturedNewUser.PidpEmail);
Assert.Equal(expectedPassword, capturedNewUser.Password);

Assert.NotNull(capturedNewKeycloakUser);
Assert.True(capturedNewKeycloakUser.Enabled);
Assert.Equal(party.FirstName, capturedNewKeycloakUser.FirstName);
Assert.Equal(party.LastName, capturedNewKeycloakUser.LastName);

Assert.Single(party.Credentials.Where(credential => credential.IdentityProvider == IdentityProviders.BCProvider));
Assert.Equal(expectedNewUserId, party.Credentials.Single(credential => credential.IdentityProvider == IdentityProviders.BCProvider).UserId);
}

public static IEnumerable<object[]> LicenceTestCases()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public async Task Handle(PlrCpnLookupFound notification, CancellationToken cance
// TODO: what to do if this fails?
foreach (var userId in notification.UserIds)
{
await this.client.UpdateUserCpn(userId, notification.Cpn);
await this.client.UpdateUser(userId, (user) => user.SetCpn(notification.Cpn));
}
}
}
Expand Down
67 changes: 58 additions & 9 deletions backend/webapi/Features/Credentials/BCProvider.Create.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ namespace Pidp.Features.Credentials;

using DomainResults.Common;
using FluentValidation;
using Flurl;
using HybridModelBinding;
using Microsoft.EntityFrameworkCore;
using System.Text.Json.Serialization;
Expand All @@ -10,6 +11,7 @@ namespace Pidp.Features.Credentials;
using Pidp.Extensions;
using Pidp.Infrastructure.Auth;
using Pidp.Infrastructure.HttpClients.BCProvider;
using Pidp.Infrastructure.HttpClients.Keycloak;
using Pidp.Infrastructure.HttpClients.Mail;
using Pidp.Infrastructure.HttpClients.Plr;
using Pidp.Models;
Expand Down Expand Up @@ -39,22 +41,28 @@ public class CommandHandler : ICommandHandler<Command, IDomainResult<string>>
{
private readonly IBCProviderClient client;
private readonly IEmailService emailService;
private readonly PidpDbContext context;
private readonly ILogger logger;
private readonly IKeycloakAdministrationClient keycloakClient;
private readonly ILogger<CommandHandler> logger;
private readonly IPlrClient plrClient;
private readonly PidpDbContext context;
private readonly Url keycloakAdministrationUrl;

public CommandHandler(
IBCProviderClient client,
IEmailService emailService,
PidpDbContext context,
IKeycloakAdministrationClient keycloakClient,
ILogger<CommandHandler> logger,
IPlrClient plrClient)
IPlrClient plrClient,
PidpConfiguration config,
PidpDbContext context)
{
this.client = client;
this.context = context;
this.emailService = emailService;
this.keycloakClient = keycloakClient;
this.logger = logger;
this.plrClient = plrClient;
this.context = context;
this.keycloakAdministrationUrl = Url.Parse(config.Keycloak.AdministrationUrl);
}

public async Task<IDomainResult<string>> HandleAsync(Command command)
Expand Down Expand Up @@ -122,17 +130,55 @@ public async Task<IDomainResult<string>> HandleAsync(Command command)
return DomainResult.Failed<string>();
}

// TODO: Domain Event! Probably should create this credential now and then Queue the keycloak User creation + updating the UserId
var userId = await this.CreateKeycloakUser(party.FirstName, party.LastName, createdUser.UserPrincipalName);
if (userId == null)
{
return DomainResult.Failed<string>();
}

this.context.Credentials.Add(new Credential
{
UserId = userId.Value,
PartyId = command.PartyId,
IdpId = createdUser.UserPrincipalName,
IdentityProvider = IdentityProviders.BCProvider
});

await this.context.SaveChangesAsync();
await this.SendBCProviderCreationEmail(newUserRep.PidpEmail, createdUser.UserPrincipalName);
await this.SendBCProviderCreationEmail(party.Email, createdUser.UserPrincipalName);

return DomainResult.Success(createdUser.UserPrincipalName);
}

return DomainResult.Success(createdUser.UserPrincipalName!);
private async Task<Guid?> CreateKeycloakUser(string firstName, string lastName, string userPrincipalName)
{
var newUser = new UserRepresentation
{
Enabled = true,
FirstName = firstName,
LastName = lastName,
Username = this.GenerateMohKeycloakUsername(userPrincipalName)
};
return await this.keycloakClient.CreateUser(newUser);
}

/// <summary>
/// The expected Ministry of Health Keycloak username for this user. The schema is {IdentityProviderIdentifier}@{IdentityProvider}.
/// Most of our Credentials come to us from Keycloak and so the username is saved as-is in the column IdpId.
/// However, we create BC Provider Credentials directly; so the User Principal Name is saved to the database without the suffix.
/// There are also two inconsistencies with how the MOH Keycloak handles BCP usernames:
/// 1. The username suffix is @bcp rather than @bcprovider_aad.
/// 2. This suffix is only added in Test and Production; there is no suffix at all for BCP Users in the Dev Keycloak.
/// </summary>
private string GenerateMohKeycloakUsername(string userPrincipalName)
{
if (this.keycloakAdministrationUrl.Host == "user-management-dev.api.hlth.gov.bc.ca")
{
return userPrincipalName;
}

return userPrincipalName + "@bcp";
}

private async Task SendBCProviderCreationEmail(string partyEmail, string userPrincipalName)
Expand All @@ -152,8 +198,11 @@ private async Task SendBCProviderCreationEmail(string partyEmail, string userPri
public static partial class BCProviderCreateLoggingExtensions
{
[LoggerMessage(1, LogLevel.Information, "Party {partyId} attempted to create a second BC Provider account.")]
public static partial void LogPartyHasBCProviderCredential(this ILogger logger, int partyId);
public static partial void LogPartyHasBCProviderCredential(this ILogger<BCProviderCreate.CommandHandler> logger, int partyId);

[LoggerMessage(2, LogLevel.Error, "Failed to create BC Provider for Party {partyId}, one or more requirements were not met. Party state: {state}.")]
public static partial void LogInvalidState(this ILogger logger, int partyId, object state);
public static partial void LogInvalidState(this ILogger<BCProviderCreate.CommandHandler> logger, int partyId, object state);

[LoggerMessage(3, LogLevel.Error, "Error when attempting to create a Keycloak User for Party {partyId} with username {username}.")]
public static partial void LogKeycloakUserCreationError(this ILogger<BCProviderCreate.CommandHandler> logger, int partyId, string username);
}
2 changes: 1 addition & 1 deletion backend/webapi/Features/Credentials/Create.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,5 @@ public static partial class CredentialCreateLoggingExtensions
public static partial void LogCredentialLinkTicketExpired(this ILogger logger, int credentialLinkTicketId);

[LoggerMessage(4, LogLevel.Error, "Credential Link Ticket {credentialLinkTicketId} expected to link to IDP {expectedIdp}, user had IDP {actualIdp} instead.")]
public static partial void LogCredentialLinkTicketIdpError(this ILogger logger, int credentialLinkTicketId, string expectedIdp, string? actualIdp);
public static partial void LogCredentialLinkTicketIdpError(this ILogger logger, int credentialLinkTicketId, string expectedIdp, string actualIdp);
}
61 changes: 45 additions & 16 deletions backend/webapi/Infrastructure/HttpClients/BaseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace Pidp.Infrastructure.HttpClients;
using DomainResults.Common;
using Flurl;
using System.Net;
using System.Net.Http.Headers;
using System.Text;
using System.Text.Json;

Expand Down Expand Up @@ -73,6 +74,18 @@ public BaseClient(HttpClient client, ILogger logger, PropertySerialization optio
/// <param name="data"></param>
protected async Task<IDomainResult<T>> PostAsync<T>(string url, object? data = null) => await this.SendCoreAsync<T>(HttpMethod.Post, url, data == null ? null : this.CreateStringContent(data), default);

/// <summary>
/// Performs a POST to the supplied Url with an optional JSON StringContent body as per the serialization settings set in the constructor.
/// Produces a DomainResult and the value of the Location response header.
/// </summary>
/// <param name="url"></param>
/// <param name="data"></param>
protected async Task<(IDomainResult Status, Uri? Location)> PostWithLocationAsync(string url, object? data = null)
{
var (status, headers) = await this.SendCoreWithHeadersAsync(HttpMethod.Post, url, data == null ? null : this.CreateStringContent(data), default);
return (status, headers?.Location);
}

/// <summary>
/// Performs a PUT to the supplied Url with an optional JSON StringContent body as per the serialization settings set in the constructor
/// </summary>
Expand All @@ -91,13 +104,13 @@ public BaseClient(HttpClient client, ILogger logger, PropertySerialization optio
/// <param name="cancellationToken"></param>
protected async Task<IDomainResult> SendCoreAsync(HttpMethod method, string url, HttpContent? content, CancellationToken cancellationToken)
{
var result = await this.SendCoreInternalAsync<object>(method, url, content, true, cancellationToken);
result.Deconstruct(out _, out var details);
var (status, _) = await this.SendCoreInternalAsync<object>(method, url, content, true, cancellationToken);
status.Deconstruct(out _, out var details);
return details;
}

/// <summary>
/// Send an HTTTP message to the API; returning:
/// Send an HTTP message to the API; returning:
/// a) a Success result with a (non-null) value of the indicated type, or
/// b) a Failure result in the case of errors, non-success status codes, or a missing/null response value.
/// </summary>
Expand All @@ -106,9 +119,25 @@ protected async Task<IDomainResult> SendCoreAsync(HttpMethod method, string url,
/// <param name="url"></param>
/// <param name="content"></param>
/// <param name="cancellationToken"></param>
protected async Task<IDomainResult<T>> SendCoreAsync<T>(HttpMethod method, string url, HttpContent? content, CancellationToken cancellationToken) => await this.SendCoreInternalAsync<T>(method, url, content, false, cancellationToken);
protected async Task<IDomainResult<T>> SendCoreAsync<T>(HttpMethod method, string url, HttpContent? content, CancellationToken cancellationToken) => (await this.SendCoreInternalAsync<T>(method, url, content, false, cancellationToken)).Status;

/// <summary>
/// Sends an HTTP message to the API; returning the response headers as well as:
/// a) a Success result, or
/// b) a Failure result in the case of errors or a non-success status code
/// </summary>
/// <param name="method"></param>
/// <param name="url"></param>
/// <param name="content"></param>
/// <param name="cancellationToken"></param>
protected async Task<(IDomainResult Status, HttpResponseHeaders? Headers)> SendCoreWithHeadersAsync(HttpMethod method, string url, HttpContent? content, CancellationToken cancellationToken)
{
var (status, headers) = await this.SendCoreInternalAsync<object>(method, url, content, true, cancellationToken);
status.Deconstruct(out _, out var details);
return (details, headers);
}

private async Task<IDomainResult<T>> SendCoreInternalAsync<T>(HttpMethod method, string url, HttpContent? content, bool ignoreResponseContent, CancellationToken cancellationToken)
private async Task<(IDomainResult<T> Status, HttpResponseHeaders? Headers)> SendCoreInternalAsync<T>(HttpMethod method, string url, HttpContent? content, bool ignoreResponseContent, CancellationToken cancellationToken)
{
try
{
Expand All @@ -131,55 +160,55 @@ private async Task<IDomainResult<T>> SendCoreInternalAsync<T>(HttpMethod method,
: "";

this.Logger.LogNonSuccessStatusCode(response.StatusCode, responseMessage);
return DomainResult.Failed<T>(response.StatusCode == HttpStatusCode.NotFound
return (DomainResult.Failed<T>(response.StatusCode == HttpStatusCode.NotFound
? $"The URL {url} was not found"
: "Did not receive a successful status code");
: "Did not receive a successful status code"), response.Headers);
}

if (ignoreResponseContent)
{
return DomainResult.Success<T>(default!);
return (DomainResult.Success<T>(default!), response.Headers);
}

if (response.Content == null)
{
this.Logger.LogNullResponseContent();
return DomainResult.Failed<T>("Response content was null");
return (DomainResult.Failed<T>("Response content was null"), response.Headers);
}

var deserializationResult = await response.Content.ReadFromJsonAsync<T>(cancellationToken: cancellationToken);
if (deserializationResult == null)
{
this.Logger.LogNullResponseContent();
return DomainResult.Failed<T>("Response content was null");
return (DomainResult.Failed<T>("Response content was null"), response.Headers);
}

return DomainResult.Success(deserializationResult);
return (DomainResult.Success(deserializationResult), response.Headers);
}
catch (HttpRequestException exception)
{
this.Logger.LogBaseClientException(exception);
return DomainResult.Failed<T>("HttpRequestException during call to API");
return (DomainResult.Failed<T>("HttpRequestException during call to API"), null);
}
catch (TimeoutException exception)
{
this.Logger.LogBaseClientException(exception);
return DomainResult.Failed<T>("TimeoutException during call to API");
return (DomainResult.Failed<T>("TimeoutException during call to API"), null);
}
catch (OperationCanceledException exception)
{
this.Logger.LogBaseClientException(exception);
return DomainResult.Failed<T>("Task was canceled during call to API");
return (DomainResult.Failed<T>("Task was canceled during call to API"), null);
}
catch (JsonException exception)
{
this.Logger.LogBaseClientException(exception);
return DomainResult.Failed<T>("Could not deserialize API response");
return (DomainResult.Failed<T>("Could not deserialize API response"), null);
}
catch (Exception exception)
{
this.Logger.LogBaseClientException(exception);
return DomainResult.Failed<T>("Unhandled exception when calling the API");
return (DomainResult.Failed<T>("Unhandled exception when calling the API"), null);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ public interface IKeycloakAdministrationClient
/// <param name="roleName"></param>
Task<bool> AssignRealmRole(Guid userId, string roleName);

/// <summary>
/// Creates a new User in Keycloak. Username must be unique.
/// Returns the User's UserId if successful.
/// </summary>
/// <param name="userRep"></param>
Task<Guid?> CreateUser(UserRepresentation userRep);

/// <summary>
/// Finds a User by Username.
/// Returns null on a failure or if no User is found.
/// </summary>
/// <param name="username"></param>
Task<UserRepresentation?> FindUser(string username);

/// <summary>
/// Gets the Keycloak Client representation by ClientId.
/// Returns null if unsuccessful.
Expand Down Expand Up @@ -86,13 +100,4 @@ public interface IKeycloakAdministrationClient
/// <param name="userId"></param>
/// <param name="updateAction"></param>
Task<bool> UpdateUser(Guid userId, Action<UserRepresentation> updateAction);

/// <summary>
/// Fetches the User and updates their CPN with the provided value. Does not update if the provided cpn is null.
/// A convenience method for UpdateUser.
/// Returns true if the operation was successful.
/// </summary>
/// <param name="userId"></param>
/// <param name="cpn"></param>
Task<bool> UpdateUserCpn(Guid userId, string? cpn);
}
Loading

0 comments on commit 6e89bb5

Please sign in to comment.