Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The blank predicate still differs from persistence, allowing some whitespace-only required values to be discarded after validation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Aligns required multivalued attribute validation with persisted values.
Changes:
- Accepts mixed blank and non-blank values.
- Adds registration coverage for checkbox multiselects.
| File | Description |
|---|---|
AttributeRequiredByMetadataValidator.java |
Updates required-value validation. |
UserProfileTest.java |
Tests validation and persistence behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
CONTRIBUTING.md asks new contributors to keep at most two pull requests open at a time, and I opened seven at once, so I'm closing this one for now. I'll reopen it once the others are done. Sorry for the noise. |
pedroigor
force-pushed
the
fix-required-multivalued-attribute-blank-values
branch
from
October 5, 2026 19:34
b73a182 to
6c91c39
Compare
A required multivalued attribute rendered as multiselect-checkboxes in the keycloak.v2 login theme is always submitted with an extra empty value from a hidden input, so the required validation failed even when options were selected. Blank values are discarded when the attribute is stored, so only fail the required check when no non-blank value is present. Closes keycloak#52776 Signed-off-by: anarch <2vir.sharma@gmail.com>
DefaultUserProfile drops values that are blank according to String.isBlank() (StringUtil), while the required check used Validation.isBlank(), which only trims ASCII whitespace. A value such as U+2003 passed the required check and was then discarded on save. Relates to keycloak#52776 Signed-off-by: anarch <2vir.sharma@gmail.com>
pedroigor
force-pushed
the
fix-required-multivalued-attribute-blank-values
branch
from
October 6, 2026 11:16
6c91c39 to
37eade2
Compare
pedroigor
approved these changes
Oct 6, 2026
Contributor
|
Thanks, @Vir-007. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #52776
In the
keycloak.v2login theme, amultiselect-checkboxesattribute also renders a hidden input (<input type="hidden" name="<attr>" value="">) so that clearing every checkbox still sends the attribute. That means the form always sends an extra empty string along with the selected options.AttributeRequiredByMetadataValidatorfailed as soon as any value was blank, so a required multiselect attribute could never pass validation during registration, whatever the user selected.Blank values are already dropped when the attribute is stored (
DefaultUserProfileremoves the attribute if every value is blank and otherwise keeps only the non-blank values). This change makes the required check follow the same rule: it now fails only when there is no non-blank value. Single-valued attributes are not affected, because an empty or blank value still fails.Tests:
UserProfileTest#testRequiredMultivaluedAttributeWithBlankValue:[""]still fails the required check, and["", "opt1", "opt2"]now passes validation and stores["opt1", "opt2"]. The test fails without the fix (error-user-attribute-required).UserProfileTest,admin.user.UserProfileTestandDeclarativeUserTestall pass (63 tests), andspotless:checkpasses onservicesandtests/base.AI disclosure: the code, tests and this description were written with the help of an AI coding agent (Claude Code).