Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BAU: Indicate whether account deleted manually in audit message #4901

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dbes-gds
Copy link
Contributor

@dbes-gds dbes-gds commented Jul 12, 2024

What

Indicate whether account deleted manually in audit message.

HMRC have indicated that certain fields are missing from the AUTH_DELETE_ACCOUNT audit event. Account deletion can either be triggered by users, or manually as required when we have to delete accounts. For manual deletion many of the usual fields such as session ids will be missing as there is not a user session. To make this explicit a new field has been added to the event stating whether the account was deleted manually or not.

How to review

  1. Code Review

@dbes-gds dbes-gds requested review from a team as code owners July 12, 2024 16:11
@dbes-gds dbes-gds force-pushed the BAU/audit-flag-manual-delete branch from 2193fd5 to 6fd223b Compare July 12, 2024 16:44
if (input.isPresent()) {
ipAddress = PersistentIdHelper.extractPersistentIdFromHeaders(input.get().getHeaders());
attachLogFieldToLogs(PERSISTENT_SESSION_ID, ipAddress);
ipAddress = IpAddressHelper.extractIpAddress(input.get());
manualDeletion = Boolean.FALSE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we are using boxed booleans rather than primitive e.g. true and false?

Comment on lines 119 to 122
var pairs =
new AuditService.MetadataPair[] {
pair("manualDeletion", manualDeletion.toString())
};
Copy link
Contributor

@mrwilson mrwilson Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable can probably be inlined as line 134 takes a varargs e.g.

auditService.submitAuditEvent(DELETE_ACCOUNT, auditContext, pair("manualDeletion", manualDeletion))

@ethanmills
Copy link
Member

I think this may make sense as a deletion_reason field, as we're going to have more in the future such as deletion of inactive accounts

@alhcomer alhcomer force-pushed the BAU/audit-flag-manual-delete branch from 991fd19 to 0824563 Compare July 17, 2024 12:53
- Changed boxed boolean to primitive boolean.
- Changed metadata pair declarations to be inline.
- Deleted no longer necessary buildMetadataPairs helper method.
@alhcomer alhcomer force-pushed the BAU/audit-flag-manual-delete branch from 0824563 to cf5acff Compare July 17, 2024 12:54
Copy link

sonarcloud bot commented Jul 17, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants