Skip to content

Commit

Permalink
Support handling LogoutResponse from SAML idP (#56316)
Browse files Browse the repository at this point in the history
SAML idP sends back a LogoutResponse at the end of the logout workflow. It can be sent via either HTTP-Redirect binding or HTTP-POST binding. Currently, the HTTP-Redirect request is simply ignored by Kibana and never reaches ES. It does not cause any obvious issue and the workflow is completed normally from user's perspective.

The HTTP-POST request results in a 404 error because POST request is not accepted by Kibana's logout end-point. This causes a non-trivial issue because it renders an error page in user's browser. In addition, some resources do not seem to be fully cleaned up due to the error, e.g. the username will be pre-filled when trying to login again after the 404 error.

This PR solves both of the above issues from ES side with a new /_security/saml/complete_logout end-point. Changes are still needed on Kibana side to relay the messages.
  • Loading branch information
ywangd authored Jun 29, 2020
1 parent e9b6d28 commit e966155
Show file tree
Hide file tree
Showing 25 changed files with 1,087 additions and 354 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.saml;

import org.elasticsearch.action.ActionType;

/**
* ActionType for completing SAML LogoutResponse
*/
public final class SamlCompleteLogoutAction extends ActionType<SamlCompleteLogoutResponse> {

public static final String NAME = "cluster:admin/xpack/security/saml/complete_logout";
public static final SamlCompleteLogoutAction INSTANCE = new SamlCompleteLogoutAction();

private SamlCompleteLogoutAction() {
super(NAME, SamlCompleteLogoutResponse::new);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.saml;

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;

import java.io.IOException;
import java.util.List;

import static org.elasticsearch.action.ValidateActions.addValidationError;

/**
* Represents a request to complete SAML LogoutResponse
*/
public final class SamlCompleteLogoutRequest extends ActionRequest {

@Nullable
private String queryString;
@Nullable
private String content;
private List<String> validRequestIds;
private String realm;

public SamlCompleteLogoutRequest(StreamInput in) throws IOException {
super(in);
}

public SamlCompleteLogoutRequest() {
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (Strings.hasText(realm) == false) {
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);
}
if (Strings.hasText(queryString) && Strings.hasText(content)) {
validationException = addValidationError("queryString and content may not both present", validationException);
}
return validationException;
}

public String getQueryString() {
return queryString;
}

public void setQueryString(String queryString) {
this.queryString = queryString;
}

public String getContent() {
return content;
}

public void setContent(String content) {
this.content = content;
}

public List<String> getValidRequestIds() {
return validRequestIds;
}

public void setValidRequestIds(List<String> validRequestIds) {
this.validRequestIds = validRequestIds;
}

public String getRealm() {
return realm;
}

public void setRealm(String realm) {
this.realm = realm;
}

public boolean isHttpRedirect() {
return queryString != null;
}

public String getPayload() {
return isHttpRedirect() ? queryString : content;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.saml;

import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;

import java.io.IOException;

/**
* A response to complete the LogoutResponse from idP
*/
public final class SamlCompleteLogoutResponse extends ActionResponse {

public SamlCompleteLogoutResponse(StreamInput in) throws IOException {
super(in);
}

public SamlCompleteLogoutResponse() {
}

@Override
public void writeTo(StreamOutput out) throws IOException {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,32 @@
*/
public final class SamlLogoutResponse extends ActionResponse {

private String redirectUrl;
private final String requestId;
private final String redirectUrl;

public SamlLogoutResponse(StreamInput in) throws IOException {
super(in);
requestId = in.readString();
redirectUrl = in.readString();
}

public SamlLogoutResponse(String redirectUrl) {
public SamlLogoutResponse(String requestId, String redirectUrl) {
this.requestId = requestId;
this.redirectUrl = redirectUrl;
}

public String getRequestId() {
return requestId;
}

public String getRedirectUrl() {
return redirectUrl;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(requestId);
out.writeString(redirectUrl);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,4 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(redirectUrl);
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

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

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.test.ESTestCase;

import static org.hamcrest.Matchers.containsString;

public class SamlCompleteLogoutRequestTests extends ESTestCase {

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"));
}

public void testValidateFailsWhenQueryAndBodyBothSet() {
final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest();
samlCompleteLogoutRequest.setRealm("realm");
samlCompleteLogoutRequest.setQueryString("queryString");
samlCompleteLogoutRequest.setContent("content");
final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate();
assertThat(validationException.getMessage(), containsString("queryString and content may not both present"));
}

public void testValidateFailsWhenRealmIsNotSet() {
final SamlCompleteLogoutRequest samlCompleteLogoutRequest = new SamlCompleteLogoutRequest();
samlCompleteLogoutRequest.setQueryString("queryString");
final ActionRequestValidationException validationException = samlCompleteLogoutRequest.validate();
assertThat(validationException.getMessage(), containsString("realm may not be empty"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
import org.elasticsearch.xpack.core.security.action.saml.SamlAuthenticateAction;
import org.elasticsearch.xpack.core.security.action.saml.SamlInvalidateSessionAction;
import org.elasticsearch.xpack.core.security.action.saml.SamlLogoutAction;
import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutAction;
import org.elasticsearch.xpack.core.security.action.saml.SamlPrepareAuthenticationAction;
import org.elasticsearch.xpack.core.security.action.token.CreateTokenAction;
import org.elasticsearch.xpack.core.security.action.token.InvalidateTokenAction;
Expand Down Expand Up @@ -167,6 +168,7 @@
import org.elasticsearch.xpack.security.action.saml.TransportSamlAuthenticateAction;
import org.elasticsearch.xpack.security.action.saml.TransportSamlInvalidateSessionAction;
import org.elasticsearch.xpack.security.action.saml.TransportSamlLogoutAction;
import org.elasticsearch.xpack.security.action.saml.TransportSamlCompleteLogoutAction;
import org.elasticsearch.xpack.security.action.saml.TransportSamlPrepareAuthenticationAction;
import org.elasticsearch.xpack.security.action.token.TransportCreateTokenAction;
import org.elasticsearch.xpack.security.action.token.TransportInvalidateTokenAction;
Expand Down Expand Up @@ -233,6 +235,7 @@
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlAuthenticateAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlInvalidateSessionAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlLogoutAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlCompleteLogoutAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlPrepareAuthenticationAction;
import org.elasticsearch.xpack.security.rest.action.user.RestChangePasswordAction;
import org.elasticsearch.xpack.security.rest.action.user.RestDeleteUserAction;
Expand Down Expand Up @@ -751,6 +754,7 @@ public void onIndexModule(IndexModule module) {
new ActionHandler<>(SamlAuthenticateAction.INSTANCE, TransportSamlAuthenticateAction.class),
new ActionHandler<>(SamlLogoutAction.INSTANCE, TransportSamlLogoutAction.class),
new ActionHandler<>(SamlInvalidateSessionAction.INSTANCE, TransportSamlInvalidateSessionAction.class),
new ActionHandler<>(SamlCompleteLogoutAction.INSTANCE, TransportSamlCompleteLogoutAction.class),
new ActionHandler<>(OpenIdConnectPrepareAuthenticationAction.INSTANCE,
TransportOpenIdConnectPrepareAuthenticationAction.class),
new ActionHandler<>(OpenIdConnectAuthenticateAction.INSTANCE, TransportOpenIdConnectAuthenticateAction.class),
Expand Down Expand Up @@ -808,6 +812,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
new RestSamlAuthenticateAction(settings, getLicenseState()),
new RestSamlLogoutAction(settings, getLicenseState()),
new RestSamlInvalidateSessionAction(settings, getLicenseState()),
new RestSamlCompleteLogoutAction(settings, getLicenseState()),
new RestOpenIdConnectPrepareAuthenticationAction(settings, getLicenseState()),
new RestOpenIdConnectAuthenticateAction(settings, getLicenseState()),
new RestOpenIdConnectLogoutAction(settings, getLicenseState()),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.security.action.saml;

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.tasks.Task;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutAction;
import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutRequest;
import org.elasticsearch.xpack.core.security.action.saml.SamlCompleteLogoutResponse;
import org.elasticsearch.xpack.security.authc.Realms;
import org.elasticsearch.xpack.security.authc.saml.SamlLogoutResponseHandler;
import org.elasticsearch.xpack.security.authc.saml.SamlRealm;
import org.elasticsearch.xpack.security.authc.saml.SamlUtils;

import java.util.List;

import static org.elasticsearch.xpack.security.authc.saml.SamlRealm.findSamlRealms;

/**
* Transport action responsible for completing SAML LogoutResponse
*/
public final class TransportSamlCompleteLogoutAction extends HandledTransportAction<SamlCompleteLogoutRequest, SamlCompleteLogoutResponse> {

private final Realms realms;

@Inject
public TransportSamlCompleteLogoutAction(TransportService transportService, ActionFilters actionFilters, Realms realms) {
super(SamlCompleteLogoutAction.NAME, transportService, actionFilters, SamlCompleteLogoutRequest::new);
this.realms = realms;
}

@Override
protected void doExecute(Task task, SamlCompleteLogoutRequest request, ActionListener<SamlCompleteLogoutResponse> listener) {
List<SamlRealm> realms = findSamlRealms(this.realms, request.getRealm(), null);
if (realms.isEmpty()) {
listener.onFailure(SamlUtils.samlException("Cannot find any matching realm with name [{}]", request.getRealm()));
} else if (realms.size() > 1) {
listener.onFailure(SamlUtils.samlException("Found multiple matching realms [{}] with name [{}]", realms, request.getRealm()));
} else {
processLogoutResponse(realms.get(0), request, listener);
}
}

private void processLogoutResponse(SamlRealm samlRealm, SamlCompleteLogoutRequest request,
ActionListener<SamlCompleteLogoutResponse> listener) {

final SamlLogoutResponseHandler logoutResponseHandler = samlRealm.getLogoutResponseHandler();
try {
logoutResponseHandler.handle(request.isHttpRedirect(), request.getPayload(), request.getValidRequestIds());
listener.onResponse(new SamlCompleteLogoutResponse());
} catch (Exception e) {
listener.onFailure(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ private SamlLogoutResponse buildResponse(Authentication authentication, Map<Stri
final String session = getMetadataString(tokenMetadata, SamlRealm.TOKEN_METADATA_SESSION);
final LogoutRequest logout = realm.buildLogoutRequest(nameId.asXml(), session);
if (logout == null) {
return new SamlLogoutResponse((String)null);
return new SamlLogoutResponse(null, null);
}
final String uri = new SamlRedirect(logout, realm.getSigningConfiguration()).getRedirectUrl();
return new SamlLogoutResponse(uri);
return new SamlLogoutResponse(logout.getID(), uri);
}

private String getMetadataString(Map<String, Object> metadata, String key) {
Expand Down
Loading

0 comments on commit e966155

Please sign in to comment.