From b31886b67aaa814dc0ffe48bb7d5c4863de0106e Mon Sep 17 00:00:00 2001 From: cweiske Date: Tue, 28 Sep 2010 22:11:59 +0000 Subject: tests for deleting bookmarks via the API. two of them fail currently because of a security issue git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@767 b3834d28-1941-0410-a4f8-b48e95affb8f --- tests/Api/PostsDeleteTest.php | 302 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 302 insertions(+) create mode 100644 tests/Api/PostsDeleteTest.php (limited to 'tests/Api/PostsDeleteTest.php') diff --git a/tests/Api/PostsDeleteTest.php b/tests/Api/PostsDeleteTest.php new file mode 100644 index 0000000..705f94e --- /dev/null +++ b/tests/Api/PostsDeleteTest.php @@ -0,0 +1,302 @@ + + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ + +require_once dirname(__FILE__) . '/../prepare.php'; +require_once 'HTTP/Request2.php'; + +if (!defined('PHPUnit_MAIN_METHOD')) { + define('PHPUnit_MAIN_METHOD', 'Api_PostsDeleteTest::main'); +} + +/** + * Unit tests for the SemanticScuttle post deletion API. + * + * @category Bookmarking + * @package SemanticScuttle + * @author Benjamin Huynh-Kim-Bang + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ +class Api_PostsDeleteTest extends TestBaseApi +{ + protected $urlPart = 'api/posts/delete'; + + + + /** + * Used to run this test class standalone + * + * @return void + */ + public static function main() + { + require_once 'PHPUnit/TextUI/TestRunner.php'; + PHPUnit_TextUI_TestRunner::run( + new PHPUnit_Framework_TestSuite(__CLASS__) + ); + } + + + + /** + * Test if authentication is required when sending no auth data + */ + public function testAuthWithoutAuthData() + { + $req = $this->getRequest(null, false); + $res = $req->send(); + $this->assertEquals(401, $res->getStatus()); + } + + + + /** + * Test if authentication is required when sending wrong user data + + */ + public function testAuthWrongCredentials() + { + $req = $this->getRequest(null, false); + $req->setAuth('user', 'password', HTTP_Request2::AUTH_BASIC); + $res = $req->send(); + $this->assertEquals(401, $res->getStatus()); + } + + + + /** + * Test if deleting an own bookmark works. + */ + public function testDeleteOwnBookmark() + { + $this->bs->deleteAll(); + + $bookmarkUrl = 'http://example.org/tag-1'; + + list($req, $uId) = $this->getAuthRequest( + '?url=' . urlencode($bookmarkUrl) + ); + + $bId = $this->addBookmark( + $uId, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + //user has one bookmark now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(1, $data['total']); + + //send request + $res = $req->send(); + + $this->assertEquals(200, $res->getStatus()); + //verify MIME content type + $this->assertEquals( + 'text/xml; charset=utf-8', + $res->getHeader('content-type') + ); + + //verify xml + $this->assertTag( + array( + 'tag' => 'result', + 'attributes' => array('code' => 'done') + ), + $res->getBody(), + null, false + ); + + //bookmark should be deleted now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(0, $data['total']); + } + + + + /** + * Test if deleting an own bookmark via POST works. + */ + public function testDeleteOwnBookmarkPost() + { + $this->bs->deleteAll(); + + $bookmarkUrl = 'http://example.org/tag-1'; + + list($req, $uId) = $this->getAuthRequest(); + + $bId = $this->addBookmark( + $uId, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + //user has one bookmark now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(1, $data['total']); + + //send request + $req->setMethod(HTTP_Request2::METHOD_POST); + $req->addPostParameter('url', $bookmarkUrl); + $res = $req->send(); + + $this->assertEquals(200, $res->getStatus()); + //verify MIME content type + $this->assertEquals( + 'text/xml; charset=utf-8', + $res->getHeader('content-type') + ); + + //verify xml + $this->assertTag( + array( + 'tag' => 'result', + 'attributes' => array('code' => 'done') + ), + $res->getBody(), + null, false + ); + + //bookmark should be deleted now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(0, $data['total']); + } + + + + /** + * Verify that deleting a bookmark of a different does not work + */ + public function testDeleteOtherBookmark() + { + $this->bs->deleteAll(); + + $bookmarkUrl = 'http://example.org/tag-1'; + + list($req, $uId) = $this->getAuthRequest( + '?url=' . urlencode($bookmarkUrl) + ); + $uId2 = $this->addUser(); + + $bId = $this->addBookmark( + $uId2, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + //user 1 has no bookmarks + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(0, $data['total']); + //user 2 has one bookmark + $data = $this->bs->getBookmarks(0, null, $uId2); + $this->assertEquals(1, $data['total']); + + //send request + $res = $req->send(); + + //401 - unauthorized + $this->assertEquals(401, $res->getStatus()); + //verify MIME content type + $this->assertEquals( + 'text/xml; charset=utf-8', + $res->getHeader('content-type') + ); + + //verify xml + $this->assertNotTag( + array( + 'tag' => 'result', + 'attributes' => array('code' => 'done') + ), + $res->getBody(), + '', false + ); + + //bookmark should still be there + $data = $this->bs->getBookmarks(0, null, $uId2); + $this->assertEquals(1, $data['total']); + } + + + + /** + * Test if deleting a bookmark works that also other users + * bookmarked. + */ + public function testDeleteBookmarkOneOfTwo() + { + $this->bs->deleteAll(); + + $bookmarkUrl = 'http://example.org/tag-1'; + + list($req, $uId) = $this->getAuthRequest( + '?url=' . urlencode($bookmarkUrl) + ); + $uId2 = $this->addUser(); + $uId3 = $this->addUser(); + + //important: the order of addition is crucial here + $this->addBookmark( + $uId2, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + $bId = $this->addBookmark( + $uId, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + $this->addBookmark( + $uId3, $bookmarkUrl, 0, + array('unittest', 'tag1') + ); + + //user one and two have a bookmark now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(1, $data['total']); + $data = $this->bs->getBookmarks(0, null, $uId2); + $this->assertEquals(1, $data['total']); + + //send request + $res = $req->send(); + + $this->assertEquals(200, $res->getStatus()); + //verify MIME content type + $this->assertEquals( + 'text/xml; charset=utf-8', + $res->getHeader('content-type') + ); + + //verify xml + $this->assertTag( + array( + 'tag' => 'result', + 'attributes' => array('code' => 'done') + ), + $res->getBody(), + '', false + ); + + //bookmark should be deleted now + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(0, $data['total']); + //user 2 should still have his + $data = $this->bs->getBookmarks(0, null, $uId2); + $this->assertEquals(1, $data['total']); + //user 3 should still have his, too + $data = $this->bs->getBookmarks(0, null, $uId3); + $this->assertEquals(1, $data['total']); + } + +} + +if (PHPUnit_MAIN_METHOD == 'Api_PostsDeleteTest::main') { + Api_PostsDeleteTest::main(); +} +?> \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 22c9a01ee845d2b92fcab6b6cb10ac6ff0eec52e Mon Sep 17 00:00:00 2001 From: cweiske Date: Tue, 28 Sep 2010 22:14:31 +0000 Subject: rewrite api/posts/delete to be more secure and add unit tests for it git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@769 b3834d28-1941-0410-a4f8-b48e95affb8f --- src/SemanticScuttle/Service/Bookmark.php | 18 ++++++--- tests/Api/PostsDeleteTest.php | 9 +++-- www/api/posts_delete.php | 63 +++++++++++++++++++++++--------- 3 files changed, 62 insertions(+), 28 deletions(-) (limited to 'tests/Api/PostsDeleteTest.php') diff --git a/src/SemanticScuttle/Service/Bookmark.php b/src/SemanticScuttle/Service/Bookmark.php index dde1df5..4e18d3f 100644 --- a/src/SemanticScuttle/Service/Bookmark.php +++ b/src/SemanticScuttle/Service/Bookmark.php @@ -176,7 +176,10 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService * Retrieves a bookmark with the given URL. * DOES NOT RESPECT PRIVACY SETTINGS! * - * @param string $address URL to get bookmarks for + * @param string $address URL to get bookmarks for + * @param boolean $all Retrieve from all users (true) + * or only bookmarks owned by the current + * user (false) * * @return mixed Array with bookmark data or false in case * of an error (i.e. not found). @@ -184,9 +187,9 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService * @uses getBookmarkByHash() * @see getBookmarkByShortname() */ - public function getBookmarkByAddress($address) + public function getBookmarkByAddress($address, $all = true) { - return $this->getBookmarkByHash($this->getHash($address)); + return $this->getBookmarkByHash($this->getHash($address), $all); } @@ -195,16 +198,19 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService * Retrieves a bookmark with the given hash. * DOES NOT RESPECT PRIVACY SETTINGS! * - * @param string $hash URL hash + * @param string $hash URL hash + * @param boolean $all Retrieve from all users (true) + * or only bookmarks owned by the current + * user (false) * * @return mixed Array with bookmark data or false in case * of an error (i.e. not found). * * @see getHash() */ - public function getBookmarkByHash($hash) + public function getBookmarkByHash($hash, $all = true) { - return $this->_getbookmark('bHash', $hash, true); + return $this->_getbookmark('bHash', $hash, $all); } diff --git a/tests/Api/PostsDeleteTest.php b/tests/Api/PostsDeleteTest.php index 705f94e..626746f 100644 --- a/tests/Api/PostsDeleteTest.php +++ b/tests/Api/PostsDeleteTest.php @@ -202,8 +202,9 @@ class Api_PostsDeleteTest extends TestBaseApi //send request $res = $req->send(); - //401 - unauthorized - $this->assertEquals(401, $res->getStatus()); + //404 - user does not have that bookmark + $this->assertEquals(404, $res->getStatus()); + //verify MIME content type $this->assertEquals( 'text/xml; charset=utf-8', @@ -211,10 +212,10 @@ class Api_PostsDeleteTest extends TestBaseApi ); //verify xml - $this->assertNotTag( + $this->assertTag( array( 'tag' => 'result', - 'attributes' => array('code' => 'done') + 'attributes' => array('code' => 'something went wrong') ), $res->getBody(), '', false diff --git a/www/api/posts_delete.php b/www/api/posts_delete.php index a63cc62..982b686 100644 --- a/www/api/posts_delete.php +++ b/www/api/posts_delete.php @@ -1,33 +1,60 @@ + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ // Force HTTP authentication first! $httpContentType = 'text/xml'; require_once 'httpauth.inc.php'; -/* Service creation: only useful services are created */ -$bookmarkservice =SemanticScuttle_Service_Factory::get('Bookmark'); - +$bs = SemanticScuttle_Service_Factory::get('Bookmark'); +$uId = $userservice->getCurrentUserId(); -// Note that del.icio.us only errors out if no URL was passed in; there's no error on attempting -// to delete a bookmark you don't have. // Error out if there's no address -if (is_null($_REQUEST['url'])) { +if (!isset($_REQUEST['url']) + || $_REQUEST['url'] == '' +) { $deleted = false; +} else if (!$bs->bookmarkExists($_REQUEST['url'], $uId)) { + //the user does not have such a bookmark + // Note that del.icio.us only errors out if no URL was passed in; + // there's no error on attempting to delete a bookmark you don't have. + // this sucks, and I don't care about being different but correct here. + header('HTTP/1.0 404 Not Found'); + $deleted = false; + } else { - $bookmark = $bookmarkservice->getBookmarkByAddress($_REQUEST['url']); - $bid = $bookmark['bId']; - $delete = $bookmarkservice->deleteBookmark($bid); - $deleted = true; + $bookmark = $bs->getBookmarkByAddress($_REQUEST['url'], false); + $bId = $bookmark['bId']; + $deleted = $bs->deleteBookmark($bId); + if (!$deleted) { + //something really went wrong + header('HTTP/1.0 500 Internal Server Error'); + } } // Set up the XML file and output the result. -echo '\r\n"; -echo ''; +echo '\r\n"; +echo ''; ?> \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 70c39a8eea7896271c0ad3f0c435ec06c64074d1 Mon Sep 17 00:00:00 2001 From: cweiske Date: Wed, 29 Sep 2010 20:49:14 +0000 Subject: delicious returns a proper error message when deleting non-existant items, which we do now, too git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@770 b3834d28-1941-0410-a4f8-b48e95affb8f --- doc/developers/api | 10 ++++++++++ tests/Api/PostsDeleteTest.php | 2 +- www/api/posts_delete.php | 14 +++++--------- 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 doc/developers/api (limited to 'tests/Api/PostsDeleteTest.php') diff --git a/doc/developers/api b/doc/developers/api new file mode 100644 index 0000000..efa05fe --- /dev/null +++ b/doc/developers/api @@ -0,0 +1,10 @@ +SemanticScuttle API +=================== + +SemanticScuttle tries to implement the delicious API v1 as closely as sensible. + +Where it makes sense and the delicious API just does things plainly wrong +(i.e. when returning a wrong status code on an error), we do it better. + +- http://www.delicious.com/help/api +- http://support.delicious.com/forum/comments.php?DiscussionID=5286&page=1 diff --git a/tests/Api/PostsDeleteTest.php b/tests/Api/PostsDeleteTest.php index 626746f..d9fb6cd 100644 --- a/tests/Api/PostsDeleteTest.php +++ b/tests/Api/PostsDeleteTest.php @@ -215,7 +215,7 @@ class Api_PostsDeleteTest extends TestBaseApi $this->assertTag( array( 'tag' => 'result', - 'attributes' => array('code' => 'something went wrong') + 'attributes' => array('code' => 'item not found') ), $res->getBody(), '', false diff --git a/www/api/posts_delete.php b/www/api/posts_delete.php index 982b686..03cc968 100644 --- a/www/api/posts_delete.php +++ b/www/api/posts_delete.php @@ -4,8 +4,6 @@ * The delicious API is implemented here. * * The delicious API behaves like that: - * - returns "done" even if the bookmark doesn't exist - * - we do it correctly * - does NOT allow the hash for the url parameter * - doesn't set the Content-Type to text/xml * - we do it correctly, too @@ -35,26 +33,24 @@ $uId = $userservice->getCurrentUserId(); if (!isset($_REQUEST['url']) || $_REQUEST['url'] == '' ) { - $deleted = false; + $msg = 'something went wrong'; } else if (!$bs->bookmarkExists($_REQUEST['url'], $uId)) { //the user does not have such a bookmark - // Note that del.icio.us only errors out if no URL was passed in; - // there's no error on attempting to delete a bookmark you don't have. - // this sucks, and I don't care about being different but correct here. header('HTTP/1.0 404 Not Found'); - $deleted = false; - + $msg = 'item not found'; } else { $bookmark = $bs->getBookmarkByAddress($_REQUEST['url'], false); $bId = $bookmark['bId']; $deleted = $bs->deleteBookmark($bId); + $msg = 'done'; if (!$deleted) { //something really went wrong header('HTTP/1.0 500 Internal Server Error'); + $msg = 'something really went wrong'; } } // Set up the XML file and output the result. echo '\r\n"; -echo ''; +echo ''; ?> \ No newline at end of file -- cgit v1.2.3-54-g00ecf