From e91a25dcb6c1c87063209f3a86c930f833fe19c8 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 18 Mar 2025 06:33:24 +0100 Subject: [PATCH] Restrict valid API user client IDs to 100 characters. (#18688) --- .../ClientCredentialsUserControllerBase.cs | 4 ++-- src/Umbraco.Core/Services/UserService.cs | 2 +- .../Services/UserServiceTests.cs | 15 ++++++++++++++- 3 files changed, 17 insertions(+), 4 deletions(-) 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 deeb8d8f0d..9a93a6814e 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/User/ClientCredentials/ClientCredentialsUserControllerBase.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Security.OperationStatus; @@ -20,7 +20,7 @@ public abstract class ClientCredentialsUserControllerBase : UserControllerBase .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 [-._~]. Furthermore, including the prefix it cannot be longer than 255 characters.") + .WithDetail("The specified client ID is invalid. A valid client ID can only contain [a-z], [A-Z], [0-9], and [-._~]. Furthermore, including the prefix it cannot be longer than 100 characters.") .Build()), _ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder .WithTitle("Unknown client credentials operation status.") diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index e92a5b0cd0..659a98ea07 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -2680,7 +2680,7 @@ internal partial class UserService : RepositoryService, IUserService } } - [GeneratedRegex(@"^[\w\d\-\._~]{1,255}$")] + [GeneratedRegex(@"^[\w\d\-\._~]{1,100}$")] private static partial Regex ValidClientId(); #endregion diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs index 92829fdf37..6896b396df 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs @@ -1004,7 +1004,6 @@ public class UserServiceTests : UmbracoIntegrationTest [TestCase("@", UserClientCredentialsOperationStatus.InvalidClientId)] [TestCase("[", UserClientCredentialsOperationStatus.InvalidClientId)] [TestCase("]", UserClientCredentialsOperationStatus.InvalidClientId)] - [TestCase("More_Than_255_characters_012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789", UserClientCredentialsOperationStatus.InvalidClientId)] public async Task Can_Use_Only_Unreserved_Characters_For_ClientId(string clientId, UserClientCredentialsOperationStatus expectedResult) { // Arrange @@ -1017,6 +1016,20 @@ public class UserServiceTests : UmbracoIntegrationTest Assert.AreEqual(expectedResult, result); } + [TestCase("Less_Than_100_characters_0123456789012345678901234567890123456789012345678901234567890123456789", UserClientCredentialsOperationStatus.Success)] + [TestCase("More_Than_100_characters_01234567890123456789012345678901234567890123456789012345678901234567890123456789", UserClientCredentialsOperationStatus.InvalidClientId)] + public async Task Cannot_Create_Too_Long_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();