Skip to content

Commit

Permalink
PIDP-1011 Linking Accounts Error fix + Business events (#593)
Browse files Browse the repository at this point in the history
* linking accounts business events

* delete link-ticket cookie when logging in

* refactor logout logic on cancelLink

* refactor pt2

* rework pipe

* tests

* magic strings

* address remaining magic strings

* error log level

* update migrations

* update logging

* regenerate migrations

---------

Co-authored-by: James Hollinger <jameshollinger.ee@gmail.com>
  • Loading branch information
Paahn and james-hollinger authored Nov 22, 2024
1 parent 73c0a89 commit 83ddef6
Show file tree
Hide file tree
Showing 10 changed files with 1,827 additions and 60 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using Microsoft.EntityFrameworkCore.Migrations;

#nullable disable

namespace Pidp.Data.Migrations
{
/// <inheritdoc />
public partial class AccountLinkingBusinessEvents : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{

}

/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{

}
}
}
63 changes: 63 additions & 0 deletions backend/webapi/Data/Migrations/PidpDbContextModelSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,47 @@ protected override void BuildModel(ModelBuilder modelBuilder)
b.HasDiscriminator().HasValue("MSTeamsClinicAddress");
});

modelBuilder.Entity("Pidp.Models.AccountLinkingFailure", b =>
{
b.HasBaseType("Pidp.Models.BusinessEvent");
b.Property<int>("PartyId")
.ValueGeneratedOnUpdateSometimes()
.HasColumnType("integer")
.HasColumnName("PartyId");
b.HasIndex("PartyId");
b.ToTable("BusinessEvent");
b.HasDiscriminator().HasValue("AccountLinkingFailure");
});

modelBuilder.Entity("Pidp.Models.AccountLinkingFailure+LinkTicketNotFound", b =>
{
b.HasBaseType("Pidp.Models.BusinessEvent");
b.ToTable("BusinessEvent");
b.HasDiscriminator().HasValue("LinkTicketNotFound");
});

modelBuilder.Entity("Pidp.Models.AccountLinkingSuccess", b =>
{
b.HasBaseType("Pidp.Models.BusinessEvent");
b.Property<int>("PartyId")
.ValueGeneratedOnUpdateSometimes()
.HasColumnType("integer")
.HasColumnName("PartyId");
b.HasIndex("PartyId");
b.ToTable("BusinessEvent");
b.HasDiscriminator().HasValue("AccountLinkingSuccess");
});

modelBuilder.Entity("Pidp.Models.BCProviderPasswordReset", b =>
{
b.HasBaseType("Pidp.Models.BusinessEvent");
Expand Down Expand Up @@ -1470,6 +1511,28 @@ protected override void BuildModel(ModelBuilder modelBuilder)
b.Navigation("Clinic");
});

modelBuilder.Entity("Pidp.Models.AccountLinkingFailure", b =>
{
b.HasOne("Pidp.Models.Party", "Party")
.WithMany()
.HasForeignKey("PartyId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Party");
});

modelBuilder.Entity("Pidp.Models.AccountLinkingSuccess", b =>
{
b.HasOne("Pidp.Models.Party", "Party")
.WithMany()
.HasForeignKey("PartyId")
.OnDelete(DeleteBehavior.Cascade)
.IsRequired();
b.Navigation("Party");
});

modelBuilder.Entity("Pidp.Models.BCProviderPasswordReset", b =>
{
b.HasOne("Pidp.Models.Party", "Party")
Expand Down
5 changes: 3 additions & 2 deletions backend/webapi/Features/Credentials/CredentialsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,12 @@ await this.AuthorizationService.SignTokenAsync(new Cookies.CredentialLinkTicket.
return result.ToActionResult();
}

[HttpDelete("/api/[controller]")]
[HttpDelete("/api/[controller]/link-ticket/cookie")]
[AllowAnonymous]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public ActionResult DeleteCredential()
public ActionResult DeleteCredentialLinkCookie()
{
this.Response.Cookies.Append(
Cookies.CredentialLinkTicket.Key,
Expand Down
77 changes: 44 additions & 33 deletions backend/webapi/Features/Discovery/Discovery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ public async Task<Model> HandleAsync(Query query)

if (query.CredentialLinkToken != null)
{
return await this.HandleAcountLinkingDiscovery(query, data?.Credential);
var model = await this.HandleAcountLinkingDiscovery(query, data?.Credential);
if (model.Status != Model.StatusCode.AccountLinkInProgress)
{
await this.context.SaveChangesAsync();
}
return model;
}

if (data == null)
Expand Down Expand Up @@ -103,52 +108,58 @@ private async Task<Model> HandleAcountLinkingDiscovery(Query query, Credential?
if (ticket == null)
{
this.logger.LogTicketNotFound(query.User.GetUserId(), query.CredentialLinkToken!.Value);
this.context.BusinessEvents.Add(AccountLinkingFailure.CreateTicketNotFound(query.User.GetUserId(), query.CredentialLinkToken.Value, this.clock.GetCurrentInstant()));
return new Model { Status = Model.StatusCode.AccountLinkingError };
}

if (credential != null)
{
// Either the Credential is already linked to the Party, or the Credential already exists on a different Party.
if (credential.PartyId == ticket.PartyId)
{
this.logger.LogCredentialAlreadyLinked(query.User.GetUserId(), ticket.Id, credential.Id);
this.context.BusinessEvents.Add(AccountLinkingFailure.CreateCredentialAlreadyLinked(ticket.PartyId, credential.Id, ticket.Id, this.clock.GetCurrentInstant()));
return new Model
{
PartyId = credential.PartyId,
Status = Model.StatusCode.AlreadyLinked
};
}
else
{
this.context.CredentialLinkErrorLogs.Add(new CredentialLinkErrorLog
{
CredentialLinkTicketId = ticket.Id,
ExistingCredentialId = credential.Id
});

this.logger.LogCredentialAlreadyExists(query.User.GetUserId(), ticket.Id, credential.Id);
this.context.BusinessEvents.Add(AccountLinkingFailure.CreateCredentialExists(ticket.PartyId, credential.Id, ticket.Id, this.clock.GetCurrentInstant()));
return new Model
{
PartyId = credential.PartyId,
Status = Model.StatusCode.CredentialExists
};
}
}

if (ticket.LinkToIdentityProvider != query.User.GetIdentityProvider())
{
this.logger.LogTicketIdpError(query.User.GetUserId(), ticket.Id, ticket.LinkToIdentityProvider, query.User.GetIdentityProvider());
this.context.BusinessEvents.Add(AccountLinkingFailure.CreateWrongIdentityProvider(ticket.PartyId, ticket.Id, query.User.GetIdentityProvider(), this.clock.GetCurrentInstant()));
return new Model { Status = Model.StatusCode.AccountLinkingError };
}
if (ticket.ExpiresAt < this.clock.GetCurrentInstant())
{
this.logger.LogTicketExpired(query.User.GetUserId(), ticket.Id);
this.context.BusinessEvents.Add(AccountLinkingFailure.CreateTicketExpired(ticket.PartyId, ticket.Id, this.clock.GetCurrentInstant()));
return new Model { Status = Model.StatusCode.TicketExpired };
}

if (credential == null)
{
return new Model
{
Status = Model.StatusCode.AccountLinkInProgress
};
}

if (credential.PartyId == ticket.PartyId)
{
this.logger.LogCredentialAlreadyLinked(query.User.GetUserId(), ticket.Id, credential.Id);
return new Model
{
PartyId = credential.PartyId,
Status = Model.StatusCode.AlreadyLinked
};
}
else
return new Model
{
this.context.CredentialLinkErrorLogs.Add(new CredentialLinkErrorLog
{
CredentialLinkTicketId = ticket.Id,
ExistingCredentialId = credential.Id
});
await this.context.SaveChangesAsync();

this.logger.LogCredentialAlreadyExists(query.User.GetUserId(), ticket.Id, credential.Id);
return new Model
{
PartyId = credential.PartyId,
Status = Model.StatusCode.CredentialExists
};
}
Status = Model.StatusCode.AccountLinkInProgress
};
}

private async Task HandleUpdatesAsync(Credential credential, bool checkPlr, ClaimsPrincipal user)
Expand Down
41 changes: 41 additions & 0 deletions backend/webapi/Models/BusinessEvent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,47 @@ public static BCProviderPasswordReset Create(int partyId, string userPrincipalNa
}
}

public class AccountLinkingSuccess : PartyBusinessEvent
{
public static AccountLinkingSuccess Create(int partyId, Instant recordedOn)
{
return new AccountLinkingSuccess
{
PartyId = partyId,
Description = $"Party successfully linked their account.",
Severity = LogLevel.Information,
RecordedOn = recordedOn
};
}
}

public class AccountLinkingFailure : PartyBusinessEvent
{
public class LinkTicketNotFound : BusinessEvent { }
public static LinkTicketNotFound CreateTicketNotFound(Guid userId, Guid credentialLinkToken, Instant recordedOn) => new()
{
Description = $"No unclaimed Credential Link Ticket found. User Id: {userId}, Credential Link Token: {credentialLinkToken}",
Severity = LogLevel.Error,
RecordedOn = recordedOn
};

public static AccountLinkingFailure CreateCredentialAlreadyLinked(int partyId, int credentialId, int ticketId, Instant recordedOn) => CreateInternal(partyId, $"Credential {credentialId} is already linked. Ticket ID {ticketId}", recordedOn);
public static AccountLinkingFailure CreateCredentialExists(int partyId, int credentialId, int ticketId, Instant recordedOn) => CreateInternal(partyId, $"Credential {credentialId} already exists on another Party. Ticket ID {ticketId}", recordedOn);
public static AccountLinkingFailure CreateTicketExpired(int partyId, int ticketId, Instant recordedOn) => CreateInternal(partyId, $"Ticket {ticketId} expired", recordedOn);
public static AccountLinkingFailure CreateWrongIdentityProvider(int partyId, int ticketId, string? actualIdp, Instant recordedOn) => CreateInternal(partyId, $"New Credential's Identity Provider {actualIdp} does not match Link Ticket {ticketId} expected IDP", recordedOn);

private static AccountLinkingFailure CreateInternal(int partyId, string failureReason, Instant recordedOn)
{
return new AccountLinkingFailure
{
PartyId = partyId,
Description = $"Party failed to link their account. Reason: {failureReason}.",
Severity = LogLevel.Error,
RecordedOn = recordedOn
};
}
}

public class CollegeLicenceSearchError : PartyBusinessEvent
{
public static CollegeLicenceSearchError Create(int partyId, CollegeCode? collegeCode, string? licenceNumber, Instant recordedOn)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,19 @@
import { HttpErrorResponse } from '@angular/common/http';
import { Inject, Injectable } from '@angular/core';
import { Injectable } from '@angular/core';

import { Observable, catchError, map, tap, throwError } from 'rxjs';
import { Observable, catchError, tap, throwError } from 'rxjs';

import { APP_CONFIG, AppConfig } from '@app/app.config';
import { ApiHttpClient } from '@app/core/resources/api-http-client.service';
import { ToastService } from '@app/core/services/toast.service';

import { AuthService } from '../../services/auth.service';

@Injectable({
providedIn: 'root',
})
export class LinkAccountConfirmResource {
public logoutRedirectUrl: string;
public constructor(
@Inject(APP_CONFIG) private config: AppConfig,
private apiResource: ApiHttpClient,
private authService: AuthService,
private toastService: ToastService,
) {
this.logoutRedirectUrl = `${this.config.applicationUrl}/`;
}
) {}

public linkAccount(): Observable<number> {
return this.apiResource.post<number>('credentials', {}).pipe(
Expand All @@ -39,9 +31,8 @@ export class LinkAccountConfirmResource {
);
}

public cancelLink(): Observable<Observable<void> | boolean> {
return this.apiResource.delete('credentials').pipe(
map(() => this.authService.logout(this.logoutRedirectUrl)),
public cancelLink(): Observable<unknown> {
return this.apiResource.delete('credentials/link-ticket/cookie').pipe(
catchError((error: HttpErrorResponse) => {
this.toastService.openErrorToast(
'Something went wrong. Please try again.',
Expand Down
Loading

0 comments on commit 83ddef6

Please sign in to comment.