From da0421890fb76ee1b7c58d60ac4105443ef86603 Mon Sep 17 00:00:00 2001 From: Benson Bird <89985506+BensonB12@users.noreply.github.com> Date: Thu, 17 Oct 2024 08:03:26 -0600 Subject: [PATCH] [PM-12777] Fixed Issue #4034, API endpoint now handles optional parameters (#4812) * resolves issue #4043 default values for itemsPerPage and startIndex * UsersController#Get now uses a queryParamModel Co-authored-by: Ahmad Mustafa Jebran Co-authored-by: Luris Solis * Test now passes, default 50 is represented --------- Co-authored-by: Jared McCannon --- .../Scim/Controllers/v2/UsersController.cs | 10 ++--- .../src/Scim/Models/GetUserQueryParamModel.cs | 12 ++++++ .../src/Scim/Users/GetUsersListQuery.cs | 13 ++++-- .../Users/Interfaces/IGetUsersListQuery.cs | 2 +- .../Controllers/v2/UsersControllerTests.cs | 40 +++++++++++++++++++ .../Scim.Test/Users/GetUsersListQueryTests.cs | 10 ++--- 6 files changed, 71 insertions(+), 16 deletions(-) create mode 100644 bitwarden_license/src/Scim/Models/GetUserQueryParamModel.cs diff --git a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs index 5e73505b9..1323205b9 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs @@ -57,17 +57,15 @@ public class UsersController : Controller [HttpGet("")] public async Task Get( Guid organizationId, - [FromQuery] string filter, - [FromQuery] int? count, - [FromQuery] int? startIndex) + [FromQuery] GetUsersQueryParamModel model) { - var usersListQueryResult = await _getUsersListQuery.GetUsersListAsync(organizationId, filter, count, startIndex); + var usersListQueryResult = await _getUsersListQuery.GetUsersListAsync(organizationId, model); var scimListResponseModel = new ScimListResponseModel { Resources = usersListQueryResult.userList.Select(u => new ScimUserResponseModel(u)).ToList(), - ItemsPerPage = count.GetValueOrDefault(usersListQueryResult.userList.Count()), + ItemsPerPage = model.Count, TotalResults = usersListQueryResult.totalResults, - StartIndex = startIndex.GetValueOrDefault(1), + StartIndex = model.StartIndex, }; return Ok(scimListResponseModel); } diff --git a/bitwarden_license/src/Scim/Models/GetUserQueryParamModel.cs b/bitwarden_license/src/Scim/Models/GetUserQueryParamModel.cs new file mode 100644 index 000000000..27d7b6d9a --- /dev/null +++ b/bitwarden_license/src/Scim/Models/GetUserQueryParamModel.cs @@ -0,0 +1,12 @@ +using System.ComponentModel.DataAnnotations; + +public class GetUsersQueryParamModel +{ + public string Filter { get; init; } = string.Empty; + + [Range(1, int.MaxValue)] + public int Count { get; init; } = 50; + + [Range(1, int.MaxValue)] + public int StartIndex { get; init; } = 1; +} diff --git a/bitwarden_license/src/Scim/Users/GetUsersListQuery.cs b/bitwarden_license/src/Scim/Users/GetUsersListQuery.cs index 1bea930f1..9bcbcbdaf 100644 --- a/bitwarden_license/src/Scim/Users/GetUsersListQuery.cs +++ b/bitwarden_license/src/Scim/Users/GetUsersListQuery.cs @@ -13,11 +13,16 @@ public class GetUsersListQuery : IGetUsersListQuery _organizationUserRepository = organizationUserRepository; } - public async Task<(IEnumerable userList, int totalResults)> GetUsersListAsync(Guid organizationId, string filter, int? count, int? startIndex) + public async Task<(IEnumerable userList, int totalResults)> GetUsersListAsync(Guid organizationId, GetUsersQueryParamModel userQueryParams) { string emailFilter = null; string usernameFilter = null; string externalIdFilter = null; + + int count = userQueryParams.Count; + int startIndex = userQueryParams.StartIndex; + string filter = userQueryParams.Filter; + if (!string.IsNullOrWhiteSpace(filter)) { var filterLower = filter.ToLowerInvariant(); @@ -56,11 +61,11 @@ public class GetUsersListQuery : IGetUsersListQuery } totalResults = userList.Count; } - else if (string.IsNullOrWhiteSpace(filter) && startIndex.HasValue && count.HasValue) + else if (string.IsNullOrWhiteSpace(filter)) { userList = orgUsers.OrderBy(ou => ou.Email) - .Skip(startIndex.Value - 1) - .Take(count.Value) + .Skip(startIndex - 1) + .Take(count) .ToList(); totalResults = orgUsers.Count; } diff --git a/bitwarden_license/src/Scim/Users/Interfaces/IGetUsersListQuery.cs b/bitwarden_license/src/Scim/Users/Interfaces/IGetUsersListQuery.cs index 265c6a8e7..f584cb8e7 100644 --- a/bitwarden_license/src/Scim/Users/Interfaces/IGetUsersListQuery.cs +++ b/bitwarden_license/src/Scim/Users/Interfaces/IGetUsersListQuery.cs @@ -4,5 +4,5 @@ namespace Bit.Scim.Users.Interfaces; public interface IGetUsersListQuery { - Task<(IEnumerable userList, int totalResults)> GetUsersListAsync(Guid organizationId, string filter, int? count, int? startIndex); + Task<(IEnumerable userList, int totalResults)> GetUsersListAsync(Guid organizationId, GetUsersQueryParamModel userQueryParams); } diff --git a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs index c0e4f3eb7..1c9b0c112 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs +++ b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs @@ -236,6 +236,46 @@ public class UsersControllerTests : IClassFixture, IAsyn AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); } + [Fact] + public async Task GetList_SearchUserNameWithoutOptionalParameters_Success() + { + string filter = "userName eq user2@example.com"; + int? itemsPerPage = null; + int? startIndex = null; + var expectedResponse = new ScimListResponseModel + { + ItemsPerPage = 50, //default value + TotalResults = 1, + StartIndex = 1, //default value + Resources = new List + { + new ScimUserResponseModel + { + Id = ScimApplicationFactory.TestOrganizationUserId2, + DisplayName = "Test User 2", + ExternalId = "UB", + Active = true, + Emails = new List + { + new BaseScimUserModel.EmailModel { Primary = true, Type = "work", Value = "user2@example.com" } + }, + Groups = new List(), + Name = new BaseScimUserModel.NameModel("Test User 2"), + UserName = "user2@example.com", + Schemas = new List { ScimConstants.Scim2SchemaUser } + } + }, + Schemas = new List { ScimConstants.Scim2SchemaListResponse } + }; + + var context = await _factory.UsersGetListAsync(ScimApplicationFactory.TestOrganizationId1, filter, itemsPerPage, startIndex); + + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + var responseModel = JsonSerializer.Deserialize>(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); + } + [Fact] public async Task Post_Success() { diff --git a/bitwarden_license/test/Scim.Test/Users/GetUsersListQueryTests.cs b/bitwarden_license/test/Scim.Test/Users/GetUsersListQueryTests.cs index b7497e281..9352e5c20 100644 --- a/bitwarden_license/test/Scim.Test/Users/GetUsersListQueryTests.cs +++ b/bitwarden_license/test/Scim.Test/Users/GetUsersListQueryTests.cs @@ -24,7 +24,7 @@ public class GetUsersListQueryTests .GetManyDetailsByOrganizationAsync(organizationId) .Returns(organizationUserUserDetails); - var result = await sutProvider.Sut.GetUsersListAsync(organizationId, null, count, startIndex); + var result = await sutProvider.Sut.GetUsersListAsync(organizationId, new GetUsersQueryParamModel { Count = count, StartIndex = startIndex }); await sutProvider.GetDependency().Received(1).GetManyDetailsByOrganizationAsync(organizationId); @@ -49,7 +49,7 @@ public class GetUsersListQueryTests .GetManyDetailsByOrganizationAsync(organizationId) .Returns(organizationUserUserDetails); - var result = await sutProvider.Sut.GetUsersListAsync(organizationId, filter, null, null); + var result = await sutProvider.Sut.GetUsersListAsync(organizationId, new GetUsersQueryParamModel { Filter = filter }); await sutProvider.GetDependency().Received(1).GetManyDetailsByOrganizationAsync(organizationId); @@ -71,7 +71,7 @@ public class GetUsersListQueryTests .GetManyDetailsByOrganizationAsync(organizationId) .Returns(organizationUserUserDetails); - var result = await sutProvider.Sut.GetUsersListAsync(organizationId, filter, null, null); + var result = await sutProvider.Sut.GetUsersListAsync(organizationId, new GetUsersQueryParamModel { Filter = filter }); await sutProvider.GetDependency().Received(1).GetManyDetailsByOrganizationAsync(organizationId); @@ -96,7 +96,7 @@ public class GetUsersListQueryTests .GetManyDetailsByOrganizationAsync(organizationId) .Returns(organizationUserUserDetails); - var result = await sutProvider.Sut.GetUsersListAsync(organizationId, filter, null, null); + var result = await sutProvider.Sut.GetUsersListAsync(organizationId, new GetUsersQueryParamModel { Filter = filter }); await sutProvider.GetDependency().Received(1).GetManyDetailsByOrganizationAsync(organizationId); @@ -120,7 +120,7 @@ public class GetUsersListQueryTests .GetManyDetailsByOrganizationAsync(organizationId) .Returns(organizationUserUserDetails); - var result = await sutProvider.Sut.GetUsersListAsync(organizationId, filter, null, null); + var result = await sutProvider.Sut.GetUsersListAsync(organizationId, new GetUsersQueryParamModel { Filter = filter }); await sutProvider.GetDependency().Received(1).GetManyDetailsByOrganizationAsync(organizationId);