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.

Sign up for GitHub

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

Jump to bottom

Watcher: Remove extraneous auth classes #32300

Merged
merged 6 commits into from
Aug 14, 2018
Merged

Watcher: Remove extraneous auth classes #32300

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a1023c
Watcher: Remove extraneous auth classes
hub-cap Jul 23, 2018
93b7248
Cleanup a short line
hub-cap Jul 23, 2018
6e2a024
Merge remote-tracking branch 'upstream/master' into remove_httpauth
hub-cap Jul 25, 2018
612a394
Merge remote-tracking branch 'upstream/master' into remove_httpauth
hub-cap Jul 26, 2018
dd97372
Merge remote-tracking branch 'upstream/master' into remove_httpauth
hub-cap Aug 2, 2018
8142993
Merge remote-tracking branch 'upstream/master' into remove_httpauth
hub-cap Aug 13, 2018
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
22 changes: 5 additions & 17 deletions x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java
View file
Open in desktop
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 @@ -264,12 +259,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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @@ -286,11 +276,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 @@ -310,7 +298,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 @@ -324,7 +312,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
5 changes: 2 additions & 3 deletions .../watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookAction.java
View file
Open in desktop
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
8 changes: 2 additions & 6 deletions ...r/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java
View file
Open in desktop
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
27 changes: 18 additions & 9 deletions ...her/common/http/auth/basic/BasicAuth.java → .../xpack/watcher/common/http/BasicAuth.java
View file
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using an object parser here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not new, it was just yanked from HttpAuthRegistry.java since its still used in ser/deser of HttpRequest and some other classes. Id be happy to do it, but id like to make it a diff commit since this is just moving the code.

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
16 changes: 9 additions & 7 deletions .../plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java
View file
Open in desktop
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
29 changes: 10 additions & 19 deletions ...plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java
View file
Open in desktop
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