KEYCLOAK-6222 Check syntax for errors on ScriptBasedOIDCProtocolMapper validation

We now explicitly check for syntax errors
during validation of ScriptBasedOIDCProtocolMappers.

server-spi-private/src/main/java/org/keycloak/scripting/ScriptCompilationException.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2018 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.keycloak.scripting;
+
+import org.keycloak.models.ScriptModel;
+
+import javax.script.ScriptException;
+
+/**
+ * Indicates compilation problems reported by a {@link ScriptException} and adds additional metadata.
+ *
+ * @author <a href="mailto:thomas.darimont@gmail.com">Thomas Darimont</a>
+ */
+public class ScriptCompilationException extends RuntimeException {
+
+    public ScriptCompilationException(ScriptModel script, Exception ex) {
+        super("Could not compile '" + script.getName() + "' problem was: " + ex.getMessage(), ex);
+    }
+}

services/src/main/java/org/keycloak/protocol/oidc/mappers/ScriptBasedOIDCProtocolMapper.java

@@ -20,18 +20,23 @@ package org.keycloak.protocol.oidc.mappers;
 import org.jboss.logging.Logger;
 import org.keycloak.common.Profile;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ProtocolMapperContainerModel;
 import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.ScriptModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
+import org.keycloak.protocol.ProtocolMapperConfigException;
 import org.keycloak.protocol.ProtocolMapperUtils;
 import org.keycloak.provider.ProviderConfigProperty;
 import org.keycloak.provider.ProviderConfigurationBuilder;
 import org.keycloak.representations.IDToken;
 import org.keycloak.scripting.EvaluatableScriptAdapter;
+import org.keycloak.scripting.ScriptCompilationException;
 import org.keycloak.scripting.ScriptingProvider;
 
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
 import java.util.List;
 
 /**
@@ -143,11 +148,29 @@ public class ScriptBasedOIDCProtocolMapper extends AbstractOIDCProtocolMapper im
     OIDCAttributeMapperHelper.mapClaim(token, mappingModel, claimValue);
   }
 
+  @Override
+  public void validateConfig(KeycloakSession session, RealmModel realm, ProtocolMapperContainerModel client, ProtocolMapperModel mapperModel) throws ProtocolMapperConfigException {
+
+    String scriptCode = mapperModel.getConfig().get(SCRIPT);
+    if (scriptCode == null) {
+      return;
+    }
+
+    ScriptingProvider scripting = session.getProvider(ScriptingProvider.class);
+    ScriptModel scriptModel = scripting.createScript(realm.getId(), ScriptModel.TEXT_JAVASCRIPT, mapperModel.getName() + "-script", scriptCode, "");
+
+    try {
+      scripting.prepareEvaluatableScript(scriptModel);
+    } catch (ScriptCompilationException  ex) {
+      throw new ProtocolMapperConfigException("error", "{0}", ex.getMessage());
+    }
+  }
+
   public static ProtocolMapperModel create(String name,
-                                                      String userAttribute,
+                                           String userAttribute,
-                                                      String tokenClaimName, String claimType,
+                                           String tokenClaimName, String claimType,
-                                                      boolean consentRequired, String consentText,
+                                           boolean consentRequired, String consentText,
-                                                      boolean accessToken, boolean idToken, String script, boolean multiValued) {
+                                           boolean accessToken, boolean idToken, String script, boolean multiValued) {
     ProtocolMapperModel mapper = OIDCAttributeMapperHelper.createClaimMapper(name, userAttribute,
       tokenClaimName, claimType,
       consentRequired, consentText,

services/src/main/java/org/keycloak/scripting/DefaultScriptingProvider.java

@@ -72,20 +72,19 @@ public class DefaultScriptingProvider implements ScriptingProvider {
         ScriptEngine engine = createPreparedScriptEngine(scriptModel);
 
         if (engine instanceof Compilable) {
-            try {
+            return new CompiledEvaluatableScriptAdapter(scriptModel, tryCompile(scriptModel, (Compilable) engine));
-                final CompiledScript compiledScript = ((Compilable) engine).compile(scriptModel.getCode());
-                return new CompiledEvaluatableScriptAdapter(scriptModel, compiledScript);
-            }
-            catch (ScriptException e) {
-                throw new ScriptExecutionException(scriptModel, e);
-            }
         }
+
         return new UncompiledEvaluatableScriptAdapter(scriptModel, engine);
     }
 
-    //TODO allow scripts to be maintained independently of other components, e.g. with dedicated persistence
+    private CompiledScript tryCompile(ScriptModel scriptModel, Compilable engine) {
-    //TODO allow script lookup by (scriptId)
+        try {
-    //TODO allow script lookup by (name, realmName)
+            return engine.compile(scriptModel.getCode());
+        } catch (ScriptException e) {
+            throw new ScriptCompilationException(scriptModel, e);
+        }
+    }
 
     @Override
     public ScriptModel createScript(String realmId, String mimeType, String scriptName, String scriptCode, String scriptDescription) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java

@@ -42,6 +42,7 @@ import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.ProtocolMapperUtil;
 
+import javax.ws.rs.core.Response;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -149,6 +150,10 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
             app.getProtocolMappers().createMapper(createRoleNameMapper("rename-app-role", "test-app.customer-user", "realm-user")).close();
             app.getProtocolMappers().createMapper(createScriptMapper("test-script-mapper1","computed-via-script", "computed-via-script", "String", true, true, "'hello_' + user.username", false)).close();
             app.getProtocolMappers().createMapper(createScriptMapper("test-script-mapper2","multiValued-via-script", "multiValued-via-script", "String", true, true, "new java.util.ArrayList(['A','B'])", true)).close();
+
+            Response response = app.getProtocolMappers().createMapper(createScriptMapper("test-script-mapper3", "syntax-error-script", "syntax-error-script", "String", true, true, "func_tion foo(){ return 'fail';} foo()", false));
+            assertThat(response.getStatusInfo().getFamily(), is(Response.Status.Family.CLIENT_ERROR));
+            response.close();
         }
 
         {