[Fusionforge-commits] r11241 - in trunk: src/common/include src/www/admin src/www/news/admin tests/func/RBAC tests/func/Testing

Roland Mas lolando at libremir.placard.fr.eu.org
Thu Oct 28 18:48:16 CEST 2010


Author: lolando
Date: 2010-10-28 18:48:16 +0200 (Thu, 28 Oct 2010)
New Revision: 11241

Modified:
   trunk/src/common/include/Group.class.php
   trunk/src/common/include/RBAC.php
   trunk/src/common/include/Role.class.php
   trunk/src/common/include/session.php
   trunk/src/www/admin/admin_utils.php
   trunk/src/www/admin/approve-pending.php
   trunk/src/www/news/admin/index.php
   trunk/tests/func/RBAC/rbacTests.php
   trunk/tests/func/Testing/SeleniumGforge.php
Log:
RBAC testsuite: checking approve_projects and approve_news permissions (with associated fixes)

Modified: trunk/src/common/include/Group.class.php
===================================================================
--- trunk/src/common/include/Group.class.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/common/include/Group.class.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -686,13 +686,9 @@
 	function setStatus(&$user, $status) {
 		global $SYS;
 
-		$perm =& $this->getPermission ();
-		if (!$perm || !is_object($perm)) {
+                if (!forge_check_global_perm ('approve_projects')) {
 			$this->setPermissionDeniedError();
 			return false;
-		} elseif (!$perm->isSuperUser()) {
-			$this->setPermissionDeniedError();
-			return false;
 		}
 
 		//	Projects in 'A' status can only go to 'H' or 'D'
@@ -2196,7 +2192,7 @@
 			$this->setError(_("Group already active"));
 			return false;
 		}
-
+		
 		db_begin();
 
 		// Step 1: Activate group and create LDAP entries
@@ -2208,7 +2204,60 @@
 		// Switch to system language for item creation
 		setup_gettext_from_sys_lang ();
 
+		// Create default roles
+		if (USE_PFO_RBAC) {
+			$idadmin_group = NULL ;
+			foreach (get_group_join_requests ($this) as $gjr) {
+				$idadmin_group = $gjr->getUserID() ;
+				break ;
+			}
+			if ($idadmin_group == NULL) {
+				$idadmin_group = $user->getID();
+			}
+		} else {
+			$admin_group = db_query_params ('SELECT user_id FROM user_group WHERE group_id=$1 AND admin_flags=$2',
+							array ($this->getID(),
+							       'A')) ;
+			if (db_numrows($admin_group) > 0) {
+				$idadmin_group = db_result($admin_group,0,'user_id');
+			} else {
+				$idadmin_group = $user->getID();
+				db_query_params ('INSERT INTO user_group (user_id, group_id, admin_flags) VALUES ($1, $2, $3)',
+						 array ($idadmin_group,
+							$this->getID(),
+							'A')) ;
+			}
+		}
 
+		$role = new Role($this);
+		$todo = array_keys($role->defaults);
+		for ($c=0; $c<count($todo); $c++) {
+			$role = new Role($this);
+			if (! ($role_id = $role->createDefault($todo[$c]))) {
+				$this->setError(sprintf(_('R%d: %s'),$c,$role->getErrorMessage()));
+				db_rollback();
+				setup_gettext_from_context();
+				return false;
+			}
+			$role = new Role($this, $role_id);
+			if ($role->getVal('projectadmin',0)=='A') {
+				$role->setUser($idadmin_group);
+			}
+		}
+		
+		if (USE_PFO_RBAC) {
+			$roles = $this->getRoles() ;
+			foreach ($roles as $r) {
+				if ($r->getSetting ('project_admin', $this->getID())) {
+					$r->addUser (user_get_object ($idadmin_group)) ;
+				}
+			}
+		}
+		
+		// Temporarily switch to the submitter's identity
+		$saved_session = session_get_user () ;
+		session_set_internal ($idadmin_group) ;
+
 		//
 		//
 		//	Tracker Integration
@@ -2316,59 +2365,8 @@
 			}
 		}
 
-		//
-		//
-		//	Set Default Roles
-		//
-		//
+		// Set permissions for roles
 		if (USE_PFO_RBAC) {
-			$idadmin_group = NULL ;
-			foreach (get_group_join_requests ($this) as $gjr) {
-				$idadmin_group = $gjr->getUserID() ;
-				break ;
-			}
-			if ($idadmin_group == NULL) {
-				$idadmin_group = $user->getID();
-			}
-		} else {
-
-		$admin_group = db_query_params ('SELECT user_id FROM user_group WHERE group_id=$1 AND admin_flags=$2',
-						array ($this->getID(),
-						       'A')) ;
-		if (db_numrows($admin_group) > 0) {
-			$idadmin_group = db_result($admin_group,0,'user_id');
-		} else {
-			$idadmin_group = $user->getID();
-			db_query_params ('INSERT INTO user_group (user_id, group_id, admin_flags) VALUES ($1, $2, $3)',
-					 array ($idadmin_group,
-						$this->getID(),
-						'A')) ;
-		}
-		}
-
-		$role = new Role($this);
-		$todo = array_keys($role->defaults);
-		for ($c=0; $c<count($todo); $c++) {
-			$role = new Role($this);
-			if (! ($role_id = $role->createDefault($todo[$c]))) {
-				$this->setError(sprintf(_('R%d: %s'),$c,$role->getErrorMessage()));
-				db_rollback();
-				setup_gettext_from_context();
-				return false;
-			}
-			$role = new Role($this, $role_id);
-			if ($role->getVal('projectadmin',0)=='A') {
-				$role->setUser($idadmin_group);
-			}
-		}
-
-		if (USE_PFO_RBAC) {
-			$roles = $this->getRoles() ;
-			foreach ($roles as $r) {
-				if ($r->getSetting ('project_admin', $this->getID())) {
-					$r->addUser (user_get_object ($idadmin_group)) ;
-				}
-			}
 			if ($this->isPublic()) {
 				$ra = RoleAnonymous::getInstance() ;
 				$rl = RoleLoggedIn::getInstance() ;
@@ -2445,6 +2443,7 @@
 		}
 
 		// Switch back to user preference
+		session_set_internal ($saved_session->getID()) ;
 		setup_gettext_from_context();
 
 		db_commit();

Modified: trunk/src/common/include/RBAC.php
===================================================================
--- trunk/src/common/include/RBAC.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/common/include/RBAC.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -897,19 +897,22 @@
 	 *
 	 *	@param	string	The name of the role.
 	 *	@param	array	A multi-dimensional array of data in this format: $data['section_name']['ref_id']=$val
+	 *      @param  boolean Perform permission checking
 	 *	@return	boolean	True on success or false on failure.
 	 */
-	function update($role_name,$data) {
+	function update($role_name,$data,$check_perms=true) {
 		global $SYS;
 		if (USE_PFO_RBAC) {
-			if ($this->getHomeProject() == NULL) {
-				if (!forge_check_global_perm ('forge_admin')) {
+			if ($check_perms) {
+				if ($this->getHomeProject() == NULL) {
+					if (!forge_check_global_perm ('forge_admin')) {
+						$this->setPermissionDeniedError();
+						return false;
+					}
+				} elseif (!forge_check_perm ('project_admin', $this->getHomeProject()->getID())) {
 					$this->setPermissionDeniedError();
 					return false;
 				}
-			} elseif (!forge_check_perm ('project_admin', $this->getHomeProject()->getID())) {
-				$this->setPermissionDeniedError();
-				return false;
 			}
 		} else {
 			$perm =& $this->Group->getPermission ();
@@ -1315,7 +1318,7 @@
 		
 		// Save
 		if (USE_PFO_RBAC) {
-			$this->update ($this->getName(), $new_pa) ;
+			$this->update ($this->getName(), $new_pa, false) ;
 		} else {
 			$this->update ($this->getName(), $new_sa) ;
 		}

Modified: trunk/src/common/include/Role.class.php
===================================================================
--- trunk/src/common/include/Role.class.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/common/include/Role.class.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -174,13 +174,19 @@
 	 *	@param	array	A multi-dimensional array of data in this format: $data['section_name']['ref_id']=$val
 	 *	@return integer	The id on success or false on failure.
 	 */
-	function create($role_name,$data) {
+	function create($role_name,$data,$newproject=false) {
 		if (USE_PFO_RBAC) {
 			if ($this->Group == NULL) {
 				if (!forge_check_global_perm ('forge_admin')) {
 					$this->setPermissionDeniedError();
 					return false;
 				}
+			}
+			if ($newproject) {
+				if (!forge_check_global_perm ('approve_projects')) {
+					$this->setPermissionDeniedError();
+					return false;
+				}
 			} elseif (!forge_check_perm ('project_admin', $this->Group->getID())) {
 				$this->setPermissionDeniedError();
 				return false;
@@ -302,7 +308,7 @@
 
 	function createDefault($name) {
 		if ($this->Group == NULL) {
-			return $this->create($name,array());
+			return $this->create($name,array(),true);
 		}
 		
 		if (array_key_exists ($name, $this->defaults)) {
@@ -349,7 +355,7 @@
 			}
 		}
 
-		return $this->create($name,$data);
+		return $this->create($name,$data,true);
 	}
 	
 	/**

Modified: trunk/src/common/include/session.php
===================================================================
--- trunk/src/common/include/session.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/common/include/session.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -391,8 +391,6 @@
 				exit_permission_denied($reason,'');
 			}
 		}
-	} else if ($req['isloggedin']) {
-		//no need to check as long as the check is present at top of function
 	} else {
 		exit_permission_denied($reason,'');
 	}
@@ -420,6 +418,10 @@
  */
 function session_require_global_perm ($section, $action = NULL, $reason='') {
 	if (!forge_check_global_perm ($section, $action)) {
+		if (!$reason) {
+			$reason = sprintf (_('Permission denied. The %s administrators will have to grant you permission to view this page.'),
+					   forge_get_config ('forge_name')) ;
+		}
 		exit_permission_denied ($reason,'');
 	}		
 }
@@ -447,10 +449,10 @@
  *	@return none
  */
 function session_set_new($user_id) {
-	global $G_SESSION,$session_ser;
+	global $session_ser;
 
 	// set session cookie
-  //
+	//
 	$cookie = session_build_session_cookie($user_id);
 	session_cookie("session_ser", $cookie, "", $GLOBALS['sys_session_expire']);
 	$session_ser=$cookie;
@@ -475,13 +477,17 @@
 	else if (db_numrows($res) < 1) {
 		exit_error(_('Could not fetch user session data'),'');
 	} else {
+		session_set_internal ($user_id, $res) ;
+	}
 
-		//set up the new user object
-		//
-		$G_SESSION = user_get_object($user_id,$res);
-		if ($G_SESSION) {
-			$G_SESSION->setLoggedIn(true);
-		}
+}
+
+function session_set_internal ($user_id, $res=false) {
+	global $G_SESSION ;
+	
+	$G_SESSION = user_get_object($user_id,$res);
+	if ($G_SESSION) {
+		$G_SESSION->setLoggedIn(true);
 	}
 
 	RBACEngine::getInstance()->invalidateRoleCaches() ;

Modified: trunk/src/www/admin/admin_utils.php
===================================================================
--- trunk/src/www/admin/admin_utils.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/www/admin/admin_utils.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -21,8 +21,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-function site_admin_header($params) {
-	session_require_global_perm ('forge_admin');
+function site_admin_header($params, $required_perm = 'forge_admin') {
+	session_require_global_perm ($required_perm);
 
 	if (version_compare(PHP_VERSION, '5.1.0', '<')) {
 		$GLOBALS['warning_msg'] = 'WARNING: Your php version must not be lower than 5.1.0, please upgrade';

Modified: trunk/src/www/admin/approve-pending.php
===================================================================
--- trunk/src/www/admin/approve-pending.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/www/admin/approve-pending.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -107,7 +107,7 @@
 	}
 }
 
-site_admin_header(array('title'=>_('Approving Pending Projects')));
+site_admin_header(array('title'=>_('Approving Pending Projects')), 'approve_projects');
 echo '<h1>' . _('Approving Pending Projects') . '</h1>';
 
 // get current information

Modified: trunk/src/www/news/admin/index.php
===================================================================
--- trunk/src/www/news/admin/index.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/src/www/news/admin/index.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -42,7 +42,9 @@
 
 $feedback = htmlspecialchars(getStringFromRequest('feedback'));
 
-if ($group_id && $group_id != forge_get_config('news_group') && user_ismember($group_id,'A')) {
+if ($group_id && $group_id != forge_get_config('news_group')) {
+	session_require_perm ('project_admin', $group_id) ;
+
 	$status = getIntFromRequest('status');
 	$summary = getStringFromRequest('summary');
 	$details = getStringFromRequest('details');
@@ -182,7 +184,7 @@
 	}
 	news_footer(array());
 
-} else if (forge_check_global_perm ('approve_news')) {
+} else {
 	/*
 
 		News uber-user admin pages
@@ -192,6 +194,8 @@
 		Admin members of forge_get_config('news_group') (news project) can edit/change/approve news items
 
 	*/
+	session_require_global_perm ('approve_news') ;
+
 	if ($post_changes) {
 		if ($approve) {
 			if ($status==1) {
@@ -340,11 +344,6 @@
 
 	}
 	news_footer(array());
-
-} else {
-
-	exit_error(sprintf(_('You have to be an admin on the project you are editing or a member of the %s News team.'), forge_get_config ('forge_name')),'news');
-
 }
 
 // Local Variables:

Modified: trunk/tests/func/RBAC/rbacTests.php
===================================================================
--- trunk/tests/func/RBAC/rbacTests.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/tests/func/RBAC/rbacTests.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -125,7 +125,62 @@
 		$this->waitForPageToLoad("30000");
 		$this->assertFalse($this->isTextPresent("projapp Lastname"));
 		$this->assertTrue($this->isTextPresent("newsmod Lastname"));
-		
+
+		// Register project as unprivileged user
+		$this->createUser ("toto") ;
+		$this->switchUser ("toto") ;
+		$this->registerProject ("TotoProject", "toto") ;
+
+		// Try approving it as two users without the right to do so
+		$this->open( ROOT . '/admin/approve-pending.php') ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isPermissionDenied()) ;
+		$this->switchUser ("newsmod") ;
+		$this->open( ROOT . '/admin/approve-pending.php') ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isPermissionDenied()) ;
+
+		// Approve it with a user that only has approve_projects
+		$this->approveProject ("TotoProject", "projapp") ;
+
+		// Submit a news in the project
+		$this->switchUser ("toto") ;
+		$this->gotoProject ("TotoProject") ;
+		$this->click("link=News") ;
+		$this->waitForPageToLoad("30000");
+		$this->click("link=Submit") ;
+		$this->waitForPageToLoad("30000");
+		$this->type("summary", "First TotoNews");
+		$this->type("details", "This is a simple news for Toto's project.");
+		$this->click("submit");
+		$this->waitForPageToLoad("30000");
+
+		// Try to push it to front page with user toto
+		$this->open( ROOT . '/news/admin/') ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isPermissionDenied()) ;
+
+		// Try to push it to front page with user projapp
+		$this->switchUser ("projapp") ;
+		$this->open( ROOT . '/news/admin/') ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isPermissionDenied()) ;
+
+		// Push it to front page with user newsmod
+		$this->switchUser ("newsmod") ;
+		$this->open( ROOT . '/news/admin/') ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isTextPresent("These items need to be approved")) ;
+		$this->assertTrue ($this->isTextPresent("First TotoNews")) ;
+		$this->click ("//a[contains(.,'First TotoNews')]") ;
+		$this->waitForPageToLoad("30000");
+		$this->click ("//input[@type='radio' and @value='1']") ;
+		$this->click ("submit") ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isTextPresent("These items were approved this past week")) ;
+		$this->open( ROOT ) ;
+		$this->waitForPageToLoad("30000");
+		$this->assertTrue ($this->isTextPresent("First TotoNews")) ;
 	}
 }
 ?>

Modified: trunk/tests/func/Testing/SeleniumGforge.php
===================================================================
--- trunk/tests/func/Testing/SeleniumGforge.php	2010-10-28 16:27:01 UTC (rev 11240)
+++ trunk/tests/func/Testing/SeleniumGforge.php	2010-10-28 16:48:16 UTC (rev 11241)
@@ -139,7 +139,9 @@
 	protected function registerProject ($name, $user) {
 		$unix_name = strtolower($name);
 
+		$saved_user = $this->logged_in ;
 		$this->switchUser ($user) ;
+
 		$this->click("link=My Page");
 		$this->waitForPageToLoad("30000");
 		$this->click("link=Register Project");
@@ -153,10 +155,14 @@
 		$this->waitForPageToLoad("30000");
 		$this->assertTrue($this->isTextPresent("Your project has been submitted"));
 		$this->assertTrue($this->isTextPresent("you will receive notification of their decision and further instructions"));
+
+		$this->switchUser ($saved_user) ;
 	}	
 
 	protected function approveProject ($name, $user) {
 		$unix_name = strtolower($name);
+
+		$saved_user = $this->logged_in ;
 		$this->switchUser ($user) ;
 
 		if ($user == 'admin') {
@@ -169,7 +175,7 @@
 			$this->waitForPageToLoad("30000");
 		}
 		$this->click("document.forms['approve.$unix_name'].submit");
-		$this->waitForPageToLoad("30000");
+		$this->waitForPageToLoad("60000");
 		$this->click("link=Home");
 		$this->waitForPageToLoad("30000");
 		$this->assertTrue($this->isTextPresent($name));
@@ -177,10 +183,14 @@
 		$this->waitForPageToLoad("30000");
 		$this->assertTrue($this->isTextPresent("This is the public description for $name."));
 		$this->assertTrue($this->isTextPresent("This project has not yet categorized itself"));
+
+		$this->switchUser ($saved_user) ;
 	}
 
 	protected function createProject ($name) {
 		$unix_name = strtolower($name);
+
+		$this->switchUser ('admin') ;
 		
 		// Create a simple project.
 		if ((!defined('PROJECTA')) || ($unix_name != "projecta")) {
@@ -229,9 +239,11 @@
 	}
 
 	protected function gotoProject($project) {
-		$this->open( ROOT . '/projects/' . $project) ;
+		$unix_name = strtolower($project);
+		
+		$this->open( ROOT . '/projects/' . $unix_name) ;
 		$this->waitForPageToLoad("30000");
-		$this->assertTrue($this->isTextPresent("This is the public description for $name."));
+		$this->assertTrue($this->isTextPresent("This is the public description for $project."));
 	}
 }
 




More information about the Fusionforge-commits mailing list