continuum-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From och...@apache.org
Subject svn commit: r1092648 - in /continuum/branches/continuum-1.3.x: ./ continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/ continuum-webapp/ continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ continuum-webapp/src/main/java...
Date Fri, 15 Apr 2011 10:01:19 GMT
Author: oching
Date: Fri Apr 15 10:01:18 2011
New Revision: 1092648

URL: http://svn.apache.org/viewvc?rev=1092648&view=rev
Log:
[CONTINUUM-2622]
o do an explicit check for a random generated value in the action on remove project group
(built-in token session interceptor doesn't work for projectGroupSummary page because
the <s:action> tag (which executes result) for getting the projects in the group in
the page causes a double submit
o enabled selenium test for remove project group csrf check

Added:
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ContinuumTokenSessionInterceptor.java
Modified:
    continuum/branches/continuum-1.3.x/continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/CSRFSecurityTest.java
    continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ProjectGroupAction.java
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/localization/Continuum.properties
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/struts.xml
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/confirmGroupRemoval.jsp
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/groupSummary.jsp
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/projectGroupSummary.jsp
    continuum/branches/continuum-1.3.x/pom.xml

Modified: continuum/branches/continuum-1.3.x/continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/CSRFSecurityTest.java
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/CSRFSecurityTest.java?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/CSRFSecurityTest.java
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp-test/src/test/testng/org/apache/continuum/web/test/CSRFSecurityTest.java
Fri Apr 15 10:01:18 2011
@@ -53,14 +53,13 @@ public class CSRFSecurityTest
         assertTextPresent( "Possible CSRF attack detected! Invalid token found in the request."
);
     }
 
-    /*
     public void testCSRFRemoveProjectGroup()
     {
         getSelenium().open( baseUrl );
         getSelenium().open( baseUrl + "/removeProjectGroup.action?projectGroupId=2" );
         assertTextPresent( "Security Alert - Invalid Token Found" );
         assertTextPresent( "Possible CSRF attack detected! Invalid token found in the request."
);
-    } */
+    } 
 
     public void testCSRFRemoveBuildResult()
     {

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml (original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml Fri Apr 15 10:01:18 2011
@@ -448,6 +448,10 @@ under the License.
       <artifactId>commons-io</artifactId>
     </dependency>
     <dependency>
+      <groupId>commons-codec</groupId>
+      <artifactId>commons-codec</artifactId>
+    </dependency>
+    <dependency>
       <groupId>commons-fileupload</groupId>
       <artifactId>commons-fileupload</artifactId>
       <version>1.2.1</version>

Added: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ContinuumTokenSessionInterceptor.java
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ContinuumTokenSessionInterceptor.java?rev=1092648&view=auto
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ContinuumTokenSessionInterceptor.java
(added)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ContinuumTokenSessionInterceptor.java
Fri Apr 15 10:01:18 2011
@@ -0,0 +1,76 @@
+package org.apache.continuum.web.interceptor;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import com.opensymphony.xwork2.ActionInvocation;
+import org.apache.struts2.ServletActionContext;
+import org.apache.struts2.interceptor.TokenInterceptor;
+import org.apache.struts2.interceptor.TokenSessionStoreInterceptor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * ContinuumTokenSessionInterceptor allows the action to pass through if <i>explicitCSRFCheck</i>
is
+ * enabled even if the TokenSessionStoreInterceptor returned invalid token error. If the
<i>explicitCSRFCheck</i>
+ * parameter is <i>true</i>, it means that the CSRF check will be done in the
action itself. Otherwise,
+ * whatever  result is returned by the TokenSessionStoreInterceptor will just be propagated.
+ *
+ * This is to handle cases in Continuum where an <s:action> tag is also present in
the page (example is projectGroupSummary.jsp)
+ * causing a double submit and the TokenSessionStoreInterceptor to fail even if the request
was actually valid.
+ *
+ * @author: Maria Odea Ching <oching@apache.org>
+ * @version:
+ * @plexus.component role="com.opensymphony.xwork2.interceptor.Interceptor" role-hint="continuumTokenSessionInterceptor"
+ */
+public class ContinuumTokenSessionInterceptor
+    extends TokenSessionStoreInterceptor
+{
+    private static final String EXPLICIT_CSRF_CHECK_PARAM = "explicitCSRFCheck";
+
+    private static final Logger logger = LoggerFactory.getLogger( ContinuumTokenSessionInterceptor.class
);
+
+    @Override
+    protected String doIntercept( ActionInvocation invocation )
+        throws Exception
+    {
+        String returnedString = super.doIntercept( invocation );
+
+        logger.debug( "TokenSessionStoreInterceptor returned '" + returnedString + "'." );
+
+        if ( TokenInterceptor.INVALID_TOKEN_CODE.equalsIgnoreCase( returnedString ) )
+        {
+            HttpServletRequest request =
+                ( HttpServletRequest ) invocation.getInvocationContext().get( ServletActionContext.HTTP_REQUEST
);
+
+            String explicitCSRFCheck = request.getParameter( EXPLICIT_CSRF_CHECK_PARAM );
+
+            if ( explicitCSRFCheck != null && Boolean.parseBoolean( explicitCSRFCheck
) )
+            {
+                logger.debug( "Explicit CSRF check flag set to true. Passing on the invocation.."
);
+                return invocation.invoke();
+            }
+        }
+
+        return returnedString;
+    }
+
+}

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ProjectGroupAction.java
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ProjectGroupAction.java?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ProjectGroupAction.java
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ProjectGroupAction.java
Fri Apr 15 10:01:18 2011
@@ -19,6 +19,7 @@ package org.apache.maven.continuum.web.a
  * under the License.
  */
 
+import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -26,9 +27,12 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Random;
 
+import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.collections.ComparatorUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.continuum.buildmanager.BuildManagerException;
@@ -46,6 +50,7 @@ import org.apache.maven.continuum.model.
 import org.apache.maven.continuum.project.ContinuumProjectState;
 import org.apache.maven.continuum.web.bean.ProjectGroupUserBean;
 import org.apache.maven.continuum.web.exception.AuthorizationRequiredException;
+import org.apache.struts2.interceptor.TokenInterceptor;
 import org.codehaus.plexus.redback.rbac.RBACManager;
 import org.codehaus.plexus.redback.rbac.RbacManagerException;
 import org.codehaus.plexus.redback.rbac.RbacObjectNotFoundException;
@@ -144,6 +149,21 @@ public class ProjectGroupAction
 
     private List<ProjectScmRoot> projectScmRoots;
 
+    private Random randomizer;
+
+    private String encodedRandomVal;
+
+    private static List<String> encodedRandomValCache =  new LinkedList();
+
+    private boolean explicitCSRFCheck = false;
+
+    private static final int CACHE_MAX_SIZE = 30;
+
+    public ProjectGroupAction()
+    {
+        randomizer = new SecureRandom();
+    }
+
     public String summary()
         throws ContinuumException
     {
@@ -244,6 +264,19 @@ public class ProjectGroupAction
             projectScmRoots = getContinuum().getProjectScmRootByProjectGroup( projectGroup.getId()
);
         }
 
+        // explicit csrf check for CONTINUUM-2622
+        encodedRandomVal = generateEncodedRandomVal();
+
+        synchronized( encodedRandomValCache )
+        {
+            // check size of cache first before adding anything in the cache
+            if( encodedRandomValCache.size() == CACHE_MAX_SIZE )
+            {
+                ( ( LinkedList ) encodedRandomValCache ).removeFirst();
+            }
+            encodedRandomValCache.add( decodeRandomVal( encodedRandomVal ) );
+        }
+
         return SUCCESS;
     }
 
@@ -315,6 +348,24 @@ public class ProjectGroupAction
         }
         else
         {
+            // explicit CSRF check for CONTINUUM-2622 - need to explicitly implement for
remove project group because <s:token/> doesn't work
+            //   in project group summary as there is a <s:action> whose result is
being executed in the page causing a double submission
+            if( explicitCSRFCheck  )
+            {
+                if( StringUtils.isEmpty( encodedRandomVal ) || !encodedRandomValCache.contains(
decodeRandomVal( encodedRandomVal ) ) )
+                {
+                    logger.error( "Token not found in cache!" );
+                    addActionError( getText( "projectGroup.remove.invalid.token", "Action
not allowed to continue - invalid token found!" ) );
+                    return TokenInterceptor.INVALID_TOKEN_CODE;
+                }
+                else
+                {
+                    logger.info( "Token found in cache.." );
+                    // remove it from the cache if found and let the action continue
+                    encodedRandomValCache.remove( decodeRandomVal( encodedRandomVal ) );
 
+                }
+            }
+
             name = getProjectGroupName();
             return CONFIRM;
         }
@@ -834,6 +885,42 @@ public class ProjectGroupAction
         } );
     }
 
+    protected String generateEncodedRandomVal()
+    {
+        String encodedRandomVale;
+
+        byte[] random =  new byte[16];
+        randomizer.nextBytes( random );
+        byte[] all = new byte[17];
+
+        for( int i = 0; i < random.length; i++ )
+        {
+            all[i] = random[i];
+        }
+
+        // include time to ensure uniqueness
+        byte time = ( byte ) System.currentTimeMillis();
+        all[16] = time;
+
+        // encode as string
+        encodedRandomVale = Base64.encodeBase64String( all );
+
+        return encodedRandomVale;
+    }
+
+    protected String decodeRandomVal( String encodedRandomVal )
+    {
+        byte[] randomValInBytes = Base64.decodeBase64( encodedRandomVal );
+
+        String decodedRandomVal = "";
+        if( randomValInBytes != null )
+        {
+            decodedRandomVal = new String( randomValInBytes );
+        }
+
+        return decodedRandomVal;
+    }
+
     public int getProjectGroupId()
     {
         return projectGroupId;
@@ -1101,4 +1188,24 @@ public class ProjectGroupAction
     {
         this.sorterProperty = sorterProperty;
     }
+
+    public String getEncodedRandomVal()
+    {
+        return encodedRandomVal;
+    }
+
+    public void setEncodedRandomVal( String encodedRandomVal )
+    {
+        this.encodedRandomVal = encodedRandomVal;
+    }
+
+    public boolean isExplicitCSRFCheck()
+    {
+        return explicitCSRFCheck;
+    }
+
+    public void setExplicitCSRFCheck( boolean explicitCSRFCheck )
+    {
+        this.explicitCSRFCheck = explicitCSRFCheck;
+    }
 }

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/localization/Continuum.properties
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/localization/Continuum.properties?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/localization/Continuum.properties
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/localization/Continuum.properties
Fri Apr 15 10:01:18 2011
@@ -198,6 +198,7 @@ projectGroup.scmRoot.title = Project Gro
 projectGroup.scmRoot.label = Scm Root URL
 projectGroup.cancelGroupBuild = Cancel Group Build
 projectGroup.invalid.id = Invalid Project Group Id: {0}
+projectGroup.remove.invalid.token = Possible CSRF attack! Invalid token found in remove project
group request. 
 
 # ----------------------------------------------------------------------
 # Page: Project Group - Members

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/struts.xml
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/struts.xml?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/struts.xml (original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/resources/struts.xml Fri
Apr 15 10:01:18 2011
@@ -31,6 +31,7 @@
 
     <interceptors>
       <interceptor name="continuumConfigurationCheck" class="forceContinuumConfigurationInterceptor"/>
+      <interceptor name="continuumTokenSessionCheck" class="continuumTokenSessionInterceptor"/>
       <interceptor name="redbackForceAdminUser" class="redbackForceAdminUserInterceptor"/>
       <interceptor name="redbackSecureActions" class="redbackSecureActionInterceptor"/>
       <interceptor name="redbackAutoLogin" class="redbackAutoLoginInterceptor"/>
@@ -47,7 +48,7 @@
         </interceptor-ref>
         <interceptor-ref name="redbackPolicyEnforcement"/>
         <interceptor-ref name="continuumConfigurationCheck"/>
-        <interceptor-ref name="tokenSession">
+        <interceptor-ref name="continuumTokenSessionCheck">
           <param name="excludeMethods">*</param>
         </interceptor-ref>
         <interceptor-ref name="validation">
@@ -67,6 +68,9 @@
         <interceptor-ref name="redbackSecureActions">
           <param name="enableReferrerCheck">false</param>
         </interceptor-ref>
+        <interceptor-ref name="continuumTokenSessionCheck">
+          <param name="excludeMethods">*</param>
+        </interceptor-ref>
         <interceptor-ref name="validation">
           <param name="excludeMethods">input,back,cancel,browse,edit</param>
         </interceptor-ref>
@@ -366,7 +370,9 @@
     </action>
 
     <action name="removeProjectGroup" class="projectGroup" method="remove">
-      <interceptor-ref name="storeStack"/>
+      <interceptor-ref name="storeStack">
+        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
+      </interceptor-ref>
       <result name="confirm">/WEB-INF/jsp/confirmGroupRemoval.jsp</result>
       <result name="success" type="redirect-action">
         <param name="actionName">groupSummary</param>
@@ -493,7 +499,7 @@
       <result name="success" type="chain">schedules</result>
       <result name="error" type="chain">schedule</result>
       <interceptor-ref name="configuredContinuumStack">
-        <param name="tokenSession.includeMethods">remove</param>
+        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
       </interceptor-ref>
     </action>
 
@@ -977,7 +983,7 @@
     
     <action name="removeRepository" class="localRepository" method="remove">
       <interceptor-ref name="storeStack">
-        <param name="tokenSession.includeMethods">remove</param>
+        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
       </interceptor-ref>
       <result name="confirm">/WEB-INF/jsp/admin/confirmDeleteLocalRepository.jsp</result>
       <result name="success" type="redirect-action">
@@ -1022,7 +1028,7 @@
         <param name="actionName">purgeConfigList</param>
       </result>
       <interceptor-ref name="configuredContinuumStack">
-        <param name="tokenSession.includeMethods">remove</param>
+        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
       </interceptor-ref>
     </action>
     

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/confirmGroupRemoval.jsp
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/confirmGroupRemoval.jsp?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/confirmGroupRemoval.jsp
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/confirmGroupRemoval.jsp
Fri Apr 15 10:01:18 2011
@@ -31,6 +31,7 @@
         <s:form action="removeProjectGroup" method="post">
           <s:hidden name="projectGroupId"/>
           <s:hidden name="confirmed" value="true"/>
+          <s:hidden name="explicitCSRFCheck" value="false"/>
           <s:token/>
           <s:actionerror/>
 

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/groupSummary.jsp
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/groupSummary.jsp?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/groupSummary.jsp
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/groupSummary.jsp
Fri Apr 15 10:01:18 2011
@@ -101,6 +101,7 @@
               <s:param name="projectGroupId">${group.id}</s:param>
               <s:param name="struts.token.name">struts.token</s:param>
               <s:param name="struts.token"><s:property value="struts.token"/></s:param>
+              <s:param name="explicitCSRFCheck">false</s:param>
             </s:url>
             <s:a href="%{removeProjectGroupUrl}">
               <img src="<s:url value='/images/delete.gif'/>" alt="<s:text name="projectGroup.deleteGroup"/>"
title="<s:text name="projectGroup.deleteGroup"/>" border="0">

Modified: continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/projectGroupSummary.jsp
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/projectGroupSummary.jsp?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/projectGroupSummary.jsp
(original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/src/main/webapp/WEB-INF/jsp/projectGroupSummary.jsp
Fri Apr 15 10:01:18 2011
@@ -171,6 +171,9 @@
             <td>
               <redback:ifAuthorized permission="continuum-remove-group" resource="${projectGroup.name}">
                 <form action="removeProjectGroup.action" method="post">
+                  <s:token/>
+                  <input type="hidden" name="encodedRandomVal" value="<s:property value="encodedRandomVal"/>"/>
+                  <input type="hidden" name="explicitCSRFCheck" value="true"/>
                   <input type="hidden" name="projectGroupId" value="<s:property value="projectGroupId"/>"/>
                   <input type="submit" name="remove" value="<s:text name="projectGroup.deleteGroup"/>"/>
                 </form>

Modified: continuum/branches/continuum-1.3.x/pom.xml
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/pom.xml?rev=1092648&r1=1092647&r2=1092648&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/pom.xml (original)
+++ continuum/branches/continuum-1.3.x/pom.xml Fri Apr 15 10:01:18 2011
@@ -1087,7 +1087,12 @@ under the License.
         <groupId>commons-io</groupId>
         <artifactId>commons-io</artifactId>
         <version>1.4</version>
-      </dependency>      
+      </dependency>
+      <dependency>
+        <groupId>commons-codec</groupId>
+        <artifactId>commons-codec</artifactId>
+        <version>1.4</version>  
+      </dependency>
       <dependency>
         <groupId>org.apache.struts</groupId>
         <artifactId>struts2-core</artifactId>



Mime
View raw message