Skip to content

Fix ClassCastException in RAR schema validation for integer keywords - #8324

Merged
pavinduLakshan merged 1 commit into
wso2:masterfrom
pavinduLakshan:fix-rar-schema-integer-keywords
Oct 8, 2026
Merged

pavinduLakshan merged 1 commit into
wso2:masterfrom
pavinduLakshan:fix-rar-schema-integer-keywords

Conversation

@pavinduLakshan

Copy link
Copy Markdown
Member

Purpose

Validating authorization_details against a registered schema that uses minItems/maxItems (and similar integer keywords) fails with ClassCastException: Double cannot be cast to Integer, regardless of the payload.

AuthorizationDetailsTypesUtil.parseSchema uses a default Gson, which parses every number in a Map<String, Object> as a Double. The Vert.x JSON schema validator used by the OAuth RAR component casts minItems, maxItems, minProperties, maxProperties, minContains and maxContains directly to Integer.

Approach

  • Add AuthorizationDetailsTypesUtil.parseSchemaForValidation, which parses schemas with a ToNumberStrategy that returns integral values (including ones written as 1.0) as Integer. Values outside the Integer range and non-integral numbers are returned as Long/Double.
  • Use it in AuthorizedAPIDAOImpl.getAuthorizedAuthorizationDetailsTypes, the read path that supplies schemas to RAR validation (DefaultAuthorizationDetailsValidator).
  • parseSchema is unchanged, so the API resource management responses keep their current format. No new configuration is introduced.

Related issues

🤖 Generated with Claude Code

Gson parses every number of a Map<String, Object> as a Double, so integer
schema keywords such as minItems and maxItems were returned as 1.0/3.0.
JSON schema validators like Vert.x cast these keywords directly to Integer,
causing a ClassCastException during RAR authorization_details validation.

Add parseSchemaForValidation, which returns integral numbers as Integer
(or Long when out of Integer range), and use it for schemas loaded for
authorization details validation. parseSchema and the API resource
management responses remain unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
.github/copilot-instructions.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: wso2/carbon-identity-framework/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 056f79ea-455a-4f5d-bf10-cdeb8ba67ae7

📥 Commits

Reviewing files that changed from the base of the PR and between 250d1ab and af6683a.

📒 Files selected for processing (4)
  • components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/util/AuthorizationDetailsTypesUtil.java
  • components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/test/java/org/wso2/carbon/identity/api/resource/mgt/AuthorizationDetailsTypesUtilTest.java
  • components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/test/resources/testng.xml
  • components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/AuthorizedAPIDAOImpl.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The change adds a validation-specific schema parser that preserves integral values as integers when possible. Authorization-details type construction now uses this parser. Tests cover the new parser and confirm the existing parser behavior.

Changes

Schema validation parsing

Layer / File(s) Summary
Add and test validation-specific parsing
components/api-resource-mgt/.../AuthorizationDetailsTypesUtil.java, components/api-resource-mgt/.../AuthorizationDetailsTypesUtilTest.java, components/api-resource-mgt/.../testng.xml
Adds a parser that returns integral values as Integer when they fit the integer range. Tests cover integral and decimal values across schema locations and confirm the existing parser's number format.
Use validation parsing when building types
components/application-mgt/.../AuthorizedAPIDAOImpl.java
Authorization-details type construction now calls parseSchemaForValidation.

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to af668

No actionable merge-blocking issue is established for the schema parsing change; it is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to af668

The change is narrowly scoped and does not establish a new access path or weaker authorization control. Downstream compatibility and error handling remain unverified, so some uncertainty remains.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is schemas loaded through application-associated authorization-details retrieval. The public Java method does not itself establish a new network entrypoint or additional authority. Effects on downstream authorization decisions cannot be bounded without the external validator.

Trust Boundaries and Controls

  • observed — The changed call receives stored schema text, not an authorization-request payload. Existing application resolution and feature gating remain upstream of parsing. These observations do not independently prove schema-registration privileges or complete tenant isolation.

Resilience and Maintainability Implications

  • inferred — A parsing failure occurs before the affected type is added to the local result list and causes no database mutation in this path. The DAO catches SQLExceptions, not parsing exceptions; external handling and fail-closed authorization behavior remain unknown.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the problem, implementation approach, affected code path, and related issue. However, it omits most required template sections, including Goals, User stories, Release note, Do… Complete the missing template sections. Use “N/A” with a brief explanation where a section does not apply, and include unit test coverage and security check details.
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing the ClassCastException during RAR schema validation for integer keywords.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation approach, affected code path, and related issue. However, it omits most required template sections, including Goals, User stories, Release note, Documentation, Training, Certification, Marketing, Automation tests, Security checks, Samples, Related PRs, Migrations, Test environment, and Learning.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 53.68%. Comparing base (250d1ab) to head (af6683a).

Files with missing lines Patch % Lines
...source/mgt/util/AuthorizationDetailsTypesUtil.java 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #8324      +/-   ##
============================================
+ Coverage     53.65%   53.68%   +0.03%     
+ Complexity    22647    22621      -26     
============================================
  Files          2264     2264              
  Lines        138114   138125      +11     
  Branches      23871    23873       +2     
============================================
+ Hits          74100    74155      +55     
+ Misses        55083    55039      -44     
  Partials       8931     8931              
Flag Coverage Δ
unit 39.81% <91.66%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@pavinduLakshan
pavinduLakshan merged commit 378f394 into wso2:master Oct 8, 2026
6 checks passed
@pavinduLakshan
pavinduLakshan deleted the fix-rar-schema-integer-keywords branch October 8, 2026 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants