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

Deprecate camelCase parameters used by SAML APIs #73984

Merged
merged 6 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,14 @@ clients. See also <<security-api-saml-authenticate,SAML authenticate API>>,
(Required, array) A json array with all the valid SAML Request Ids that the caller of
the API has for the current user.

`queryString`::
`query_string`::
(Optional, string) If the SAML IdP sends the logout response with the HTTP-Redirect
binding, this field must be set to the query string of the redirect URI.

`queryString`::
deprecated:[7.14.0, "Use query_string instead"]
See `query_string`

`content`::
(Optional, string) If the SAML IdP sends the logout response with the HTTP-Post
binding, this field must be set to the value of the `SAMLResponse` form parameter
Expand All @@ -67,7 +71,7 @@ POST /_security/saml/complete_logout
{
"realm": "saml1",
"ids": [ "_1c368075e0b3..." ],
"queryString": "SAMLResponse=fZHLasMwEEVbfb1bf...&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=CuCmFn%2BLqnaZGZJqK..."
"query_string": "SAMLResponse=fZHLasMwEEVbfb1bf...&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=CuCmFn%2BLqnaZGZJqK..."
}
--------------------------------------------------
// TEST[skip:can't test this without a valid SAML Logout Response]
Expand Down
10 changes: 7 additions & 3 deletions x-pack/docs/en/rest-api/security/saml-invalidate-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,20 @@ clients. See also <<security-api-saml-authenticate,SAML authenticate API>>,
(Optional, string) The Assertion Consumer Service URL that matches the one of the SAML
realm in {es} that should be used. You must specify either this parameter or the `realm` parameter.

`queryString`::
`query_string`::
(Required, string) The query part of the URL that the user was redirected to by the SAML
IdP to initiate the Single Logout. This query should include a single
parameter named `SAMLRequest` that contains a SAML logout request that is
deflated and Base64 encoded. If the SAML IdP has signed the logout request,
the URL should include two extra parameters named `SigAlg` and `Signature`
that contain the algorithm used for the signature and the signature value itself.
In order for {es} to be able to verify the IdP's signature, the value of the queryString field must be an exact match to the string provided by the browser.
In order for {es} to be able to verify the IdP's signature, the value of the query_string field must be an exact match to the string provided by the browser.
The client application must not attempt to parse or process the string in any way.

`queryString`::
deprecated:[7.14.0, "Use query_string instead"]
See `query_string`.

`realm`::
(Optional, string) The name of the SAML realm in {es} the configuration. You must specify
either this parameter or the `acs` parameter.
Expand Down Expand Up @@ -78,7 +82,7 @@ the user that is identified in the SAML Logout Request:
--------------------------------------------------
POST /_security/saml/invalidate
{
"queryString" : "SAMLRequest=nZFda4MwFIb%2FiuS%2BmviRpqFaClKQdbvo2g12M2KMraCJ9cRR9utnW4Wyi13sMie873MeznJ1aWrnS3VQGR0j4mLkKC1NUeljjA77zYyhVbIE0dR%2By7fmaHq7U%2BdegXWGpAZ%2B%2F4pR32luBFTAtWgUcCv56%2Fp5y30X87Yz1khTIycdgpUW9kY7WdsC9zxoXTvMvWuVV98YyMnSGH2SYE5pwALBIr9QKiwDGpW0oGVUznGeMyJZKFkQ4jBf5HnhUymjIhzCAL3KNFihbYx8TBYzzGaY7EnIyZwHzCWMfiDnbRIftkSjJr%2BFu0e9v%2B0EgOquRiiZjKpiVFp6j50T4WXoyNJ%2FEWC9fdqc1t%2F1%2B2F3aUpjzhPiXpqMz1%2FHSn4A&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256&Signature=MsAYz2NFdovMG2mXf6TSpu5vlQQyEJAg%2B4KCwBqJTmrb3yGXKUtIgvjqf88eCAK32v3eN8vupjPC8LglYmke1ZnjK0%2FKxzkvSjTVA7mMQe2AQdKbkyC038zzRq%2FYHcjFDE%2Bz0qISwSHZY2NyLePmwU7SexEXnIz37jKC6NMEhus%3D",
"query_string" : "SAMLRequest=nZFda4MwFIb%2FiuS%2BmviRpqFaClKQdbvo2g12M2KMraCJ9cRR9utnW4Wyi13sMie873MeznJ1aWrnS3VQGR0j4mLkKC1NUeljjA77zYyhVbIE0dR%2By7fmaHq7U%2BdegXWGpAZ%2B%2F4pR32luBFTAtWgUcCv56%2Fp5y30X87Yz1khTIycdgpUW9kY7WdsC9zxoXTvMvWuVV98YyMnSGH2SYE5pwALBIr9QKiwDGpW0oGVUznGeMyJZKFkQ4jBf5HnhUymjIhzCAL3KNFihbYx8TBYzzGaY7EnIyZwHzCWMfiDnbRIftkSjJr%2BFu0e9v%2B0EgOquRiiZjKpiVFp6j50T4WXoyNJ%2FEWC9fdqc1t%2F1%2B2F3aUpjzhPiXpqMz1%2FHSn4A&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256&Signature=MsAYz2NFdovMG2mXf6TSpu5vlQQyEJAg%2B4KCwBqJTmrb3yGXKUtIgvjqf88eCAK32v3eN8vupjPC8LglYmke1ZnjK0%2FKxzkvSjTVA7mMQe2AQdKbkyC038zzRq%2FYHcjFDE%2Bz0qISwSHZY2NyLePmwU7SexEXnIz37jKC6NMEhus%3D",
"realm" : "saml1"
}
--------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ public ActionRequestValidationException validate() {
validationException = addValidationError("realm may not be empty", validationException);
}
if (Strings.hasText(queryString) == false && Strings.hasText(content) == false) {
validationException = addValidationError("queryString and content may not both be empty", validationException);
validationException = addValidationError("query_string and content may not both be empty", validationException);
}
if (Strings.hasText(queryString) && Strings.hasText(content)) {
validationException = addValidationError("queryString and content may not both present", validationException);
validationException = addValidationError("query_string and content may not both present", validationException);
}
return validationException;
}
Expand All @@ -56,7 +56,11 @@ public String getQueryString() {
}

public void setQueryString(String queryString) {
this.queryString = queryString;
if (this.queryString == null) {
this.queryString = queryString;
} else {
throw new IllegalArgumentException("Must use either [query_string] or [queryString], not both at the same time");
}
}

public String getContent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public SamlInvalidateSessionRequest() {
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (Strings.isNullOrEmpty(queryString)) {
validationException = addValidationError("queryString is missing", validationException);
validationException = addValidationError("query_string is missing", validationException);
}
return validationException;
}
Expand All @@ -50,7 +50,11 @@ public String getQueryString() {
}

public void setQueryString(String queryString) {
this.queryString = queryString;
if (this.queryString == null) {
this.queryString = queryString;
} else {
throw new IllegalArgumentException("Must use either [query_string] or [queryString], not both at the same time");
}
}

public String getRealmName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void testValidateFailsWhenQueryAndBodyBothNotExist() {
final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest();
samlCompleteLogoutRequest.setRealm("realm");
final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate();
assertThat(validationException.getMessage(), containsString("queryString and content may not both be empty"));
assertThat(validationException.getMessage(), containsString("query_string and content may not both be empty"));
}

public void testValidateFailsWhenQueryAndBodyBothSet() {
Expand All @@ -27,7 +27,7 @@ public void testValidateFailsWhenQueryAndBodyBothSet() {
samlCompleteLogoutRequest.setQueryString("queryString");
samlCompleteLogoutRequest.setContent("content");
final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate();
assertThat(validationException.getMessage(), containsString("queryString and content may not both present"));
assertThat(validationException.getMessage(), containsString("query_string and content may not both present"));
}

public void testValidateFailsWhenRealmIsNotSet() {
Expand All @@ -36,4 +36,12 @@ public void testValidateFailsWhenRealmIsNotSet() {
final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate();
assertThat(validationException.getMessage(), containsString("realm may not be empty"));
}

public void testCannotSetQueryStringTwice() {
final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest();
samlCompleteLogoutRequest.setQueryString("query_string");
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> samlCompleteLogoutRequest.setQueryString("queryString"));
assertThat(e.getMessage(), containsString("Must use either [query_string] or [queryString], not both at the same time"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.core.security.action.saml;

import org.elasticsearch.test.ESTestCase;

import static org.hamcrest.Matchers.containsString;

public class SamlInvalidateSessionRequestTests extends ESTestCase {

public void testCannotSetQueryStringTwice() {
final SamlInvalidateSessionRequest samlInvalidateSessionRequest = new SamlInvalidateSessionRequest();
samlInvalidateSessionRequest.setQueryString("query_string");
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> samlInvalidateSessionRequest.setQueryString("queryString"));
assertThat(e.getMessage(), containsString("Must use either [query_string] or [queryString], not both at the same time"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class RestSamlCompleteLogoutAction extends SamlBaseRestHandler{
PARSER = new ObjectParser<>("saml_complete_logout", SamlCompleteLogoutRequest::new);

static {
PARSER.declareStringOrNull(SamlCompleteLogoutRequest::setQueryString, new ParseField("queryString"));
PARSER.declareStringOrNull(SamlCompleteLogoutRequest::setQueryString, new ParseField("query_string", "queryString"));
PARSER.declareStringOrNull(SamlCompleteLogoutRequest::setContent, new ParseField("content"));
PARSER.declareStringArray(SamlCompleteLogoutRequest::setValidRequestIds, new ParseField("ids"));
PARSER.declareString(SamlCompleteLogoutRequest::setRealm, new ParseField("realm"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class RestSamlInvalidateSessionAction extends SamlBaseRestHandler {
new ObjectParser<>("saml_invalidate_session", SamlInvalidateSessionRequest::new);

static {
PARSER.declareString(SamlInvalidateSessionRequest::setQueryString, new ParseField("queryString"));
PARSER.declareString(SamlInvalidateSessionRequest::setQueryString, new ParseField("query_string", "queryString"));
PARSER.declareString(SamlInvalidateSessionRequest::setAssertionConsumerServiceURL, new ParseField("acs"));
PARSER.declareString(SamlInvalidateSessionRequest::setRealmName, new ParseField("realm"));
}
Expand Down