Skip to content

Commit

Permalink
[IdP plugin] Surface SP-init flow errors in the same way that IdP-ini…
Browse files Browse the repository at this point in the history 
…t flow does (#101855)

CP-4958

This PR updates the IdP plugin's /init handler to surface SP-init
errors (e.g. with authorisation in) the same way as IdP-init errors,
through an Exception that results in a non-200 HTTP status code. At the same
, it continues to send the SAML response object with a failed SAML
message that that flow is currently returning. In this way, both IdP and SP
init flows have unified error handling, while leaving us the flexibility of
using a SAML (error) message-based flow if we wish to in the future.

It does this by:
* Introducing a new `SamlInitiateSingleSignOnException` class that
  can hold a `SamlInitiateSingleSignOnResponse` with the failed SAML message
  * This exception has an override of `metadataToXContent` that serialises
    its SamlInitiateSingleSignOnResponse if it is present.
* Build `SamlInitiateSingleSignOnException` for handling responses in `TransportSamlInitiateSingleSignOnAction`
* Updating `TransportSamlInitiateSingleSignOnAction` to consistently use the
  `listener.onFailure(ex)` mechanism for handling failures

Example of the new error response in the SP-init flow from the IT:

```json
{
    "error": {
        "root_cause": [
            {
                "type": "saml_initiate_single_sign_on_exception",
                "reason": "User [es_user] is not permitted to access service [ec:abcdef:123456]",
                "saml_initiate_single_sign_on_response": {
                    "post_url": "https://AVoMOJLJfbru.elastic-cloud.com/saml/acs",
                    "saml_response": "<?xml version=\"1.0\" encoding=\"UTF-8\"?><saml2p:Response xmlns:saml2p=\"urn:oasis:names:tc:SAML:2.0:protocol\" Destination=\"https://AVoMOJLJfbru.elastic-cloud.com/saml/acs\" ID=\"_d73186163618586bd9a671c7ad3d9e399f18b775\" InResponseTo=\"_d7dfe67845acbd717c8f07e7018d99b576d57967\" IssueInstant=\"2023-11-07T08:03:52.193Z\" Version=\"2.0\"><saml2:Issuer xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\">urn:elastic:cloud:idp</saml2:Issuer><saml2p:Status><saml2p:StatusCode Value=\"urn:oasis:names:tc:SAML:2.0:status:Requester\"/></saml2p:Status></saml2p:Response>",
                    "saml_status": "urn:oasis:names:tc:SAML:2.0:status:Requester",
                    "error": "User [es_user] is not permitted to access service [ec:abcdef:123456]",
                    "service_provider": {
                        "entity_id": "ec:abcdef:123456"
                    }
                }
            }
        ],
        "type": "saml_initiate_single_sign_on_exception",
        "reason": "User [es_user] is not permitted to access service [ec:abcdef:123456]",
        "saml_initiate_single_sign_on_response": {
            "post_url": "https://AVoMOJLJfbru.elastic-cloud.com/saml/acs",
            "saml_response": "<?xml version=\"1.0\" encoding=\"UTF-8\"?><saml2p:Response xmlns:saml2p=\"urn:oasis:names:tc:SAML:2.0:protocol\" Destination=\"https://AVoMOJLJfbru.elastic-cloud.com/saml/acs\" ID=\"_d73186163618586bd9a671c7ad3d9e399f18b775\" InResponseTo=\"_d7dfe67845acbd717c8f07e7018d99b576d57967\" IssueInstant=\"2023-11-07T08:03:52.193Z\" Version=\"2.0\"><saml2:Issuer xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\">urn:elastic:cloud:idp</saml2:Issuer><saml2p:Status><saml2p:StatusCode Value=\"urn:oasis:names:tc:SAML:2.0:status:Requester\"/></saml2p:Status></saml2p:Response>",
            "saml_status": "urn:oasis:names:tc:SAML:2.0:status:Requester",
            "error": "User [es_user] is not permitted to access service [ec:abcdef:123456]",
            "service_provider": {
                "entity_id": "ec:abcdef:123456"
            }
        }
    },
    "status": 403
}
```

Signed-off-by: lloydmeta <lloydmeta@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
@lloydmeta @elasticmachine
lloydmeta and elasticmachine committed Nov 16, 2023
1 parent 7117ac5 commit 7ef539c
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 67 deletions.
16 changes: 10 additions & 6 deletions ...nternalClusterTest/java/org/elasticsearch/xpack/idp/action/SamlIdentityProviderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,17 +276,21 @@ public void testSpInitiatedSsoFailsForUserWithNoAccess() throws Exception {
initRequest.setJsonEntity(Strings.format("""
{"entity_id":"%s", "acs":"%s","authn_state":%s}
""", entityId, acsUrl, Strings.toString(authnStateBuilder)));
Response initResponse = getRestClient().performRequest(initRequest);
ObjectPath initResponseObject = ObjectPath.createFromResponse(initResponse);
assertThat(initResponseObject.evaluate("post_url").toString(), equalTo(acsUrl));
final String body = initResponseObject.evaluate("saml_response").toString();
ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(initRequest));
Response response = e.getResponse();
assertThat(response.getStatusLine().getStatusCode(), equalTo(403));
ObjectPath initResponseObject = ObjectPath.createFromResponse(response);
assertThat(initResponseObject.evaluate("status"), equalTo(403));
final String baseSamlResponseObjectPath = "error.saml_initiate_single_sign_on_response.";
assertThat(initResponseObject.evaluate(baseSamlResponseObjectPath + "post_url").toString(), equalTo(acsUrl));
final String body = initResponseObject.evaluate(baseSamlResponseObjectPath + "saml_response").toString();
assertThat(body, containsString("<saml2p:StatusCode Value=\"urn:oasis:names:tc:SAML:2.0:status:Requester\"/>"));
assertThat(body, containsString("InResponseTo=\"" + expectedInResponeTo + "\""));
Map<String, String> sp = initResponseObject.evaluate("service_provider");
Map<String, String> sp = initResponseObject.evaluate(baseSamlResponseObjectPath + "service_provider");
assertThat(sp, hasKey("entity_id"));
assertThat(sp.get("entity_id"), equalTo(entityId));
assertThat(
initResponseObject.evaluate("error"),
initResponseObject.evaluate(baseSamlResponseObjectPath + "error"),
equalTo("User [" + SAMPLE_USER_NAME + "] is not permitted to access service [" + entityId + "]")
);
}
Expand Down
11 changes: 11 additions & 0 deletions ...er/src/main/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;

Expand Down Expand Up @@ -72,4 +73,14 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(samlStatus);
out.writeOptionalString(error);
}

public void toXContent(XContentBuilder builder) throws IOException {
builder.field("post_url", this.getPostUrl());
builder.field("saml_response", this.getSamlResponse());
builder.field("saml_status", this.getSamlStatus());
builder.field("error", this.getError());
builder.startObject("service_provider");
builder.field("entity_id", this.getEntityId());
builder.endObject();
}
}
147 changes: 94 additions & 53 deletions ...main/java/org/elasticsearch/xpack/idp/action/TransportSamlInitiateSingleSignOnAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.LoggerMessageFormat;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.tasks.Task;
Expand All @@ -29,6 +30,7 @@
import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProvider;
import org.elasticsearch.xpack.idp.saml.support.SamlAuthenticationState;
import org.elasticsearch.xpack.idp.saml.support.SamlFactory;
import org.elasticsearch.xpack.idp.saml.support.SamlInitiateSingleSignOnException;
import org.opensaml.saml.saml2.core.Response;
import org.opensaml.saml.saml2.core.StatusCode;

Expand Down Expand Up @@ -80,47 +82,51 @@ protected void doExecute(
false,
ActionListener.wrap(sp -> {
if (null == sp) {
final String message = "Service Provider with Entity ID ["
+ request.getSpEntityId()
+ "] and ACS ["
+ request.getAssertionConsumerService()
+ "] is not known to this Identity Provider";
possiblyReplyWithSamlFailure(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.RESPONDER,
new IllegalArgumentException(message),
listener
writeFailureResponse(
listener,
buildSamlInitiateSingleSignOnException(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.RESPONDER,
RestStatus.BAD_REQUEST,
"Service Provider with Entity ID [{}] and ACS [{}] is not known to this Identity Provider",
request.getSpEntityId(),
request.getAssertionConsumerService()
)

);
return;
}
final SecondaryAuthentication secondaryAuthentication = SecondaryAuthentication.readFromContext(securityContext);
if (secondaryAuthentication == null) {
possiblyReplyWithSamlFailure(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.REQUESTER,
new ElasticsearchSecurityException("Request is missing secondary authentication", RestStatus.FORBIDDEN),
listener
writeFailureResponse(
listener,
buildSamlInitiateSingleSignOnException(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.REQUESTER,
RestStatus.FORBIDDEN,
"Request is missing secondary authentication"
)
);
return;
}
buildUserFromAuthentication(secondaryAuthentication, sp, ActionListener.wrap(user -> {
if (user == null) {
possiblyReplyWithSamlFailure(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.REQUESTER,
new ElasticsearchSecurityException(
"User [{}] is not permitted to access service [{}]",
writeFailureResponse(
listener,
buildSamlInitiateSingleSignOnException(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.REQUESTER,
RestStatus.FORBIDDEN,
"User [{}] is not permitted to access service [{}]",
secondaryAuthentication.getUser().principal(),
sp.getEntityId()
),
listener
)
);
return;
}
Expand All @@ -144,23 +150,25 @@ protected void doExecute(
listener.onFailure(e);
}
},
e -> possiblyReplyWithSamlFailure(
e -> writeFailureResponse(
listener,
buildResponderSamlInitiateSingleSignOnException(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
e
)
)
));
},
e -> writeFailureResponse(
listener,
buildResponderSamlInitiateSingleSignOnException(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.RESPONDER,
e,
listener
e
)
));
},
e -> possiblyReplyWithSamlFailure(
authenticationState,
request.getSpEntityId(),
request.getAssertionConsumerService(),
StatusCode.RESPONDER,
e,
listener
)
)
);
Expand Down Expand Up @@ -194,27 +202,60 @@ private void buildUserFromAuthentication(
});
}

private void possiblyReplyWithSamlFailure(
SamlAuthenticationState authenticationState,
String spEntityId,
String acsUrl,
String statusCode,
Exception e,
ActionListener<SamlInitiateSingleSignOnResponse> listener
private void writeFailureResponse(
final ActionListener<SamlInitiateSingleSignOnResponse> listener,
final SamlInitiateSingleSignOnException ex
) {
logger.debug("Failed to generate a successful SAML response: ", ex);
listener.onFailure(ex);
}

private SamlInitiateSingleSignOnException buildSamlInitiateSingleSignOnException(
final SamlAuthenticationState authenticationState,
final String spEntityId,
final String acsUrl,
final String statusCode,
final RestStatus restStatus,
final String messageFormatStr,
final Object... args
) {
logger.debug("Failed to generate a successful SAML response: ", e);
final SamlInitiateSingleSignOnException ex;
String exceptionMessage = LoggerMessageFormat.format(messageFormatStr, args);
if (authenticationState != null) {
final FailedAuthenticationResponseMessageBuilder builder = new FailedAuthenticationResponseMessageBuilder(
samlFactory,
Clock.systemUTC(),
identityProvider
).setInResponseTo(authenticationState.getAuthnRequestId()).setAcsUrl(acsUrl).setPrimaryStatusCode(statusCode);
final Response response = builder.build();
listener.onResponse(
new SamlInitiateSingleSignOnResponse(spEntityId, acsUrl, samlFactory.getXmlContent(response), statusCode, e.getMessage())
ex = new SamlInitiateSingleSignOnException(
exceptionMessage,
restStatus,
new SamlInitiateSingleSignOnResponse(spEntityId, acsUrl, samlFactory.getXmlContent(response), statusCode, exceptionMessage)
);
} else {
listener.onFailure(e);
ex = new SamlInitiateSingleSignOnException(exceptionMessage, restStatus);
}
return ex;
}

private SamlInitiateSingleSignOnException buildResponderSamlInitiateSingleSignOnException(
final SamlAuthenticationState authenticationState,
final String spEntityId,
final String acsUrl,
final Exception cause
) {
final String exceptionMessage = cause.getMessage();
final RestStatus restStatus = ExceptionsHelper.status(cause);
final SamlInitiateSingleSignOnException ex = buildSamlInitiateSingleSignOnException(
authenticationState,
spEntityId,
acsUrl,
StatusCode.RESPONDER,
restStatus,
exceptionMessage
);
ex.initCause(cause);
return ex;
}
}
8 changes: 1 addition & 7 deletions ...java/org/elasticsearch/xpack/idp/saml/rest/action/RestSamlInitiateSingleSignOnAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien
@Override
public RestResponse buildResponse(SamlInitiateSingleSignOnResponse response, XContentBuilder builder) throws Exception {
builder.startObject();
builder.field("post_url", response.getPostUrl());
builder.field("saml_response", response.getSamlResponse());
builder.field("saml_status", response.getSamlStatus());
builder.field("error", response.getError());
builder.startObject("service_provider");
builder.field("entity_id", response.getEntityId());
builder.endObject();
response.toXContent(builder);
builder.endObject();
return new RestResponse(RestStatus.OK, builder);
}
Expand Down
46 changes: 46 additions & 0 deletions ...main/java/org/elasticsearch/xpack/idp/saml/support/SamlInitiateSingleSignOnException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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.idp.saml.support;

import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xpack.idp.action.SamlInitiateSingleSignOnResponse;

import java.io.IOException;

public class SamlInitiateSingleSignOnException extends ElasticsearchSecurityException {

private SamlInitiateSingleSignOnResponse samlInitiateSingleSignOnResponse;

public SamlInitiateSingleSignOnException(
String msg,
RestStatus status,
SamlInitiateSingleSignOnResponse samlInitiateSingleSignOnResponse
) {
super(msg, status);
this.samlInitiateSingleSignOnResponse = samlInitiateSingleSignOnResponse;
}

public SamlInitiateSingleSignOnException(String msg, RestStatus status) {
super(msg, status);
}

@Override
protected void metadataToXContent(XContentBuilder builder, Params params) throws IOException {
if (this.samlInitiateSingleSignOnResponse != null) {
builder.startObject("saml_initiate_single_sign_on_response");
this.samlInitiateSingleSignOnResponse.toXContent(builder);
builder.endObject();
}
}

public SamlInitiateSingleSignOnResponse getSamlInitiateSingleSignOnResponse() {
return samlInitiateSingleSignOnResponse;
}
}
5 changes: 4 additions & 1 deletion ...java/org/elasticsearch/xpack/idp/action/TransportSamlInitiateSingleSignOnActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.xpack.idp.saml.sp.WildcardServiceProviderResolver;
import org.elasticsearch.xpack.idp.saml.support.SamlAuthenticationState;
import org.elasticsearch.xpack.idp.saml.support.SamlFactory;
import org.elasticsearch.xpack.idp.saml.support.SamlInitiateSingleSignOnException;
import org.elasticsearch.xpack.idp.saml.test.IdpSamlTestCase;
import org.mockito.Mockito;
import org.opensaml.saml.saml2.core.StatusCode;
Expand Down Expand Up @@ -112,7 +113,9 @@ public void testGetResponseWithoutSecondaryAuthenticationInSpInitiatedFlow() thr
final TransportSamlInitiateSingleSignOnAction action = setupTransportAction(false);
action.doExecute(mock(Task.class), request, future);

final SamlInitiateSingleSignOnResponse response = future.get();
final SamlInitiateSingleSignOnException ex = (SamlInitiateSingleSignOnException) expectThrows(Exception.class, future::get)
.getCause();
final SamlInitiateSingleSignOnResponse response = ex.getSamlInitiateSingleSignOnResponse();
assertThat(response.getError(), equalTo("Request is missing secondary authentication"));
assertThat(response.getSamlStatus(), equalTo(StatusCode.REQUESTER));
assertThat(response.getPostUrl(), equalTo("https://sp.some.org/saml/acs"));
Expand Down

0 comments on commit 7ef539c

Please sign in to comment.