Improve permission checking in objects, fix some initialization bugs

admin/adminMaintainers.php

@@ -24,7 +24,6 @@ if ($_REQUEST['sub'])
     if($_REQUEST['sub'] == 'delete')
     {
         $sQuery = "DELETE FROM appMaintainers WHERE maintainerId = ".$_REQUEST['maintainerId'].";";
-        echo "$sQuery";
         $hResult = query_appdb($sQuery);
         echo html_frame_start("Delete maintainer: ".$_REQUEST['maintainerId'],400,"",0);
         if($hResult)

admin/deleteAny.php

@@ -29,39 +29,25 @@ if($_REQUEST['what'])
     case "category":
         // delete category and the apps in it
         $oCategory = new Category($_REQUEST['catId']);
-            if( !$_SESSION['current']->hasPriv("admin") )
+        if(!$oCategory->delete())
-            {
             errorpage();
-            } else
+        else
-            {
-                $oCategory->delete();
             redirect(BASE."appbrowse.php");
-            }
         break;
     case "appFamily":
         // delete app family & all its versions
         $oApp = new Application($_REQUEST['appId']);
-            if( !$_SESSION['current']->hasPriv("admin") )
+        if(!$oApp->delete())
-            {
             errorpage();
-            } else
+        else
-            {
-                $oApp->delete();
             redirect(BASE."appbrowse.php");
-            }
         break;
     case "appVersion":
-	    // delete a version
         $oVersion = new Version($_REQUEST['versionId']);
-            if( !$_SESSION['current']->isSuperMaintainer($oVersion->iAppId) 
+        if(!$oVersion->delete())
-             && !$_SESSION['current']->hasPriv("admin") )
-            {
             errorpage();
-            } else
+        else
-            {
-                $oVersion->delete();
             redirect(BASE."appview.php?appId=".$_REQUEST['appId']);
-            }
         break;
     }
 }

include/application.php

@@ -33,43 +33,7 @@ class Application {
         // we are working on an existing application
         if(is_numeric($iAppId))
         {
-            /*
+            /* fetch this applications information */
-             * We fetch application data and versionsIds. 
-             */
-            $sQuery = "SELECT appFamily.*, appVersion.versionId AS versionId
-                       FROM appFamily, appVersion 
-                       WHERE appFamily.appId = appVersion.appId 
-                       AND appVersion.queued='false'
-                       AND appFamily.appId = ".$iAppId." ORDER BY versionName";
-            if($hResult = query_appdb($sQuery))
-            {
-                $this->aVersionsIds = array();
-                while($oRow = mysql_fetch_object($hResult))
-                {
-                    if(!$this->iAppId)
-                    {
-                        $this->iAppId = $iAppId;
-                        $this->iVendorId = $oRow->vendorId;
-                        $this->iCatId = $oRow->catId;
-                        $this->iSubmitterId = $oRow->submitterId;
-                        $this->sSubmitTime = $oRow->submitTime;
-                        $this->sDate = $oRow->submitTime;
-                        $this->sName = $oRow->appName;
-                        $this->sKeywords = $oRow->keywords;
-                        $this->sDescription = $oRow->description;
-                        $this->sWebpage = $oRow->webPage;
-                        $this->sQueued = $oRow->queued;
-                    }
-                    $this->aVersionsIds[] = $oRow->versionId;
-                }
-            }
-
-            /*
-             * Then we fetch the data related to this application if the first query didn't return anything.
-             * This can happen if an application has no version linked to it.
-             */
-            if(!$this->appId)
-            {
             $sQuery = "SELECT *
                        FROM appFamily 
                        WHERE appId = ".$iAppId;
@@ -88,7 +52,19 @@ class Application {
                 $this->sWebpage = $oRow->webPage;
                 $this->sQueued = $oRow->queued;
             }
+
+            /* fetch versions of this application, if there are any */
+            $this->aVersionsIds = array();
+            $sQuery = "SELECT versionId FROM appVersion WHERE
+                       appId =".$this->iAppId;
+            if($hResult = query_appdb($sQuery))
+            {
+                while($oRow = mysql_fetch_object($hResult))
+                {
+                    $this->aVersionsIds[] = $oRow->versionId;
                 }
+            }
+
 
             /*
              * We fetch urlsIds. 
@@ -115,8 +91,10 @@ class Application {
      */
     function create()
     {
-        // Security, if we are not an administrator the application must be queued.
+        if(!$_SESSION['current']->canCreateApplication())
-        if(!($_SESSION['current']->hasPriv("admin")))
+            return;
+
+        if($_SESSION['current']->appCreatedMustBeQueued())
             $this->sQueued = 'true';
         else
             $this->sQueued = 'false';
@@ -138,10 +116,11 @@ class Application {
             $this->application($this->iAppId);
             $this->SendNotificationMail();  // Only administrators will be mailed as no supermaintainers exist for this app.
             return true;
-        }
+        } else
-        else
+        {
             return false;
         }
+    }
 
 
     /**
@@ -152,6 +131,10 @@ class Application {
     {
         $sWhatChanged = "";
 
+        /* if the user doesn't have permission to modify this application, don't let them */
+        if(!$_SESSION['current']->canModifyApplication($this))
+            return;
+
         /* create an instance of ourselves so we can see what has changed */
         $oApp = new Application($this->iAppId);
 
@@ -211,16 +194,16 @@ class Application {
         return true;
     }
 
-
     /**    
      * Deletes the application from the database. 
      * and request the deletion of linked elements.
      */
     function delete($bSilent=false)
     {
-        /* don't let non-admins delete applications */
+        /* make sure the current user has the appropriate permission to delete
-        if(!($_SESSION['current']->hasPriv("admin")))
+           this application */
-            return;
+        if(!$_SESSION['current']->canDeleteApplication($this))
+            return false;
 
         foreach($this->aVersionsIds as $iVersionId)
         {
@@ -250,6 +233,8 @@ class Application {
 
         if(!$bSilent)
             $this->SendNotificationMail("delete");
+
+        return true;
     }
 
 
@@ -258,6 +243,9 @@ class Application {
      */
     function unQueue()
     {
+        if(!$_SESSION['current']->canUnQueueApplication())
+            return;
+
         // If we are not in the queue, we can't move the application out of the queue.
         if(!$this->sQueued == 'true')
             return false;
@@ -278,6 +266,9 @@ class Application {
 
     function Reject()
     {
+        if(!$_SESSION['current']->canRejectApplication($this))
+            return;
+
         // If we are not in the queue, we can't move the application out of the queue.
         if(!$this->sQueued == 'true')
             return false;
@@ -296,6 +287,9 @@ class Application {
     }
     function ReQueue()
     {
+        if(!$_SESSION->canRequeueApplication())
+            return false;
+
         // If we are not in the rejected, we can't move the application into the queue.
         if(!$this->sQueued == 'rejected')
             return false;

include/category.php

@@ -132,6 +132,9 @@ class Category {
      */
     function delete($bSilent=false)
     {
+        if(!$_SESSION['current']->canDeleteCategory($this))
+            return false;
+
         if(sizeof($this->aApplicationsIds)>0)
         {
             addmsg("The category has not been deleted because there are still applications linked to it.", "red");
@@ -143,6 +146,8 @@ class Category {
             query_appdb($sQuery);
             addmsg("The category has been deleted.", "green");
         }
+
+        return true;
     }
 
 
@@ -232,8 +237,7 @@ function make_cat_path($path, $appId = '', $versionId = '')
             $oVersion = new Version($versionId);
             $str .= " > ".html_ahref($oApp->sName,"appview.php?appId=$appId");
             $str .= " > ".$oVersion->sName;
-        }
+        } else
-        else
         {
             $str .= " > ".$oApp->sName;
         }

include/user.php

@@ -492,48 +492,14 @@ class User {
       */
      function deleteAppData($iAppDataId)
      {
-         $isMaintainer = false;
+         if(!$_SESSION['current']->canDeleteAppDataId($iAppDataId))
-
-         /* if we aren't an admin we should see if we can find any results */
-         /* for a query based on this appDataId, if we can then */
-         /* we have permission to delete the entry */
-         if(!$this->hasPriv("admin"))
-         {
-             $hResult = $this->getAppDataQuery($iAppDataId, false, false);
-             if(!$hResult)
              return false;
 
-             echo "result rows:".mysql_num_row($hResult);
-
-             if(mysql_num_rows($hResult) > 0)
-                 $isMaintainer = true;
-         }
-
-         /* do we have permission to delete this item? */
-         if($this->hasPriv("admin") || $isMaintainer)
-         {
          $sQuery = "DELETE from appData where id = ".$iAppDataId."
                         LIMIT 1;";
          $hResult = query_appdb($sQuery);
          if($hResult)
              return true;
-         }
-
-         return false;
-     }
-
-     /**
-      * Returns true or false depending on whether the user can view the image
-      */
-     function canViewImage($iImageId)
-     {
-         $oScreenshot = new Screenshot($iImageId);
-
-         if(!$oScreenshot->bQueued ||
-            ($oScreenshot->bQueued && ($this->hasPriv("admin") ||
-                                       $this->isMaintainer($oScreenshot->iVersionId) ||
-                                       $this->isSuperMaintainer($oScreenshot->iAppId))))
-             return true;
 
          return false;
      }
@@ -631,27 +597,6 @@ class User {
          return $retval;
      }
 
-     /**
-      * Does the user have permission to modify on this version?
-      */
-     function hasAppVersionModifyPermission($iVersionId)
-     {
-         if($this->hasPriv("admin"))
-             return true;
-
-         $sQuery = "SELECT appVersion.versionId FROM appVersion, appFamily, appMaintainers
-                      WHERE appFamily.appId = appVersion.appId 
-                      AND appFamily.appId = appMaintainers.appId
-                      AND appMaintainers.superMaintainer = '1'
-                      AND appMaintainers.userId = '".$this->iUserId."'
-                      AND appVersion.versionId = '".$iVersionId."';";
-         $hResult = query_appdb($sQuery);
-         if(mysql_num_rows($hResult))
-             return true;
-         else
-             return false;
-     }
-
      function isAppSubmitter($iAppId)
      {
          $sQuery = "SELECT appId FROM appFamily
@@ -729,6 +674,288 @@ class User {
 
          return true;
      }
+
+
+
+
+
+
+     /************************/
+     /* Permission functions */
+     /************************/
+
+     function canDeleteCategory($oCategory)
+     {
+        if($this->hasPriv("admin"))
+            return true;
+
+        return false;
+     }
+
+     /**
+      * Returns true or false depending on whether the user can view the image
+      */
+     function canViewImage($iImageId)
+     {
+         $oScreenshot = new Screenshot($iImageId);
+
+         if(!$oScreenshot->bQueued ||
+            ($oScreenshot->bQueued && ($this->hasPriv("admin") ||
+                                       $this->isMaintainer($oScreenshot->iVersionId) ||
+                                       $this->isSuperMaintainer($oScreenshot->iAppId))))
+             return true;
+
+         return false;
+     }
+
+     function canDeleteAppDataId($iAppDataId)
+     {
+         /* admins can delete anything */
+         if($this->hasPriv("admin"))
+             return true;
+
+         $isMaintainer = false;
+
+         /* if we aren't an admin we should see if we can find any results */
+         /* for a query based on this appDataId, if we can then */
+         /* we have permission to delete the entry */
+         $hResult = $this->getAppDataQuery($iAppDataId, false, false);
+         if(!$hResult)
+             return false;
+
+         if(mysql_num_rows($hResult) > 0)
+             $isMaintainer = true;
+
+         /* if this user maintains the app data, they can delete it */
+         if($isMaintainer)
+             return true;
+
+         return false;
+     }
+
+     /***************************/
+     /* application permissions */
+     function canViewApplication($oApp)
+     {
+         /* if the application isn't queued */
+         if($oApp->sQueued == 'false')
+             return true;
+
+         if($this->hasPriv("admin"))
+             return true;
+
+         /* if this user is the submitter and the application is queued */
+         if(($this->iUserId == $oApp->iSubmitterId) &&
+            ($oApp->sQueued != 'false'))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Does the user have permission to modify this application?
+      */
+     function canModifyApplication($oApp)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         /* is this user a super maintainer of this app? */
+         if($this->isSuperMaintainer($oApp->iAppId))
+             return true;
+         
+         /* if the user is the submitter of the application */
+         /* and the application is still queued */
+         /* the user can modify the app */
+         if(($this->iUserId == $oApp->iSubmitterId) &&
+            ($oApp->sQueued != 'false'))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Can this user create applications?
+      */
+     function canCreateApplication()
+     {
+         return isLoggedIn();
+     }
+
+     /**
+      * Returns 'true' if the current user has the permission to delete
+      * this application, 'false' otherwise
+      */
+     function canDeleteApplication($oApp)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         /* is this the user that submitted the application and is still queued */
+         if(($oApp->sQueued != 'false') && ($oApp->iSubmitterId == $this->iUserId))
+             return true;
+
+         return false;
+     }
+
+     /* Can this user unQueue applications? */
+     function canUnQueueApplication()
+     {
+         return $this->hasPriv("admin");
+     }
+
+     /* Can this user Requeue an application? */
+     function canRequeueApplication()
+     {
+         return $this->hasPriv("admin");
+     }
+
+     /* Can the user reject application? */
+     function canRejectApplication()
+     {
+         return $this->hasPriv("admin");
+     }
+
+     /**
+      * Does the created application have to be queued for admin processing?
+      */
+     function appCreatedMustBeQueued()
+     {
+         return !$this->hasPriv("admin");
+     }
+
+
+     /***********************/
+     /* version permissions */
+
+     function canViewVersion($oVersion)
+     {
+         /* if the version isn't queued */
+         if($oVersion->sQueued == 'false')
+             return true;
+
+         if($this->hasPriv("admin"))
+             return true;
+
+         /* if the user is the submitter and the version is still queued */
+         if(($this->iUserId == $oVersion->iSubmitterId) &&
+            ($oVersion->sQueued != 'false'))
+             return true;
+
+         /* if this user supermaintains the application this version belongs to */
+         if($this->isSupermaintainer($oVersion->iAppId))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Does the user have permission to modify on this version?
+      */
+     function hasAppVersionModifyPermission($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         $sQuery = "SELECT appVersion.versionId FROM appVersion, appFamily, appMaintainers
+                      WHERE appFamily.appId = appVersion.appId 
+                      AND appFamily.appId = appMaintainers.appId
+                      AND appMaintainers.superMaintainer = '1'
+                      AND appMaintainers.userId = '".$this->iUserId."'
+                      AND appVersion.versionId = '".$oVersion->iVersionId."';";
+         $hResult = query_appdb($sQuery);
+         if(mysql_num_rows($hResult))
+             return true;
+         else
+             return false;
+     }
+
+     /**
+      * Can this user create a version?
+      */
+     function canCreateVersion()
+     {
+         return $this->isLoggedIn();
+     }
+
+     function versionCreatedMustBeQueued($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return false;
+
+         if($this->isSupermaintainer($oVersion->iAppId))
+             return false;
+
+         return true;
+     }
+
+
+     /**
+      * Returns 'true' if the current user has the permission to delete
+      * this version, 'false' otherwise
+      */
+     function canDeleteVersion($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         /* if the app is anything other than not queued and if the user is the submitter */
+         /* then allow the user to delete the app */
+         if(($oVersion->sQueued != 'false') && ($oVersion->iSubmitterId == $this->iUserId))
+             return true;
+         
+         /* is this user a supermaintainer of the application this version is under? */
+         if($this->isSuperMaintainer($oVersion->iAppId))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Can the user unqueue this version?
+      */
+     function canUnQueueVersion($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+         
+         if($this->hasAppVersionModifyPermission($oVersion->iVersionId))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Can the user reject this version?
+      */
+     function canRejectVersion($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         if($this->hasAppVersionModifyPermission($oVersion->iVersionId))
+             return true;
+
+         return false;
+     }
+
+     /**
+      * Can the user reject this version?
+      */
+     function canRequeueVersion($oVersion)
+     {
+         if($this->hasPriv("admin"))
+             return true;
+
+         if($this->hasAppVersionModifyPermission($oVersion->iVersionId))
+             return true;
+
+         if(($this->iUserId == $oVersion->iSubmitterId) &&
+            ($oVersion->sQueued != 'false'))
+             return true;
+
+         return false;
+     }
 }
 
 

include/version.php

@@ -139,8 +139,10 @@ class Version {
      */
     function create()
     {
-        // Security, if we are not an administrator or an appmaintainer the version must be queued.
+        if(!$_SESSION['current']->canCreateVersion())
-        if(!($_SESSION['current']->hasPriv("admin") || $_SESSION['current']->isSupermaintainer($iAppId)))
+            return;
+
+        if($_SESSION['current']->versionCreatedMustBeQueued($this))
             $this->sQueued = 'true';
         else
             $this->sQueued = 'false';
@@ -178,6 +180,9 @@ class Version {
     {
         $sWhatChanged = "";
 
+        if(!$_SESSION['current']->hasAppVersionModifyPermission($this))
+            return;
+
         $oVersion = new Version($this->iVersionId);
 
         if ($this->sName && ($this->sName!=$oVersion->sName))
@@ -247,12 +252,8 @@ class Version {
     function delete($bSilent=false)
     {
         /* is the current user allowed to delete this version? */
-        if(!$_SESSION['current']->hasPriv("admin") && 
+        if(!$_SESSION['current']->canDeleteVersion($this))
-           !$_SESSION['current']->hasAppVersionModifyPermission($this->iVersionId) &&
+            return false;
-           !(($_SESSION['current']->iUserId == $this->iSubmitterId) && ($this->sQueued == 'rejected')))
-        {
-            return;
-        }
 
         /* remove all of the items this version contains */
         foreach($this->aNotesIds as $iNoteId)
@@ -275,7 +276,7 @@ class Version {
             $oUrl = new Url($iUrlId);
             $oUrl->delete($bSilent);
         }
-        foreach($this->$aBuglinkIds as $iBug_id)
+        foreach($this->aBuglinkIds as $iBug_id)
         {
             $oBug = new bug($iBug_id);
             $oBug->delete($bSilent);
@@ -301,6 +302,8 @@ class Version {
             $this->SendNotificationMail("delete");
 
         $this->mailSubmitter("delete");
+
+        return true;
     }
 
 
@@ -309,11 +312,8 @@ class Version {
      */
     function unQueue()
     {
-        /* is the current user allowed to delete this version? */
+        if(!$_SESSION['current']->canUnQueueVersion($this))
-        if(!$_SESSION['current']->hasPriv("admin") && !$_SESSION['current']->hasAppVersionModifyPermission($this->iVersionId))
-        {
             return;
-        }
 
         // If we are not in the queue, we can't move the version out of the queue.
         if(!$this->sQueued == 'true')
@@ -334,11 +334,8 @@ class Version {
 
     function Reject($bSilent=false)
     {
-        /* is the current user allowed to delete this version? */
+        if(!$_SESSION['current']->canRejectVersion($this))
-        if(!$_SESSION['current']->hasPriv("admin") && !$_SESSION['current']->hasAppVersionModifyPermission($this->iVersionId))
-        {
             return;
-        }
 
         // If we are not in the queue, we can't move the version out of the queue.
         if(!$this->sQueued == 'true')
@@ -361,13 +358,8 @@ class Version {
 
     function ReQueue()
     {
-        /* is the current user allowed to delete this version? */
+        if(!$_SESSION['current']->canRequeueVersion($this))
-        if(!$_SESSION['current']->hasPriv("admin") &&
-           !$_SESSION['current']->hasAppVersionModifyPermission($this->iVersionId) &&
-           !$_SESSION['current']->iUserId == $this->iSubmitterId)
-        {
             return;
-        }
 
         $sUpdate = compile_update_string(array('queued'    => "true"));
         if(query_appdb("UPDATE appVersion SET ".$sUpdate." WHERE versionId = ".$this->iVersionId))