Skip to content

Conversation

@lauraneto
Copy link
Contributor

@lauraneto lauraneto commented May 7, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #17149

Description

When using the IContentService it is possible to pass in user id 0 (special Unknown/System user). This user does not actually exist in the database, and was therefore causing an exception to be thrown in IUserIdKeyResolver, which was called when mapping the audit logs.
It was not mentioned in the reported issue, but when reproducing the issue I noticed it only happened with user 0, any other non-existing id would throw a foreign key error when trying to insert, so this was a special case.

Reproduction steps

  1. Create a node in the backoffice.
  2. Add code to use the ContentService to publish the node, using user id 0, and call it.
    In my case I added a simple API endpoint that I could easily call as needed, and added the following code. You can also check the reproduction sample provided in the issue.
IContent content = _contentService.GetById(Guid.Parse("3b2f86e4-5839-440f-985e-4e0e02c05289")) ?? throw new InvalidOperationException();
content.SetValue("field", Guid.NewGuid().ToString());
_contentService.Save(content, 0);
_contentService.Publish(content, ["*"], 0);
  1. Open the node in the backoffice and go to the info tab.

Current behavior

The audit logs do not load and an error toast is shown.
image

New behavior

The audit logs load and show the entries with id 0 as Unknown.
image

@lauraneto lauraneto force-pushed the v16/bugfix/17149-can-publish-with-a-non-existing-user branch from 581c275 to 2424d4a Compare May 7, 2025 09:18
@lauraneto lauraneto force-pushed the v16/bugfix/17149-can-publish-with-a-non-existing-user branch from 2424d4a to af9f130 Compare May 7, 2025 09:23
private AuditLogResponseModel CreateAuditLogViewModel(IAuditItem auditItem)
{
Guid userKey = _userIdKeyResolver.GetAsync(auditItem.UserId).GetAwaiter().GetResult();
IUser user = _userService.GetAsync(userKey).GetAwaiter().GetResult()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the user service does no extra validations when retrieving the user, I removed this call entirely, as it seemed unnecessary as we only need the key.

@umbraco umbraco deleted a comment from github-actions bot May 7, 2025
@lauraneto lauraneto marked this pull request as ready for review May 7, 2025 09:42
@lauraneto lauraneto requested a review from Copilot May 7, 2025 10:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the audit log functionality to properly handle audit entries for the special Unknown/System user (user id 0).

  • Updates the IUserIdKeyResolver documentation to indicate that an exception is thrown when a user is not found.
  • Modifies the AuditLogPresentationFactory to return an empty user reference when the user id is the unknown/system user, avoiding an exception.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Umbraco.Core/Services/IUserIdKeyResolver.cs Updated XML comments to document the new exception-based behavior when a user is not found.
src/Umbraco.Cms.Api.Management/Factories/AuditLogPresentationFactory.cs Refactored audit log view model creation to handle user id 0 gracefully using a switch expression.

@lauraneto lauraneto requested a review from Zeegaan May 8, 2025 08:13
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

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

Looks good, tests good 🎉

@Zeegaan Zeegaan merged commit b42888d into main May 12, 2025
25 checks passed
@Zeegaan Zeegaan deleted the v16/bugfix/17149-can-publish-with-a-non-existing-user branch May 12, 2025 10:31
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.

I can publish content as non-existing user

3 participants