Skip to content

Commit

Permalink
Merge pull request #690 from JCgH4164838Gh792C124B5/localS2_62_Securi…
Browse files Browse the repository at this point in the history 
…tyMemberTestUpdate1

WW-5288 follow-up test case updates
  • Loading branch information
@lukaszlenart
lukaszlenart committed May 29, 2023
2 parents 6d0a4bb + 269a102 commit 3ef7747
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 6 deletions.
3 changes: 2 additions & 1 deletion core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
return false;
}

if (isClassExcluded(targetClass)) {
if (targetClass != memberClass && isClassExcluded(targetClass)) {
// Optimization: Already checked memberClass exclusion, so if-and-only-if targetClass == memberClass, this check is redundant.
LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
return false;
}
Expand Down
167 changes: 162 additions & 5 deletions core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Pattern;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class SecurityMemberAccessTest {

Expand Down Expand Up @@ -383,8 +385,9 @@ public void testAccessStaticField() throws Exception {
}

@Test
public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception {
public void testBlockedStaticFieldWhenFlagIsTrue() throws Exception {
// given
assignNewSma(true);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
Expand Down Expand Up @@ -479,6 +482,104 @@ public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception {
assertFalse("Access to private final static field is allowed?", actual);
}

@Test
public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception {
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
Member method = StaticTester.class.getField("MAX_VALUE");
boolean actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to public static field is allowed when flag false?", actual);

// public static final test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.class.getField("MIN_VALUE");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to public final static field is allowed when flag is false?", actual);

// package static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("PACKAGE_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to package static field is allowed?", actual);

// package final static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("FINAL_PACKAGE_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to package final static field is allowed?", actual);

// protected static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("PROTECTED_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to protected static field is allowed?", actual);

// protected final static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("FINAL_PROTECTED_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to protected final static field is allowed?", actual);

// private static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("PRIVATE_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to private static field is allowed?", actual);

// private final static test
// given
assignNewSma(false);
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
method = StaticTester.getFieldByName("FINAL_PRIVATE_STRING");
actual = sma.isAccessible(context, null, method, null);

// then
assertFalse("Access to private final static field is allowed?", actual);
}

@Test
public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception {
// given
Expand Down Expand Up @@ -506,7 +607,7 @@ public void testBlockStaticMethodAccess() throws Exception {
}

@Test
public void testBlockStaticAccessIfClassIsExcluded() throws Exception {
public void testBlockAccessIfClassIsExcluded() throws Exception {
// given
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

Expand All @@ -515,11 +616,25 @@ public void testBlockStaticAccessIfClassIsExcluded() throws Exception {
boolean actual = sma.isAccessible(context, Class.class, method, null);

// then
assertFalse("Access to static method of excluded class isn't blocked!", actual);
assertFalse("Access to method of excluded class isn't blocked!", actual);
}

@Test
public void testBlockAccessIfClassIsExcluded_2() throws Exception {
// given
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class)));

// when
Member method = ClassLoader.class.getMethod("loadClass", String.class);
ClassLoader classLoaderTarget = this.getClass().getClassLoader();
boolean actual = sma.isAccessible(context, classLoaderTarget, method, null);

// then
assertFalse("Invalid test! Access to method of excluded class isn't blocked!", actual);
}

@Test
public void testAllowStaticAccessIfClassIsNotExcluded() throws Exception {
public void testAllowAccessIfClassIsNotExcluded() throws Exception {
// given
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class)));

Expand All @@ -528,7 +643,26 @@ public void testAllowStaticAccessIfClassIsNotExcluded() throws Exception {
boolean actual = sma.isAccessible(context, Class.class, method, null);

// then
assertTrue("Invalid test! Access to static method of excluded class is blocked!", actual);
assertTrue("Invalid test! Access to method of non-excluded class is blocked!", actual);
}

@Test
public void testIllegalArgumentExceptionExpectedForTargetMemberMismatch() throws Exception {
// given
sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class)));

// when
Member method = ClassLoader.class.getMethod("loadClass", String.class);
String mismatchTarget = "misMatchTargetObject";
try {
boolean actual = sma.isAccessible(context, mismatchTarget, method, null);

// then
assertFalse("Invalid test! Access to method of excluded class isn't blocked!", actual);
fail("Mismatch between target and member did not cause IllegalArgumentException?");
} catch (IllegalArgumentException iex) {
// Expected result is this exception
}
}

@Test
Expand Down Expand Up @@ -686,10 +820,12 @@ public void setStringField(String stringField) {
this.stringField = stringField;
}

@Override
public String fooLogic() {
return "fooLogic";
}

@Override
public String barLogic() {
return "barLogic";
}
Expand All @@ -699,6 +835,27 @@ public int hashCode() {
return 1;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
final FooBar other = (FooBar) obj;
if (this.intField != other.intField) {
return false;
}
if (!Objects.equals(this.stringField, other.stringField)) {
return false;
}
return Objects.equals(this.doubleField, other.doubleField);
}

public int getIntField() {
return intField;
}
Expand Down

0 comments on commit 3ef7747

Please sign in to comment.