You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by "kusalk (via GitHub)" <gi...@apache.org> on 2023/11/05 09:23:36 UTC
[PR] WW-5350 Refactor SecurityMemberAccess [struts]
kusalk opened a new pull request, #780:
URL: https://github.com/apache/struts/pull/780
WW-5350
--
See comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #780:
URL: https://github.com/apache/struts/pull/780#discussion_r1382547971
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,168 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target != null) {
+ // Special case: Target is a Class object but not Class.class
+ if (Class.class.equals(target.getClass()) && !Class.class.equals(target)) {
Review Comment:
Validate and throw exceptions if arguments are not what we expect as the following logic depends on these assumptions to be accurate
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk merged PR #780:
URL: https://github.com/apache/struts/pull/780
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1793685163
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=780)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list) [75.4% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #780:
URL: https://github.com/apache/struts/pull/780#discussion_r1382538846
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
Review Comment:
Throw an exception here because if this ceases to be the case, it means the following logic may need changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1793698505
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=780)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list) [75.2% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #780:
URL: https://github.com/apache/struts/pull/780#discussion_r1382540169
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
Review Comment:
Set to null as there is no more useful information to extract here, and it simplifies the checks/logic to follow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #780:
URL: https://github.com/apache/struts/pull/780#discussion_r1382538745
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
Review Comment:
Wanted to be a bit more deliberate about how we handle the arguments in case OGNL behaviour changes
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
Review Comment:
Throw an exception here because if this ceases to be the case, it means the following logic may need changes
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
Review Comment:
Extract this into its own protected method for consistency
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!", member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", targetClass);
- return false;
- }
- if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
Review Comment:
Extracted all the exclusion list checks into this method
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
Review Comment:
Tried to standardise around just passing the object and/or member to these protected methods
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
Review Comment:
This is essentially an exemption to `checkStaticMethodAccess` so I moved it within that method - we shouldn't be relying on method call order here
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
Review Comment:
Removed redundant argument
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!", member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", targetClass);
- return false;
- }
- if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getDeclaringClass();
+ if (isClassExcluded(memberClass)) {
+ LOG.warn("Declaring class of member type [{}] is excluded!", memberClass);
+ return false;
+ }
+ if (isPackageExcluded(memberClass)) {
LOG.warn("Package [{}] of target class [{}] of target [{}] is excluded!",
- targetClass.getPackage(),
- targetClass,
+ memberClass.getPackage(),
+ memberClass,
target);
return false;
}
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (isClassExcluded(targetClass)) {
+ LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ return false;
+ }
+ if (isPackageExcluded(targetClass)) {
+ LOG.warn("Package [{}] of member [{}] are excluded!", targetClass.getPackage(), member);
+ return false;
+ }
+ return true;
+ }
- if (targetClass != memberClass && isPackageExcluded(memberClass)) {
- LOG.warn("Package [{}] of member [{}] are excluded!", memberClass.getPackage(), member);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkDefaultPackageAccess(Object target, Member member) {
+ if (!disallowDefaultPackageAccess) {
+ return true;
+ }
+ Class<?> memberClass = member.getDeclaringClass();
+ if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", memberClass);
+ return false;
+ }
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", targetClass);
return false;
}
+ return true;
+ }
- return isAcceptableProperty(propertyName);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkProxyMemberAccess(Object target, Member member) {
+ return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target));
}
/**
* Check access for static method (via modifiers).
- *
+ * <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- *
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticMethodAccess(Member member, int memberModifiers) {
- return !Modifier.isStatic(memberModifiers) || member instanceof Field;
+ protected boolean checkStaticMethodAccess(Member member) {
+ if (checkEnumAccess(member)) {
+ LOG.trace("Exempting Enum#values from static method check: class [{}]", member.getDeclaringClass());
+ return true;
+ }
+ return member instanceof Field || !isStatic(member);
+ }
+
+ private static boolean isStatic(Member member) {
+ return Modifier.isStatic(member.getModifiers());
}
/**
* Check access for static field (via modifiers).
* <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticFieldAccess(Member member, int memberModifiers) {
- if (Modifier.isStatic(memberModifiers) && member instanceof Field) {
- return allowStaticFieldAccess;
- } else {
+ protected boolean checkStaticFieldAccess(Member member) {
+ if (allowStaticFieldAccess) {
Review Comment:
Bypass computation if static field access is permitted
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
Review Comment:
Set to null as there is no more useful information to extract here, and it simplifies the checks/logic to follow
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!", member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
Review Comment:
Extracted this into its own method
##########
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java:
##########
@@ -104,126 +105,164 @@ public void restore(Map context, Object target, Member member, String propertyNa
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
- throw new IllegalArgumentException("Target does not match member!");
+ if (target instanceof Class) { // Target may be of type Class for static members
+ if (!member.getDeclaringClass().equals(target)) {
+ throw new IllegalArgumentException("Target class does not match static member!");
+ }
+ target = null;
+ } else {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
+ throw new IllegalArgumentException("Target does not match member!");
+ }
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member)) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!", member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", targetClass);
- return false;
- }
- if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getDeclaringClass();
+ if (isClassExcluded(memberClass)) {
+ LOG.warn("Declaring class of member type [{}] is excluded!", memberClass);
+ return false;
+ }
+ if (isPackageExcluded(memberClass)) {
LOG.warn("Package [{}] of target class [{}] of target [{}] is excluded!",
- targetClass.getPackage(),
- targetClass,
+ memberClass.getPackage(),
+ memberClass,
target);
return false;
}
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (isClassExcluded(targetClass)) {
+ LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ return false;
+ }
+ if (isPackageExcluded(targetClass)) {
+ LOG.warn("Package [{}] of member [{}] are excluded!", targetClass.getPackage(), member);
+ return false;
+ }
+ return true;
+ }
- if (targetClass != memberClass && isPackageExcluded(memberClass)) {
- LOG.warn("Package [{}] of member [{}] are excluded!", memberClass.getPackage(), member);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkDefaultPackageAccess(Object target, Member member) {
+ if (!disallowDefaultPackageAccess) {
+ return true;
+ }
+ Class<?> memberClass = member.getDeclaringClass();
+ if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", memberClass);
+ return false;
+ }
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", targetClass);
return false;
}
+ return true;
+ }
- return isAcceptableProperty(propertyName);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkProxyMemberAccess(Object target, Member member) {
+ return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target));
}
/**
* Check access for static method (via modifiers).
- *
+ * <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- *
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticMethodAccess(Member member, int memberModifiers) {
- return !Modifier.isStatic(memberModifiers) || member instanceof Field;
+ protected boolean checkStaticMethodAccess(Member member) {
+ if (checkEnumAccess(member)) {
+ LOG.trace("Exempting Enum#values from static method check: class [{}]", member.getDeclaringClass());
+ return true;
+ }
+ return member instanceof Field || !isStatic(member);
+ }
+
+ private static boolean isStatic(Member member) {
+ return Modifier.isStatic(member.getModifiers());
}
/**
* Check access for static field (via modifiers).
* <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticFieldAccess(Member member, int memberModifiers) {
- if (Modifier.isStatic(memberModifiers) && member instanceof Field) {
- return allowStaticFieldAccess;
- } else {
+ protected boolean checkStaticFieldAccess(Member member) {
+ if (allowStaticFieldAccess) {
return true;
}
+ return !(member instanceof Field) || !isStatic(member);
}
/**
* Check access for public members (via modifiers)
- * <p>
- * Returns true if-and-only-if the member is public.
*
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkPublicMemberAccess(int memberModifiers) {
- return Modifier.isPublic(memberModifiers);
+ protected boolean checkPublicMemberAccess(Member member) {
+ return Modifier.isPublic(member.getModifiers());
}
- protected boolean checkEnumAccess(Object target, Member member) {
- if (target instanceof Class) {
- final Class<?> clazz = (Class<?>) target;
- return Enum.class.isAssignableFrom(clazz) && member.getName().equals("values");
- }
- return false;
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkEnumAccess(Member member) {
+ return member.getDeclaringClass().isEnum()
+ && isStatic(member)
+ && member instanceof Method
+ && member.getName().equals("values")
+ && ((Method) member).getParameterCount() == 0;
Review Comment:
Closed a minor hole where another static `values` method could also be exempted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1807061451
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=780)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list) [76.2% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1807061357
@lukaszlenart I'm going to make it extensible as part of WW-5343 so that applications can add even stronger checks if they so desire - so will leave it not `final`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1793701515
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=780)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list) [76.0% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5350 Refactor SecurityMemberAccess [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #780:
URL: https://github.com/apache/struts/pull/780#issuecomment-1793706015
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=780)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=780&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=780&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list) [76.2% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=780&metric=new_duplicated_lines_density&view=list)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org