Add in-response-to field to saml successful response by piotrsulkowski-elastic · Pull Request #137599 · elastic/elasticsearch

piotrsulkowski-elastic · 2025-11-05T02:08:31Z

Return in-response-to field in all successful authentication attempts, as this information is useful to the client

fixes #128179

elasticsearchmachine · 2025-11-10T09:55:54Z

Pinging @elastic/es-security (Team:Security)

elasticsearchmachine · 2025-11-10T10:01:15Z

Hi @piotrsulkowski-elastic, I've created a changelog YAML for you.

elasticsearchmachine · 2025-11-10T10:03:37Z

Hi @piotrsulkowski-elastic, I've updated the changelog YAML for you.

Copilot

Pull Request Overview

This PR adds support for returning the in-response-to field in SAML successful authentication responses, making this information available to clients for correlation purposes.

Key changes:

Added inResponseTo field to SamlAttributes class to capture and propagate the SAML response correlation ID
Updated token metadata creation to include the in-response-to value when present
Enhanced test coverage to verify the new field is properly returned in authentication results

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
SamlAttributes.java	Added `inResponseTo` field with getter method and updated constructor/toString
SamlAuthenticator.java	Modified to extract and pass `inResponseTo` from SAML response to `SamlAttributes`
SamlRealm.java	Added constant `TOKEN_METADATA_IN_RESPONSE_TO` and updated `createTokenMetadata` to include the field
TransportSamlInvalidateSessionAction.java	Updated `createTokenMetadata` call to pass `null` for `inResponseTo`
TransportSamlLogoutActionTests.java	Updated test to pass `null` for new `inResponseTo` parameter
TransportSamlInvalidateSessionActionTests.java	Updated test to pass `null` for new `inResponseTo` parameter
SamlRealmTests.java	Added test assertions to verify `inResponseTo` is returned in token metadata
SamlAttributesTests.java	Updated test to include `inResponseTo` in assertions
137599.yaml	Added changelog entry for the enhancement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jfreden

Great job on this PR!

I think you're missing a changelog since this is an >enhancement, maybe the autogenerated one was overwritten during a force-push?

Optional: You might want to add a REST test that verifies that the metadata field indeed makes it all the way to the client. I haven't found a great place to add that but maybe by adding a test to SamlServiceProviderMetadataIT that test is more for testing unreliable metadata but it has all the components. WDYT?

elasticsearchmachine · 2025-11-14T15:39:04Z

Hi @piotrsulkowski-elastic, I've created a changelog YAML for you.

jfreden

Great job! I have a couple of minor comments.

jfreden · 2025-12-08T12:41:58Z

...rc/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlAuthenticateResponse.java


+    // Constructor used in Serverless
    public SamlAuthenticateResponse(Authentication authentication, String tokenString, String refreshToken, TimeValue expiresIn) {
+        this(authentication, tokenString, refreshToken, expiresIn, /* inResponseTo= */ null);


nit: I don't think the /* inResponseTo= */ is needed here.

jfreden · 2025-12-08T12:42:15Z

...rc/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlAuthenticateResponse.java

+        String tokenString,
+        String refreshToken,
+        TimeValue expiresIn,
+        String inResponseTo


nit: this can be @Nullable

jfreden · 2025-12-08T13:15:14Z

...rc/main/java/org/elasticsearch/xpack/core/security/action/saml/SamlAuthenticateResponse.java

        out.writeString(refreshToken);
        out.writeTimeValue(expiresIn);
        authentication.writeTo(out);
+        out.writeString(inResponseTo);


I was kind surprised by not finding a corresponding StreamInput constructor and found some information about it here: #107666 (comment)

This is dead code per my understanding, the only scenario is if we send this to an old node and don't expect a response:

That argument isn't totally watertight, there could be situations where we're required to send a particular kind of message (e.g. a response to an older remote node) but will never receive such a message (e.g. because we never make the corresponding request).

If that's the only potential use case (which we probably don't support for SamlAuthenticationResponse) we shouldn't add to it, since the remote node wouldn't know how to parse it.

The reason we don't need transport serialization for SamlAuthenticationResponse is because we're using a NodeClient in RestSamlAuthenticateAction that will be executed on the local node (so not sent over the wire).

jfreden · 2025-12-08T13:22:50Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java

    }

-    public Map<String, Object> createTokenMetadata(SamlNameId nameId, String session) {
+    public Map<String, Object> createTokenMetadata(SamlNameId nameId, String session, String inResponseTo) {


nit: nameId and inResponseTo are @Nullable

jfreden · 2025-12-08T13:28:06Z

...ts/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/saml/SamlInResponseToIT.java

        String requestId = generateRandomRequestId();
        var response = authUser(username, requestId, requestId);
        assertThat(response, hasKey("access_token"));
+        assertThat(response, hasKey("in_response_to"));


Nice to be able to extend this! 👍

jfreden

LGTM!

Return in-response-to field in all successful authentication attempts, as this information is useful to the client part of elastic#128179

elasticsearchmachine added needs:triage Requires assignment of a team area label v9.3.0 labels Nov 5, 2025

lukewhiting added the :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) label Nov 10, 2025

elasticsearchmachine added Team:Security Meta label for security team and removed needs:triage Requires assignment of a team area label labels Nov 10, 2025

lukewhiting added the >enhancement label Nov 10, 2025

lukewhiting requested a review from Copilot November 10, 2025 10:09

Copilot AI reviewed Nov 10, 2025

View reviewed changes

tvernum requested a review from a team November 11, 2025 04:12

piotrsulkowski-elastic force-pushed the saml-in-response-to-successful branch 3 times, most recently from 3fc5fb5 to 3a0316c Compare November 12, 2025 11:07

jfreden reviewed Nov 13, 2025

View reviewed changes

piotrsulkowski-elastic force-pushed the saml-in-response-to-successful branch 2 times, most recently from 7d96928 to eb89101 Compare November 13, 2025 21:50

piotrsulkowski-elastic added >enhancement and removed >enhancement labels Nov 14, 2025

piotrsulkowski-elastic force-pushed the saml-in-response-to-successful branch from f8ae8fe to 9e93cd8 Compare December 3, 2025 02:45

piotrsulkowski-elastic changed the title ~~in-response-to in saml successful response~~ Dec 3, 2025

piotrsulkowski-elastic force-pushed the saml-in-response-to-successful branch 5 times, most recently from b5c786a to 9c461f9 Compare December 8, 2025 11:07

jfreden reviewed Dec 8, 2025

View reviewed changes

jfreden approved these changes Dec 8, 2025

View reviewed changes

piotrsulkowski-elastic added 7 commits December 8, 2025 18:36

in-response-to in saml successful response

87e434a

Return in-response-to field in all successful authentication attempts, as this information is useful to the client part of elastic#128179

Update docs/changelog/137599.yaml

885d9cd

fix missing rest serialization

7d067aa

add test

706a096

fix compilation issue

a8d34a6

code review fixes

c7b487f

add nullable annotations

6a1e254

piotrsulkowski-elastic force-pushed the saml-in-response-to-successful branch from 9aa9604 to 6a1e254 Compare December 8, 2025 18:36

piotrsulkowski-elastic merged commit 0fe2853 into elastic:main Dec 8, 2025
40 checks passed

piotrsulkowski-elastic deleted the saml-in-response-to-successful branch December 9, 2025 11:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add in-response-to field to saml successful response#137599

Add in-response-to field to saml successful response#137599
piotrsulkowski-elastic merged 7 commits intoelastic:mainfrom
piotrsulkowski-elastic:saml-in-response-to-successful

piotrsulkowski-elastic commented Nov 5, 2025 •

edited by lukewhiting

Loading

elasticsearchmachine commented Nov 10, 2025

elasticsearchmachine commented Nov 10, 2025

elasticsearchmachine commented Nov 10, 2025

Copilot AI left a comment

jfreden left a comment

elasticsearchmachine commented Nov 14, 2025

jfreden left a comment

jfreden Dec 8, 2025

jfreden Dec 8, 2025

jfreden Dec 8, 2025 •

edited

Loading

jfreden Dec 8, 2025

jfreden Dec 8, 2025

jfreden Dec 8, 2025

jfreden left a comment

Uh oh!

Labels

5 participants

Conversation

piotrsulkowski-elastic commented Nov 5, 2025 • edited by lukewhiting Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Nov 10, 2025

elasticsearchmachine commented Nov 10, 2025

elasticsearchmachine commented Nov 10, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

jfreden left a comment

Choose a reason for hiding this comment

elasticsearchmachine commented Nov 14, 2025

jfreden left a comment

Choose a reason for hiding this comment

jfreden Dec 8, 2025

Choose a reason for hiding this comment

jfreden Dec 8, 2025

Choose a reason for hiding this comment

jfreden Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

jfreden Dec 8, 2025

Choose a reason for hiding this comment

jfreden Dec 8, 2025

Choose a reason for hiding this comment

jfreden Dec 8, 2025

Choose a reason for hiding this comment

jfreden left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

5 participants

piotrsulkowski-elastic commented Nov 5, 2025 •

edited by lukewhiting

Loading

jfreden Dec 8, 2025 •

edited

Loading