UX Improvements

Redirect to login form if lacking permissions
Better redirectBack if we can guess from the comment
This commit is contained in:
Damian Mooyman 2015-03-30 18:57:29 +13:00
parent 974b4554fb
commit b5a53451b3
2 changed files with 49 additions and 8 deletions

View File

@ -20,6 +20,7 @@ class CommentingController extends Controller {
private $baseClass = ""; private $baseClass = "";
private $ownerRecord = ""; private $ownerRecord = "";
private $ownerController = ""; private $ownerController = "";
protected $fallbackReturnURL = null;
public function setBaseClass($class) { public function setBaseClass($class) {
$this->baseClass = $class; $this->baseClass = $class;
@ -132,7 +133,9 @@ class CommentingController extends Controller {
public function delete() { public function delete() {
$comment = $this->getComment(); $comment = $this->getComment();
if(!$comment) return $this->httpError(404); if(!$comment) return $this->httpError(404);
if(!$comment->canDelete()) return $this->httpError(403); if(!$comment->canDelete()) {
return Security::permissionFailure($this, 'You do not have permission to delete this comment');
}
if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400); if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400);
$comment->delete(); $comment->delete();
@ -148,7 +151,9 @@ class CommentingController extends Controller {
public function spam() { public function spam() {
$comment = $this->getComment(); $comment = $this->getComment();
if(!$comment) return $this->httpError(404); if(!$comment) return $this->httpError(404);
if(!$comment->canEdit()) return $this->httpError(403); if(!$comment->canEdit()) {
return Security::permissionFailure($this, 'You do not have permission to edit this comment');
}
if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400); if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400);
$comment->IsSpam = true; $comment->IsSpam = true;
@ -166,7 +171,9 @@ class CommentingController extends Controller {
public function ham() { public function ham() {
$comment = $this->getComment(); $comment = $this->getComment();
if(!$comment) return $this->httpError(404); if(!$comment) return $this->httpError(404);
if(!$comment->canEdit()) return $this->httpError(403); if(!$comment->canEdit()) {
return Security::permissionFailure($this, 'You do not have permission to edit this comment');
}
if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400); if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400);
$comment->IsSpam = false; $comment->IsSpam = false;
@ -184,7 +191,9 @@ class CommentingController extends Controller {
public function approve() { public function approve() {
$comment = $this->getComment(); $comment = $this->getComment();
if(!$comment) return $this->httpError(404); if(!$comment) return $this->httpError(404);
if(!$comment->canEdit()) return $this->httpError(403); if(!$comment->canEdit()) {
return Security::permissionFailure($this, 'You do not have permission to approve this comment');
}
if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400); if(!$comment->getSecurityToken()->checkRequest($this->request)) return $this->httpError(400);
$comment->IsSpam = false; $comment->IsSpam = false;
@ -209,6 +218,7 @@ class CommentingController extends Controller {
$comment = DataObject::get_by_id('Comment', $id); $comment = DataObject::get_by_id('Comment', $id);
if($comment) { if($comment) {
$this->fallbackReturnURL = $comment->Link();
return $comment; return $comment;
} }
} }
@ -420,4 +430,35 @@ class CommentingController extends Controller {
return $this->doPostComment($data, $form); return $this->doPostComment($data, $form);
} }
public function redirectBack() {
// Don't cache the redirect back ever
HTTP::set_cache_age(0);
$url = null;
// In edge-cases, this will be called outside of a handleRequest() context; in that case,
// redirect to the homepage - don't break into the global state at this stage because we'll
// be calling from a test context or something else where the global state is inappropraite
if($this->request) {
if($this->request->requestVar('BackURL')) {
$url = $this->request->requestVar('BackURL');
} else if($this->request->isAjax() && $this->request->getHeader('X-Backurl')) {
$url = $this->request->getHeader('X-Backurl');
} else if($this->request->getHeader('Referer')) {
$url = $this->request->getHeader('Referer');
}
}
if(!$url) $url = $this->fallbackReturnURL;
if(!$url) $url = Director::baseURL();
// absolute redirection URLs not located on this site may cause phishing
if(Director::is_site_url($url)) {
return $this->redirect($url);
} else {
return false;
}
}
} }

View File

@ -95,7 +95,7 @@ class CommentsTest extends FunctionalTest {
$comment = $this->objFromFixture('Comment', 'firstComA'); $comment = $this->objFromFixture('Comment', 'firstComA');
$commentID = $comment->ID; $commentID = $comment->ID;
$this->assertFalse($comment->DeleteLink(), 'No permission to see delete link'); $this->assertFalse($comment->DeleteLink(), 'No permission to see delete link');
$delete = $this->get('CommentingController/delete/'.$comment->ID); $delete = $this->get('CommentingController/delete/'.$comment->ID.'?ajax=1');
$this->assertEquals(403, $delete->getStatusCode()); $this->assertEquals(403, $delete->getStatusCode());
$check = DataObject::get_by_id('Comment', $commentID); $check = DataObject::get_by_id('Comment', $commentID);
$this->assertTrue($check && $check->exists()); $this->assertTrue($check && $check->exists());
@ -134,7 +134,7 @@ class CommentsTest extends FunctionalTest {
$comment = $this->objFromFixture('Comment', 'firstComA'); $comment = $this->objFromFixture('Comment', 'firstComA');
$commentID = $comment->ID; $commentID = $comment->ID;
$this->assertFalse($comment->SpamLink(), 'No permission to see mark as spam link'); $this->assertFalse($comment->SpamLink(), 'No permission to see mark as spam link');
$spam = $this->get('CommentingController/spam/'.$comment->ID); $spam = $this->get('CommentingController/spam/'.$comment->ID.'?ajax=1');
$this->assertEquals(403, $spam->getStatusCode()); $this->assertEquals(403, $spam->getStatusCode());
$check = DataObject::get_by_id('Comment', $commentID); $check = DataObject::get_by_id('Comment', $commentID);
$this->assertEquals(0, $check->IsSpam, 'No permission to mark as spam'); $this->assertEquals(0, $check->IsSpam, 'No permission to mark as spam');
@ -176,7 +176,7 @@ class CommentsTest extends FunctionalTest {
$comment = $this->objFromFixture('Comment', 'secondComC'); $comment = $this->objFromFixture('Comment', 'secondComC');
$commentID = $comment->ID; $commentID = $comment->ID;
$this->assertFalse($comment->HamLink(), 'No permission to see mark as ham link'); $this->assertFalse($comment->HamLink(), 'No permission to see mark as ham link');
$ham = $this->get('CommentingController/ham/'.$comment->ID); $ham = $this->get('CommentingController/ham/'.$comment->ID.'?ajax=1');
$this->assertEquals(403, $ham->getStatusCode()); $this->assertEquals(403, $ham->getStatusCode());
$check = DataObject::get_by_id('Comment', $commentID); $check = DataObject::get_by_id('Comment', $commentID);
$this->assertEquals(1, $check->IsSpam, 'No permission to mark as ham'); $this->assertEquals(1, $check->IsSpam, 'No permission to mark as ham');
@ -218,7 +218,7 @@ class CommentsTest extends FunctionalTest {
$comment = $this->objFromFixture('Comment', 'secondComB'); $comment = $this->objFromFixture('Comment', 'secondComB');
$commentID = $comment->ID; $commentID = $comment->ID;
$this->assertFalse($comment->ApproveLink(), 'No permission to see approve link'); $this->assertFalse($comment->ApproveLink(), 'No permission to see approve link');
$approve = $this->get('CommentingController/approve/'.$comment->ID); $approve = $this->get('CommentingController/approve/'.$comment->ID.'?ajax=1');
$this->assertEquals(403, $approve->getStatusCode()); $this->assertEquals(403, $approve->getStatusCode());
$check = DataObject::get_by_id('Comment', $commentID); $check = DataObject::get_by_id('Comment', $commentID);
$this->assertEquals(0, $check->Moderated, 'No permission to approve'); $this->assertEquals(0, $check->Moderated, 'No permission to approve');