Draft
Fix SAML v2 API: null/omitted string attributes now remove model attributes#51252
Conversation
When a SAML client attribute is set to null (via PATCH with explicit null or PUT omitting the field), the v2 mapper now removes the corresponding model attribute instead of silently ignoring the null value. Previously, `setAttributeIfNotNull` and `setBooleanAttributeIfNotNull` would skip null values, leaving stale attributes (e.g. signingCertificate) in place. This was a security concern since an administrator who PATCHed `signingCertificate: null` would receive a 200 but the old certificate would still be trusted for SAML signature verification. Closes #51213
Copilot
AI
changed the title
[WIP] Fix SAML string-attribute mapper to remove attributes on null
Fix SAML v2 API: null/omitted string attributes now remove model attributes
Jul 28, 2026
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.
In the Admin API v2 SAML mapper,
setAttributeIfNotNullandsetBooleanAttributeIfNotNullsilently ignored null values, soPATCHwith an explicitnullor aPUTomitting a field (e.g.signingCertificate) left the underlying model attribute intact. A stale signing certificate would remain trusted for SAML signature verification despite the API returning 200.Changes
SAMLClientModelMapper: BothsetAttributeIfNotNullandsetBooleanAttributeIfNotNullnow callmodel.removeAttribute(key)when the value isnull, aligning with the expected merge-patch and PUT replace semantics.SAMLClientModelMapperTest: UpdatedtoModel_doesNotSetNullAttributes→toModel_removesAttributesWhenNullto assert the corrected behavior; also addssigningCertificateto the verified attributes.Behavior
PATCH"signingCertificate": nullPUTPATCHreaderForUpdating)Note: Jackson's
readerForUpdatingpreserves absent fields from the base representation during merge-patch, so omitted PATCH fields are never null whentoModelis called — only explicitly nulled fields are.