A better fix for the blog security issue.

Now permissions are checked in the blog edit/view pages, rather than in the constructor, which was only later going to end in pain and suffering. This will mean that views with blogs in them will work again, as per bug #1168.

htdocs/artefact/blog/attachedfiles.json.php

@@ -36,6 +36,7 @@ $id = param_integer('blogpost', 0);
 
 if ($id) {
     $blogpost = new ArtefactTypeBlogPost($id);
+    $blogpost->check_permission();
     if (!$filelist = $blogpost->get_attached_files()) {
         $filelist = array();
     }

htdocs/artefact/blog/index.json.php

@@ -50,13 +50,13 @@ if ($action == 'list') {
 }
 else if ($action == 'delete') {
     $blog = artefact_instance_from_id($id);
-    if ($blog instanceof ArtefactTypeBlog && $blog->get('owner') == $USER->get('id')) {
+    if ($blog instanceof ArtefactTypeBlog) {
+        $blog->check_permission();
         $blog->delete();
+        json_reply(false, get_string('blogdeleted', 'artefact.blog'));
     }
-    echo json_encode(array(
-        'error'   => false,
-        'message' => get_string('blogdeleted', 'artefact.blog'),
-    ));
+
+    throw new ArtefactNotFoundException(get_string('blogdoesnotexist', 'artefact.blog'));
 }
 
 ?>

htdocs/artefact/blog/lang/en.utf8/artefact.blog.php

@@ -38,6 +38,7 @@ $string['attachments'] = 'Attachments';
 $string['blogdesc'] = 'Description';
 $string['blogdescdesc'] = 'e.g., ‘A record of Jill\'s experiences and reflections’.';
 $string['blogdoesnotexist'] = 'You are trying to access a blog that does not exist';
+$string['blogpostdoesnotexist'] = 'You are trying to access a blog post that does not exist';
 $string['blogfilesdirdescription'] = 'Files uploaded as blog post attachments';
 $string['blogfilesdirname'] = 'blogfiles';
 $string['blogpost'] = 'blog post';

htdocs/artefact/blog/lib.php

@@ -137,7 +137,6 @@ class ArtefactTypeBlog extends ArtefactType {
      * @param object
      */
     public function __construct($id = 0, $data = null) {
-        global $USER;
         parent::__construct($id, $data);
 
         if (!$data) {
@@ -159,9 +158,6 @@ class ArtefactTypeBlog extends ArtefactType {
         if (empty($this->id)) {
             $this->container = 1;
         }
-        else if ($this->owner != $USER->get('id')) {
-            throw new AccessDeniedException(get_string('youarenottheownerofthisblogpost', 'artefact.blog'));
-        }
     }
 
     public function is_container() {
@@ -220,6 +216,19 @@ class ArtefactTypeBlog extends ArtefactType {
         parent::delete();
     }
 
+    /**
+     * Checks that the person viewing this blog is the owner. If not, throws an 
+     * AccessDeniedException. Used in the blog section to ensure only the 
+     * owners of the blogs can view or change them there. Other people see 
+     * blogs when they are placed in views.
+     */
+    public function check_permission() {
+        global $USER;
+        if ($USER->get('id') != $this->owner) {
+            throw new AccessDeniedException(get_string('youarenottheownerofthisblog', 'artefact.blog'));
+        }
+    }
+
     /**
      * This function overrides the default functionality for listing children,
      * using a smarty template and tablerenderer stuff.
@@ -437,7 +446,6 @@ class ArtefactTypeBlogPost extends ArtefactType {
      * @param object
      */
     public function __construct($id = 0, $data = null) {
-        global $USER;
         parent::__construct($id, $data);
 
         if (!$data) {
@@ -455,10 +463,6 @@ class ArtefactTypeBlogPost extends ArtefactType {
                 }
             }
         }
-
-        if ($this->id && $this->owner != $USER->get('id')) {
-            throw new AccessDeniedException(get_string('youarenottheownerofthisblogpost', 'artefact.blog'));
-        }
     }
 
     /**
@@ -505,6 +509,19 @@ class ArtefactTypeBlogPost extends ArtefactType {
       
         parent::delete();
     }
+
+    /**
+     * Checks that the person viewing this blog is the owner. If not, throws an 
+     * AccessDeniedException. Used in the blog section to ensure only the 
+     * owners of the blogs can view or change them there. Other people see 
+     * blogs when they are placed in views.
+     */
+    public function check_permission() {
+        global $USER;
+        if ($USER->get('id') != $this->owner) {
+            throw new AccessDeniedException(get_string('youarenottheownerofthisblogpost', 'artefact.blog'));
+        }
+    }
   
     public function describe_size() {
         return $this->count_attachments() . ' ' . get_string('attachments', 'artefact.blog');

htdocs/artefact/blog/post.php

@@ -68,6 +68,7 @@ if (!$blogpost) {
 }
 else {
     $blogpostobj = new ArtefactTypeBlogPost($blogpost);
+    $blogpostobj->check_permission();
     $blog = $blogpostobj->get('parent');
     $title = $blogpostobj->get('title');
     $description = $blogpostobj->get('description');

htdocs/artefact/blog/settings/index.php

@@ -33,6 +33,7 @@ safe_require('artefact', 'blog');
 
 $id = param_integer('id');
 $blog = new ArtefactTypeBlog($id);
+$blog->check_permission();
 
 $form = pieform(array(
     'name' => 'editblog',

htdocs/artefact/blog/view/delete.json.php

@@ -35,10 +35,7 @@ json_headers();
 $id = param_integer('id');
 
 $blogpost = new ArtefactTypeBlogPost($id);
-if ($blogpost->get('owner') != $USER->get('id')) {
-    json_reply('local', get_string('youarenottheownerofthisblogpost', 'artefact.blog'));
-}
-
+$blogpost->check_permission();
 $blogpost->delete();
 
 json_reply(false, get_string('blogpostdeleted', 'artefact.blog'));

htdocs/artefact/blog/view/index.php

@@ -36,6 +36,7 @@ safe_require('artefact', 'blog');
 
 $id = param_integer('id');
 $blog = new ArtefactTypeBlog($id);
+$blog->check_permission();
 
 // This javascript is used to generate a list of blog posts.
 $js = require('index.js.php'); 

htdocs/artefact/blog/view/publish.json.php

@@ -34,10 +34,8 @@ safe_require('artefact', 'blog');
 $id = param_integer('id');
 
 $blogpost = new ArtefactTypeBlogPost($id);
+$blogpost->check_permission();
 
-if ($blogpost->get('owner') != $USER->get('id')) {
-    json_reply('local', get_string('youarenottheownerofthisblogpost', 'artefact.blog'));
-}
 if (!$blogpost->publish()) {
     json_reply('local', get_string('publishfailed', 'artefact.blog'));
 }