Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the logging properties to opt-out of the prefix events #844 #845

Merged
merged 10 commits into from
Sep 5, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@ public interface EsapiPropertyLoader {
*/
public Boolean getBooleanProp(String propertyName) throws ConfigurationException;

/**
* Get any Boolean type property from security configuration.
* If property does not exist in configuration or has incorrect type, defaultValue is returned
* @return property value.
*/
public Boolean getBooleanProp(String propertyName, Boolean defaultValue);

/**
* Get any property from security configuration. As every property can be returned as string, this method
* throws exception only when property does not exist.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,6 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
throw new ConfigurationException("Could not find property " + propertyName + " in configuration");
}

/**
* {@inheritDoc}
*/
@Override
public Boolean getBooleanProp(String propertyName, Boolean defaultValue) {
for (AbstractPrioritizedPropertyLoader loader : loaders) {
return loader.getBooleanProp(propertyName, defaultValue);
}
return defaultValue;
}


/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,6 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
}
}

/**
* {@inheritDoc}
*/
@Override
public Boolean getBooleanProp(String propertyName, Boolean defaultValue) {
String property = properties.getProperty(propertyName);
if (property == null) {
return defaultValue;
}
if (property.equalsIgnoreCase("true") || property.equalsIgnoreCase("yes")) {
return true;
}
if (property.equalsIgnoreCase("false") || property.equalsIgnoreCase("no")) {
return false;
}
return defaultValue;
}

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,6 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
}
}

/**
* {@inheritDoc}
*/
@Override
public Boolean getBooleanProp(String propertyName, Boolean defaultValue) {
String property = properties.getProperty(propertyName);
if (property == null) {
return defaultValue;
}
if (property.equalsIgnoreCase("true") || property.equalsIgnoreCase("yes")) {
return true;
}
if (property.equalsIgnoreCase("false") || property.equalsIgnoreCase("no")) {
return false;
}
return defaultValue;
}

/**
* {@inheritDoc}
*/
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/org/owasp/esapi/logging/java/JavaLogFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,16 @@ public class JavaLogFactory implements LogFactory {
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(LOG_APPLICATION_NAME);
String appName = ESAPI.securityConfiguration().getStringProp(APPLICATION_NAME);
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(LOG_SERVER_IP);
boolean logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX, true);

boolean logPrefix = true;
try {
logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX);
} catch (ConfigurationException ex) {
System.out.println("ESAPI: Failed to read Log Prefix configuration. Defaulting to enabled" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the output to something that explicitly mentions the property name on the output sent to stderr. E.g., something like this:

 System.out.println("ESAPI: Failed to read Log Prefix configuration, " + LOG_PREFIX + ". Defaulting to enabled" +

That will make it easier for those getting this failure to know exactly what property to look for if they wish to change it in their ESAPI.properties file.

". Caught " + ex.getClass().getName() +
"; exception message was: " + ex);
}

JAVA_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);

Map<Integer, JavaLogLevelHandler> levelLookup = new HashMap<>();
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/owasp/esapi/logging/slf4j/Slf4JLogFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.owasp.esapi.LogFactory;
import org.owasp.esapi.Logger;
import org.owasp.esapi.codecs.HTMLEntityCodec;
import org.owasp.esapi.errors.ConfigurationException;
import org.owasp.esapi.logging.appender.LogAppender;
import org.owasp.esapi.logging.appender.LogPrefixAppender;
import org.owasp.esapi.logging.cleaning.CodecLogScrubber;
Expand Down Expand Up @@ -70,7 +71,16 @@ public class Slf4JLogFactory implements LogFactory {
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(LOG_APPLICATION_NAME);
String appName = ESAPI.securityConfiguration().getStringProp(APPLICATION_NAME);
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(LOG_SERVER_IP);
boolean logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX, true);

boolean logPrefix = true;
try {
logPrefix = ESAPI.securityConfiguration().getBooleanProp(LOG_PREFIX);
} catch (ConfigurationException ex) {
System.out.println("ESAPI: Failed to read Log Prefix configuration. Defaulting to enabled" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment.

". Caught " + ex.getClass().getName() +
"; exception message was: " + ex);
}

SLF4J_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName, logPrefix);

Map<Integer, Slf4JLogLevelHandler> levelLookup = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1456,32 +1456,6 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
}
}

/**
* {@inheritDoc}
* Looks for property in three configuration files in following order:
* 1.) In file defined as org.owasp.esapi.opsteam system property
* 2.) In file defined as org.owasp.esapi.devteam system property
* 3.) In ESAPI.properties
*/
@Override
public Boolean getBooleanProp(String propertyName, Boolean defaultValue) {
try {
return esapiPropertyManager.getBooleanProp(propertyName);
} catch (ConfigurationException ex) {
String property = properties.getProperty(propertyName);
if ( property == null ) {
return defaultValue;
}
if ( property.equalsIgnoreCase("true") || property.equalsIgnoreCase("yes") ) {
return true;
}
if ( property.equalsIgnoreCase("false") || property.equalsIgnoreCase("no") ) {
return false;
}
return defaultValue;
}
}

/**
* {@inheritDoc}
* Looks for property in three configuration files in following order:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,11 +558,6 @@ public Boolean getBooleanProp(String propertyName) throws ConfigurationException
return wrapped.getBooleanProp(propertyName);
}

@Override
public Boolean getBooleanProp(String propertyName, Boolean defaultValue) {
return wrapped.getBooleanProp(propertyName, defaultValue);
}

@Override
public String getStringProp(String propertyName) throws ConfigurationException {
return wrapped.getStringProp(propertyName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,46 +288,6 @@ public void testBooleanPropFoundInLoader() {
assertEquals(expectedPropertyValue, propertyValue);
}


@Test
public void testBooleanPropFoundInLoaderWithDefaultValueTrue() {
// given
System.setProperty(EsapiConfiguration.DEVTEAM_ESAPI_CFG.getConfigName(), xmlFilename1);
String propertyKey = "boolean_property";
boolean expectedPropertyValue = true;

// when
try {
testPropertyManager = new EsapiPropertyManager();
} catch (IOException e) {
fail(e.getMessage());
}
boolean propertyValue = testPropertyManager.getBooleanProp(propertyKey, true);

// then
assertEquals(expectedPropertyValue, propertyValue);
}

@Test
public void testBooleanPropFoundInLoaderWithDefaultValueFalse() {
// given
System.setProperty(EsapiConfiguration.DEVTEAM_ESAPI_CFG.getConfigName(), xmlFilename1);
String propertyKey = "boolean_property";
boolean expectedPropertyValue = true;

// when
try {
testPropertyManager = new EsapiPropertyManager();
} catch (IOException e) {
fail(e.getMessage());
}
boolean propertyValue = testPropertyManager.getBooleanProp(propertyKey, false);

// then
assertEquals(expectedPropertyValue, propertyValue);
}


@Test(expected = ConfigurationException.class)
public void testBooleanPropertyNotFoundByLoaderAndThrowException() {
// given
Expand All @@ -344,44 +304,6 @@ public void testBooleanPropertyNotFoundByLoaderAndThrowException() {
// then expect exception
}

@Test
public void testBooleanPropertyNotFoundByLoaderWithDefaultValueTrue() {
// given
String propertyKey = "non.existing.property";
boolean expectedPropertyValue = true;

// when
try {
testPropertyManager = new EsapiPropertyManager();
} catch (IOException e) {
fail(e.getMessage());
}

boolean propertyValue = testPropertyManager.getBooleanProp(propertyKey, true);

// then
assertEquals(expectedPropertyValue, propertyValue);
}

@Test
public void testBooleanPropertyNotFoundByLoaderWithDefaultValueFalse() {
// given
String propertyKey = "non.existing.property";
boolean expectedPropertyValue = false;

// when
try {
testPropertyManager = new EsapiPropertyManager();
} catch (IOException e) {
fail(e.getMessage());
}

boolean propertyValue = testPropertyManager.getBooleanProp(propertyKey, false);

// then
assertEquals(expectedPropertyValue, propertyValue);
}

@Test
public void testByteArrayPropFoundInLoader() {
// given
Expand Down
Loading