Introduce user credential management API #26425
Conversation
WalkthroughAdds Credential Management API dependencies and servlet registrations, and introduces integration test infrastructure: a test base, positive/negative test suites, REST client, DTOs, and testng registration. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java (1)
31-57: Clean DTO for credential response; optionally addtoStringThe DTO is minimal and correct: Jackson mapping, getters/setters, and
equals/hashCodeall align on thecredentialsfield. For easier assertion/debugging in tests, consider adding atoString()implementation, but this is optional.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
30-78: Credential model looks correct and consistentField definitions, Jackson annotations, and
equals/hashCodeare all coherent and symmetric acrosscredentialId,type, anddisplayName. As withCredentialResponse, atoString()could make test failures more readable, but is not required.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java (3)
92-155: OAuth2 REST helpers are idiomatic; duplication is acceptable for testsThe GET/POST/DELETE/PUT helpers around RestAssured are straightforward and correctly apply preemptive OAuth2 auth, JSON content type, and conditional logging. Given this is test code, the small amount of duplication is acceptable and keeps each method clear.
If you find yourself adding more verbs in future, consider extracting a small helper that configures the common
given()chain and takes the HTTP method as a parameter.
163-171: Clarify authorized API list vs credential management endpoints
getAuthorizedAPIList()is documented as "APIs that need to be authorized for credential management operations", but currently only/api/server/v1/organizationswith organization scopes is returned. If credential management endpoints (e.g.,/api/server/v1/users/.../credentials) also require explicit scopes for these tests, consider adding them here; otherwise, consider rewording the Javadoc to clarify that this list is only for the org‑setup part of the scenario.Please double‑check how
getAuthorizedAPIList()is consumed inRESTAPIServerTestBaseto ensure credential‑management calls aren’t unintentionally running without the expected scope setup.
179-186: MakeconvertToOrgBasePathlogic consistent across tenantsThe method builds the org base path differently for super tenant vs others:
- Super:
"/t/" + tenant + "/o" + basePath- Non‑super:
basePath.replace(tenant, tenant + "/o")Depending on what
basePathcontains (with or without/t/{tenant}), this can either duplicate the"/t/{tenant}"segment or fail to add it for non‑super tenants. A more explicit, path‑based transformation would be safer and easier to read.For example:
- protected String convertToOrgBasePath(String basePath) { - - if (StringUtils.equals(MultitenantConstants.SUPER_TENANT_DOMAIN_NAME, tenant)) { - return TENANTED_URL_PATH_SPECIFIER + tenant + ORGANIZATION_PATH_SPECIFIER + basePath; - } else { - return basePath.replace(tenant, tenant + ORGANIZATION_PATH_SPECIFIER); - } - } + protected String convertToOrgBasePath(String basePath) { + // Expect basePath in the form "/t/{tenant}/...". + final String tenantPrefix = TENANTED_URL_PATH_SPECIFIER + tenant; + if (!basePath.startsWith(tenantPrefix)) { + // Fallback: leave unchanged rather than risk mangling the path. + return basePath; + } + return tenantPrefix + ORGANIZATION_PATH_SPECIFIER + basePath.substring(tenantPrefix.length()); + }This keeps behavior consistent for both super and non‑super tenants and avoids relying on a plain
String.replaceon the tenant name.Verify how
basePathis initialized inRESTAPIServerTestBase(with or without the/t/{tenant}prefix) to confirm the above transformation matches the expected URL layout in your tests.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (3)
67-79: Consider reusing ObjectMapper instance.Creating a new
ObjectMapperon every call is inefficient. Consider making it a class-level constant sinceObjectMapperis thread-safe for reading operations.public class CredentialManagementRestClient extends RestBaseClient { + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new JsonFactory()); private static final String CREDENTIAL_MANAGEMENT_API_PATH = "users";Then use
OBJECT_MAPPER.readValue(...)instead of creating a new instance.
89-97: Avoid using test assertions in REST client methods.Using
Assert.assertEqualsin a client method couples the client to the test framework and is inconsistent withgetUserCredentials, which throws an exception on failure. The client should throw an exception, letting the caller handle test assertions.public void deleteUserCredential(String userId, String type, String credentialId) throws Exception { String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH + PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId; try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) { - Assert.assertEquals(response.getStatusLine().getStatusCode(), HTTP_NO_CONTENT, - "Credential deletion failed for user: " + userId); + if (response.getStatusLine().getStatusCode() != HTTP_NO_CONTENT) { + throw new Exception("Error code " + response.getStatusLine().getStatusCode() + + " occurred while deleting credential for user: " + userId); + } } }
118-127: Specify charset for consistent encoding.
getBytes()without a charset uses the platform default, which may vary across environments. UseStandardCharsets.UTF_8for consistent behavior.+import java.nio.charset.StandardCharsets;headerList[1] = new BasicHeader(AUTHORIZATION_ATTRIBUTE, BASIC_AUTHORIZATION_ATTRIBUTE + - Base64.encodeBase64String((username + ":" + password).getBytes()).trim()); + Base64.encodeBase64String((username + ":" + password).getBytes(StandardCharsets.UTF_8)).trim());modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (2)
48-52: Remove unused constantINVALID_USER_ID.The constant
INVALID_USER_IDis declared but never used in any test method.// Invalid test data private static final String NON_EXISTENT_USER_ID = "non-existent-user-12345"; - private static final String INVALID_USER_ID = "invalid@user#id"; private static final String NON_EXISTENT_CREDENTIAL_ID = "non-existent-credential-12345"; private static final String INVALID_CREDENTIAL_TYPE = "INVALID_TYPE";
198-205: Consider polling instead of fixed sleep.
Thread.sleep(2000)is fragile—it may be insufficient on slow systems or wasteful on fast ones. Consider a polling mechanism that waits until the organizations are confirmed created, with a timeout.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (2)
145-161: Positive tests accepting error codes dilute test value.Tests named "positive" should verify successful operations, but this test accepts
400(Bad Request) and404(Not Found) as valid outcomes alongside204(No Content). This makes the test pass even when the operation fails. Consider setting up actual credentials in@BeforeClassand asserting only204for true positive validation, or rename these to reflect they are smoke/tolerance tests.
92-94: ReplaceSystem.out.printlnwith proper logging.Using
System.out.printlnfor warnings makes it harder to filter and manage test output. Consider using a logging framework (e.g., SLF4J/Log4j) consistent with the rest of the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
modules/api-resources/api-resources-full/pom.xml(1 hunks)modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml(2 hunks)modules/api-resources/pom.xml(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml(1 hunks)pom.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-79)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (1)
CredentialManagementRestClient(43-128)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (1)
OrgMgtRestClient(69-492)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-79)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (1)
CredentialManagementRestClient(43-128)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (1)
OrgMgtRestClient(69-492)
🔇 Additional comments (6)
modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml (2)
83-84: Credential management UsersApi correctly wired into /server/v1 servletAdding
org.wso2.carbon.identity.api.server.credential.management.v1.UsersApialongside other server APIs is consistent with existing patterns and should expose the new endpoints under/server/v1. No issues spotted here.
176-177: Organization-aware servlet also registers credential management UsersApiRegistering the same
UsersApiunderOrganizationServerV1Servletkeeps the/o/server/v1surface aligned with/server/v1. This mirrors how other server APIs are exposed. Looks good.modules/api-resources/api-resources-full/pom.xml (1)
599-606: Credential management server API dependencies are wired consistentlyAdding:
org.wso2.carbon.identity.api.server.credential.management.v1org.wso2.carbon.identity.api.server.credential.management.commonto
api-resources-fullmatches how other server APIs are pulled into the WAR and aligns with the corresponding entries in the parentmodules/api-resources/pom.xml. No structural issues noticed.Please confirm that both artifacts are present for
${identity.server.api.version}in your Maven repo and that no extra exclusions or packaging rules are needed beyond the existingWEB-INF/lib/org.wso2.carbon.identity.api.*.jarpattern.modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml (1)
295-296: Credential management REST tests correctly added to TestNG suiteRegistering
CredentialManagementPositiveTestandCredentialManagementNegativeTestunder theis-test-rest-apitest block is consistent with how other server REST API suites are organized and ensures the new endpoints are covered in CI.modules/api-resources/pom.xml (1)
633-642: Credential management artifacts added to dependencyManagement in line with other server APIsThe two new credential‑management artifacts are correctly declared with
${identity.server.api.version}, matching the convention used for the rest of theorg.wso2.carbon.identity.server.apimodules. This keeps versioning centralized and consistent across modules.It’s worth confirming that
${identity.server.api.version}already includes these two new artifacts (e.g., by checking your internal Maven repo or the BOM) so that consuming modules don’t hit resolution errors.pom.xml (1)
2850-2850: The version propertyidentity.integration.ui.templates.versionis set to1.0.28(not1.0.27as initially stated), and is correctly referenced by 16 template ZIP dependencies in dependencyManagement. No action needed if this version was intentional; if a change to1.0.27is planned, verify artifact availability at that version.Likely an incorrect or invalid review comment.
| try { | ||
| orgMgtRestClient = new OrgMgtRestClient(context, context.getContextTenant(), serverURL, | ||
| getAuthorizedAPIList()); | ||
| createOrganizationHierarchy(); | ||
| } catch (Exception e) { | ||
| System.out.println("Warning: Organization setup failed: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd modules/integration/tests-integration/tests-backend && find . -name "CredentialManagementNegativeTest.java" -type fRepository: wso2/product-is
Length of output: 188
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | head -150Repository: wso2/product-is
Length of output: 7402
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | grep -A 5 -B 5 "parentOrgId\|secondaryOrgId"Repository: wso2/product-is
Length of output: 2433
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | grep -E "testCrossOrganization|parentOrgId|secondaryOrgId"Repository: wso2/product-is
Length of output: 758
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | tail -100Repository: wso2/product-is
Length of output: 5046
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | grep -E "SECONDARY_ORG_USER_ID|TEST_PASSKEY_CREDENTIAL_ID"Repository: wso2/product-is
Length of output: 401
🏁 Script executed:
cd modules/integration/tests-integration/tests-backend && find . -name "CredentialManagementTestBase.java" -type fRepository: wso2/product-is
Length of output: 184
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java | head -100Repository: wso2/product-is
Length of output: 4344
🏁 Script executed:
grep -n "parentOrgId\|secondaryOrgId" modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.javaRepository: wso2/product-is
Length of output: 568
🏁 Script executed:
cd modules/integration/tests-integration/tests-backend && find . -path "*/credential/management/v1/*.java" -type f | grep -i testRepository: wso2/product-is
Length of output: 680
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTest.java | grep -A 5 -B 5 "createOrganizationHierarchy\|parentOrgId\|secondaryOrgId" | head -40Repository: wso2/product-is
Length of output: 262
🏁 Script executed:
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java | grep -B 5 -A 10 "createOrganizationHierarchy\|parentOrgId"Repository: wso2/product-is
Length of output: 3381
🏁 Script executed:
cd modules/integration/tests-integration/tests-backend && find . -path "*/rest/api/server/*.java" -type f | head -5 | xargs grep -l "logger\|LOG" | head -3Repository: wso2/product-is
Length of output: 41
Use proper logging instead of System.out.println in exception handlers.
The exception handling at lines 83-90 catches setup failures silently using System.out.println instead of a proper logger. While the test methods (testCrossOrganizationGetCredentialsIsolation, etc.) use hardcoded user ID constants rather than the dynamically created organization IDs, adopting standard logging practices improves maintainability and debuggability of test failures.
🤖 Prompt for AI Agents
In
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java
around lines 83-90, replace the System.out.println in the catch block with a
proper SLF4J logger call and include the caught exception so stacktrace is
preserved; add a private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CredentialManagementNegativeTest.class) at the
top of the class if not present, then call logger.warn("Organization setup
failed", e) (or logger.error if more appropriate) instead of printing to stdout.
| // Create sub-organization | ||
| String subOrgHandle = System.currentTimeMillis() + "-" + SUB_ORG_HANDLE; | ||
| subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, subOrgHandle); | ||
|
|
There was a problem hiding this comment.
Incorrect argument passed to addSubOrganization.
addSubOrganization expects (String orgName, String parentOrgId) based on the OrgMgtRestClient API, but the code passes subOrgHandle (a handle string) instead of parentOrgId. This will fail to create the sub-organization under the correct parent.
// Create sub-organization
- String subOrgHandle = System.currentTimeMillis() + "-" + SUB_ORG_HANDLE;
- subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, subOrgHandle);
+ subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, parentOrgId);| if (subOrgId != null) { | ||
| try { | ||
| orgMgtRestClient.deleteOrganization(subOrgId); | ||
| } catch (Exception e) { | ||
| System.out.println("Warning: Failed to delete sub org: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use deleteSubOrganization for sub-organization cleanup.
The sub-organization was created using addSubOrganization, but cleanup uses deleteOrganization. Based on OrgMgtRestClient, sub-organizations should be deleted using deleteSubOrganization(orgId, parentOrgId).
if (subOrgId != null) {
try {
- orgMgtRestClient.deleteOrganization(subOrgId);
+ orgMgtRestClient.deleteSubOrganization(subOrgId, parentOrgId);
} catch (Exception e) {
System.out.println("Warning: Failed to delete sub org: " + e.getMessage());
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java
around lines 320-326, the cleanup calls
orgMgtRestClient.deleteOrganization(subOrgId) but the sub-organization was
created with addSubOrganization and must be removed with deleteSubOrganization;
change the cleanup to call orgMgtRestClient.deleteSubOrganization(subOrgId,
parentOrgId) (or the appropriate parent org id variable in this test) and handle
exceptions the same way so the sub-org is deleted correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (2)
291-293: Fix the incorrect argument to addSubOrganization.The method signature for
addSubOrganizationexpects(String orgName, String parentOrgId), but Line 293 passessubOrgHandle(a timestamp-based string) instead ofparentOrgId. This will cause the sub-organization to be created without the correct parent relationship.Apply this diff to fix the issue:
- String subOrgHandle = System.currentTimeMillis() + "-" + SUB_ORG_HANDLE; - subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, subOrgHandle); + subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, parentOrgId);
320-326: Use deleteSubOrganization for sub-organization cleanup.The sub-organization was created using
addSubOrganization, so it should be deleted usingdeleteSubOrganization(orgId, parentOrgId)instead ofdeleteOrganization(orgId)to maintain API consistency.Apply this diff to fix the cleanup:
if (subOrgId != null) { try { - orgMgtRestClient.deleteOrganization(subOrgId); + orgMgtRestClient.deleteSubOrganization(subOrgId, parentOrgId); } catch (Exception e) { System.out.println("Warning: Failed to delete sub org: " + e.getMessage()); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
modules/api-resources/api-resources-full/pom.xml(1 hunks)modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml(2 hunks)modules/api-resources/pom.xml(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java(1 hunks)modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java
- modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
🧰 Additional context used
🧬 Code graph analysis (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/serviceextensions/model/Tenant.java (1)
Tenant(26-75)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-79)
🔇 Additional comments (12)
modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml (1)
83-84: Service registration placement is consistent.The
UsersApifrom the credential management module is properly registered in both servlet configurations with correct formatting and trailing comma preservation.Also applies to: 176-177
modules/api-resources/api-resources-full/pom.xml (1)
599-606: Dependency structure follows established patterns.The credential management API dependencies are correctly added as a v1 and common pair, consistent with other API modules in the file. Version inheritance from parent POM is properly applied.
modules/api-resources/pom.xml (1)
633-642: DependencyManagement declarations are well-structured.The credential management API dependencies are correctly declared in the parent POM with explicit version property
${identity.server.api.version}, maintaining consistency with other server API modules and enabling proper version management across the build.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
28-78: LGTM!The model class is well-structured with proper Jackson annotations, standard accessors, and correct equals/hashCode implementations.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (3)
67-79: LGTM!The method correctly handles credential retrieval with proper error handling and JSON deserialization to Credential array.
89-97: LGTM!The delete operation is properly implemented with correct path construction and status code validation.
109-127: LGTM!Helper methods correctly handle tenant-aware path construction and Basic Authentication header setup.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java (1)
29-57: LGTM!The response wrapper class is correctly implemented with proper Jackson binding for the credentials list.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (1)
217-277: LGTM!The credential isolation and access control tests are well-designed, properly testing cross-organization boundaries and admin privileges within tenant hierarchies.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java (3)
58-83: LGTM!The static initialization and lifecycle hooks properly manage Swagger definition loading and RestAssured base path configuration.
92-155: LGTM!The OAuth2-enabled HTTP helper methods are consistently implemented with proper authentication, content type handling, and logging.
163-186: LGTM!The helper methods correctly handle organization scope authorization and tenant-aware path conversion for both super tenant and regular tenant contexts.
...egration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml:
- Around line 83-86: Remove the duplicate registration of
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi from
the ServerV1ApiServlet service classes list so each API class is listed only
once; locate the duplicate entries for WorkflowInstancesApi in the service
classes block and delete the redundant line, leaving the other API registrations
(e.g., VcTemplatesApi, UsersApi) intact.
In
@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java:
- Around line 140-150: The test
testGetCredentialsUsingRestClientWithNonExistentUser currently only asserts
inside the catch block so it will silently pass if
credentialManagementRestClient.getUserCredentials(NON_EXISTENT_USER_ID) does not
throw; modify the test to call getUserCredentials and capture its return (or
wrap call), then if no exception is thrown explicitly fail the test (e.g., call
Assert.fail with a clear message) or assert the returned value indicates
non-existence (e.g., empty array/list), and keep the existing catch block to
assert the expected error text; reference test method
testGetCredentialsUsingRestClientWithNonExistentUser and the call to
credentialManagementRestClient.getUserCredentials to locate where to add the
Assert.fail or explicit assertion.
🧹 Nitpick comments (6)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (2)
89-97: Using assertion in REST client may obscure actual error details.
Assert.assertEqualswill throwAssertionErrorwithout the actual HTTP response body, making debugging harder. Consider throwing an exception with response details instead.Suggested improvement for better error diagnostics
public void deleteUserCredential(String userId, String type, String credentialId) throws Exception { String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH + PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId; try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) { - Assert.assertEquals(response.getStatusLine().getStatusCode(), HTTP_NO_CONTENT, - "Credential deletion failed for user: " + userId); + if (response.getStatusLine().getStatusCode() != HTTP_NO_CONTENT) { + String responseBody = EntityUtils.toString(response.getEntity()); + throw new Exception("Credential deletion failed for user: " + userId + + ". Status: " + response.getStatusLine().getStatusCode() + + ". Response: " + responseBody); + } } }
104-107: Missing null check before closing HTTP client.If the client initialization failed, calling
close()could throw an NPE.Suggested fix
public void closeHttpClient() throws IOException { + if (client != null) { client.close(); + } }modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (4)
78-90: Consider using a logger instead of System.out.println for warnings.Test infrastructure code should use proper logging for better observability and consistency.
152-173: Test assertion logic is overly permissive.Accepting 200, 403, or 404 as valid responses doesn't clearly validate cross-organization isolation. If 200 is acceptable, the empty array check is a secondary fallback that may mask issues.
Consider documenting the expected API behavior more clearly or tightening the assertions once the expected behavior is confirmed.
198-205: Avoid using Thread.sleep in tests.
Thread.sleep(2000)introduces flakiness and slows down tests. Consider polling for organization readiness or using a retry mechanism.Suggested approach
private void createOrganizationHierarchy() throws Exception { String parentOrgHandle = System.currentTimeMillis() + "-" + PARENT_ORG_HANDLE; parentOrgId = orgMgtRestClient.addOrganization(PARENT_ORG_NAME, parentOrgHandle); String secondaryOrgHandle = System.currentTimeMillis() + "-" + SECONDARY_ORG_HANDLE; secondaryOrgId = orgMgtRestClient.addOrganization(SECONDARY_ORG_NAME, secondaryOrgHandle); - Thread.sleep(2000); + // TODO: Replace with polling mechanism to verify organization creation is complete + waitForOrganizationReady(parentOrgId); + waitForOrganizationReady(secondaryOrgId); }
49-50: Unused constant INVALID_USER_ID.The constant
INVALID_USER_IDis declared but never used in any test method.Either remove it or add a test case that validates the API's behavior with malformed user IDs.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
modules/api-resources/api-resources-full/pom.xmlmodules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xmlmodules/api-resources/pom.xmlmodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.javamodules/integration/tests-integration/tests-backend/src/test/resources/testng.xmlpom.xml
✅ Files skipped from review due to trivial changes (1)
- pom.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
- modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/serviceextensions/model/Tenant.java (1)
Tenant(26-75)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-79)
🔇 Additional comments (11)
modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml (1)
178-179: LGTM!The
UsersApifor credential management is correctly added toOrganizationServerV1Servlet, enabling the credential management API for organization-level access.modules/api-resources/api-resources-full/pom.xml (1)
598-606: LGTM!The credential management API dependencies are correctly added, following the established pattern of pairing the v1 API module with its common module. These dependencies support the servlet registrations in web.xml.
modules/api-resources/pom.xml (1)
633-642: LGTM!The credential management dependencies are correctly added to
dependencyManagementwith version${identity.server.api.version}, following the established pattern for server API modules.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java (3)
42-56: LGTM on constants and static swagger loading.The constants are well-defined and the static initializer properly handles the swagger definition loading with appropriate test failure on error.
67-83: LGTM on lifecycle methods.The lifecycle methods correctly delegate to the parent class and reset RestAssured state between tests.
92-155: LGTM on OAuth2 HTTP helper methods.The helper methods follow RestAssured patterns consistently and correctly use preemptive OAuth2 authentication.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java (1)
29-57: LGTM on CredentialResponse model.The DTO follows standard patterns with proper Jackson annotation, accessors, and object contract methods (
equals/hashCode).modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (2)
52-58: LGTM on constructor.The constructor correctly initializes tenant info and builds the credential management base path.
67-79: LGTM on getUserCredentials method.The method properly uses try-with-resources for the HTTP response and throws a descriptive exception on failure.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (1)
59-76: LGTM on data provider and factory constructor.The multi-tenant test setup using TestNG's
@Factoryand@DataProviderfollows the established pattern in the codebase.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
28-78: LGTM on Credential model.The model class follows standard patterns with proper Jackson annotations, accessors, and correctly implemented
equals/hashCodemethods.
| org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi, | ||
| org.wso2.carbon.identity.api.server.vc.template.management.v1.VcTemplatesApi | ||
| org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi, | ||
| org.wso2.carbon.identity.api.server.credential.management.v1.UsersApi |
There was a problem hiding this comment.
Critical: Duplicate WorkflowInstancesApi entry.
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi is registered twice in the ServerV1ApiServlet service classes (line 83 and line 85). This duplicate entry should be removed as it could cause unexpected behavior during servlet initialization.
🔧 Proposed fix
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowsApi,
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowAssociationsApi,
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi,
- org.wso2.carbon.identity.api.server.vc.template.management.v1.VcTemplatesApi
- org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi,
- org.wso2.carbon.identity.api.server.credential.management.v1.UsersApi
+ org.wso2.carbon.identity.api.server.vc.template.management.v1.VcTemplatesApi,
+ org.wso2.carbon.identity.api.server.credential.management.v1.UsersApi🤖 Prompt for AI Agents
In @modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml
around lines 83 - 86, Remove the duplicate registration of
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi from
the ServerV1ApiServlet service classes list so each API class is listed only
once; locate the duplicate entries for WorkflowInstancesApi in the service
classes block and delete the redundant line, leaving the other API registrations
(e.g., VcTemplatesApi, UsersApi) intact.
...egration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java
Show resolved
Hide resolved
| protected String convertToOrgBasePath(String basePath) { | ||
|
|
||
| if (StringUtils.equals(MultitenantConstants.SUPER_TENANT_DOMAIN_NAME, tenant)) { | ||
| return TENANTED_URL_PATH_SPECIFIER + tenant + ORGANIZATION_PATH_SPECIFIER + basePath; | ||
| } else { | ||
| return basePath.replace(tenant, tenant + ORGANIZATION_PATH_SPECIFIER); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "convertToOrgBasePath|TENANTED_URL_PATH_SPECIFIER|ORGANIZATION_PATH_SPECIFIER" --type java -B 2 -A 2Repository: wso2/product-is
Length of output: 31140
🏁 Script executed:
rg -n "SUPER_TENANT_DOMAIN_NAME" --type java | head -20Repository: wso2/product-is
Length of output: 4866
🏁 Script executed:
find . -name "MultitenantConstants.java" -type f | head -3Repository: wso2/product-is
Length of output: 41
🏁 Script executed:
rg -n "SUPER_TENANT_DOMAIN_NAME\s*=" --type javaRepository: wso2/product-is
Length of output: 41
🏁 Script executed:
# Search for test methods that call convertToOrgBasePath to understand the basePath values
rg -B 5 "convertToOrgBasePath" --type java | grep -A 10 "basePath\|tenant"Repository: wso2/product-is
Length of output: 20308
🏁 Script executed:
# Find test classes that extend CredentialManagementTestBase
rg "extends CredentialManagementTestBase" --type javaRepository: wso2/product-is
Length of output: 588
🏁 Script executed:
# Look for usage of convertToOrgBasePath in the test classes
rg -A 5 -B 5 "convertToOrgBasePath" modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java | head -50Repository: wso2/product-is
Length of output: 41
🏁 Script executed:
# Read the full CredentialManagementTestBase to understand the context
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java | head -200Repository: wso2/product-is
Length of output: 7848
🏁 Script executed:
# Read RESTAPIServerTestBase to understand tenant and basePath
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/common/RESTAPIServerTestBase.java | grep -A 10 -B 2 "protected.*tenant\|protected.*basePath"Repository: wso2/product-is
Length of output: 931
🏁 Script executed:
# Check ISIntegrationTest.getTenantedRelativePath method
rg -A 10 "getTenantedRelativePath" --type java modules/integration/tests-common/integration-test-utils/src/main/java/org/wso2/identity/integration/common/utils/ISIntegrationTest.javaRepository: wso2/product-is
Length of output: 545
Path construction for organization base paths is inconsistent between tenant types.
The super tenant branch explicitly prepends /t/{tenant}/o to the base path, while the regular tenant branch uses string replacement to insert /o after the tenant name. This inconsistency could be error-prone if basePath ever contains multiple occurrences of the tenant identifier, as replace() would modify all instances.
Consider standardizing both branches to use explicit string concatenation/prepending instead of relying on replace(), similar to how the super tenant case is handled.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java`:
- Around line 95-105: Both negative tests
(testDeleteCredentialWithNonExistentCredentialId and
testDeleteCredentialWithInvalidCredentialType) call
credentialManagementRestClient.deleteUserCredential(...) without asserting the
response; update each test to capture the HTTP response (or catch the thrown
exception if the REST client is configured to throw on non-2xx) from
credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY,
NON_EXISTENT_CREDENTIAL_ID) and
credentialManagementRestClient.deleteUserCredential(VALID_USER_ID,
INVALID_CREDENTIAL_TYPE, VALID_CREDENTIAL_ID), then assert the status code is
the expected error (e.g., 400 or 404) and validate the error body/message
contains the expected error identifier; if the client throws, assert the
exception type and extract/assert the status and error message from the
exception instead.
- Around line 115-128: The tests testCrossOrganizationGetCredentialsIsolation
and testCrossOrganizationDeleteCredentialsIsolation are using a hardcoded
SECONDARY_ORG_USER_ID constant instead of real users created under parentOrgId
and secondaryOrgId, so they pass due to non-existent users; fix by creating test
users during setup (e.g., createUserInOrg or similar helper) in both parentOrgId
and secondaryOrgId, create credentials for the secondary-org user via the same
test helpers, then replace SECONDARY_ORG_USER_ID with the dynamically returned
user ID for that created user and assert that
credentialManagementRestClient.getUserCredentials(createdSecondaryUserId)
returns the expected credentials when called in the secondary org context and
returns none / throws authorization when called from the parent org context, and
similarly call
credentialManagementRestClient.deleteUserCredential(createdSecondaryUserId,
TYPE_PASSKEY, createdCredentialId) from the parent org context and assert it is
forbidden or fails rather than silently succeeding; also remove reliance on the
hardcoded "secondary-org-user-789" constant.
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java`:
- Around line 137-144: In testDeletePushAuthCredentialForSubOrgUser remove the
duplicate RestAssured.basePath assignment (only call
convertToOrgBasePath(this.basePath) once) and change the credential type passed
to credentialManagementRestClient.deleteUserCredential from TYPE_PASSKEY to
TYPE_PUSH_AUTH so the call matches the test name and deletes a push-auth
credential (update the call signature using SUB_ORG_USER_ID, TYPE_PUSH_AUTH,
SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID).
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java`:
- Around line 89-99: The deleteUserCredential method currently treats HTTP 400
as a success which hides client errors; change behavior so that only
HttpStatus.SC_NO_CONTENT is treated as success and any non-204 (including 400)
throws a descriptive exception (or returns a rich result object) containing the
response status and body to callers; update the assertion/response handling in
deleteUserCredential to throw an exception when responseStatusCode !=
HttpStatus.SC_NO_CONTENT and include the response entity or status in the
exception message, and fix the spacing typo in the comparison
(responseStatusCode == HttpStatus.SC_BAD_REQUEST) if you keep any special-case
checks.
♻️ Duplicate comments (2)
modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml (1)
84-86: Critical: DuplicateWorkflowInstancesApiand missing comma.
WorkflowInstancesApiis already registered on line 83 and duplicated on line 85. Additionally, line 84 (VcTemplatesApi) is missing a trailing comma, which will cause the service classes to be malformed.🔧 Proposed fix
org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi, - org.wso2.carbon.identity.api.server.vc.template.management.v1.VcTemplatesApi - org.wso2.carbon.identity.rest.api.server.workflow.v1.WorkflowInstancesApi, - org.wso2.carbon.identity.api.server.credential.management.v1.UsersApi + org.wso2.carbon.identity.api.server.vc.template.management.v1.VcTemplatesApi, + org.wso2.carbon.identity.api.server.credential.management.v1.UsersApimodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java (1)
108-115: Inconsistent path construction between tenant types.The super tenant branch explicitly prepends
/t/{tenant}/o, while the regular tenant branch usesreplace()which could modify multiple occurrences if the tenant name appears elsewhere in the path.
🧹 Nitpick comments (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.java (1)
34-34: Unused static import.The
givenimport from RestAssured is not used in this base class.🔧 Suggested fix
-import static io.restassured.RestAssured.given;modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (1)
96-106: Duplicate testInit/testFinish methods from base class.These methods are identical to the ones in
CredentialManagementTestBase(lines 74-84). Since this class extends the base class, these overrides are unnecessary.🔧 Suggested fix - remove duplicate methods
- `@BeforeMethod`(alwaysRun = true) - public void testInit() { - - RestAssured.basePath = basePath; - } - - `@AfterMethod`(alwaysRun = true) - public void testFinish() { - - RestAssured.basePath = StringUtils.EMPTY; - }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
modules/api-resources/api-resources-full/pom.xmlmodules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xmlmodules/api-resources/pom.xmlmodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.javamodules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- modules/api-resources/pom.xml
- modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java
- modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
- modules/api-resources/api-resources-full/pom.xml
🧰 Additional context used
🧬 Code graph analysis (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-85)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (1)
CredentialManagementRestClient(43-130)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (1)
OrgMgtRestClient(69-492)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-85)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (1)
CredentialManagementRestClient(43-130)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (1)
OrgMgtRestClient(69-492)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/serviceextensions/model/Tenant.java (1)
Tenant(26-75)modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/Credential.java (1)
Credential(28-85)
🔇 Additional comments (4)
modules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xml (1)
178-179: LGTM!The
UsersApifor credential management is correctly added to theOrganizationServerV1Servlet, and the trailing comma was appropriately added toScopesApito maintain proper formatting.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/model/CredentialResponse.java (1)
29-57: LGTM!The DTO implementation is correct with proper Jackson annotation, equals/hashCode implementation using
Objectsutility, and follows standard model class patterns.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java (1)
67-79: LGTM!The
getUserCredentialsmethod properly handles error responses by throwing an exception on non-200 status codes, allowing callers to handle errors appropriately.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java (1)
82-93: LGTM!The cleanup logic properly handles null checks for both REST clients and organization IDs before attempting to close/delete resources.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @Test(description = "Test delete credential with non-existent credential ID") | ||
| public void testDeleteCredentialWithNonExistentCredentialId() throws Exception { | ||
|
|
||
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY, NON_EXISTENT_CREDENTIAL_ID); | ||
| } | ||
|
|
||
| @Test(description = "Test delete credential with invalid credential type") | ||
| public void testDeleteCredentialWithInvalidCredentialType() throws Exception { | ||
|
|
||
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, INVALID_CREDENTIAL_TYPE, VALID_CREDENTIAL_ID); | ||
| } |
There was a problem hiding this comment.
Negative delete tests lack assertions to verify expected behavior.
Both testDeleteCredentialWithNonExistentCredentialId and testDeleteCredentialWithInvalidCredentialType simply call the delete method without any assertion. Since deleteUserCredential accepts HTTP 400 as valid, these tests will pass regardless of whether the API correctly rejects the invalid requests or not.
Negative tests should verify that the API returns the expected error response (e.g., 400 or 404) with an appropriate error message.
🐛 Proposed fix to add proper assertions
If the REST client is updated to throw on error responses:
`@Test`(description = "Test delete credential with non-existent credential ID")
-public void testDeleteCredentialWithNonExistentCredentialId() throws Exception {
+public void testDeleteCredentialWithNonExistentCredentialId() {
- credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY, NON_EXISTENT_CREDENTIAL_ID);
+ try {
+ credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY, NON_EXISTENT_CREDENTIAL_ID);
+ Assert.fail("Expected an error for non-existent credential ID but none was thrown");
+ } catch (Exception e) {
+ Assert.assertTrue(e.getMessage().contains("404") || e.getMessage().contains("400"),
+ "Expected 404 or 400 error but got: " + e.getMessage());
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test(description = "Test delete credential with non-existent credential ID") | |
| public void testDeleteCredentialWithNonExistentCredentialId() throws Exception { | |
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY, NON_EXISTENT_CREDENTIAL_ID); | |
| } | |
| @Test(description = "Test delete credential with invalid credential type") | |
| public void testDeleteCredentialWithInvalidCredentialType() throws Exception { | |
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, INVALID_CREDENTIAL_TYPE, VALID_CREDENTIAL_ID); | |
| } | |
| `@Test`(description = "Test delete credential with non-existent credential ID") | |
| public void testDeleteCredentialWithNonExistentCredentialId() { | |
| try { | |
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY, NON_EXISTENT_CREDENTIAL_ID); | |
| Assert.fail("Expected an error for non-existent credential ID but none was thrown"); | |
| } catch (Exception e) { | |
| Assert.assertTrue(e.getMessage().contains("404") || e.getMessage().contains("400"), | |
| "Expected 404 or 400 error but got: " + e.getMessage()); | |
| } | |
| } | |
| `@Test`(description = "Test delete credential with invalid credential type") | |
| public void testDeleteCredentialWithInvalidCredentialType() throws Exception { | |
| credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, INVALID_CREDENTIAL_TYPE, VALID_CREDENTIAL_ID); | |
| } |
🤖 Prompt for AI Agents
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java`
around lines 95 - 105, Both negative tests
(testDeleteCredentialWithNonExistentCredentialId and
testDeleteCredentialWithInvalidCredentialType) call
credentialManagementRestClient.deleteUserCredential(...) without asserting the
response; update each test to capture the HTTP response (or catch the thrown
exception if the REST client is configured to throw on non-2xx) from
credentialManagementRestClient.deleteUserCredential(VALID_USER_ID, TYPE_PASSKEY,
NON_EXISTENT_CREDENTIAL_ID) and
credentialManagementRestClient.deleteUserCredential(VALID_USER_ID,
INVALID_CREDENTIAL_TYPE, VALID_CREDENTIAL_ID), then assert the status code is
the expected error (e.g., 400 or 404) and validate the error body/message
contains the expected error identifier; if the client throws, assert the
exception type and extract/assert the status and error message from the
exception instead.
| @Test(description = "Test that admin from organization A cannot access credentials of users in organization B") | ||
| public void testCrossOrganizationGetCredentialsIsolation() throws Exception { | ||
|
|
||
| Credential[] credentials = credentialManagementRestClient.getUserCredentials(SECONDARY_ORG_USER_ID); | ||
| Assert.assertEquals(credentials.length, 0, | ||
| "Should not return credentials from secondary organization"); | ||
| } | ||
|
|
||
| @Test(description = "Test that admin from organization A cannot delete credentials of users in organization B") | ||
| public void testCrossOrganizationDeleteCredentialsIsolation() throws Exception { | ||
|
|
||
| credentialManagementRestClient.deleteUserCredential(SECONDARY_ORG_USER_ID, TYPE_PASSKEY, | ||
| TEST_PASSKEY_CREDENTIAL_ID); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the test class structure and find the base class
cd modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/
# Check the file and its class hierarchy
head -50 CredentialManagementNegativeTest.java | grep -E "^(public|class|extends|import)"Repository: wso2/product-is
Length of output: 695
🏁 Script executed:
# Search for SECONDARY_ORG_USER_ID definition and where it's used
rg "SECONDARY_ORG_USER_ID" modules/integration/tests-integration/tests-backend/src/test/java/ -B 2 -A 2Repository: wso2/product-is
Length of output: 4846
🏁 Script executed:
# Check for test setup methods that might create users in organizations
rg "@Before|setUp|createOrganization|createUser" modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/ -A 5Repository: wso2/product-is
Length of output: 10311
🏁 Script executed:
# Look for other cross-org isolation tests to understand the pattern
rg "CrossOrganization|testCrossOrg|isolat" modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/ -B 2 -A 8Repository: wso2/product-is
Length of output: 6958
🏁 Script executed:
# Check the full CredentialManagementTestBase.java file to see all setup
wc -l modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.javaRepository: wso2/product-is
Length of output: 238
🏁 Script executed:
# Get the full base test class
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementTestBase.javaRepository: wso2/product-is
Length of output: 5170
🏁 Script executed:
# Check if there's any user creation in the NegativeTest class
cat -n modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java | head -200Repository: wso2/product-is
Length of output: 7796
🏁 Script executed:
# Search for any code that creates users in organizations
rg "createUser|addUser" modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/ -B 2 -A 3Repository: wso2/product-is
Length of output: 41
Cross-organization isolation tests use hardcoded user IDs unrelated to created organizations.
The tests create parentOrgId and secondaryOrgId organizations but then use hardcoded constants like SECONDARY_ORG_USER_ID ("secondary-org-user-789") that are never created within those organizations. These tests pass because the user doesn't exist, not because cross-organization isolation is enforced. The test logic is identical to testGetCredentialsUsingRestClientWithNonExistentUser() (line 107-113), which explicitly tests non-existent users.
For meaningful isolation tests, create actual test users within each organization during setup and use the dynamically created user IDs to verify:
- Credentials exist for a user in their own organization
- Those credentials are inaccessible when querying from another organization context
🤖 Prompt for AI Agents
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementNegativeTest.java`
around lines 115 - 128, The tests testCrossOrganizationGetCredentialsIsolation
and testCrossOrganizationDeleteCredentialsIsolation are using a hardcoded
SECONDARY_ORG_USER_ID constant instead of real users created under parentOrgId
and secondaryOrgId, so they pass due to non-existent users; fix by creating test
users during setup (e.g., createUserInOrg or similar helper) in both parentOrgId
and secondaryOrgId, create credentials for the secondary-org user via the same
test helpers, then replace SECONDARY_ORG_USER_ID with the dynamically returned
user ID for that created user and assert that
credentialManagementRestClient.getUserCredentials(createdSecondaryUserId)
returns the expected credentials when called in the secondary org context and
returns none / throws authorization when called from the parent org context, and
similarly call
credentialManagementRestClient.deleteUserCredential(createdSecondaryUserId,
TYPE_PASSKEY, createdCredentialId) from the parent org context and assert it is
forbidden or fails rather than silently succeeding; also remove reliance on the
hardcoded "secondary-org-user-789" constant.
| @Test(description = "Test delete push-auth credential for sub-org user") | ||
| public void testDeletePushAuthCredentialForSubOrgUser() throws Exception { | ||
|
|
||
| RestAssured.basePath = convertToOrgBasePath(this.basePath); | ||
| RestAssured.basePath = convertToOrgBasePath(this.basePath); | ||
| credentialManagementRestClient.deleteUserCredential(SUB_ORG_USER_ID, TYPE_PASSKEY, | ||
| SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID); | ||
| } |
There was a problem hiding this comment.
Duplicate basePath assignment and wrong credential type.
Two issues in this test method:
- Line 140-141:
RestAssured.basePathis set twice (duplicate call) - Line 142: Uses
TYPE_PASSKEYinstead ofTYPE_PUSH_AUTHfor a push-auth credential delete test, which contradicts the test name and description
🐛 Proposed fix
`@Test`(description = "Test delete push-auth credential for sub-org user")
public void testDeletePushAuthCredentialForSubOrgUser() throws Exception {
RestAssured.basePath = convertToOrgBasePath(this.basePath);
- RestAssured.basePath = convertToOrgBasePath(this.basePath);
- credentialManagementRestClient.deleteUserCredential(SUB_ORG_USER_ID, TYPE_PASSKEY,
+ credentialManagementRestClient.deleteUserCredential(SUB_ORG_USER_ID, TYPE_PUSH_AUTH,
SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test(description = "Test delete push-auth credential for sub-org user") | |
| public void testDeletePushAuthCredentialForSubOrgUser() throws Exception { | |
| RestAssured.basePath = convertToOrgBasePath(this.basePath); | |
| RestAssured.basePath = convertToOrgBasePath(this.basePath); | |
| credentialManagementRestClient.deleteUserCredential(SUB_ORG_USER_ID, TYPE_PASSKEY, | |
| SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID); | |
| } | |
| `@Test`(description = "Test delete push-auth credential for sub-org user") | |
| public void testDeletePushAuthCredentialForSubOrgUser() throws Exception { | |
| RestAssured.basePath = convertToOrgBasePath(this.basePath); | |
| credentialManagementRestClient.deleteUserCredential(SUB_ORG_USER_ID, TYPE_PUSH_AUTH, | |
| SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID); | |
| } |
🤖 Prompt for AI Agents
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/credential/management/v1/CredentialManagementPositiveTest.java`
around lines 137 - 144, In testDeletePushAuthCredentialForSubOrgUser remove the
duplicate RestAssured.basePath assignment (only call
convertToOrgBasePath(this.basePath) once) and change the credential type passed
to credentialManagementRestClient.deleteUserCredential from TYPE_PASSKEY to
TYPE_PUSH_AUTH so the call matches the test name and deletes a push-auth
credential (update the call signature using SUB_ORG_USER_ID, TYPE_PUSH_AUTH,
SUB_ORG_USER_PUSH_AUTH_CREDENTIAL_ID).
| @Test(description = "Test that credentials from different orgs are isolated") | ||
| public void testCredentialIsolationBetweenOrgs() throws Exception { | ||
|
|
||
| // Get credentials for parent org user | ||
| Credential[] credentials = credentialManagementRestClient.getUserCredentials(PARENT_ORG_USER_ID); | ||
| Assert.assertNotNull(credentials, "Credentials array should not be null"); | ||
|
|
||
| // Get credentials for secondary org user | ||
| Credential[] secondaryUserCredentials = credentialManagementRestClient.getUserCredentials(SECONDARY_ORG_USER_ID); | ||
| Assert.assertEquals(secondaryUserCredentials.length, 0, | ||
| "Should not return credentials from secondary organization"); | ||
| } |
There was a problem hiding this comment.
Credential isolation test uses hardcoded user IDs.
Similar to the negative tests, testCredentialIsolationBetweenOrgs uses hardcoded constants (PARENT_ORG_USER_ID, SECONDARY_ORG_USER_ID) that don't correspond to actual users in the created organizations. The test verifies that SECONDARY_ORG_USER_ID returns empty credentials, but this could be because the user simply doesn't exist anywhere, not because of isolation.
For a meaningful isolation test, create actual users in each organization during setup.
| /** | ||
| * Create organization hierarchy for testing. | ||
| * Creates: Parent Org -> Sub Org, and Secondary Org (separate) | ||
| * | ||
| * @throws Exception If an error occurs while creating organizations. | ||
| */ | ||
| private void createOrganizationHierarchy() throws Exception { | ||
|
|
||
| parentOrgId = orgMgtRestClient.addOrganization(PARENT_ORG_NAME); | ||
| secondaryOrgId = orgMgtRestClient.addOrganization(SECONDARY_ORG_NAME); | ||
| } |
There was a problem hiding this comment.
Organization hierarchy does not create the sub-organization referenced in tests.
The Javadoc claims "Creates: Parent Org -> Sub Org, and Secondary Org" but the implementation only creates parentOrg and secondaryOrg. No sub-organization is created, yet several tests (testDeletePasskeyCredentialForSubOrgUser, testDeletePushAuthCredentialForSubOrgUser, testGetCredentialsForSubOrgUser) use SUB_ORG_USER_ID which implies a sub-organization should exist.
Either:
- Add sub-organization creation using
orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, parentOrgId) - Update the tests and Javadoc to reflect the actual organization structure
🔧 Example fix to add sub-organization
+ private static final String SUB_ORG_NAME = "subOrganization";
+ private String subOrgId;
private void createOrganizationHierarchy() throws Exception {
parentOrgId = orgMgtRestClient.addOrganization(PARENT_ORG_NAME);
+ subOrgId = orgMgtRestClient.addSubOrganization(SUB_ORG_NAME, parentOrgId);
secondaryOrgId = orgMgtRestClient.addOrganization(SECONDARY_ORG_NAME);
}
private void deleteOrganizationHierarchy() throws Exception {
if (orgMgtRestClient != null) {
// Delete in reverse order (sub-org first)
+ if (subOrgId != null) {
+ orgMgtRestClient.deleteSubOrganization(subOrgId, parentOrgId);
+ }
if (secondaryOrgId != null) {
orgMgtRestClient.deleteOrganization(secondaryOrgId);
}
if (parentOrgId != null) {
orgMgtRestClient.deleteOrganization(parentOrgId);
}
}
}| public void deleteUserCredential(String userId, String type, String credentialId) throws Exception { | ||
|
|
||
| String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH + | ||
| PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId; | ||
| try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) { | ||
| int responseStatusCode = response.getStatusLine().getStatusCode(); | ||
| Assert.assertTrue(responseStatusCode == HttpStatus.SC_NO_CONTENT || | ||
| responseStatusCode== HttpStatus.SC_BAD_REQUEST, | ||
| "Expected 204 or 400 but got: " + responseStatusCode); | ||
| } | ||
| } |
There was a problem hiding this comment.
Accepting HTTP 400 as a valid response in a REST client is problematic.
The deleteUserCredential method accepts both 204 No Content and 400 Bad Request as valid responses. This design makes it impossible for callers to distinguish between a successful deletion and a client error. A REST client should propagate error conditions to the caller rather than silently accepting them.
Consider either:
- Throwing an exception on 400 and let the caller handle it
- Returning a result object that indicates success/failure with details
Additionally, there's a minor formatting issue on line 96 (missing space before ==).
🐛 Proposed fix to properly handle error responses
public void deleteUserCredential(String userId, String type, String credentialId) throws Exception {
String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH +
PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId;
try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) {
int responseStatusCode = response.getStatusLine().getStatusCode();
- Assert.assertTrue(responseStatusCode == HttpStatus.SC_NO_CONTENT ||
- responseStatusCode== HttpStatus.SC_BAD_REQUEST,
- "Expected 204 or 400 but got: " + responseStatusCode);
+ if (responseStatusCode != HttpStatus.SC_NO_CONTENT) {
+ throw new Exception("Error code " + responseStatusCode +
+ " occurred while deleting credential: " + credentialId + " for user: " + userId);
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void deleteUserCredential(String userId, String type, String credentialId) throws Exception { | |
| String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH + | |
| PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId; | |
| try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) { | |
| int responseStatusCode = response.getStatusLine().getStatusCode(); | |
| Assert.assertTrue(responseStatusCode == HttpStatus.SC_NO_CONTENT || | |
| responseStatusCode== HttpStatus.SC_BAD_REQUEST, | |
| "Expected 204 or 400 but got: " + responseStatusCode); | |
| } | |
| } | |
| public void deleteUserCredential(String userId, String type, String credentialId) throws Exception { | |
| String credentialPath = credentialManagementBasePath + PATH_SEPARATOR + userId + CREDENTIALS_PATH + | |
| PATH_SEPARATOR + type + PATH_SEPARATOR + credentialId; | |
| try (CloseableHttpResponse response = getResponseOfHttpDelete(credentialPath, getHeadersWithBasicAuth())) { | |
| int responseStatusCode = response.getStatusLine().getStatusCode(); | |
| if (responseStatusCode != HttpStatus.SC_NO_CONTENT) { | |
| throw new Exception("Error code " + responseStatusCode + | |
| " occurred while deleting credential: " + credentialId + " for user: " + userId); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/CredentialManagementRestClient.java`
around lines 89 - 99, The deleteUserCredential method currently treats HTTP 400
as a success which hides client errors; change behavior so that only
HttpStatus.SC_NO_CONTENT is treated as success and any non-204 (including 400)
throws a descriptive exception (or returns a rich result object) containing the
response status and body to callers; update the assertion/response handling in
deleteUserCredential to throw an exception when responseStatusCode !=
HttpStatus.SC_NO_CONTENT and include the response entity or status in the
exception message, and fix the spacing typo in the comparison
(responseStatusCode == HttpStatus.SC_BAD_REQUEST) if you keep any special-case
checks.
1 participant
This pull request introduces support for the Credential Management API to the project, including new dependencies, configuration updates, and integration tests. The changes ensure that the API is properly wired into the application and that negative scenarios are tested for robustness and security.
Credential Management API integration
Added new dependencies for
org.wso2.carbon.identity.api.server.credential.management.v1andorg.wso2.carbon.identity.api.server.credential.management.commonto bothmodules/api-resources/pom.xmlandmodules/api-resources/api-resources-full/pom.xml, enabling the use of credential management features. [1] [2]Registered the new
UsersApifrom the credential management module inmodules/api-resources/api-resources-full/src/main/webapp/WEB-INF/web.xmlunder two servlet initialization parameters, ensuring the API is available in the web application. [1] [2]Testing and validation
CredentialManagementNegativeTest.javato verify error handling, access isolation, and invalid request scenarios for the Credential Management API. This includes tests for invalid credential types, non-existent users and credentials, and cross-organization access restrictions.Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.