Hide information about walls the user doesn't have access to

mozilla · Mar 26, 2013 · 346102a · 346102a
1 parent dcb6dd0
commit 346102a
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 11 deletions.
diff --git a/wall/lib/walls.inc b/wall/lib/walls.inc
@@ -372,13 +372,13 @@ class Wall {
     return $config['editor']['url'] . $path;
   }
 
-  protected function canAdminister() {
+  public function canAdminister() {
     // In the future we will check if the wall has been shared with the current 
     // user or not
     return $this->isOwner();
   }
 
-  protected function isOwner() {
+  public function isOwner() {
     if (!isset($this->email) || $this->ownerEmail === null)
       return false;
     return $this->email == $this->ownerEmail;

diff --git a/wall/public/api/doWalls.php b/wall/public/api/doWalls.php
@@ -51,6 +51,20 @@
     if ($wall === null)
       bailWithError('not-found');
 
+    // Walls::getById will filter out sensitive information if the supplied 
+    // email address does not have access to administer the wall.
+    //
+    // However, for now we disallow all access if the user doesn't have 
+    // administration rights since a user may want to keep their event private 
+    // from others for various reasons.
+    //
+    // In the future we will probably fine tune this so that walls which are 
+    // marked for display in the public gallery can be reached from this API 
+    // since we won't be exposing any information via this API that isn't 
+    // available by browsing the gallery.
+    if (!$wall->canAdminister())
+      bailWithError('no-auth');
+
     $result = $wall->asArray();
     break;
 

diff --git a/wall/tests/api/TestCreateWall.php b/wall/tests/api/TestCreateWall.php
@@ -112,8 +112,7 @@ function testEmail() {
 
     // Try a bad email
     $this->logout();
-    $this->userEmail = 'abc';
-    $this->login();
+    $this->login('abc');
     $wall = $this->createWall('Test wall', $this->testDesignId);
     $this->assertEqual(@$wall['error_key'], 'bad-email');
   }

diff --git a/wall/tests/api/TestGetWall.php b/wall/tests/api/TestGetWall.php
@@ -80,7 +80,22 @@ function testNotFound() {
     $this->assertEqual(@$wall['error_key'], 'not-found');
   }
 
-  // XXX Check we can't get the information of someone else's wall
+  function testSomeoneElsesWall() {
+    // Create wall
+    $this->login();
+    $wall = $this->createWall('Test wall', $this->testDesignId);
+    $wallId = $wall['wallId'];
+    $this->logout();
+
+    // Login as someone else
+    $this->login('abc@abc.org');
+    $wall = $this->getWall($wallId);
+    $this->assertEqual(@$wall['error_key'], 'no-auth');
+    $this->logout();
+
+    // Tidy up
+    $this->removeWall($wallId);
+  }
 
   function looksLikeAUrl($url) {
     $parts = parse_url($url);

diff --git a/wall/tests/api/WallMakerTestCase.php b/wall/tests/api/WallMakerTestCase.php
@@ -24,6 +24,8 @@
  */
 abstract class WallMakerTestCase extends WallTestCase {
 
+  const DEFAULT_USER_EMAIL = 'test@test.org';
+
   static private $updatedSessionSettings = false;
 
   protected $sessionId        = null;
@@ -41,27 +43,27 @@ function __construct($name = false) {
 
   function setUp() {
     $this->sessionId = null;
-
-    $this->userEmail = "test@test.org";
+    $this->userEmail = null;
     $this->createTestDesign(array('test.jpg'));
   }
 
   function tearDown() {
     if ($this->sessionId) {
       $this->logout();
     }
-
-    $this->userEmail = null;
     $this->removeTestDesign();
   }
 
-  function login() {
+  function login($email = null) {
     session_name(WALLMAKER_SESSION_NAME);
     session_cache_limiter(''); // Prevent warnings about not being able to send 
                                // cache limiting headers
     session_start();
 
-    $_SESSION['email'] = $this->userEmail;
+    $email = $email ? $email : self::DEFAULT_USER_EMAIL;
+
+    $_SESSION['email'] = $email;
+    $this->userEmail   = $email;
 
     // We're about to call into the wall server which will want to access the 
     // same session but session files are opened exclusively so we store the 
@@ -83,6 +85,7 @@ function logout() {
 
     // Clear local state
     $this->sessionId = null;
+    $this->userEmail = null;
 
     // When you create cookies without an expiry date they are treated as 
     // temporary cookies.