Skip to content

fix(sso): tell an administrator how to accept a repeated SAML attribute name - #3283

Merged
marevol merged 1 commit into
masterfrom
fix/saml-actionable-login-diagnostics
Aug 13, 2026
Merged

fix(sso): tell an administrator how to accept a repeated SAML attribute name#3283
marevol merged 1 commit into
masterfrom
fix/saml-actionable-login-diagnostics

Conversation

@marevol

@marevol marevol commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

An assertion that carries the same attribute name on more than one element is refused by
java-saml, and the login failed with nothing but the fact:

Authentication failed: ValidationException: Found an Attribute element with duplicated Name

The setting that accepts the repeats and merges their values exists —
saml.security.allow_duplicated_attribute_name, which reaches the library's
onelogin.saml2.security.allow_duplicated_attribute_name through the usual saml. prefix
mapping — but nothing in Fess mentions it, so the deployment is a dead end.

This is not a rare shape. Keycloak is listed as a supported IdP in the documentation and
produces it on a stock configuration
: its role and group mappers emit one <Attribute> element
per value unless their single option is enabled, and every Keycloak account carries several
default realm roles (default-roles-*, offline_access, uma_authorization,
manage-account, …). Every login of every user therefore fails.

The failure does not depend on Fess mapping those attributes either — a deployment that sets no
saml.attribute.role.name at all fails identically, because the refusal happens inside the
library before Fess is given anything to map.

It is also raised from Auth#processResponse after SamlResponse#isValid has already returned
true, while the attributes are being read, so the signature, the InResponseTo comparison and the
replay check all passed. An administrator told only that an assertion was refused reasonably
suspects the certificate or the clock, none of which is involved.

Change

Give that one failure its own warning, naming both remedies:

The IdP repeated an attribute name in the SAML assertion, which is refused, so the login failed
while the attributes were being read; the assertion itself passed validation and no group or role
was mapped. An IdP that emits one <Attribute> element per value produces this: Keycloak does
unless the "single" option of its role and group mappers is enabled, and every Keycloak account
carries several default roles. Either aggregate each attribute into a single element at the IdP,
or set saml.security.allow_duplicated_attribute_name=true in system.properties to accept the
repeats and merge their values.

It is matched on ValidationException#DUPLICATED_ATTRIBUTE_NAME_FOUND, not on the library's
message text, and split out the same way the SameSite / TTL / expired-session diagnoses already
are. The pending AuthnRequest ID is not consumed by this failure, so a login retried after either
side is reconfigured still has its ID to match against.

The class javadoc gains the same note next to the existing attribute-mapping settings.

Verification

Against a real Keycloak 26.4 with its default role list mapper and a stock Fess SAML
configuration:

  • before the change, the login fails with the bare Found an Attribute element with duplicated Name line;
  • after the change, the login fails with the new line, and following its instruction —
    setting saml.security.allow_duplicated_attribute_name=true — makes the same login succeed
    with the repeated values merged and group/role permissions mapped as expected.

Both new tests were mutation-checked: dropping the branch, and matching the wrong error code,
each turn the suite red.

mvn -o test -Dtest=SamlAuthenticatorTest          → 56/56
mvn -o test -Dtest='org.codelibs.fess.sso.**'     → 288/288
mvn -o clean javadoc:jar                          → no errors

Follow-up (not in this PR)

Documentation for the same problem is being prepared separately for fess-docs, since Keycloak
is advertised as supported there.

Whether saml.security.allow_duplicated_attribute_name should simply default to true is a
separate question worth deciding on its own: java-saml merges the repeated values into one list,
which is what Fess wants, but that changes a security default and is not something this PR
assumes.

…te name

An assertion that carries the same attribute name on more than one element is
refused by the library, and the login failed with nothing but the fact:

    Authentication failed: ValidationException: Found an Attribute element with duplicated Name

The setting that accepts the repeats and merges their values exists
(saml.security.allow_duplicated_attribute_name, the library's
onelogin.saml2.security.allow_duplicated_attribute_name), but nothing in Fess
mentions it, so the deployment is a dead end.

It is not a rare shape. Keycloak is listed as a supported IdP and produces it
on a stock configuration: its role and group mappers emit one <Attribute>
element per value unless their "single" option is enabled, and every Keycloak
account carries several default realm roles, so every login of every user
fails. The failure does not depend on Fess mapping those attributes either --
a deployment that sets no saml.attribute.role.name at all fails identically,
because the refusal happens in the library before Fess is given anything to
map.

The refusal is also raised from Auth#processResponse after
SamlResponse#isValid has already returned true, while the attributes are being
read, so the signature, the InResponseTo comparison and the replay check all
passed. An administrator told only that an assertion was refused reasonably
suspects the certificate or the clock, none of which is involved.

Give that one failure its own warning, naming both remedies, and match it on
ValidationException#DUPLICATED_ATTRIBUTE_NAME_FOUND rather than on the
library's message text. The pending AuthnRequest ID is not consumed by this
failure, so a login retried after either side is reconfigured still has its ID
to match against.

Verified against Keycloak 26.4 with its default role list mapper: the login
fails with the new line, and setting the named property makes the same login
succeed with the repeated values merged.
@marevol marevol added this to the 15.8.0 milestone Aug 13, 2026
@marevol marevol self-assigned this Aug 13, 2026
@marevol marevol added the task label Aug 13, 2026
@marevol
marevol merged commit 70cbd8d into master Aug 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant