continuum-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From och...@apache.org
Subject svn commit: r1099015 - in /continuum/branches/continuum-1.3.x: ./ continuum-webapp/ continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/ continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ continuum-webapp/src/main/res...
Date Tue, 03 May 2011 11:30:12 GMT
Author: oching
Date: Tue May  3 11:30:11 2011
New Revision: 1099015

URL: http://svn.apache.org/viewvc?rev=1099015&view=rev
Log:
[CONTINUUM-2622]
o revert changes made in -r1092648 in csrf check for remove project group
o check only on actual delete, do not check on confirm delete -- separated remove project
group and confirm remove project group into separate actions

Removed:
    continuum/branches/continuum-1.3.x/continuum-webapp/src/main/java/org/apache/continuum/web/interceptor/
Modified:
    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/pom.xml
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml?rev=1099015&r1=1099014&r2=1099015&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml (original)
+++ continuum/branches/continuum-1.3.x/continuum-webapp/pom.xml Tue May  3 11:30:11 2011
@@ -448,10 +448,6 @@ 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>

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=1099015&r1=1099014&r2=1099015&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
Tue May  3 11:30:11 2011
@@ -19,7 +19,6 @@ 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;
@@ -27,12 +26,9 @@ 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;
@@ -50,7 +46,6 @@ 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;
@@ -110,8 +105,6 @@ public class ProjectGroupAction
 
     private Map<Integer, String> projectGroups = new HashMap<Integer, String>();
 
-    private boolean confirmed;
-
     private boolean projectInCOQueue = false;
 
     private Collection<Project> projectList;
@@ -149,21 +142,6 @@ 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
     {
@@ -264,19 +242,6 @@ 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;
     }
 
@@ -333,41 +298,15 @@ public class ProjectGroupAction
             return REQUIRES_AUTHORIZATION;
         }
 
-        if ( confirmed )
+        try
         {
-            try
-            {
-                getContinuum().removeProjectGroup( projectGroupId );
-            }
-            catch ( ContinuumException e )
-            {
-                logger.error( "Error while removing project group with id " + projectGroupId,
e );
-                addActionError( getText( "projectGroup.delete.error", "Unable to remove project
group",
-                                         Integer.toString( projectGroupId ) ) );
-            }
+            getContinuum().removeProjectGroup( projectGroupId );
         }
-        else
+        catch ( ContinuumException e )
         {
-            // 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;
+            logger.error( "Error while removing project group with id " + projectGroupId,
e );
+            addActionError( getText( "projectGroup.delete.error", "Unable to remove project
group",
+                                     Integer.toString( projectGroupId ) ) );
         }
 
         AuditLog event = new AuditLog( "Project Group id=" + projectGroupId, AuditLogConstants.REMOVE_PROJECT_GROUP
);
@@ -378,6 +317,23 @@ public class ProjectGroupAction
         return SUCCESS;
     }
 
+    public String confirmRemove()
+        throws ContinuumException
+    {
+        try
+        {
+            checkRemoveProjectGroupAuthorization( getProjectGroupName() );
+        }
+        catch ( AuthorizationRequiredException authzE )
+        {
+            addActionError( authzE.getMessage() );
+            return REQUIRES_AUTHORIZATION;
+        }
+
+        name = getProjectGroupName();
+        return CONFIRM;
+    }
+
     private void initialize()
         throws ContinuumException
     {
@@ -885,42 +841,6 @@ 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;
@@ -941,16 +861,6 @@ public class ProjectGroupAction
         this.projectGroup = projectGroup;
     }
 
-    public boolean isConfirmed()
-    {
-        return confirmed;
-    }
-
-    public void setConfirmed( boolean confirmed )
-    {
-        this.confirmed = confirmed;
-    }
-
     public String getDescription()
     {
         return description;
@@ -1188,24 +1098,4 @@ 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=1099015&r1=1099014&r2=1099015&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
Tue May  3 11:30:11 2011
@@ -198,7 +198,6 @@ 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=1099015&r1=1099014&r2=1099015&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 Tue
May  3 11:30:11 2011
@@ -31,7 +31,6 @@
 
     <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"/>
@@ -48,7 +47,7 @@
         </interceptor-ref>
         <interceptor-ref name="redbackPolicyEnforcement"/>
         <interceptor-ref name="continuumConfigurationCheck"/>
-        <interceptor-ref name="continuumTokenSessionCheck">
+        <interceptor-ref name="tokenSession">
           <param name="excludeMethods">*</param>
         </interceptor-ref>
         <interceptor-ref name="validation">
@@ -68,9 +67,6 @@
         <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>
@@ -369,11 +365,15 @@
       <result name="to_summary_page" type="chain">groupSummary</result>
     </action>
 
+    <action name="confirmRemoveProjectGroup" class="projectGroup" method="confirmRemove">
+      <interceptor-ref name="storeStack"/>
+      <result name="confirm">/WEB-INF/jsp/confirmGroupRemoval.jsp</result>
+    </action>
+
     <action name="removeProjectGroup" class="projectGroup" method="remove">
       <interceptor-ref name="storeStack">
-        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
+        <param name="tokenSession.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>
       </result>
@@ -499,7 +499,7 @@
       <result name="success" type="chain">schedules</result>
       <result name="error" type="chain">schedule</result>
       <interceptor-ref name="configuredContinuumStack">
-        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
+        <param name="tokenSession.includeMethods">remove</param>
       </interceptor-ref>
     </action>
 
@@ -983,7 +983,7 @@
     
     <action name="removeRepository" class="localRepository" method="remove">
       <interceptor-ref name="storeStack">
-        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
+        <param name="tokenSession.includeMethods">remove</param>
       </interceptor-ref>
       <result name="confirm">/WEB-INF/jsp/admin/confirmDeleteLocalRepository.jsp</result>
       <result name="success" type="redirect-action">
@@ -1028,7 +1028,7 @@
         <param name="actionName">purgeConfigList</param>
       </result>
       <interceptor-ref name="configuredContinuumStack">
-        <param name="continuumTokenSessionCheck.includeMethods">remove</param>
+        <param name="tokenSession.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=1099015&r1=1099014&r2=1099015&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
Tue May  3 11:30:11 2011
@@ -30,8 +30,6 @@
         <div class="axial">
         <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=1099015&r1=1099014&r2=1099015&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
Tue May  3 11:30:11 2011
@@ -97,11 +97,8 @@
         <ec:column property="removeProjectGroupAction" title="&nbsp;" width="1%">
           <redback:ifAuthorized permission="continuum-remove-group" resource="${group.name}">
             <s:token/>
-            <s:url id="removeProjectGroupUrl" action="removeProjectGroup" namespace="/"
includeParams="none">
+            <s:url id="removeProjectGroupUrl" action="confirmRemoveProjectGroup" namespace="/"
includeParams="none">
               <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=1099015&r1=1099014&r2=1099015&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
Tue May  3 11:30:11 2011
@@ -170,10 +170,7 @@
             </td>
             <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"/>
+                <form action="confirmRemoveProjectGroup.action" method="post">
                   <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=1099015&r1=1099014&r2=1099015&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/pom.xml (original)
+++ continuum/branches/continuum-1.3.x/pom.xml Tue May  3 11:30:11 2011
@@ -1087,12 +1087,7 @@ under the License.
         <groupId>commons-io</groupId>
         <artifactId>commons-io</artifactId>
         <version>1.4</version>
-      </dependency>
-      <dependency>
-        <groupId>commons-codec</groupId>
-        <artifactId>commons-codec</artifactId>
-        <version>1.4</version>  
-      </dependency>
+      </dependency>      
       <dependency>
         <groupId>org.apache.struts</groupId>
         <artifactId>struts2-core</artifactId>



Mime
View raw message