Make the date of birth future-date rule fixed instead of configurable - #8250
Conversation
PR wso2#8212 gated the DateValidator client rule on a disallowFutureDate claim property seeded from claim-config.xml. That property only lands when a tenant is provisioned, so existing tenants never received it and the dynamic registration flow showed no inline error for them, even though SCIMUserOperationListener already rejects a future date of birth for every tenant unconditionally. The client hint and the server rule disagreed. Resolve the rule in code instead. NO_FUTURE_DATE_CLAIMS holds the claims that can never carry a future date; it is deliberately limited to date of birth, since other date claims such as expiry or renewal dates are legitimately in the future. This drops the disallowFutureDate property, its claim-config.xml entry, and the per-field claim metadata lookup that ran on every flow render.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe flow validation engine now applies future-date validation only to the date-of-birth claim. It no longer reads the ChangesDate-of-birth validation
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change makes date-of-birth future-date validation consistent for all tenants and removes an unnecessary metadata lookup; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
|
PR builder started |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8250 +/- ##
============================================
+ Coverage 52.56% 53.85% +1.29%
- Complexity 20949 21701 +752
============================================
Files 2236 2234 -2
Lines 137313 131933 -5380
Branches 21696 20368 -1328
============================================
- Hits 72179 71059 -1120
+ Misses 56245 52218 -4027
+ Partials 8889 8656 -233
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
PR builder completed |
|
PR builder started |
|
PR builder started |
|
PR builder completed |
|
PR builder completed |
jenkins-is-staging
left a comment
jenkins-is-staging
left a comment
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/32006412235
3 participants
Purpose
Related issue: https://github.com/wso2-enterprise/wso2-iam-internal/issues/7266
Follow-up to: #8212
#8212 made the dynamic registration flow emit a
DateValidatorclient rule only when the backing claim carried adisallowFutureDate=trueproperty, seeded fromclaim-config.xml.That gate turned out to be the wrong mechanism:
claim-config.xmlseeds claim metadata at tenant provisioning. Tenants created before the property existed keep their old metadata, so they never get the rule. Verified against a real dev-tier tenant: the dynamic flow accepted a future date of birth with no inline error.SCIMUserOperationListeneralready rejects a future date of birth for every tenant, unconditionally, with no configuration (identity-inbound-provisioning-scim2#800). Gating only the client hint meant a user on an unseeded tenant got no inline error, submitted, and then hit a server error.This resolves the rule in code so it applies to every tenant as soon as the change is deployed, and matches what the server already enforces.
Changes
InputValidationServiceNO_FUTURE_DATE_CLAIMS, the set of claims whose value can never be a future date. It contains only the date of birth claim.applyDateValidationIfNeedednow checks membership in that set instead of reading claim metadata. TheClaimMetadataManagementServicelookup, its null-service guard, and theClaimMetadataExceptionhandling are gone, along with thetenantDomainplumbing Emit DateValidator client rule for date claims that disallow future values #8212 threaded throughprocessComponentValidations.Constants— addedDOB_CLAIM_URI, alongside the existingUSERNAME_CLAIM_URI.ClaimConstants— removedDISALLOW_FUTURE_DATE_PROPERTY.claim-config.xml— removed the<disallowFutureDate>entry from the date of birth claim.Net effect is 61 fewer lines, and one fewer claim metadata fetch per date field on every flow render.
Scoping note
The set is deliberately limited to date of birth rather than applied to every
DATEvariant input. The flow builder lets an administrator map a date input to any claim, and claims such as expiry or renewal dates are legitimately in the future.testPrepareStepInputsNoDateValidatorForOtherDateClaimsguards this.Tradeoff
Adding another no-future date claim now needs a code change rather than configuration, and the rule can no longer be switched off per tenant. That is intended here: a date of birth is never legitimately in the future, and the server already enforces it unconditionally, so there is no tenant for which the client should say otherwise.
Leftover data
Tenants provisioned while #8212 was in place will still have
disallowFutureDate=truein their claim metadata. Nothing reads it after this change, so it is inert and needs no migration.Testing
InputValidationServiceTestpass on JDK 21: 63 tests, 0 failures.DATEvariant, not added for an input with no claim mapping, appended alongside existing rules, and resolved without any claim metadata lookup.Related PRs
DateValidator, unchanged by this; it still enforces whatever rule it is handed