Skip to content

Commit 0814425

Browse files
hub-caphub-cap
committed
Watcher: Remove extraneous auth classes (#32300)
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.
1 parent dbd5f50 commit 0814425

36 files changed

+139
-474
lines changed

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,7 @@
9292
import org.elasticsearch.xpack.watcher.actions.webhook.WebhookAction;
9393
import org.elasticsearch.xpack.watcher.actions.webhook.WebhookActionFactory;
9494
import org.elasticsearch.xpack.watcher.common.http.HttpClient;
95-
import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate;
9695
import org.elasticsearch.xpack.watcher.common.http.HttpSettings;
97-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthFactory;
98-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
99-
import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuth;
100-
import org.elasticsearch.xpack.watcher.common.http.auth.basic.BasicAuthFactory;
10196
import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;
10297
import org.elasticsearch.xpack.watcher.condition.ArrayCompareCondition;
10398
import org.elasticsearch.xpack.watcher.condition.CompareCondition;
@@ -283,12 +278,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
283278
new WatcherIndexTemplateRegistry(settings, clusterService, threadPool, client);
284279

285280
// http client
286-
Map<String, HttpAuthFactory> httpAuthFactories = new HashMap<>();
287-
httpAuthFactories.put(BasicAuth.TYPE, new BasicAuthFactory(cryptoService));
288-
// TODO: add more auth types, or remove this indirection
289-
HttpAuthRegistry httpAuthRegistry = new HttpAuthRegistry(httpAuthFactories);
290-
HttpRequestTemplate.Parser httpTemplateParser = new HttpRequestTemplate.Parser(httpAuthRegistry);
291-
httpClient = new HttpClient(settings, httpAuthRegistry, getSslService());
281+
httpClient = new HttpClient(settings, getSslService(), cryptoService);
292282

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

306296
TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService);
307297
Map<String, EmailAttachmentParser> emailAttachmentParsers = new HashMap<>();
308-
emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, httpTemplateParser,
309-
templateEngine));
298+
emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, templateEngine));
310299
emailAttachmentParsers.put(DataAttachmentParser.TYPE, new DataAttachmentParser());
311-
emailAttachmentParsers.put(ReportingAttachmentParser.TYPE, new ReportingAttachmentParser(settings, httpClient, templateEngine,
312-
httpAuthRegistry));
300+
emailAttachmentParsers.put(ReportingAttachmentParser.TYPE, new ReportingAttachmentParser(settings, httpClient, templateEngine));
313301
EmailAttachmentsParser emailAttachmentsParser = new EmailAttachmentsParser(emailAttachmentParsers);
314302

315303
// conditions
@@ -329,7 +317,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
329317
// actions
330318
final Map<String, ActionFactory> actionFactoryMap = new HashMap<>();
331319
actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser));
332-
actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, httpTemplateParser, templateEngine));
320+
actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, templateEngine));
333321
actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client));
334322
actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine));
335323
actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService));
@@ -343,7 +331,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
343331
final Map<String, InputFactory> inputFactories = new HashMap<>();
344332
inputFactories.put(SearchInput.TYPE, new SearchInputFactory(settings, client, xContentRegistry, scriptService));
345333
inputFactories.put(SimpleInput.TYPE, new SimpleInputFactory(settings));
346-
inputFactories.put(HttpInput.TYPE, new HttpInputFactory(settings, httpClient, templateEngine, httpTemplateParser));
334+
inputFactories.put(HttpInput.TYPE, new HttpInputFactory(settings, httpClient, templateEngine));
347335
inputFactories.put(NoneInput.TYPE, new NoneInputFactory(settings));
348336
inputFactories.put(TransformInput.TYPE, new TransformInputFactory(settings, transformRegistry));
349337
final InputRegistry inputRegistry = new InputRegistry(settings, inputFactories);

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookAction.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
5555
return requestTemplate.toXContent(builder, params);
5656
}
5757

58-
public static WebhookAction parse(String watchId, String actionId, XContentParser parser,
59-
HttpRequestTemplate.Parser requestParser) throws IOException {
58+
public static WebhookAction parse(String watchId, String actionId, XContentParser parser) throws IOException {
6059
try {
61-
HttpRequestTemplate request = requestParser.parse(parser);
60+
HttpRequestTemplate request = HttpRequestTemplate.Parser.parse(parser);
6261
return new WebhookAction(request);
6362
} catch (ElasticsearchParseException pe) {
6463
throw new ElasticsearchParseException("could not parse [{}] action [{}/{}]. failed parsing http request template", pe, TYPE,

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,25 @@
1010
import org.elasticsearch.common.xcontent.XContentParser;
1111
import org.elasticsearch.xpack.core.watcher.actions.ActionFactory;
1212
import org.elasticsearch.xpack.watcher.common.http.HttpClient;
13-
import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate;
1413
import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;
1514

1615
import java.io.IOException;
1716

1817
public class WebhookActionFactory extends ActionFactory {
1918

2019
private final HttpClient httpClient;
21-
private final HttpRequestTemplate.Parser requestTemplateParser;
2220
private final TextTemplateEngine templateEngine;
2321

24-
public WebhookActionFactory(Settings settings, HttpClient httpClient, HttpRequestTemplate.Parser requestTemplateParser,
25-
TextTemplateEngine templateEngine) {
22+
public WebhookActionFactory(Settings settings, HttpClient httpClient, TextTemplateEngine templateEngine) {
2623

2724
super(Loggers.getLogger(ExecutableWebhookAction.class, settings));
2825
this.httpClient = httpClient;
29-
this.requestTemplateParser = requestTemplateParser;
3026
this.templateEngine = templateEngine;
3127
}
3228

3329
@Override
3430
public ExecutableWebhookAction parseExecutable(String watchId, String actionId, XContentParser parser) throws IOException {
35-
return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser, requestTemplateParser),
31+
return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser),
3632
actionLogger, httpClient, templateEngine);
3733

3834
}

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/auth/basic/BasicAuth.java‎ renamed to ‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/BasicAuth.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,22 @@
33
* or more contributor license agreements. Licensed under the Elastic License;
44
* you may not use this file except in compliance with the Elastic License.
55
*/
6-
package org.elasticsearch.xpack.watcher.common.http.auth.basic;
6+
package org.elasticsearch.xpack.watcher.common.http;
77

88
import org.elasticsearch.ElasticsearchParseException;
99
import org.elasticsearch.common.ParseField;
10+
import org.elasticsearch.common.xcontent.ToXContentObject;
1011
import org.elasticsearch.common.xcontent.XContentBuilder;
1112
import org.elasticsearch.common.xcontent.XContentParser;
1213
import org.elasticsearch.xpack.core.watcher.common.secret.Secret;
1314
import org.elasticsearch.xpack.core.watcher.crypto.CryptoService;
1415
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherParams;
1516
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherXContentParser;
16-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuth;
1717

1818
import java.io.IOException;
1919
import java.util.Objects;
2020

21-
public class BasicAuth implements HttpAuth {
21+
public class BasicAuth implements ToXContentObject {
2222

2323
public static final String TYPE = "basic";
2424

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

37-
@Override
38-
public String type() {
39-
return TYPE;
40-
}
41-
4237
public String getUsername() {
4338
return username;
4439
}
@@ -74,7 +69,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
7469
return builder.endObject();
7570
}
7671

77-
public static BasicAuth parse(XContentParser parser) throws IOException {
72+
public static BasicAuth parseInner(XContentParser parser) throws IOException {
7873
String username = null;
7974
Secret password = null;
8075

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

101+
public static BasicAuth parse(XContentParser parser) throws IOException {
102+
String type = null;
103+
XContentParser.Token token;
104+
BasicAuth auth = null;
105+
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
106+
if (token == XContentParser.Token.FIELD_NAME) {
107+
type = parser.currentName();
108+
} else if (token == XContentParser.Token.START_OBJECT && type != null) {
109+
auth = BasicAuth.parseInner(parser);
110+
}
111+
}
112+
return auth;
113+
}
114+
106115
interface Field {
107116
ParseField USERNAME = new ParseField("username");
108117
ParseField PASSWORD = new ParseField("password");

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.apache.http.HttpHost;
1111
import org.apache.http.NameValuePair;
1212
import org.apache.http.auth.AuthScope;
13+
import org.apache.http.auth.Credentials;
14+
import org.apache.http.auth.UsernamePasswordCredentials;
1315
import org.apache.http.client.AuthCache;
1416
import org.apache.http.client.CredentialsProvider;
1517
import org.apache.http.client.config.RequestConfig;
@@ -42,8 +44,7 @@
4244
import org.elasticsearch.xpack.core.common.socket.SocketAccess;
4345
import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
4446
import org.elasticsearch.xpack.core.ssl.SSLService;
45-
import org.elasticsearch.xpack.watcher.common.http.auth.ApplicableHttpAuth;
46-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
47+
import org.elasticsearch.xpack.core.watcher.crypto.CryptoService;
4748

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

69-
private final HttpAuthRegistry httpAuthRegistry;
7070
private final CloseableHttpClient client;
7171
private final HttpProxy settingsProxy;
7272
private final TimeValue defaultConnectionTimeout;
7373
private final TimeValue defaultReadTimeout;
7474
private final ByteSizeValue maxResponseSize;
75+
private final CryptoService cryptoService;
7576

76-
public HttpClient(Settings settings, HttpAuthRegistry httpAuthRegistry, SSLService sslService) {
77+
public HttpClient(Settings settings, SSLService sslService, CryptoService cryptoService) {
7778
super(settings);
78-
this.httpAuthRegistry = httpAuthRegistry;
7979
this.defaultConnectionTimeout = HttpSettings.CONNECTION_TIMEOUT.get(settings);
8080
this.defaultReadTimeout = HttpSettings.READ_TIMEOUT.get(settings);
8181
this.maxResponseSize = HttpSettings.MAX_HTTP_RESPONSE_SIZE.get(settings);
8282
this.settingsProxy = getProxyFromSettings();
83+
this.cryptoService = cryptoService;
8384

8485
HttpClientBuilder clientBuilder = HttpClientBuilder.create();
8586

@@ -139,9 +140,10 @@ public HttpResponse execute(HttpRequest request) throws IOException {
139140
HttpClientContext localContext = HttpClientContext.create();
140141
// auth
141142
if (request.auth() != null) {
142-
ApplicableHttpAuth applicableAuth = httpAuthRegistry.createApplicable(request.auth);
143143
CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
144-
applicableAuth.apply(credentialsProvider, new AuthScope(request.host, request.port));
144+
Credentials credentials = new UsernamePasswordCredentials(request.auth().username,
145+
new String(request.auth().password.text(cryptoService)));
146+
credentialsProvider.setCredentials(new AuthScope(request.host, request.port), credentials);
145147
localContext.setCredentialsProvider(credentialsProvider);
146148

147149
// preemptive auth, no need to wait for a 401 first

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpRequest.java‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import org.elasticsearch.xpack.core.watcher.support.WatcherUtils;
2222
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherParams;
2323
import org.elasticsearch.xpack.core.watcher.support.xcontent.WatcherXContentParser;
24-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuth;
25-
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
2624

2725
import java.io.ByteArrayInputStream;
2826
import java.io.ByteArrayOutputStream;
@@ -50,15 +48,15 @@ public class HttpRequest implements ToXContentObject {
5048
@Nullable final String path;
5149
final Map<String, String> params;
5250
final Map<String, String> headers;
53-
@Nullable final HttpAuth auth;
51+
@Nullable final BasicAuth auth;
5452
@Nullable final String body;
5553
@Nullable final TimeValue connectionTimeout;
5654
@Nullable final TimeValue readTimeout;
5755
@Nullable final HttpProxy proxy;
5856

5957
public HttpRequest(String host, int port, @Nullable Scheme scheme, @Nullable HttpMethod method, @Nullable String path,
6058
@Nullable Map<String, String> params, @Nullable Map<String, String> headers,
61-
@Nullable HttpAuth auth, @Nullable String body, @Nullable TimeValue connectionTimeout,
59+
@Nullable BasicAuth auth, @Nullable String body, @Nullable TimeValue connectionTimeout,
6260
@Nullable TimeValue readTimeout, @Nullable HttpProxy proxy) {
6361
this.host = host;
6462
this.port = port;
@@ -102,7 +100,7 @@ public Map<String, String> headers() {
102100
return headers;
103101
}
104102

105-
public HttpAuth auth() {
103+
public BasicAuth auth() {
106104
return auth;
107105
}
108106

@@ -166,7 +164,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params toX
166164
}
167165
if (auth != null) {
168166
builder.startObject(Field.AUTH.getPreferredName())
169-
.field(auth.type(), auth, toXContentParams)
167+
.field(BasicAuth.TYPE, auth, toXContentParams)
170168
.endObject();
171169
}
172170
if (body != null) {
@@ -234,7 +232,7 @@ public String toString() {
234232
sb.append("], ");
235233
}
236234
if (auth != null) {
237-
sb.append("auth=[").append(auth.type()).append("], ");
235+
sb.append("auth=[").append(BasicAuth.TYPE).append("], ");
238236
}
239237
sb.append("connection_timeout=[").append(connectionTimeout).append("], ");
240238
sb.append("read_timeout=[").append(readTimeout).append("], ");
@@ -254,14 +252,7 @@ static Builder builder() {
254252
}
255253

256254
public static class Parser {
257-
258-
private final HttpAuthRegistry httpAuthRegistry;
259-
260-
public Parser(HttpAuthRegistry httpAuthRegistry) {
261-
this.httpAuthRegistry = httpAuthRegistry;
262-
}
263-
264-
public HttpRequest parse(XContentParser parser) throws IOException {
255+
public static HttpRequest parse(XContentParser parser) throws IOException {
265256
Builder builder = new Builder();
266257
XContentParser.Token token;
267258
String currentFieldName = null;
@@ -275,7 +266,7 @@ public HttpRequest parse(XContentParser parser) throws IOException {
275266
throw new ElasticsearchParseException("could not parse http request. could not parse [{}] field", currentFieldName);
276267
}
277268
} else if (Field.AUTH.match(currentFieldName, parser.getDeprecationHandler())) {
278-
builder.auth(httpAuthRegistry.parse(parser));
269+
builder.auth(BasicAuth.parse(parser));
279270
} else if (HttpRequest.Field.CONNECTION_TIMEOUT.match(currentFieldName, parser.getDeprecationHandler())) {
280271
builder.connectionTimeout(TimeValue.timeValueMillis(parser.longValue()));
281272
} else if (HttpRequest.Field.CONNECTION_TIMEOUT_HUMAN.match(currentFieldName, parser.getDeprecationHandler())) {
@@ -302,7 +293,7 @@ public HttpRequest parse(XContentParser parser) throws IOException {
302293
builder.setHeaders((Map) WatcherUtils.flattenModel(parser.map()));
303294
} else if (Field.PARAMS.match(currentFieldName, parser.getDeprecationHandler())) {
304295
builder.setParams((Map) WatcherUtils.flattenModel(parser.map()));
305-
} else if (Field.BODY.match(currentFieldName, parser.getDeprecationHandler())) {
296+
} else if (Field.BODY.match(currentFieldName, parser.getDeprecationHandler())) {
306297
builder.body(parser.text());
307298
} else {
308299
throw new ElasticsearchParseException("could not parse http request. unexpected object field [{}]",
@@ -360,7 +351,7 @@ public static class Builder {
360351
private String path;
361352
private Map<String, String> params = new HashMap<>();
362353
private Map<String, String> headers = new HashMap<>();
363-
private HttpAuth auth;
354+
private BasicAuth auth;
364355
private String body;
365356
private TimeValue connectionTimeout;
366357
private TimeValue readTimeout;
@@ -421,7 +412,7 @@ public Builder setHeader(String key, String value) {
421412
return this;
422413
}
423414

424-
public Builder auth(HttpAuth auth) {
415+
public Builder auth(BasicAuth auth) {
425416
this.auth = auth;
426417
return this;
427418
}

0 commit comments

Comments
 (0)