fix(config-api): error message not indicating exact cause that SP name already exists by pujavs · Pull Request #12830 · JanssenProject/jans (original) (raw)

@pujavs pujavs commented

Dec 12, 2025

•

edited by coderabbitaiBot

Prepare

Read PR guidelines
Read license information

Description

Issue#12822: Error message not indicating exact cause that SP name already exists

Target issue

closes #12822

Implementation Details

Handled exception and throwing appropriate error message

Test and Document the changes

Static code analysis has been run locally and issues have been fixed
Relevant unit and integration tests have been added/updated
Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

New Features
- Token entities now include accessToken and logoutStatusJwt boolean properties to provide clearer token classification and improved semantics in API responses for better token handling.
Bug Fixes
- Enhanced error handling for trust relationship creation and updates to return more specific HTTP status codes and error details instead of generic server errors, with improved logging.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: pujavs pujas.works@gmail.com

… cause that SP name already exists #12822

Signed-off-by: pujavs pujas.works@gmail.com

📝 Walkthrough

Walkthrough

Updates the TokenEntity schema in the API specification by adding two optional boolean properties (accessToken and logoutStatusJwt). Enhances error handling and logging in trust relationship REST resource for creation and update operations, with specific handling for WebApplicationException cases.

Changes

Cohort / File(s)	Summary
API Schema Extensionjans-config-api/docs/jans-config-api-swagger.yaml	Added accessToken and logoutStatusJwt as optional boolean properties to the TokenEntity schema.
Error Handling & Loggingjans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/rest/TrustRelationshipResource.java	Enhanced error handling in createTrustRelationshipWithFile and updateTrustRelationship methods with specific catch blocks for WebApplicationException that return the exception status and entity directly. Added debug and error logging around trust relationship creation, including detection of duplicate names. Updated log messages to clarify operation flow. Generic exception fallback retained.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Verify that the WebApplicationException handling correctly propagates HTTP status and response body in both methods
Confirm that the new debug and error log messages for duplicate name detection align with the issue's requirement to display appropriate duplicate name error messages
Ensure the fallback generic exception handler preserves intended error behavior for non-WebApplicationException cases

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The schema update to TokenEntity appears out of scope relative to the stated objective of fixing SP name duplicate error messaging.	Remove the unrelated TokenEntity schema changes (adding accessToken and logoutStatusJwt properties) from this PR, as they do not address issue #12822.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main issue: improving error messaging when an SP name already exists during SAML SP creation.
Description check	✅ Passed	The description follows the template structure with target issue reference (#12822), implementation details mentioning exception handling, and documentation confirmation checkbox checked.
Linked Issues check	✅ Passed	The PR implementation addresses the core requirement from issue #12822: enhanced exception handling now provides clearer error messages for duplicate SP names.

✨ Finishing touches

📝 Generate docstrings 🧪 Generate unit tests (beta)
Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch jans-config-api-12813

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

[ coderabbitai[bot] ](/apps/coderabbitai)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09506a1 and a9a3031.

📒 Files selected for processing (2)

jans-config-api/docs/jans-config-api-swagger.yaml (3 hunks)
jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/rest/TrustRelationshipResource.java (3 hunks) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
GitHub Check: run-tests (MYSQL)
GitHub Check: run-tests (PGSQL)
GitHub Check: prep-matrix
GitHub Check: sonar scan (jans-lock)
GitHub Check: sonar scan (jans-pycloudlib)
GitHub Check: sonar scan (jans-config-api) 🔇 Additional comments (3)

jans-config-api/docs/jans-config-api-swagger.yaml (1)

17720-17729: The logoutStatusJwt field name is correct and consistent with the codebase; no action needed.

Verification confirms that logoutStatusJwt (camelCase with lowercase "wt") is the exact naming convention used throughout the server implementation (AppConfiguration, ClientAttributes, AuthorizationChallengeResponse, and related services). No naming mismatches or serialization issues exist.

Regarding descriptions: all fields in the TokenEntity and TokenAttributes schemas lack descriptions in the swagger.yaml. Adding descriptions only to accessToken and logoutStatusJwt would create inconsistency with the established schema pattern. If schema-wide documentation is desired, that would require a separate, comprehensive update.

jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/rest/TrustRelationshipResource.java (2)

135-135: LGTM: Logging enhancements aid troubleshooting.

The added debug and error logs improve observability when duplicate SP names are detected.

Also applies to: 140-140

157-164: Fix correctly preserves the duplicate-name error message.

By catching WebApplicationException separately before the generic Exception handler, the specific error from line 141 (NAME_CONFLICT_MSG) is now returned directly instead of being replaced by a generic internal server error. The throwBadRequestException method throws BadRequestException, which is a subclass of WebApplicationException, so the catch block at line 158 properly intercepts it and returns the original error response.

[ coderabbitai[bot] ](/apps/coderabbitai)

yurem deleted the jans-config-api-12813 branch

December 15, 2025 13:02

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})