Skip to content

Commit 1cff689

Browse files
jkakavasjkakavas
authored
Add error message in JSON response (#54389) (#54562)
When the SAML authentication is not successful, we return a SAML Response with a status that indicates a failure. This commit adds an error message in the REST API response along with the SAML Response XML string so that the caller of the API can identify that this is an unsuccessful response without needing to parse the XML.
1 parent d75571f commit 1cff689

File tree

11 files changed

+172
-145
lines changed

11 files changed

+172
-145
lines changed

‎x-pack/plugin/identity-provider/qa/idp-rest-tests/src/test/java/org/elasticsearch/xpack/idp/WildcardServiceProviderRestIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ private String initSso(String entityId, String acs, UsernamePasswordToken second
9898

9999
final Map<String, Object> map = entityAsMap(response);
100100
assertThat(map, notNullValue());
101-
assertThat(map.keySet(), containsInAnyOrder("post_url", "saml_response", "service_provider"));
101+
assertThat(map.keySet(), containsInAnyOrder("post_url", "saml_response", "saml_status", "service_provider", "error"));
102102
assertThat(map.get("post_url"), equalTo(acs));
103103
assertThat(map.get("saml_response"), instanceOf(String.class));
104104

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnRequest.java

-10
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import org.elasticsearch.action.ActionRequest;
99
import org.elasticsearch.action.ActionRequestValidationException;
1010
import org.elasticsearch.common.Strings;
11-
import org.elasticsearch.common.ValidationException;
1211
import org.elasticsearch.common.io.stream.StreamInput;
1312
import org.elasticsearch.common.io.stream.StreamOutput;
1413
import org.elasticsearch.xpack.idp.saml.support.SamlAuthenticationState;
@@ -42,15 +41,6 @@ public ActionRequestValidationException validate() {
4241
if (Strings.isNullOrEmpty(assertionConsumerService)) {
4342
validationException = addValidationError("acs is missing", validationException);
4443
}
45-
if (samlAuthenticationState != null) {
46-
final ValidationException authnStateException = samlAuthenticationState.validate();
47-
if (authnStateException != null && authnStateException.validationErrors().isEmpty() == false) {
48-
if (validationException == null) {
49-
validationException = new ActionRequestValidationException();
50-
}
51-
validationException.addValidationErrors(authnStateException.validationErrors());
52-
}
53-
}
5444
return validationException;
5545
}
5646

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnResponse.java

+22-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.idp.action;
77

88
import org.elasticsearch.action.ActionResponse;
9+
import org.elasticsearch.common.Nullable;
910
import org.elasticsearch.common.io.stream.StreamInput;
1011
import org.elasticsearch.common.io.stream.StreamOutput;
1112

@@ -16,18 +17,25 @@ public class SamlInitiateSingleSignOnResponse extends ActionResponse {
1617
private String postUrl;
1718
private String samlResponse;
1819
private String entityId;
20+
private String samlStatus;
21+
private String error;
1922

2023
public SamlInitiateSingleSignOnResponse(StreamInput in) throws IOException {
2124
super(in);
25+
this.entityId = in.readString();
2226
this.postUrl = in.readString();
2327
this.samlResponse = in.readString();
24-
this.entityId = in.readString();
28+
this.samlStatus = in.readString();
29+
this.error = in.readOptionalString();
2530
}
2631

27-
public SamlInitiateSingleSignOnResponse(String postUrl, String samlResponse, String entityId) {
32+
public SamlInitiateSingleSignOnResponse(String entityId, String postUrl, String samlResponse, String samlStatus,
33+
@Nullable String error) {
34+
this.entityId = entityId;
2835
this.postUrl = postUrl;
2936
this.samlResponse = samlResponse;
30-
this.entityId = entityId;
37+
this.samlStatus = samlStatus;
38+
this.error = error;
3139
}
3240

3341
public String getPostUrl() {
@@ -42,10 +50,20 @@ public String getEntityId() {
4250
return entityId;
4351
}
4452

53+
public String getError() {
54+
return error;
55+
}
56+
57+
public String getSamlStatus() {
58+
return samlStatus;
59+
}
60+
4561
@Override
4662
public void writeTo(StreamOutput out) throws IOException {
63+
out.writeString(entityId);
4764
out.writeString(postUrl);
4865
out.writeString(samlResponse);
49-
out.writeString(entityId);
66+
out.writeString(samlStatus);
67+
out.writeOptionalString(error);
5068
}
5169
}

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/TransportSamlInitiateSingleSignOnAction.java

+55-49
Original file line numberDiff line numberDiff line change
@@ -62,50 +62,53 @@ protected void doExecute(Task task, SamlInitiateSingleSignOnRequest request,
6262
request.getAssertionConsumerService(),
6363
false,
6464
ActionListener.wrap(
65-
sp -> {
66-
if (null == sp) {
67-
final String message = "Service Provider with Entity ID [" + request.getSpEntityId() + "] and ACS ["
68-
+ request.getAssertionConsumerService() + "] is not known to this Identity Provider";
69-
logger.debug(message);
70-
possiblyReplyWithSamlFailure(authenticationState, StatusCode.RESPONDER, new IllegalArgumentException(message),
71-
listener);
72-
return;
73-
}
74-
final SecondaryAuthentication secondaryAuthentication = SecondaryAuthentication.readFromContext(securityContext);
75-
if (secondaryAuthentication == null) {
76-
possiblyReplyWithSamlFailure(authenticationState,
77-
StatusCode.REQUESTER,
78-
new ElasticsearchSecurityException("Request is missing secondary authentication", RestStatus.FORBIDDEN),
79-
listener);
80-
return;
81-
}
82-
buildUserFromAuthentication(secondaryAuthentication, sp, ActionListener.wrap(
83-
user -> {
84-
if (user == null) {
85-
possiblyReplyWithSamlFailure(authenticationState,
86-
StatusCode.REQUESTER,
87-
new ElasticsearchSecurityException("User [{}] is not permitted to access service [{}]",
88-
RestStatus.FORBIDDEN, secondaryAuthentication.getUser(), sp),
89-
listener);
90-
return;
91-
}
92-
final SuccessfulAuthenticationResponseMessageBuilder builder =
93-
new SuccessfulAuthenticationResponseMessageBuilder(samlFactory, Clock.systemUTC(), identityProvider);
94-
try {
95-
final Response response = builder.build(user, authenticationState);
96-
listener.onResponse(new SamlInitiateSingleSignOnResponse(
97-
user.getServiceProvider().getAssertionConsumerService().toString(),
98-
samlFactory.getXmlContent(response),
99-
user.getServiceProvider().getEntityId()));
100-
} catch (ElasticsearchException e) {
101-
listener.onFailure(e);
102-
}
103-
},
104-
e -> possiblyReplyWithSamlFailure(authenticationState, StatusCode.RESPONDER, e, listener)
105-
));
106-
},
107-
e -> possiblyReplyWithSamlFailure(authenticationState, StatusCode.RESPONDER, e, listener)
108-
));
65+
sp -> {
66+
if (null == sp) {
67+
final String message = "Service Provider with Entity ID [" + request.getSpEntityId() + "] and ACS ["
68+
+ request.getAssertionConsumerService() + "] is not known to this Identity Provider";
69+
possiblyReplyWithSamlFailure(authenticationState, request.getSpEntityId(), request.getAssertionConsumerService(),
70+
StatusCode.RESPONDER, new IllegalArgumentException(message), listener);
71+
return;
72+
}
73+
final SecondaryAuthentication secondaryAuthentication = SecondaryAuthentication.readFromContext(securityContext);
74+
if (secondaryAuthentication == null) {
75+
possiblyReplyWithSamlFailure(authenticationState, request.getSpEntityId(), request.getAssertionConsumerService(),
76+
StatusCode.REQUESTER,
77+
new ElasticsearchSecurityException("Request is missing secondary authentication", RestStatus.FORBIDDEN),
78+
listener);
79+
return;
80+
}
81+
buildUserFromAuthentication(secondaryAuthentication, sp, ActionListener.wrap(
82+
user -> {
83+
if (user == null) {
84+
possiblyReplyWithSamlFailure(authenticationState, request.getSpEntityId(),
85+
request.getAssertionConsumerService(), StatusCode.REQUESTER,
86+
new ElasticsearchSecurityException("User [{}] is not permitted to access service [{}]",
87+
RestStatus.FORBIDDEN, secondaryAuthentication.getUser().principal(), sp.getEntityId()),
88+
listener);
89+
return;
90+
}
91+
final SuccessfulAuthenticationResponseMessageBuilder builder =
92+
new SuccessfulAuthenticationResponseMessageBuilder(samlFactory, Clock.systemUTC(), identityProvider);
93+
try {
94+
final Response response = builder.build(user, authenticationState);
95+
listener.onResponse(new SamlInitiateSingleSignOnResponse(
96+
user.getServiceProvider().getEntityId(),
97+
user.getServiceProvider().getAssertionConsumerService().toString(),
98+
samlFactory.getXmlContent(response),
99+
StatusCode.SUCCESS,
100+
null));
101+
} catch (ElasticsearchException e) {
102+
listener.onFailure(e);
103+
}
104+
},
105+
e -> possiblyReplyWithSamlFailure(authenticationState, request.getSpEntityId(),
106+
request.getAssertionConsumerService(), StatusCode.RESPONDER, e, listener)
107+
));
108+
},
109+
e -> possiblyReplyWithSamlFailure(authenticationState, request.getSpEntityId(), request.getAssertionConsumerService(),
110+
StatusCode.RESPONDER, e, listener)
111+
));
109112
}
110113

111114
private void buildUserFromAuthentication(SecondaryAuthentication secondaryAuthentication, SamlServiceProvider serviceProvider,
@@ -129,20 +132,23 @@ private void buildUserFromAuthentication(SecondaryAuthentication secondaryAuthen
129132
);
130133
}
131134

132-
private void possiblyReplyWithSamlFailure(SamlAuthenticationState authenticationState, String statusCode, Exception e,
135+
private void possiblyReplyWithSamlFailure(SamlAuthenticationState authenticationState, String spEntityId,
136+
String acsUrl, String statusCode, Exception e,
133137
ActionListener<SamlInitiateSingleSignOnResponse> listener) {
138+
logger.debug("Failed to generate a successful SAML response: ", e);
134139
if (authenticationState != null) {
135140
final FailedAuthenticationResponseMessageBuilder builder =
136141
new FailedAuthenticationResponseMessageBuilder(samlFactory, Clock.systemUTC(), identityProvider)
137142
.setInResponseTo(authenticationState.getAuthnRequestId())
138-
.setAcsUrl(authenticationState.getRequestedAcsUrl())
143+
.setAcsUrl(acsUrl)
139144
.setPrimaryStatusCode(statusCode);
140145
final Response response = builder.build();
141-
//TODO: Log and indicate SAML Response status is failure in the response
142146
listener.onResponse(new SamlInitiateSingleSignOnResponse(
143-
authenticationState.getRequestedAcsUrl(),
147+
spEntityId,
148+
acsUrl,
144149
samlFactory.getXmlContent(response),
145-
authenticationState.getEntityId()));
150+
statusCode,
151+
e.getMessage()));
146152
} else {
147153
listener.onFailure(e);
148154
}

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java

-2
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ private void validateAuthnRequest(AuthnRequest authnRequest, SamlServiceProvider
181181
checkDestination(authnRequest);
182182
final String acs = checkAcs(authnRequest, sp, authnState);
183183
validateNameIdPolicy(authnRequest, sp, authnState);
184-
authnState.put(SamlAuthenticationState.Fields.ENTITY_ID.getPreferredName(), sp.getEntityId());
185184
authnState.put(SamlAuthenticationState.Fields.AUTHN_REQUEST_ID.getPreferredName(), authnRequest.getID());
186185
final SamlValidateAuthnRequestResponse response = new SamlValidateAuthnRequestResponse(sp.getEntityId(), acs,
187186
authnRequest.isForceAuthn(), authnState);
@@ -268,7 +267,6 @@ private String checkAcs(AuthnRequest request, SamlServiceProvider sp, Map<String
268267
throw new ElasticsearchSecurityException("The registered ACS URL for this Service Provider is [{}] but the authentication " +
269268
"request contained [{}]", RestStatus.BAD_REQUEST, sp.getAssertionConsumerService(), acs);
270269
}
271-
authnState.put(SamlAuthenticationState.Fields.ACS_URL.getPreferredName(), acs);
272270
return acs;
273271
}
274272

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/rest/action/RestSamlInitiateSingleSignOnAction.java

+2
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ public RestResponse buildResponse(SamlInitiateSingleSignOnResponse response, XCo
6666
builder.startObject();
6767
builder.field("post_url", response.getPostUrl());
6868
builder.field("saml_response", response.getSamlResponse());
69+
builder.field("saml_status", response.getSamlStatus());
70+
builder.field("error", response.getError());
6971
builder.startObject("service_provider");
7072
builder.field("entity_id", response.getEntityId());
7173
builder.endObject();

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlAuthenticationState.java

+2-44
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import org.elasticsearch.common.Nullable;
99
import org.elasticsearch.common.ParseField;
1010
import org.elasticsearch.common.Strings;
11-
import org.elasticsearch.common.ValidationException;
1211
import org.elasticsearch.common.io.stream.StreamInput;
1312
import org.elasticsearch.common.io.stream.StreamOutput;
1413
import org.elasticsearch.common.io.stream.Writeable;
@@ -27,8 +26,6 @@
2726
* these.
2827
*/
2928
public class SamlAuthenticationState implements Writeable, ToXContentObject {
30-
private String entityId;
31-
private String requestedAcsUrl;
3229
@Nullable
3330
private String requestedNameidFormat;
3431
@Nullable
@@ -39,8 +36,6 @@ public SamlAuthenticationState() {
3936
}
4037

4138
public SamlAuthenticationState(StreamInput in) throws IOException {
42-
entityId = in.readString();
43-
requestedAcsUrl = in.readString();
4439
requestedNameidFormat = in.readOptionalString();
4540
authnRequestId = in.readOptionalString();
4641
}
@@ -61,39 +56,10 @@ public void setAuthnRequestId(String authnRequestId) {
6156
this.authnRequestId = authnRequestId;
6257
}
6358

64-
public String getEntityId() {
65-
return entityId;
66-
}
67-
68-
public void setEntityId(String entityId) {
69-
this.entityId = entityId;
70-
}
71-
72-
public String getRequestedAcsUrl() {
73-
return requestedAcsUrl;
74-
}
75-
76-
public void setRequestedAcsUrl(String requestedAcsUrl) {
77-
this.requestedAcsUrl = requestedAcsUrl;
78-
}
79-
80-
public ValidationException validate() {
81-
final ValidationException validation = new ValidationException();
82-
if (Strings.isNullOrEmpty(entityId)) {
83-
validation.addValidationError("field [" + Fields.ENTITY_ID + "] is required, but was [" + entityId + "]");
84-
}
85-
if (Strings.isNullOrEmpty(requestedAcsUrl)) {
86-
validation.addValidationError("field [" + Fields.ACS_URL + "] is required, but was [" + requestedAcsUrl + "]");
87-
}
88-
return validation;
89-
}
90-
9159
public static final ObjectParser<SamlAuthenticationState, SamlAuthenticationState> PARSER
9260
= new ObjectParser<>("saml_authn_state", true, SamlAuthenticationState::new);
9361

9462
static {
95-
PARSER.declareString(SamlAuthenticationState::setEntityId, Fields.ENTITY_ID);
96-
PARSER.declareString(SamlAuthenticationState::setRequestedAcsUrl, Fields.ACS_URL);
9763
PARSER.declareStringOrNull(SamlAuthenticationState::setRequestedNameidFormat, Fields.NAMEID_FORMAT);
9864
PARSER.declareStringOrNull(SamlAuthenticationState::setAuthnRequestId, Fields.AUTHN_REQUEST_ID);
9965
}
@@ -103,8 +69,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
10369
builder.startObject();
10470
builder.field(Fields.NAMEID_FORMAT.getPreferredName(), requestedNameidFormat);
10571
builder.field(Fields.AUTHN_REQUEST_ID.getPreferredName(), authnRequestId);
106-
builder.field(Fields.ENTITY_ID.getPreferredName(), entityId);
107-
builder.field(Fields.ACS_URL.getPreferredName(), requestedAcsUrl);
10872
return builder.endObject();
10973
}
11074

@@ -116,14 +80,10 @@ public static SamlAuthenticationState fromXContent(XContentParser parser) throws
11680
public interface Fields {
11781
ParseField NAMEID_FORMAT = new ParseField("nameid_format");
11882
ParseField AUTHN_REQUEST_ID = new ParseField("authn_request_id");
119-
ParseField ENTITY_ID = new ParseField("entity_id");
120-
ParseField ACS_URL = new ParseField("acs_url");
12183
}
12284

12385
@Override
12486
public void writeTo(StreamOutput out) throws IOException {
125-
out.writeString(entityId);
126-
out.writeString(requestedAcsUrl);
12787
out.writeOptionalString(requestedNameidFormat);
12888
out.writeOptionalString(authnRequestId);
12989
}
@@ -138,14 +98,12 @@ public boolean equals(Object o) {
13898
if (this == o) return true;
13999
if (o == null || getClass() != o.getClass()) return false;
140100
SamlAuthenticationState that = (SamlAuthenticationState) o;
141-
return entityId.equals(that.entityId) &&
142-
requestedAcsUrl.equals(that.requestedAcsUrl) &&
143-
Objects.equals(requestedNameidFormat, that.requestedNameidFormat) &&
101+
return Objects.equals(requestedNameidFormat, that.requestedNameidFormat) &&
144102
Objects.equals(authnRequestId, that.authnRequestId);
145103
}
146104

147105
@Override
148106
public int hashCode() {
149-
return Objects.hash(entityId, requestedAcsUrl, requestedNameidFormat, authnRequestId);
107+
return Objects.hash(requestedNameidFormat, authnRequestId);
150108
}
151109
}

0 commit comments

Comments
 (0)