You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@continuum.apache.org by oc...@apache.org on 2011/05/03 13:30:12 UTC
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...
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=" " 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>