Deepthi M|Updated to show details of error message only when a global property is enabled,Fixed failing test cases and added new tests for the updated method. (#591)

* Deepthi M|Updated to show details of error message only when a  global property is enabled,Fixed failing test cases and added new tests for the updated method.

* Deepthi M|Fixed global property

* Deepthi M|removing white spaces

* Deepthi M|Updated description of global property and also added a default value to global property as false when no value is given to it to avoid NPE

* Deepthi M|RESTWS-921|Updated as per the review comments-Renamed variables,updated description in config.xml and added spaces after comma.

Author: deepthi-mantena

omod-common/src/main/java/org/openmrs/module/webservices/rest/web/RestConstants.java

@@ -206,4 +206,12 @@ public class RestConstants {
 	 * Constants used for the Server Log REST Service privilege checking
 	 */
 	public static final String PRIV_GET_SERVER_LOGS = "Get Server Logs";
+	/**
+	 * Global property name used to enable or disable the inclusion of stack trace details
+	 * in the error response.
+	 *
+	 * When this property is set to 'true', stack trace details will be included in error
+	 * responses. When set to 'false', stack trace details will be omitted.
+	 */
+	public static String ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME = MODULE_ID + ".enableStackTraceDetails";
 }

omod-common/src/main/java/org/openmrs/module/webservices/rest/web/RestUtil.java

@@ -828,16 +828,21 @@ public static SimpleObject wrapErrorResponse(Exception ex, String reason) {
 		} else {
 			map.put("message", "[" + message + "]");
 		}
-		StackTraceElement[] steElements = ex.getStackTrace();
-		if (steElements.length > 0) {
-			StackTraceElement ste = ex.getStackTrace()[0];
-			map.put("code", ste.getClassName() + ":" + ste.getLineNumber());
-			map.put("detail", ExceptionUtils.getStackTrace(ex));
+		StackTraceElement[] stackTraceElements = ex.getStackTrace();
+		if (stackTraceElements.length > 0) {
+			StackTraceElement stackTraceElement = ex.getStackTrace()[0];
+			String stackTraceDetailsEnabledGp = Context.getAdministrationService()
+					.getGlobalPropertyValue(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "false");
+			map.put("code", stackTraceElement.getClassName() + ":" + stackTraceElement.getLineNumber());
+			if ("true".equalsIgnoreCase(stackTraceDetailsEnabledGp)) {
+				map.put("detail", ExceptionUtils.getStackTrace(ex));
+			} else {
+				map.put("detail", "");
+			}
 		} else {
 			map.put("code", "");
 			map.put("detail", "");
 		}
-		
 		return new SimpleObject().add("error", map);
 	}
 

omod-common/src/test/java/org/openmrs/module/webservices/rest/web/RestUtilTest.java

@@ -18,13 +18,16 @@
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
+import org.openmrs.GlobalProperty;
+import org.openmrs.api.context.Context;
 import org.openmrs.module.webservices.rest.SimpleObject;
+import org.openmrs.web.test.BaseModuleWebContextSensitiveTest;
 import org.springframework.mock.web.MockHttpServletRequest;
 
 /**
  * Tests for the {@link RestUtil} class.
  */
-public class RestUtilTest {
+public class RestUtilTest extends BaseModuleWebContextSensitiveTest {
 
 	/**
 	 * @see RestUtil#ipMatches(String,List)
@@ -201,5 +204,24 @@ public void wrapErrorResponse_shouldSetStackTraceCodeAndDetailEmptyIfNotAvailabl
 		Assert.assertEquals("", errorResponseMap.get("code"));
 		Assert.assertEquals("", errorResponseMap.get("detail"));
 	}
-	
+	@Test
+	public void wrapErrorResponse_shouldSetStackTraceDetailsIfGlobalPropEnabled() throws Exception {
+		Context.getAdministrationService().saveGlobalProperty(
+				new GlobalProperty(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "true"));
+		Exception ex = new Exception("exceptionmessage");
+		SimpleObject returnObject = RestUtil.wrapErrorResponse(ex, "wraperrorresponsemessage");
+
+		LinkedHashMap errorResponseMap = (LinkedHashMap) returnObject.get("error");
+		Assert.assertNotEquals("", errorResponseMap.get("detail"));
+	}
+	@Test
+	public void wrapErrorResponse_shouldSetNoStackTraceDetailsIfGlobalPropDisabled() throws Exception {
+		Context.getAdministrationService().saveGlobalProperty(
+				new GlobalProperty(RestConstants.ENABLE_STACK_TRACE_DETAILS_GLOBAL_PROPERTY_NAME, "false"));
+		Exception ex = new Exception("exceptionmessage");
+		SimpleObject returnObject = RestUtil.wrapErrorResponse(ex, "wraperrorresponsemessage");
+
+		LinkedHashMap errorResponseMap = (LinkedHashMap) returnObject.get("error");
+		Assert.assertEquals("", errorResponseMap.get("detail"));
+	}
 }

omod/src/main/resources/config.xml

@@ -82,6 +82,11 @@
 		<defaultValue>true</defaultValue>
 		<description>If the value of this setting is "true", then nothing is logged while the Swagger specification is being generated.</description>
 	</globalProperty>
+	<globalProperty>
+		<property>@MODULE_ID@.enableStackTraceDetails</property>
+		<defaultValue>true</defaultValue>
+		<description>If the value of this setting is "true", then the details of the stackTrace would be shown in the error response. However, the recommendation is to keep it as "false", from the Security perspective, to avoid leaking implementation details.</description>
+	</globalProperty>
 
     <!-- DWR -->