Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-11 01:27:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

WW-4118 Adds logic to allow validate defined roles and changes precedence that disallowedRoles are examined first

Browse Source 
git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@1533359 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Lukasz Lenart
2013-10-18 08:33:22 +00:00
parent 01584fabc7
commit b6e013a600
2 changed files with 158 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/interceptor/RolesInterceptor.java
+71 -31
View File
@@ -23,19 +23,20 @@ package org.apache.struts2.interceptor;
import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
import com.opensymphony.xwork2.util.logging.Logger;
import com.opensymphony.xwork2.util.logging.LoggerFactory;
import org.apache.struts2.ServletActionContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
/**
* <!-- START SNIPPET: description --> This interceptor ensures that the action
* will only be executed if the user has the correct role. <!--
* END SNIPPET: description -->
* <!-- START SNIPPET: description -->
* This interceptor ensures that the action will only be executed if the user has the correct role.
* <!-- END SNIPPET: description -->
*
* <p/> <u>Interceptor parameters:</u>
*
@@ -49,15 +50,26 @@ import java.util.List;
*
* </ul>
*
* <p>
* When both allowedRoles and disallowedRoles are configured, then disallowedRoles
* takes precedence, applying the following logic:
* (if ((inRole(role1) || inRole(role2) || ... inRole(roleN)) &&
* !inRole(roleA) && !inRole(roleB) && ... !inRole(roleZ))
* { //permit ...
* </p>
* <!-- END SNIPPET: parameters -->
*
* <!-- START SNIPPET: extending --> There are two extensions to the
* existing interceptor:
* <!-- START SNIPPET: extending -->
* There are three extensions to the existing interceptor:
* <ul>
* <li>isAllowed(HttpServletRequest,Object) - whether or not to allow
* the passed action execution with this request</li>
* <li>handleRejection(ActionInvocation) - handles an unauthorized
* request.</li>
* <li>areRolesValid(List<String> roles) - allows subclasses to lookup roles
* to ensure they are valid. If not valid, RolesInterceptor will log the error and
* cease to function. This helps prevent security misconfiguration flaws.
*
* </ul>
* <!-- END SNIPPET: extending -->
*
@@ -76,27 +88,42 @@ import java.util.List;
*/
public class RolesInterceptor extends AbstractInterceptor {
protected List<String> allowedRoles = new ArrayList<String>();
protected List<String> disallowedRoles = new ArrayList<String>();
private static final Logger LOG = LoggerFactory.getLogger(RolesInterceptor.class);
private boolean isProperlyConfigured = true;
protected List<String> allowedRoles = Collections.emptyList();
protected List<String> disallowedRoles = Collections.emptyList();
public void setAllowedRoles(String roles) {
this.allowedRoles = stringToList(roles);
allowedRoles = stringToList(roles);
checkRoles(allowedRoles);
}
public void setDisallowedRoles(String roles) {
this.disallowedRoles = stringToList(roles);
disallowedRoles = stringToList(roles);
checkRoles(disallowedRoles);
}
private void checkRoles(List<String> roles){
if (!areRolesValid(roles)){
LOG.fatal("An unknown Role was configured: #0", roles.toString());
isProperlyConfigured = false;
throw new IllegalArgumentException("An unknown role was configured: " + roles);
}
}
public String intercept(ActionInvocation invocation) throws Exception {
HttpServletRequest request = ServletActionContext.getRequest();
HttpServletResponse response = ServletActionContext.getResponse();
String result;
if (!isAllowed(request, invocation.getAction())) {
result = handleRejection(invocation, response);
} else {
result = invocation.invoke();
if (!isProperlyConfigured) {
throw new IllegalArgumentException("RolesInterceptor is misconfigured, check logs for erroneous configuration!");
}
if (!isAllowed(request, invocation.getAction())) {
return handleRejection(invocation, response);
} else {
return invocation.invoke();
}
return result;
}
/**
@@ -119,23 +146,23 @@ public class RolesInterceptor extends AbstractInterceptor {
* @return True if allowed, false otherwise
*/
protected boolean isAllowed(HttpServletRequest request, Object action) {
if (allowedRoles.size() > 0) {
boolean result = false;
for (String role : allowedRoles) {
if (request.isUserInRole(role)) {
result = true;
break;
}
}
return result;
} else if (disallowedRoles.size() > 0) {
for (String role : disallowedRoles) {
if (request.isUserInRole(role)) {
return false;
}
for (String role : disallowedRoles) {
if (request.isUserInRole(role)) {
return false;
}
}
return true;
if (allowedRoles.isEmpty()){
return true;
}
for (String role : allowedRoles) {
if (request.isUserInRole(role)) {
return true;
}
}
return false;
}
/**
@@ -151,4 +178,17 @@ public class RolesInterceptor extends AbstractInterceptor {
response.sendError(HttpServletResponse.SC_FORBIDDEN);
return null;
}
/**
* Extension point for sub-classes to test if configured roles are known valid roles.
* Implementations are encouraged to implement this method to prevent misconfigured roles.
* If this method returns false, the RolesInterceptor will be disabled and block all requests.
*
* @param roles allowed and disallowed roles
* @return whether the roles are valid or not (always true for the default implementation)
*/
protected boolean areRolesValid(List<String> roles){
return true;
}
}
core/src/test/java/org/apache/struts2/interceptor/RolesInterceptorTest.java
+87
View File
@@ -21,8 +21,12 @@
package org.apache.struts2.interceptor;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import javax.security.auth.login.FailedLoginException;
import org.apache.struts2.StrutsTestCase;
import com.mockobjects.servlet.MockHttpServletRequest;
@@ -78,10 +82,93 @@ public class RolesInterceptorTest extends StrutsTestCase {
}
public void testIsAllowed_userAllowedAndGuestDisallowed() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest() {
public boolean isUserInRole(String role) {
return "user".equals(role) || "guest".equals(role);
}
};
interceptor.setAllowedRoles("user"); //has to be a user
interceptor.setDisallowedRoles("guest"); //and not a guest
assertFalse(interceptor.isAllowed(request, null));
}
public void testIsAllowed_adminAllowedExceptManager() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest() {
public boolean isUserInRole(String role) {
return "admin".equals(role);
}
};
interceptor.setAllowedRoles("admin");//allow all
interceptor.setDisallowedRoles("manager");
assertTrue(interceptor.isAllowed(request, null));
}
public void testIsAllowed_sameRoleAllowedAndDisallowed() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest() {
public boolean isUserInRole(String role) {
return "admin".equals(role);
}
};
interceptor.setAllowedRoles("admin");
interceptor.setDisallowedRoles("admin");
assertFalse(interceptor.isAllowed(request, null));
}
public void testIsAllowed_emptyAllowedAndDisallowed() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest() {
public boolean isUserInRole(String role) {
return "admin".equals(role);
}
};
interceptor.setAllowedRoles("");//allow all
interceptor.setDisallowedRoles("admin");
assertFalse(interceptor.isAllowed(request, null));
}
public void testHandleRejection() throws Exception {
MockHttpServletResponse response = new MockHttpServletResponse();
response.setExpectedError(response.SC_FORBIDDEN);
interceptor.handleRejection(null, response);
response.verify();
}
public void testAreRolesValid() throws Exception {
RolesInterceptor roleCheckInterceptor = new RolesInterceptor(){
List<String> validRoles = Arrays.asList(new String[]{"admin","user"});
@Override
public boolean areRolesValid(List<String> roles){
return validRoles.containsAll(roles);
}
};
try {
roleCheckInterceptor.setAllowedRoles("admin, user");
roleCheckInterceptor.setDisallowedRoles("admin, user");
} catch (Exception e){
fail("Valid roles should not throw an exception");
}
try {
roleCheckInterceptor.setAllowedRoles("hacker, abuser");
fail("Invalid roles should throw an exception");
} catch (Exception e){
//expected
}
try {
roleCheckInterceptor.setAllowedRoles("nonadmin, nonuser");
fail("Invalid roles should throw an exception");
} catch (Exception e){
//expected
}
try {
roleCheckInterceptor.intercept(null);
fail("A misconfigured should always throw an exception");
} catch (Exception e){
//expected;
}
}
}