Update libomb, fix some omb handling stuff, improve error handling.

7 files changed, 130 insertions, 55 deletions

diff --git a/actions/finishremotesubscribe.php b/actions/finishremotesubscribe.php
index 13f367823..da563cb29 100644
--- a/actions/finishremotesubscribe.php
+++ b/actions/finishremotesubscribe.php
@@ -76,11 +76,10 @@ class FinishremotesubscribeAction extends Action
 
         /* Create user objects for both users. Do it early for request
            validation. */
-        $listenee = $service->getListeneeURI();
-        $user = User::staticGet('uri', $listenee);
+        $user = User::staticGet('uri', $service->getListeneeURI());
 
         if (!$user) {
-            $this->clientError(_('User being listened to doesn\'t exist.'));
+            $this->clientError(_('User being listened to does not exist.'));
             return;
         }
 
@@ -91,21 +90,31 @@ class FinishremotesubscribeAction extends Action
             return;
         }
 
+        $remote = Remote_profile::staticGet('uri', $service->getListenerURI());
+
+        $profile = Profile::staticGet($remote->id);
+
+        if ($user->hasBlocked($profile)) {
+            $this->clientError(_('That user has blocked you from subscribing.'));
+            return;
+        }
+
         /* Perform the handling itself via libomb. */
         try {
-            $service->finishAuthorization($listenee);
+            $service->finishAuthorization();
         } catch (OAuthException $e) {
             if ($e->getMessage() == 'The authorized token does not equal the ' .
                                     'submitted token.') {
-                $this->clientError(_('Not authorized.'));
+                $this->clientError(_('You are not authorized.'));
                 return;
             } else {
-                $this->clientError(_('Couldn\'t convert request token to ' .
+                $this->clientError(_('Could not convert request token to ' .
                                      'access token.'));
                 return;
             }
         } catch (OMB_RemoteServiceException $e) {
-            $this->clientError(_('Unknown version of OMB protocol.'));
+            $this->clientError(_('Remote service uses unknown version of ' .
+                                 'OMB protocol.'));
             return;
         } catch (Exception $e) {
             common_debug('Got exception ' . print_r($e, true), __FILE__);
@@ -115,8 +124,6 @@ class FinishremotesubscribeAction extends Action
 
         /* The service URLs are not accessible from datastore, so setting them
            after insertion of the profile. */
-        $remote = Remote_profile::staticGet('uri', $service->getListenerURI());
-
         $orig_remote = clone($remote);
 
         $remote->postnoticeurl    =
diff --git a/actions/postnotice.php b/actions/postnotice.php
index 74be47119..97e887c46 100644
--- a/actions/postnotice.php
+++ b/actions/postnotice.php
@@ -47,12 +47,28 @@ require_once INSTALLDIR.'/extlib/libomb/service_provider.php';
  */
 class PostnoticeAction extends Action
 {
+    /**
+     * For initializing members of the class.
+     *
+     * @param array $argarray misc. arguments
+     *
+     * @return boolean true
+     */
+    function prepare($argarray)
+    {
+        parent::prepare($argarray);
+        try {
+            $this->checkNotice();
+        } catch (Exception $e) {
+            $this->clientError($e->getMessage());
+            return false;
+        }
+        return true;
+    }
+
     function handle($args)
     {
         parent::handle($args);
-        if (!$this->checkNotice()) {
-            return;
-        }
         try {
             $srv = new OMB_Service_Provider(null, omb_oauth_datastore(),
                                             omb_oauth_server());
@@ -67,10 +83,15 @@ class PostnoticeAction extends Action
     {
         $content = common_shorten_links($_POST['omb_notice_content']);
         if (mb_strlen($content) > 140) {
-            $this->clientError(_('Invalid notice content'), 400);
-            return false;
+            throw new Exception(_('The notice content is too long.'));
+        }
+        $license      = $_POST['omb_notice_license'];
+        $site_license = common_config('license', 'url');
+        if ($license && !common_compatible_license($license, $site_license)) {
+            throw new Exception(sprintf(_('Notice license ‘%s’ is not ' .
+                                          'compatible with site license ‘%s’.'),
+                                        $license, $site_license));
         }
-        return true;
     }
 }
 ?>
diff --git a/actions/remotesubscribe.php b/actions/remotesubscribe.php
index 5122c1172..353717beb 100644
--- a/actions/remotesubscribe.php
+++ b/actions/remotesubscribe.php
@@ -153,12 +153,11 @@ class RemotesubscribeAction extends Action
         $this->profile_url = $this->trimmed('profile_url');
 
         if (!$this->profile_url) {
-            $this->showForm(_('No such user.'));
+            $this->showForm(_('No such user'));
             return;
         }
 
-        if (!Validate::uri($this->profile_url,
-                           array('allowed_schemes' => array('http', 'https')))) {
+        if (!common_valid_http_url($this->profile_url)) {
             $this->showForm(_('Invalid profile URL (bad format)'));
             return;
         }
@@ -176,14 +175,14 @@ class RemotesubscribeAction extends Action
         if ($service->getServiceURI(OAUTH_ENDPOINT_REQUEST) ==
             common_local_url('requesttoken') ||
             User::staticGet('uri', $service->getRemoteUserURI())) {
-            $this->showForm(_('That\'s a local profile! Login to subscribe.'));
+            $this->showForm(_('That’s a local profile! Login to subscribe.'));
             return;
         }
 
         try {
             $service->requestToken();
         } catch (OMB_RemoteServiceException $e) {
-            $this->showForm(_('Couldn\'t get a request token.'));
+            $this->showForm(_('Couldn’t get a request token.'));
             return;
         }
 
diff --git a/actions/updateprofile.php b/actions/updateprofile.php
index 345c28b8d..b10554e8b 100644
--- a/actions/updateprofile.php
+++ b/actions/updateprofile.php
@@ -48,9 +48,31 @@ require_once INSTALLDIR.'/extlib/libomb/service_provider.php';
 class UpdateprofileAction extends Action
 {
 
+    /**
+     * For initializing members of the class.
+     *
+     * @param array $argarray misc. arguments
+     *
+     * @return boolean true
+     */
+    function prepare($argarray)
+    {
+        parent::prepare($argarray);
+        $license      = $_POST['omb_listenee_license'];
+        $site_license = common_config('license', 'url');
+        if (!common_compatible_license($license, $site_license)) {
+            $this->clientError(sprintf(_('Listenee stream license ‘%s’ is not '.
+                                          'compatible with site license ‘%s’.'),
+                                       $license, $site_license);
+            return false;
+        }
+        return true;
+    }
+
     function handle($args)
     {
         parent::handle($args);
+
         try {
             $srv = new OMB_Service_Provider(null, omb_oauth_datastore(),
                                             omb_oauth_server());
diff --git a/actions/userauthorization.php b/actions/userauthorization.php
index d5b6a6998..54e0ee920 100644
--- a/actions/userauthorization.php
+++ b/actions/userauthorization.php
@@ -80,7 +80,7 @@ class UserauthorizationAction extends Action
             try {
                 $this->validateOmb();
                 $srv = new OMB_Service_Provider(
-                        profile_to_omb_profile($_GET['omb_listener'], $profile),
+                        profile_to_omb_profile($user->uri, $profile),
                         omb_oauth_datastore());
 
                 $remote_user = $srv->handleUserAuth();
@@ -111,8 +111,8 @@ class UserauthorizationAction extends Action
     {
         $this->element('p', null, _('Please check these details to make sure '.
                                     'that you want to subscribe to this ' .
-                                    'user\'s notices. If you didn\'t just ask ' .
-                                    'to subscribe to someone\'s notices, '.
+                                    'user’s notices. If you didn’t just ask ' .
+                                    'to subscribe to someone’s notices, '.
                                     'click “Reject”.'));
     }
 
@@ -249,7 +249,7 @@ class UserauthorizationAction extends Action
         common_show_header(_('Subscription authorized'));
         $this->element('p', null,
                        _('The subscription has been authorized, but no '.
-                         'callback URL was passed. Check with the site\'s ' .
+                         'callback URL was passed. Check with the site’s ' .
                          'instructions for details on how to authorize the ' .
                          'subscription. Your subscription token is:'));
         $this->element('blockquote', 'token', $tok);
@@ -261,7 +261,7 @@ class UserauthorizationAction extends Action
         common_show_header(_('Subscription rejected'));
         $this->element('p', null,
                        _('The subscription has been rejected, but no '.
-                         'callback URL was passed. Check with the site\'s ' .
+                         'callback URL was passed. Check with the site’s ' .
                          'instructions for details on how to fully reject ' .
                          'the subscription.'));
         common_show_footer();
@@ -295,16 +295,19 @@ class UserauthorizationAction extends Action
 
         $user = User::staticGet('uri', $listener);
         if (!$user) {
-            throw new Exception("Listener URI '$listener' not found here");
+            throw new Exception(sprintf(_('Listener URI ‘%s’ not found here'),
+                                        $listener));
         }
-        $cur = common_current_user();
-        if ($cur->id != $user->id) {
-            throw new Exception('Can\'t subscribe for another user!');
+
+        if (strlen($listenee) > 255) {
+            throw new Exception(sprintf(_('Listenee URI ‘%s’ is too long.'),
+                                        $listenee));
         }
 
         $other = User::staticGet('uri', $listenee);
         if ($other) {
-            throw new Exception("Listenee URI '$listenee' is local user");
+            throw new Exception(sprintf(_('Listenee URI ‘%s’ is a local user.'),
+                                        $listenee));
         }
 
         $remote = Remote_profile::staticGet('uri', $listenee);
@@ -318,27 +321,34 @@ class UserauthorizationAction extends Action
         }
 
         if ($profile == common_profile_url($nickname)) {
-            throw new Exception("Profile URL '$profile' is for a local user.");
+            throw new Exception(sprintf(_('Profile URL ‘%s’ is for a local user.'),
+                                        $profile));
+
         }
 
         $license      = $_GET['omb_listenee_license'];
         $site_license = common_config('license', 'url');
         if (!common_compatible_license($license, $site_license)) {
-            throw new Exception("Listenee stream license '$license' is not " .
-                                "compatible with site license '$site_license'.");
+            throw new Exception(sprintf(_('Listenee stream license ‘%s’ is not ' .
+                                          'compatible with site license ‘%s’.'),
+                                        $license, $site_license));
         }
+
         $avatar = $_GET['omb_listenee_avatar'];
         if ($avatar) {
             if (!common_valid_http_url($avatar) || strlen($avatar) > 255) {
-                throw new Exception("Invalid avatar URL '$avatar'");
+                throw new Exception(sprintf(_('Avatar URL ‘%s’ is not valid.'),
+                                            $avatar));
             }
             $size = @getimagesize($avatar);
             if (!$size) {
-                throw new Exception("Can't read avatar URL '$avatar'.");
+                throw new Exception(sprintf(_('Can’t read avatar URL ‘%s’.'),
+                                            $avatar));
             }
             if (!in_array($size[2], array(IMAGETYPE_GIF, IMAGETYPE_JPEG,
                                           IMAGETYPE_PNG))) {
-                throw new Exception("Wrong image type for '$avatar'");
+                throw new Exception(sprintf(_('Wrong image type for avatar URL '.
+                                              '‘%s’.'), $avatar));
             }
         }
     }
diff --git a/extlib/libomb/datastore.php b/extlib/libomb/datastore.php
index ac51a4ab8..ab52de547 100755
--- a/extlib/libomb/datastore.php
+++ b/extlib/libomb/datastore.php
@@ -5,26 +5,28 @@ require_once 'OAuth.php';
 /**
  * Data access interface
  *
- * This interface specifies data access methods libomb needs. It
- * should be implemented by libomb users.
- * OMB_Datastore is libomb’s main interface to the application’s data.
+ * This interface specifies data access methods libomb needs. It should be
+ * implemented by libomb users. OMB_Datastore is libomb’s main interface to the
+ * application’s data. Objects corresponding to this interface are used in
+ * OMB_Service_Provider and OMB_Service_Consumer.
+ *
+ * Note that it’s implemented as a class since OAuthDataStore is as well a
+ * class, though only declaring methods.
+ *
+ * OMB_Datastore extends OAuthDataStore with two OAuth-related methods for token
+ * revoking and authorizing and all OMB-related methods.
+ * Refer to OAuth.php for a complete specification of OAuth-related methods.
  *
  * It is the user’s duty to signal and handle errors. libomb does not check
  * return values nor handle exceptions. It is suggested to use exceptions.
  * Note that lookup_token and getProfile return null if the requested object
  * is not available. This is NOT an error and should not raise an exception.
  * Same applies for lookup_nonce which returns a boolean value. These methods
- * may nevertheless throw an exception, for example in case of a storage error.
+ * may nevertheless throw an exception, for example in case of a storage errors.
  *
- * Objects corresponding to this interface are used in OMB_Service_Provider and
- * OMB_Service_Consumer.
- *
- * OMB_Datastore extends OAuthDataStore with two OAuth-related methods for token
- * revoking and authorizing and all OMB-related methods.
- * Refer to OAuth.php for a complete specification of OAuth-related methods.
- *
- * Note that it’s implemented as a class since OAuthDataStore is as well a
- * class, though only declaring methods.
+ * Most of the parameters passed to these methods are unescaped and unverified
+ * user input. Therefore they should be handled with extra care to avoid
+ * security problems like SQL injections.
  *
  * PHP version 5
  *
@@ -59,7 +61,7 @@ class OMB_Datastore extends OAuthDataStore {
    * Revokes the authorization token specified by $token_key.
    * Throws exceptions in case of error.
    *
-   * @param string $token_key The token to be revoked
+   * @param string $token_key The key of the token to be revoked
    *
    * @access public
    **/
@@ -73,7 +75,7 @@ class OMB_Datastore extends OAuthDataStore {
    * Authorizes the authorization token specified by $token_key.
    * Throws exceptions in case of error.
    *
-   * @param string $token_key The token to be authorized
+   * @param string $token_key The key of the token to be authorized
    *
    * @access public
    **/
diff --git a/extlib/libomb/service_provider.php b/extlib/libomb/service_provider.php
index b3ad53753..753152713 100755
--- a/extlib/libomb/service_provider.php
+++ b/extlib/libomb/service_provider.php
@@ -111,6 +111,12 @@ class OMB_Service_Provider {
    * Throws exceptions on failures. Returns an OMB_Profile object representing
    * the remote user.
    *
+   * The OMB_Profile passed to the constructor of OMB_Service_Provider should
+   * not represent the user specified in the authorization request, but the one
+   * currently logged in to the service. This condition being satisfied,
+   * handleUserAuth will check whether the listener specified in the request is
+   * identical to the logged in user.
+   *
    * @access public
    *
    * @return OMB_Profile The profile of the soon-to-be subscribed, i. e. remote
@@ -150,6 +156,10 @@ class OMB_Service_Provider {
     /* Store given callback for later use. */
     if (isset($_GET['oauth_callback']) && $_GET['oauth_callback'] !== '') {
       $this->callback = $_GET['oauth_callback'];
+      if (!OMB_Helper::validateURL($this->callback)) {
+        throw OMB_RemoteServiceException::forRequest(OAUTH_ENDPOINT_AUTHORIZE,
+                                              'Invalid callback URL specified');
+      }
     }
     $this->remote_user = OMB_Profile::fromParameters($_GET, 'omb_listenee');
 
@@ -205,13 +215,16 @@ class OMB_Service_Provider {
   /**
    * Echo an access token
    *
-   * Outputs an access token for the query found in $_GET or $_POST.
+   * Outputs an access token for the query found in $_POST. OMB 0.1 specifies
+   * that the access token request has to be a POST even if OAuth allows GET as
+   * well.
    *
    * @access public
    **/
   public function writeAccessToken() {
     OMB_Helper::removeMagicQuotesFromRequest();
-    echo $this->getOAuthServer()->fetch_access_token(OAuthRequest::from_request());
+    echo $this->getOAuthServer()->fetch_access_token(
+                                            OAuthRequest::from_request('POST'));
   }
 
   /**
@@ -235,7 +248,8 @@ class OMB_Service_Provider {
   /**
    * Handle a postnotice request
    *
-   * Handles a postnotice request posted to this service.
+   * Handles a postnotice request posted to this service. Saves the notice
+   * through the OMB_Datastore.
    *
    * @access public
    *
@@ -264,7 +278,7 @@ class OMB_Service_Provider {
   protected function handleOMBRequest($uri) {
 
     OMB_Helper::removeMagicQuotesFromRequest();
-    $req = OAuthRequest::from_request();
+    $req = OAuthRequest::from_request('POST');
     $listenee =  $req->get_parameter('omb_listenee');
 
     try {