[PM-28871] Default startIndex and count values on SCIM groups list API

[kspearrin]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-28871

📔 Objective

If no query params were provided, no results were returned from the SCIM Groups list API. This is because the Groups GetUsersListAsync query did not support default values for startIndex or count. This PR adds default values for startIndex (1) and count (50) through a new query params model class, similar to what we do on the Users list API.

[github-actions]

Checkmarx One – Scan Summary & Details – c5c458c5-cc96-4fe5-afab-ec105204f02e

Fixed Issues (2)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 89
	CSRF	/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 99

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.28%. Comparing base (219993c) to head (290d522).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6648   +/-   ##
=======================================
  Coverage   53.28%   53.28%           
=======================================
  Files        1906     1907    +1     
  Lines       84973    84979    +6     
  Branches     7639     7639           
=======================================
+ Hits        45275    45282    +7     
  Misses      37944    37944           
+ Partials     1754     1753    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @kspearrin's task —— View job

---

Code Review Complete

• Read project guidelines and context
• Review PR title and description quality
• Examine changed files
• Check for integration test (per reviewer request)
• Identify bugs, security issues, and improvements
• Post comprehensive review

---

Summary

This PR successfully mirrors the Users list API pattern for the Groups list API by introducing default values through a query param model. The implementation is clean and consistent with existing patterns.

Findings

Finding 1: ❌ Missing integration test for default parameter behavior (bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/GroupsControllerTests.cs)

The reviewer correctly identified that an integration test mirroring the Users API test is missing. The Users API has GetList_SearchUserNameWithoutOptionalParameters_Success (line 243) that verifies default values work when no query params are provided. A similar test should be added to GroupsControllerTests.

Suggested test implementation

[Fact]
public async Task GetList_SearchDisplayNameWithoutOptionalParameters_Success()
{
    var organizationId = ScimApplicationFactory.TestOrganizationId1;
    string filter = "displayName eq Test Group 2";
    int? itemsPerPage = null;
    int? startIndex = null;
    var expectedResponse = new ScimListResponseModel<ScimGroupResponseModel>
    {
        ItemsPerPage = 50, //default value
        TotalResults = 1,
        StartIndex = 1, //default value
        Resources = new List<ScimGroupResponseModel>
        {
            new ScimGroupResponseModel
            {
                Id = ScimApplicationFactory.TestGroupId2,
                DisplayName = "Test Group 2",
                ExternalId = "B",
                Schemas = new List<string> { ScimConstants.Scim2SchemaGroup }
            }
        },
        Schemas = new List<string> { ScimConstants.Scim2SchemaListResponse }
    };

    var context = await _factory.GroupsGetListAsync(organizationId, filter, itemsPerPage, startIndex);

    Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);

    var responseModel = JsonSerializer.Deserialize<ScimListResponseModel<ScimGroupResponseModel>>(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase });
    AssertHelper.AssertPropertyEqual(expectedResponse, responseModel);
}

This test should be added after line 176 in GroupsControllerTests.cs to verify that the default values (StartIndex=1, Count=50) are correctly applied when parameters are omitted.

---

Good Practices

• Consistent parameter validation using [Range(1, int.MaxValue)] attributes
• Parallel implementation matching Users API pattern
• Proper unit test coverage for the query logic
• Clear documentation in PR objective

---

PR Title and Description Assessment

💭 The PR title and description meet minimum requirements but could be enhanced:

Current title: [PM-28871] Default startIndex and count values on SCIM groups list API

Suggested improvement: [PM-28871] Add default values for SCIM Groups list API pagination parameters

Current objective: Adequately explains the issue and solution, but could benefit from:

• Mentioning the specific default values (startIndex=1, count=50) in the objective section
• Adding a test plan section describing how the fix was verified

---

[r-tome]

This looks great. I think we should also add an integration test to cover what we're fixing here. There’s an identical case tested in UsersController that we can mirror