#25587 adding feedback

dotCMS · Aug 29, 2023 · 0e6d6a3 · 0e6d6a3
1 parent ae2fd76
commit 0e6d6a3
Showing 1 changed file with 26 additions and 27 deletions.
diff --git a/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/SecretTool.java b/dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/SecretTool.java
@@ -9,6 +9,7 @@
 import com.dotmarketing.portlets.contentlet.model.Contentlet;
 import com.dotmarketing.util.Config;
 import com.dotmarketing.util.Logger;
+import com.dotmarketing.util.UtilMethods;
 import com.liferay.portal.model.User;
 import org.apache.velocity.context.Context;
 import org.apache.velocity.context.InternalContextAdapterImpl;
@@ -45,9 +46,7 @@ public void init(final Object initData) {
 	 */
 	public Object get(final String key) {
 
-        if(!canUserEvaluate()){
-            throw new SecurityException("External scripting is disabled in your dotcms instance.");
-        }
+		canUserEvaluate();
 
 		final HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
 		final Optional<DotVelocitySecretAppConfig> config = DotVelocitySecretAppConfig.config(request);
@@ -74,18 +73,14 @@ public Object getSystemSecret (final String key) {
 	public Object getSystemSecret (final String key,
 								   final Object defaultValue) {
 
-        if(!canUserEvaluate()){
-            throw new SecurityException("External scripting is disabled in your dotcms instance.");
-        }
+		canUserEvaluate();
 		final Optional<DotVelocitySecretAppConfig> config = DotVelocitySecretAppConfig.config(APILocator.systemHost());
 		return config.isPresent()? config.get().getStringOrNull(key, null!= defaultValue? defaultValue.toString():null) : defaultValue;
 	}
 
 	public char[] getCharArray(final String key) {
 
-        if(!canUserEvaluate()){
-            throw new SecurityException("External scripting is disabled in your dotcms instance.");
-        }
+		canUserEvaluate();
 		final HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
 		final Optional<DotVelocitySecretAppConfig> config = DotVelocitySecretAppConfig.config(request);
 		return config.isPresent()? config.get().getCharArrayOrNull(key) : null;
@@ -99,9 +94,7 @@ public char[] getCharArraySystemSecret (final String key) {
 	public char[] getCharArraySystemSecret (final String key,
 								   final char[] defaultValue) {
 
-        if(!canUserEvaluate()){
-            throw new SecurityException("External scripting is disabled in your dotcms instance.");
-        }
+		canUserEvaluate();
 		final Optional<DotVelocitySecretAppConfig> config = DotVelocitySecretAppConfig.config(APILocator.systemHost());
 		return config.isPresent()? config.get().getCharArrayOrNull(key, defaultValue) : defaultValue;
 	}
@@ -114,39 +107,45 @@ public char[] getCharArraySystemSecret (final String key,
 	 * 2) otherwise check if the last modified user has the scripting role
      * @return boolean
      */
-    protected boolean canUserEvaluate() {
+    protected void canUserEvaluate() {
 
         if(!ENABLE_SCRIPTING) {
 
             Logger.warn(this.getClass(), "Scripting called and ENABLE_SCRIPTING set to false");
-            return false;
+			throw new SecurityException("External scripting is disabled in your dotcms instance.");
         }
 
         try {
 
 			boolean hasScriptingRole = false;
-			final User user = WebAPILocator.getUserWebAPI().getUser(this.request);
 			final Role scripting = APILocator.getRoleAPI().loadRoleByKey(Role.SCRIPTING_DEVELOPER);
-			// try with the current user
-			if (null != user) {
-
-				hasScriptingRole = APILocator.getRoleAPI().doesUserHaveRole(user, scripting);
-			}
-
-			if (!hasScriptingRole) {
 
-				this.internalContextAdapter    = new InternalContextAdapterImpl(context);
-				final String fieldResourceName = this.internalContextAdapter.getCurrentTemplateName();
+			this.internalContextAdapter    = new InternalContextAdapterImpl(context);
+			final String fieldResourceName = this.internalContextAdapter.getCurrentTemplateName();
+			if (UtilMethods.isSet(fieldResourceName)) {
 				final String contentletFileAssetInode = fieldResourceName.substring(fieldResourceName.indexOf("/") + 1, fieldResourceName.indexOf("_"));
 				final Contentlet contentlet = APILocator.getContentletAPI().find(contentletFileAssetInode, APILocator.systemUser(), true);
 				final User lastModifiedUser = APILocator.getUserAPI().loadUserById(contentlet.getModUser(), APILocator.systemUser(), true);
 				hasScriptingRole = APILocator.getRoleAPI().doesUserHaveRole(lastModifiedUser, scripting);
 			}
 
-			return hasScriptingRole;
-        } catch(Exception e){
+			if (!hasScriptingRole) {
+				final User user = WebAPILocator.getUserWebAPI().getUser(this.request);
+				// try with the current user
+				if (null != user) {
+
+					hasScriptingRole = APILocator.getRoleAPI().doesUserHaveRole(user, scripting);
+				}
+			}
+
+			if (!hasScriptingRole) {
+
+				throw new SecurityException("External scripting is disabled in your dotcms instance.");
+			}
+        } catch(Exception e) {
+
             Logger.warn(this.getClass(), "Scripting called with error" + e);
-            return false;
+			throw new SecurityException("External scripting is disabled in your dotcms instance.", e);
         }
     } // canUserEvaluate.
 }