Disallow lang to specified in requests where a stored script is used. (elastic/x-pack-elasticsearch#1949)

Requests that execute a stored script will no longer be allowed to specify the lang of the script. This information is stored in the cluster state making only an id necessary to execute against. Putting a stored script will still require a lang.

Original commit: elastic/x-pack-elasticsearch@926a7b2d86

plugin/src/main/java/org/elasticsearch/xpack/common/text/TextTemplate.java

@@ -47,7 +47,7 @@ public class TextTemplate implements ToXContent {
         if (params == null) {
             params = new HashMap<>();
         }
-        this.script = new Script(type, Script.DEFAULT_TEMPLATE_LANG, template, options, params);
+        this.script = new Script(type, type == ScriptType.STORED ? null : Script.DEFAULT_TEMPLATE_LANG, template, options, params);
         this.inlineTemplate = null;
     }
 
@@ -116,13 +116,6 @@ public class TextTemplate implements ToXContent {
             return new TextTemplate(parser.text());
         } else {
             Script template = Script.parse(parser, Script.DEFAULT_TEMPLATE_LANG);
-
-            // for deprecation of stored script namespaces the default lang is ignored,
-            // so the template lang must be set for a stored script
-            if (template.getType() == ScriptType.STORED) {
-                template = new Script(ScriptType.STORED, Script.DEFAULT_TEMPLATE_LANG, template.getIdOrCode(), template.getParams());
-            }
-
             return new TextTemplate(template);
         }
     }

plugin/src/main/java/org/elasticsearch/xpack/common/text/TextTemplateEngine.java

@@ -51,7 +51,8 @@ public class TextTemplateEngine extends AbstractComponent {
 
             options.put(Script.CONTENT_TYPE_OPTION, mediaType);
         }
-        Script script = new Script(textTemplate.getType(), "mustache", template, options, mergedModel);
+        Script script = new Script(textTemplate.getType(),
+                textTemplate.getType() == ScriptType.STORED ? null : "mustache", template, options, mergedModel);
         TemplateScript.Factory compiledTemplate = service.compile(script, Watcher.SCRIPT_TEMPLATE_CONTEXT);
         return compiledTemplate.newInstance(model).execute();
     }

plugin/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java

@@ -58,6 +58,7 @@ import org.elasticsearch.index.shard.ShardUtils;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.script.TemplateScript;
 import org.elasticsearch.xpack.security.authc.Authentication;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
@@ -272,7 +273,8 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
                 userModel.put("metadata", Collections.unmodifiableMap(user.metadata()));
                 params.put("_user", userModel);
                 // Always enforce mustache script lang:
-                script = new Script(script.getType(), "mustache", script.getIdOrCode(), script.getOptions(), params);
+                script = new Script(script.getType(),
+                        script.getType() == ScriptType.STORED ? null : "mustache", script.getIdOrCode(), script.getOptions(), params);
                 TemplateScript compiledTemplate = scriptService.compile(script, TemplateScript.CONTEXT).newInstance(script.getParams());
                 return compiledTemplate.execute();
             } else {

plugin/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java

@@ -56,7 +56,6 @@ public class WatcherSearchTemplateRequest implements ToXContentObject {
 
     public WatcherSearchTemplateRequest(String[] indices, String[] types, SearchType searchType, IndicesOptions indicesOptions,
                                         Script template) {
-        assert template == null || Script.DEFAULT_TEMPLATE_LANG.equals(template.getLang());
         this.indices = indices;
         this.types = types;
         this.searchType = searchType;
@@ -248,13 +247,6 @@ public class WatcherSearchTemplateRequest implements ToXContentObject {
                             DEFAULT_INDICES_OPTIONS);
                 } else if (TEMPLATE_FIELD.match(currentFieldName)) {
                     template = Script.parse(parser, Script.DEFAULT_TEMPLATE_LANG);
-
-                    // for deprecation of stored script namespaces the default lang is ignored,
-                    // so the template lang must be set for a stored script
-                    if (template.getType() == ScriptType.STORED) {
-                        template = new Script(
-                                ScriptType.STORED, Script.DEFAULT_TEMPLATE_LANG, template.getIdOrCode(), template.getParams());
-                    }
                 } else {
                     throw new ElasticsearchParseException("could not read search request. unexpected object field [" +
                             currentFieldName + "]");

plugin/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateService.java

@@ -14,6 +14,7 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.script.TemplateScript;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.xpack.watcher.Watcher;
@@ -48,7 +49,8 @@ public class WatcherSearchTemplateService extends AbstractComponent {
             watcherContextParams.putAll(source.getParams());
         }
         // Templates are always of lang mustache:
-        Script template = new Script(source.getType(), "mustache", source.getIdOrCode(), source.getOptions(), watcherContextParams);
+        Script template = new Script(source.getType(), source.getType() == ScriptType.STORED ? null : "mustache",
+                source.getIdOrCode(), source.getOptions(), watcherContextParams);
         TemplateScript.Factory compiledTemplate = scriptService.compile(template, Watcher.SCRIPT_TEMPLATE_CONTEXT);
         return compiledTemplate.newInstance(template.getParams()).execute();
     }

plugin/src/test/java/org/elasticsearch/xpack/common/text/TextTemplateTests.java

@@ -62,7 +62,7 @@ public class TextTemplateTests extends ESTestCase {
                     }
                 };
 
-        when(service.compile(new Script(type, lang, templateText,
+        when(service.compile(new Script(type, type == ScriptType.STORED ? null : lang, templateText,
                 type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null,
                 merged), Watcher.SCRIPT_TEMPLATE_CONTEXT)).thenReturn(compiledTemplate);
 
@@ -84,7 +84,7 @@ public class TextTemplateTests extends ESTestCase {
                 }
             };
 
-        when(service.compile(new Script(type, lang, templateText,
+        when(service.compile(new Script(type, type == ScriptType.STORED ? null : lang, templateText,
                 type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null,
                 model), Watcher.SCRIPT_TEMPLATE_CONTEXT)).thenReturn(compiledTemplate);
 
@@ -114,7 +114,8 @@ public class TextTemplateTests extends ESTestCase {
 
     public void testParser() throws Exception {
         ScriptType type = randomScriptType();
-        TextTemplate template = templateBuilder(type, "_template", singletonMap("param_key", "param_val"));
+        TextTemplate template =
+                templateBuilder(type, "_template", singletonMap("param_key", "param_val"));
         XContentBuilder builder = jsonBuilder().startObject();
         switch (type) {
             case INLINE:
@@ -134,7 +135,9 @@ public class TextTemplateTests extends ESTestCase {
     }
 
     public void testParserParserSelfGenerated() throws Exception {
-        TextTemplate template = templateBuilder(randomScriptType(), "_template", singletonMap("param_key", "param_val"));
+        ScriptType type = randomScriptType();
+        TextTemplate template =
+                templateBuilder(type, "_template", singletonMap("param_key", "param_val"));
 
         XContentBuilder builder = jsonBuilder().value(template);
         BytesReference bytes = builder.bytes();

plugin/src/test/java/org/elasticsearch/xpack/watcher/support/WatcherUtilsTests.java

@@ -96,6 +96,7 @@ public class WatcherUtilsTests extends ESTestCase {
         BytesReference expectedSource = null;
         Script expectedTemplate = null;
         WatcherSearchTemplateRequest request;
+        boolean stored = false;
         if (randomBoolean()) {
             Map<String, Object> params = new HashMap<>();
             if (randomBoolean()) {
@@ -106,7 +107,8 @@ public class WatcherUtilsTests extends ESTestCase {
             }
             String text = randomAlphaOfLengthBetween(1, 5);
             ScriptType scriptType = randomFrom(ScriptType.values());
-            expectedTemplate = new Script(scriptType, "mustache", text, params);
+            stored = scriptType == ScriptType.STORED;
+            expectedTemplate = new Script(scriptType, stored ? null : "mustache", text, params);
             request = new WatcherSearchTemplateRequest(expectedIndices, expectedTypes, expectedSearchType,
                     expectedIndicesOptions, expectedTemplate);
         } else {
@@ -130,7 +132,7 @@ public class WatcherUtilsTests extends ESTestCase {
         assertThat(result.getSearchType(), equalTo(expectedSearchType));
 
         assertNotNull(result.getTemplate());
-        assertThat(result.getTemplate().getLang(), equalTo("mustache"));
+        assertThat(result.getTemplate().getLang(), equalTo(stored ? null : "mustache"));
         if (expectedSource == null) {
             assertThat(result.getTemplate().getIdOrCode(), equalTo(expectedTemplate.getIdOrCode()));
             assertThat(result.getTemplate().getType(), equalTo(expectedTemplate.getType()));
@@ -194,6 +196,7 @@ public class WatcherUtilsTests extends ESTestCase {
             builder.rawField("body", source);
         }
         Script template = null;
+        boolean stored = false;
         if (randomBoolean()) {
             Map<String, Object> params = new HashMap<>();
             if (randomBoolean()) {
@@ -204,7 +207,8 @@ public class WatcherUtilsTests extends ESTestCase {
             }
             String text = randomAlphaOfLengthBetween(1, 5);
             ScriptType scriptType = randomFrom(ScriptType.values());
-            template = new Script(scriptType, "mustache", text, params);
+            stored = scriptType == ScriptType.STORED;
+            template = new Script(scriptType, stored ? null : "mustache", text, params);
             builder.field("template", template);
         }
         builder.endObject();
@@ -228,7 +232,7 @@ public class WatcherUtilsTests extends ESTestCase {
             assertThat(result.getTemplate().getIdOrCode(), equalTo(template.getIdOrCode()));
             assertThat(result.getTemplate().getType(), equalTo(template.getType()));
             assertThat(result.getTemplate().getParams(), equalTo(template.getParams()));
-            assertThat(result.getTemplate().getLang(), equalTo("mustache"));
+            assertThat(result.getTemplate().getLang(), equalTo(stored ? null : "mustache"));
         }
     }
 

plugin/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java

@@ -20,7 +20,7 @@ public class WatcherSearchTemplateRequestTests extends ESTestCase {
 
     public void testFromXContentWithTemplateDefaultLang() throws IOException {
         String source = "{\"template\":{\"id\":\"default-script\", \"params\":{\"foo\":\"bar\"}}}";
-        assertTemplate(source, "default-script", "mustache", singletonMap("foo", "bar"));
+        assertTemplate(source, "default-script", null, singletonMap("foo", "bar"));
     }
 
     public void testFromXContentWithTemplateCustomLang() throws IOException {

plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java

@@ -235,7 +235,7 @@ public class BasicWatcherTests extends AbstractWatcherIntegrationTestCase {
                 .setContent(jsonBuilder().startObject().field("template").value(searchSourceBuilder).endObject().bytes(), XContentType.JSON)
                 .get());
 
-        Script template = new Script(ScriptType.STORED, "mustache", "my-template", Collections.emptyMap());
+        Script template = new Script(ScriptType.STORED, null, "my-template", Collections.emptyMap());
         WatcherSearchTemplateRequest searchRequest = new WatcherSearchTemplateRequest(new String[]{"events"}, new String[0],
                 SearchType.DEFAULT, WatcherSearchTemplateRequest.DEFAULT_INDICES_OPTIONS, template);
         testConditionSearch(searchRequest);

plugin/src/test/java/org/elasticsearch/xpack/watcher/transform/TransformIntegrationTests.java

@@ -123,7 +123,7 @@ public class TransformIntegrationTests extends AbstractWatcherIntegrationTestCas
                     .setLang("mockscript")
                     .setContent(new BytesArray("{\"script\" : \"['key3' : ctx.payload.key1 + ctx.payload.key2]\"}"), XContentType.JSON)
                     .get());
-            script = new Script(ScriptType.STORED, "mockscript", "my-script", Collections.emptyMap());
+            script = new Script(ScriptType.STORED, null, "my-script", Collections.emptyMap());
         }
 
         // put a watch that has watch level transform:

plugin/src/test/java/org/elasticsearch/xpack/watcher/transform/script/ScriptTransformTests.java

@@ -58,7 +58,7 @@ public class ScriptTransformTests extends ESTestCase {
         ScriptService service = mock(ScriptService.class);
         ScriptType type = randomFrom(ScriptType.values());
         Map<String, Object> params = Collections.emptyMap();
-        Script script = new Script(type, "_lang", "_script", params);
+        Script script = new Script(type, type == ScriptType.STORED ? null : "_lang", "_script", params);
         ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
         when(service.compile(script, Watcher.SCRIPT_EXECUTABLE_CONTEXT)).thenReturn(factory);
         ExecutableScriptTransform transform = new ExecutableScriptTransform(new ScriptTransform(script), logger, service);
@@ -86,7 +86,7 @@ public class ScriptTransformTests extends ESTestCase {
         ScriptService service = mock(ScriptService.class);
         ScriptType type = randomFrom(ScriptType.values());
         Map<String, Object> params = Collections.emptyMap();
-        Script script = new Script(type, "_lang", "_script", params);
+        Script script = new Script(type, type == ScriptType.STORED ? null : "_lang", "_script", params);
         ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
         when(service.compile(script, Watcher.SCRIPT_EXECUTABLE_CONTEXT)).thenReturn(factory);
         ExecutableScriptTransform transform = new ExecutableScriptTransform(new ScriptTransform(script), logger, service);
@@ -112,7 +112,7 @@ public class ScriptTransformTests extends ESTestCase {
         ScriptService service = mock(ScriptService.class);
         ScriptType type = randomFrom(ScriptType.values());
         Map<String, Object> params = Collections.emptyMap();
-        Script script = new Script(type, "_lang", "_script", params);
+        Script script = new Script(type, type == ScriptType.STORED ? null : "_lang", "_script", params);
         ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
         when(service.compile(script, Watcher.SCRIPT_EXECUTABLE_CONTEXT)).thenReturn(factory);
         ExecutableScriptTransform transform = new ExecutableScriptTransform(new ScriptTransform(script), logger, service);
@@ -186,10 +186,9 @@ public class ScriptTransformTests extends ESTestCase {
 
     public void testScriptConditionParserBadLang() throws Exception {
         ScriptTransformFactory transformFactory = new ScriptTransformFactory(Settings.builder().build(), createScriptService(tp));
-        ScriptType scriptType = randomFrom(ScriptType.values());
         String script = "return true";
         XContentBuilder builder = jsonBuilder().startObject()
-                .field(scriptTypeField(scriptType), script)
+                .field(scriptTypeField(ScriptType.INLINE), script)
                 .field("lang", "not_a_valid_lang")
                 .startObject("params").field("key", "value").endObject()
                 .endObject();
@@ -199,13 +198,7 @@ public class ScriptTransformTests extends ESTestCase {
         parser.nextToken();
         ScriptTransform scriptCondition = transformFactory.parseTransform("_watch", parser);
         Exception e = expectThrows(IllegalArgumentException.class, () -> transformFactory.createExecutable(scriptCondition));
-        if (scriptType == ScriptType.STORED) {
+        assertThat(e.getMessage(), containsString("script_lang not supported [not_a_valid_lang]"));
-            assertThat(e.getMessage(), containsString("unable to get stored script with unsupported lang [not_a_valid_lang]"));
-            assertWarnings("specifying the field [lang] for executing stored scripts is deprecated;" +
-                    " use only the field [id] to specify an <id>");
-        } else {
-            assertThat(e.getMessage(), containsString("script_lang not supported [not_a_valid_lang]"));
-        }
     }
 
     static String scriptTypeField(ScriptType type) {

qa/smoke-test-watcher-with-mustache/src/test/resources/rest-api-spec/test/watcher_mustache/30_search_input.yml

@@ -157,5 +157,4 @@ setup:
   # makes sure that the mustache template snippets have been resolved  correctly:
   - match: { "watch_record.result.input.search.request.body.query.bool.must.0.term.value": "val_2" }
   - match: { "watch_record.result.input.search.request.template.id": "search-template" }
-  - match: { "watch_record.result.input.search.request.template.lang": "mustache" }
   - match: { "watch_record.result.input.search.request.template.params.num": 2 }