Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle RelayState in preparing a SAMLAuthN Request #46534

Merged
merged 3 commits into from
Sep 25, 2019

Conversation

jkakavas
Copy link
Member

This change allows for the caller of the saml/prepare API to pass
a relay_state parameter that will then be part of the redirect
URL in the response as the RelayState query parameter.

The SAML IdP is required to reflect back the value of that relay
state when sending a SAML Response. The caller of the APIs can
then, when receiving the SAML Response, read and consume the value
as it see fit.

Resolves: #46232

This change allows for the caller of the `saml/prepare` API to pass
a `relay_state` parameter that will then be part of the redirect
URL in the response as the `RelayState` query parameter.

The SAML IdP is required to reflect back the value of that relay
state when sending a SAML Response. The caller of the APIs can
then, when receiving the SAML Response, read and consume the value
as it see fit.
@jkakavas jkakavas added >non-issue :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.5.0 labels Sep 10, 2019
@jkakavas jkakavas requested a review from tvernum September 10, 2019 11:00
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@tvernum
Copy link
Contributor

tvernum commented Sep 11, 2019

At first glance this looks fine, but let's wait and see how Kibana wants to deal with the length limit on RelayState before we commit the change.

@jkakavas
Copy link
Member Author

I agree to wait for Kibana but I think the possibility for the caller of the API to set the relaystate we send with the authentication request is useful to have ( Since we do already handle the possibility of a relay state parameter internally ) even if kibana doesn't use this now for passing the target url

@jkakavas
Copy link
Member Author

@elasticmachine update branch

@@ -67,5 +83,8 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeOptionalString(realmName);
out.writeOptionalString(assertionConsumerServiceURL);
if (out.getVersion().onOrAfter(Version.V_7_5_0)){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (out.getVersion().onOrAfter(Version.V_7_5_0)){
if (out.getVersion().onOrAfter(Version.V_7_5_0)) {

@jkakavas jkakavas merged commit 5e14788 into elastic:master Sep 25, 2019
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Sep 25, 2019
This change allows for the caller of the `saml/prepare` API to pass
a `relay_state` parameter that will then be part of the redirect
URL in the response as the `RelayState` query parameter.

The SAML IdP is required to reflect back the value of that relay
state when sending a SAML Response. The caller of the APIs can
then, when receiving the SAML Response, read and consume the value
as it see fit.
jkakavas added a commit that referenced this pull request Sep 25, 2019
This change allows for the caller of the `saml/prepare` API to pass
a `relay_state` parameter that will then be part of the redirect
URL in the response as the `RelayState` query parameter.

The SAML IdP is required to reflect back the value of that relay
state when sending a SAML Response. The caller of the APIs can
then, when receiving the SAML Response, read and consume the value
as it see fit.
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Sep 25, 2019
Re-enable BWC tests now that elastic#46534 has been backported to 7.x
@jkakavas jkakavas mentioned this pull request Sep 25, 2019
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Sep 25, 2019
Re-enable BWC tests now that elastic#46534 has been backported to 7.x
@jkakavas jkakavas mentioned this pull request Sep 25, 2019
jkakavas added a commit that referenced this pull request Sep 25, 2019
Re-enable BWC tests now that #46534 has been backported to 7.x
jkakavas added a commit that referenced this pull request Sep 25, 2019
Re-enable BWC tests now that #46534 has been backported to 7.x
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Aug 26, 2021
Support for RelayState  was introduced in elastic#46534 but the docs
were not updated at the time.
jkakavas added a commit that referenced this pull request Oct 20, 2021
Support for RelayState  was introduced in #46534 but the docs
were not updated at the time.
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Oct 20, 2021
Support for RelayState  was introduced in elastic#46534 but the docs
were not updated at the time.
elasticsearchmachine pushed a commit that referenced this pull request Oct 20, 2021
Support for RelayState  was introduced in #46534 but the docs
were not updated at the time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.5.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SAML _security/saml/prepare API should allow consumer to specify RelayState
4 participants