-
Notifications
You must be signed in to change notification settings - Fork 810
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
WW-5439 Move DevMode security configuration to SecurityMemberAccess #979
Conversation
03ece5c
to
f6cb249
Compare
} | ||
|
||
private void useDevModeConfiguration() { | ||
if (!isDevMode || isDevModeInit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isDevModeInit
check isn't thread-safe but it doesn't need to be as there's no negative consequence of running this method more than once when DevMode is enabled.
Foo foo = new Foo(); | ||
|
||
Exception expected = null; | ||
try { | ||
ognlUtil.setExcludedClasses(Runtime.class.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exclusion list isn't checked here as it's already blocked by the static method check
@@ -1166,9 +1171,11 @@ public void testAvoidCallingMethodsOnObjectClass() { | |||
public void testAllowCallingMethodsOnObjectClassInDevModeTrue() { | |||
Exception expected = null; | |||
try { | |||
ognlUtil.setExcludedClasses(Foo.class.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods don't do anything, so we inject the configuration instead
f6cb249
to
af9aa1b
Compare
af9aa1b
to
81b4943
Compare
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
WW-5439
I noticed that after my previous refactor of
SecurityMemberAccess
- the devMode security configuration stopped working as I happened to break the test for it too.To be fair, this capability is probably not too useful anymore from 7.0, given the allowlist is enabled by default and that has no DevMode allowances.
However, we should still fix this feature for now and we can consider a better solution if necessary later.