Skip to content

Commit

Permalink
Watcher: Remove extraneous auth classes (#32300)
Browse files Browse the repository at this point in the history
The auth.basic package was an example of a single implementation
interface that leaked into many different classes. In order to clean
this up, the HttpAuth interface, factories, and Registries all were
removed and the single implementation, BasicAuth, was substituted in all
cases. This removes some dependenies between Auth and the Templates,
which can now use static methods on BasicAuth. BasicAuth was also moved
into the http package and all of the other classes were removed.
  • Loading branch information
hub-cap committed Aug 15, 2018
1 parent dbd5f50 commit 0814425
Show file tree
Hide file tree
Showing 36 changed files with 139 additions and 474 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,7 @@
import org.elasticsearch.xpack.watcher.actions.webhook.WebhookAction;
import org.elasticsearch.xpack.watcher.actions.webhook.WebhookActionFactory;
import org.elasticsearch.xpack.watcher.common.http.HttpClient;
import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate;
import org.elasticsearch.xpack.watcher.common.http.HttpSettings;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthFactory;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuth;
import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuthFactory;
import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;
import org.elasticsearch.xpack.watcher.condition.ArrayCompareCondition;
import org.elasticsearch.xpack.watcher.condition.CompareCondition;
Expand Down Expand Up @@ -283,12 +278,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
new WatcherIndexTemplateRegistry(settings, clusterService, threadPool, client);

// http client
Map<String, HttpAuthFactory> httpAuthFactories = new HashMap<>();
httpAuthFactories.put(BasicAuth.TYPE, new BasicAuthFactory(cryptoService));
// TODO: add more auth types, or remove this indirection
HttpAuthRegistry httpAuthRegistry = new HttpAuthRegistry(httpAuthFactories);
HttpRequestTemplate.Parser httpTemplateParser = new HttpRequestTemplate.Parser(httpAuthRegistry);
httpClient = new HttpClient(settings, httpAuthRegistry, getSslService());
httpClient = new HttpClient(settings, getSslService(), cryptoService);

// notification
EmailService emailService = new EmailService(settings, cryptoService, clusterService.getClusterSettings());
Expand All @@ -305,11 +295,9 @@ public Collection<Object> createComponents(Client client, ClusterService cluster

TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService);
Map<String, EmailAttachmentParser> emailAttachmentParsers = new HashMap<>();
emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, httpTemplateParser,
templateEngine));
emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, templateEngine));
emailAttachmentParsers.put(DataAttachmentParser.TYPE, new DataAttachmentParser());
emailAttachmentParsers.put(ReportingAttachmentParser.TYPE, new ReportingAttachmentParser(settings, httpClient, templateEngine,
httpAuthRegistry));
emailAttachmentParsers.put(ReportingAttachmentParser.TYPE, new ReportingAttachmentParser(settings, httpClient, templateEngine));
EmailAttachmentsParser emailAttachmentsParser = new EmailAttachmentsParser(emailAttachmentParsers);

// conditions
Expand All @@ -329,7 +317,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
// actions
final Map<String, ActionFactory> actionFactoryMap = new HashMap<>();
actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser));
actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, httpTemplateParser, templateEngine));
actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, templateEngine));
actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client));
actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine));
actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService));
Expand All @@ -343,7 +331,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
final Map<String, InputFactory> inputFactories = new HashMap<>();
inputFactories.put(SearchInput.TYPE, new SearchInputFactory(settings, client, xContentRegistry, scriptService));
inputFactories.put(SimpleInput.TYPE, new SimpleInputFactory(settings));
inputFactories.put(HttpInput.TYPE, new HttpInputFactory(settings, httpClient, templateEngine, httpTemplateParser));
inputFactories.put(HttpInput.TYPE, new HttpInputFactory(settings, httpClient, templateEngine));
inputFactories.put(NoneInput.TYPE, new NoneInputFactory(settings));
inputFactories.put(TransformInput.TYPE, new TransformInputFactory(settings, transformRegistry));
final InputRegistry inputRegistry = new InputRegistry(settings, inputFactories);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return requestTemplate.toXContent(builder, params);
}

public static WebhookAction parse(String watchId, String actionId, XContentParser parser,
HttpRequestTemplate.Parser requestParser) throws IOException {
public static WebhookAction parse(String watchId, String actionId, XContentParser parser) throws IOException {
try {
HttpRequestTemplate request = requestParser.parse(parser);
HttpRequestTemplate request = HttpRequestTemplate.Parser.parse(parser);
return new WebhookAction(request);
} catch (ElasticsearchParseException pe) {
throw new ElasticsearchParseException("could not parse [{}] action [{}/{}]. failed parsing http request template", pe, TYPE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,25 @@
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.watcher.actions.ActionFactory;
import org.elasticsearch.xpack.watcher.common.http.HttpClient;
import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate;
import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;

import java.io.IOException;

public class WebhookActionFactory extends ActionFactory {

private final HttpClient httpClient;
private final HttpRequestTemplate.Parser requestTemplateParser;
private final TextTemplateEngine templateEngine;

public WebhookActionFactory(Settings settings, HttpClient httpClient, HttpRequestTemplate.Parser requestTemplateParser,
TextTemplateEngine templateEngine) {
public WebhookActionFactory(Settings settings, HttpClient httpClient, TextTemplateEngine templateEngine) {

super(Loggers.getLogger(ExecutableWebhookAction.class, settings));
this.httpClient = httpClient;
this.requestTemplateParser = requestTemplateParser;
this.templateEngine = templateEngine;
}

@Override
public ExecutableWebhookAction parseExecutable(String watchId, String actionId, XContentParser parser) throws IOException {
return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser, requestTemplateParser),
return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser),
actionLogger, httpClient, templateEngine);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
* 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.watcher.common.http.auth.basic;
package org.elasticsearch.xpack.watcher.common.http;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.watcher.common.secret.Secret;
import org.elasticsearch.xpack.core.watcher.crypto.CryptoService;
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherParams;
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherXContentParser;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuth;

import java.io.IOException;
import java.util.Objects;

public class BasicAuth implements HttpAuth {
public class BasicAuth implements ToXContentObject {

public static final String TYPE = "basic";

Expand All @@ -34,11 +34,6 @@ public BasicAuth(String username, Secret password) {
this.password = password;
}

@Override
public String type() {
return TYPE;
}

public String getUsername() {
return username;
}
Expand Down Expand Up @@ -74,7 +69,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return builder.endObject();
}

public static BasicAuth parse(XContentParser parser) throws IOException {
public static BasicAuth parseInner(XContentParser parser) throws IOException {
String username = null;
Secret password = null;

Expand Down Expand Up @@ -103,6 +98,20 @@ public static BasicAuth parse(XContentParser parser) throws IOException {
return new BasicAuth(username, password);
}

public static BasicAuth parse(XContentParser parser) throws IOException {
String type = null;
XContentParser.Token token;
BasicAuth auth = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
type = parser.currentName();
} else if (token == XContentParser.Token.START_OBJECT && type != null) {
auth = BasicAuth.parseInner(parser);
}
}
return auth;
}

interface Field {
ParseField USERNAME = new ParseField("username");
ParseField PASSWORD = new ParseField("password");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import org.apache.http.HttpHost;
import org.apache.http.NameValuePair;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.Credentials;
import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.AuthCache;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.config.RequestConfig;
Expand Down Expand Up @@ -42,8 +44,7 @@
import org.elasticsearch.xpack.core.common.socket.SocketAccess;
import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.watcher.common.http.auth.ApplicableHttpAuth;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
import org.elasticsearch.xpack.core.watcher.crypto.CryptoService;

import javax.net.ssl.HostnameVerifier;
import java.io.ByteArrayOutputStream;
Expand All @@ -66,20 +67,20 @@ public class HttpClient extends AbstractComponent implements Closeable {
// you are querying a remote Elasticsearch cluster
private static final int MAX_CONNECTIONS = 500;

private final HttpAuthRegistry httpAuthRegistry;
private final CloseableHttpClient client;
private final HttpProxy settingsProxy;
private final TimeValue defaultConnectionTimeout;
private final TimeValue defaultReadTimeout;
private final ByteSizeValue maxResponseSize;
private final CryptoService cryptoService;

public HttpClient(Settings settings, HttpAuthRegistry httpAuthRegistry, SSLService sslService) {
public HttpClient(Settings settings, SSLService sslService, CryptoService cryptoService) {
super(settings);
this.httpAuthRegistry = httpAuthRegistry;
this.defaultConnectionTimeout = HttpSettings.CONNECTION_TIMEOUT.get(settings);
this.defaultReadTimeout = HttpSettings.READ_TIMEOUT.get(settings);
this.maxResponseSize = HttpSettings.MAX_HTTP_RESPONSE_SIZE.get(settings);
this.settingsProxy = getProxyFromSettings();
this.cryptoService = cryptoService;

HttpClientBuilder clientBuilder = HttpClientBuilder.create();

Expand Down Expand Up @@ -139,9 +140,10 @@ public HttpResponse execute(HttpRequest request) throws IOException {
HttpClientContext localContext = HttpClientContext.create();
// auth
if (request.auth() != null) {
ApplicableHttpAuth applicableAuth = httpAuthRegistry.createApplicable(request.auth);
CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
applicableAuth.apply(credentialsProvider, new AuthScope(request.host, request.port));
Credentials credentials = new UsernamePasswordCredentials(request.auth().username,
new String(request.auth().password.text(cryptoService)));
credentialsProvider.setCredentials(new AuthScope(request.host, request.port), credentials);
localContext.setCredentialsProvider(credentialsProvider);

// preemptive auth, no need to wait for a 401 first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
import org.elasticsearch.xpack.core.watcher.support.WatcherUtils;
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherParams;
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherXContentParser;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuth;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand Down Expand Up @@ -50,15 +48,15 @@ public class HttpRequest implements ToXContentObject {
@Nullable final String path;
final Map<String, String> params;
final Map<String, String> headers;
@Nullable final HttpAuth auth;
@Nullable final BasicAuth auth;
@Nullable final String body;
@Nullable final TimeValue connectionTimeout;
@Nullable final TimeValue readTimeout;
@Nullable final HttpProxy proxy;

public HttpRequest(String host, int port, @Nullable Scheme scheme, @Nullable HttpMethod method, @Nullable String path,
@Nullable Map<String, String> params, @Nullable Map<String, String> headers,
@Nullable HttpAuth auth, @Nullable String body, @Nullable TimeValue connectionTimeout,
@Nullable BasicAuth auth, @Nullable String body, @Nullable TimeValue connectionTimeout,
@Nullable TimeValue readTimeout, @Nullable HttpProxy proxy) {
this.host = host;
this.port = port;
Expand Down Expand Up @@ -102,7 +100,7 @@ public Map<String, String> headers() {
return headers;
}

public HttpAuth auth() {
public BasicAuth auth() {
return auth;
}

Expand Down Expand Up @@ -166,7 +164,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params toX
}
if (auth != null) {
builder.startObject(Field.AUTH.getPreferredName())
.field(auth.type(), auth, toXContentParams)
.field(BasicAuth.TYPE, auth, toXContentParams)
.endObject();
}
if (body != null) {
Expand Down Expand Up @@ -234,7 +232,7 @@ public String toString() {
sb.append("], ");
}
if (auth != null) {
sb.append("auth=[").append(auth.type()).append("], ");
sb.append("auth=[").append(BasicAuth.TYPE).append("], ");
}
sb.append("connection_timeout=[").append(connectionTimeout).append("], ");
sb.append("read_timeout=[").append(readTimeout).append("], ");
Expand All @@ -254,14 +252,7 @@ static Builder builder() {
}

public static class Parser {

private final HttpAuthRegistry httpAuthRegistry;

public Parser(HttpAuthRegistry httpAuthRegistry) {
this.httpAuthRegistry = httpAuthRegistry;
}

public HttpRequest parse(XContentParser parser) throws IOException {
public static HttpRequest parse(XContentParser parser) throws IOException {
Builder builder = new Builder();
XContentParser.Token token;
String currentFieldName = null;
Expand All @@ -275,7 +266,7 @@ public HttpRequest parse(XContentParser parser) throws IOException {
throw new ElasticsearchParseException("could not parse http request. could not parse [{}] field", currentFieldName);
}
} else if (Field.AUTH.match(currentFieldName, parser.getDeprecationHandler())) {
builder.auth(httpAuthRegistry.parse(parser));
builder.auth(BasicAuth.parse(parser));
} else if (HttpRequest.Field.CONNECTION_TIMEOUT.match(currentFieldName, parser.getDeprecationHandler())) {
builder.connectionTimeout(TimeValue.timeValueMillis(parser.longValue()));
} else if (HttpRequest.Field.CONNECTION_TIMEOUT_HUMAN.match(currentFieldName, parser.getDeprecationHandler())) {
Expand All @@ -302,7 +293,7 @@ public HttpRequest parse(XContentParser parser) throws IOException {
builder.setHeaders((Map) WatcherUtils.flattenModel(parser.map()));
} else if (Field.PARAMS.match(currentFieldName, parser.getDeprecationHandler())) {
builder.setParams((Map) WatcherUtils.flattenModel(parser.map()));
} else if (Field.BODY.match(currentFieldName, parser.getDeprecationHandler())) {
} else if (Field.BODY.match(currentFieldName, parser.getDeprecationHandler())) {
builder.body(parser.text());
} else {
throw new ElasticsearchParseException("could not parse http request. unexpected object field [{}]",
Expand Down Expand Up @@ -360,7 +351,7 @@ public static class Builder {
private String path;
private Map<String, String> params = new HashMap<>();
private Map<String, String> headers = new HashMap<>();
private HttpAuth auth;
private BasicAuth auth;
private String body;
private TimeValue connectionTimeout;
private TimeValue readTimeout;
Expand Down Expand Up @@ -421,7 +412,7 @@ public Builder setHeader(String key, String value) {
return this;
}

public Builder auth(HttpAuth auth) {
public Builder auth(BasicAuth auth) {
this.auth = auth;
return this;
}
Expand Down
Loading

0 comments on commit 0814425

Please sign in to comment.