diff --git a/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs index 9a30da3811..1751564b37 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs @@ -18,6 +18,10 @@ public abstract class ClientCredentialsUserControllerBase : UserControllerBase .WithTitle("Duplicate client ID") .WithDetail("The specified client ID is already in use. Choose another client ID.") .Build()), + BackOfficeUserClientCredentialsOperationStatus.InvalidClientId => BadRequest(problemDetailsBuilder + .WithTitle("Invalid client ID") + .WithDetail("The specified client ID is invalid. A valid client ID can only contain [a-z], [A-Z], [0-9], and [-._~].") + .Build()), _ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder .WithTitle("Unknown client credentials operation status.") .Build()), diff --git a/src/Umbraco.Core/Services/OperationStatus/UserClientCredentialsOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/UserClientCredentialsOperationStatus.cs index c33fd829b4..585d87c47a 100644 --- a/src/Umbraco.Core/Services/OperationStatus/UserClientCredentialsOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/UserClientCredentialsOperationStatus.cs @@ -4,5 +4,6 @@ public enum UserClientCredentialsOperationStatus { Success, DuplicateClientId, - InvalidUser + InvalidUser, + InvalidClientId } diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 07da9f631a..0aa900c840 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -4,6 +4,7 @@ using System.Linq.Expressions; using Microsoft.Extensions.DependencyInjection; using System.Security.Claims; using System.Security.Cryptography; +using System.Text.RegularExpressions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Cache; @@ -35,7 +36,7 @@ namespace Umbraco.Cms.Core.Services; /// Represents the UserService, which is an easy access to operations involving , /// and eventually Backoffice Users. /// -internal class UserService : RepositoryService, IUserService +internal partial class UserService : RepositoryService, IUserService { private readonly GlobalSettings _globalSettings; private readonly SecuritySettings _securitySettings; @@ -2483,6 +2484,11 @@ internal class UserService : RepositoryService, IUserService public async Task AddClientIdAsync(Guid userKey, string clientId) { + if (ValidClientId().IsMatch(clientId) is false) + { + return UserClientCredentialsOperationStatus.InvalidClientId; + } + using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true); IEnumerable currentClientIds = _userRepository.GetAllClientIds(); @@ -2670,5 +2676,8 @@ internal class UserService : RepositoryService, IUserService } } + [GeneratedRegex(@"^[\w\d\-\._~]*$")] + private static partial Regex ValidClientId(); + #endregion } diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserClientCredentialsManager.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserClientCredentialsManager.cs index 7036fa0e88..21afdd0165 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserClientCredentialsManager.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserClientCredentialsManager.cs @@ -35,6 +35,7 @@ public sealed class BackOfficeUserClientCredentialsManager : ClientCredentialsMa { UserClientCredentialsOperationStatus.InvalidUser => Attempt.Fail(BackOfficeUserClientCredentialsOperationStatus.InvalidUser), UserClientCredentialsOperationStatus.DuplicateClientId => Attempt.Fail(BackOfficeUserClientCredentialsOperationStatus.DuplicateClientId), + UserClientCredentialsOperationStatus.InvalidClientId => Attempt.Fail(BackOfficeUserClientCredentialsOperationStatus.InvalidClientId), _ => throw new ArgumentOutOfRangeException($"Unsupported client ID operation status: {result}") }; } diff --git a/src/Umbraco.Infrastructure/Security/OperationStatus/BackOfficeUserClientCredentialsOperationStatus.cs b/src/Umbraco.Infrastructure/Security/OperationStatus/BackOfficeUserClientCredentialsOperationStatus.cs index fd86ce68cb..f0370a13a3 100644 --- a/src/Umbraco.Infrastructure/Security/OperationStatus/BackOfficeUserClientCredentialsOperationStatus.cs +++ b/src/Umbraco.Infrastructure/Security/OperationStatus/BackOfficeUserClientCredentialsOperationStatus.cs @@ -4,5 +4,6 @@ public enum BackOfficeUserClientCredentialsOperationStatus { Success, DuplicateClientId, - InvalidUser + InvalidUser, + InvalidClientId } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs index 67048e2900..e20fcb5310 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs @@ -13,6 +13,7 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -967,6 +968,54 @@ public class UserServiceTests : UmbracoIntegrationTest } } + [TestCase(UserKind.Default, UserClientCredentialsOperationStatus.InvalidUser)] + [TestCase(UserKind.Api, UserClientCredentialsOperationStatus.Success)] + public async Task Can_Assign_ClientId_To_Api_User(UserKind userKind, UserClientCredentialsOperationStatus expectedResult) + { + // Arrange + var user = await CreateTestUser(userKind); + + // Act + var result = await UserService.AddClientIdAsync(user.Key, "client-one"); + + // Assert + Assert.AreEqual(expectedResult, result); + } + + [TestCase("abcdefghijklmnopqrstuvwxyz", UserClientCredentialsOperationStatus.Success)] + [TestCase("ABCDEFGHIJKLMNOPQRSTUVWXYZ", UserClientCredentialsOperationStatus.Success)] + [TestCase("1234567890", UserClientCredentialsOperationStatus.Success)] + [TestCase(".-_~", UserClientCredentialsOperationStatus.Success)] + [TestCase("!", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("#", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("$", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("&", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("'", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("(", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase(")", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("*", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("+", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase(",", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("/", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase(":", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase(";", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("=", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("?", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("@", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("[", UserClientCredentialsOperationStatus.InvalidClientId)] + [TestCase("]", UserClientCredentialsOperationStatus.InvalidClientId)] + public async Task Can_Use_Only_Unreserved_Characters_For_ClientId(string clientId, UserClientCredentialsOperationStatus expectedResult) + { + // Arrange + var user = await CreateTestUser(UserKind.Api); + + // Act + var result = await UserService.AddClientIdAsync(user.Key, clientId); + + // Assert + Assert.AreEqual(expectedResult, result); + } + private Content[] BuildContentItems(int numberToCreate) { var template = TemplateBuilder.CreateTextPageTemplate(); @@ -999,6 +1048,25 @@ public class UserServiceTests : UmbracoIntegrationTest return user; } + private async Task CreateTestUser(UserKind userKind) + { + var userGroup = CreateTestUserGroup(); + + var result = await UserService.CreateAsync( + Constants.Security.SuperUserKey, + new UserCreateModel + { + Email = "test1@test.com", + Id = Guid.NewGuid(), + Kind = userKind, + Name = "test1@test.com", + UserName = "test1@test.com", + UserGroupKeys = new HashSet { userGroup.Key } + }); + Assert.IsTrue(result.Success); + return result.Result.CreatedUser!; + } + private List CreateTestUsers(int[] startContentIds, IUserGroup userGroup, int numberToCreate) { var users = new List();