Better token revocation

actions/apioauthauthorize.php

@@ -99,24 +99,17 @@ class ApiOauthAuthorizeAction extends ApiOauthAction
 
         } else {
 
-            // XXX: make better error messages
-
             if (empty($this->oauth_token)) {
-
+                $this->clientError(_('No oauth_token parameter provided.'));
-                common_debug("No request token found.");
-
-                $this->clientError(_('Bad request.'));
                 return;
             }
 
             if (empty($this->app)) {
-                common_debug('No app for that token.');
+                $this->clientError(_('Invalid token.'));
-                $this->clientError(_('Bad request.'));
                 return;
             }
 
             $name = $this->app->name;
-            common_debug("Requesting auth for app: " . $name);
 
             $this->showForm();
         }
@@ -124,8 +117,6 @@ class ApiOauthAuthorizeAction extends ApiOauthAction
 
     function handlePost()
     {
-        common_debug("handlePost()");
-
         // check session token for CSRF protection.
 
         $token = $this->trimmed('token');
@@ -210,13 +201,9 @@ class ApiOauthAuthorizeAction extends ApiOauthAction
 
             if (!empty($this->callback)) {
 
-                // XXX: Need better way to build this redirect url.
-
                 $target_url = $this->getCallback($this->callback,
                                                  array('oauth_token' => $this->oauth_token));
 
-                common_debug("Doing callback to $target_url");
-
                 common_redirect($target_url, 303);
             } else {
                 common_debug("callback was empty!");
@@ -236,9 +223,12 @@ class ApiOauthAuthorizeAction extends ApiOauthAction
 
         } else if ($this->arg('deny')) {
 
+            $datastore = new ApiStatusNetOAuthDataStore();
+            $datastore->revoke_token($this->oauth_token, 0);
+
             $this->elementStart('p');
 
-            $this->raw(sprintf(_("The request token %s has been denied."),
+            $this->raw(sprintf(_("The request token %s has been denied and revoked."),
                                $this->oauth_token));
 
             $this->elementEnd('p');

actions/oauthconnectionssettings.php

@@ -33,6 +33,7 @@ if (!defined('STATUSNET') && !defined('LACONICA')) {
 
 require_once INSTALLDIR . '/lib/connectsettingsaction.php';
 require_once INSTALLDIR . '/lib/applicationlist.php';
+require_once INSTALLDIR . '/lib/apioauthstore.php';
 
 /**
  * Show connected OAuth applications
@@ -71,11 +72,6 @@ class OauthconnectionssettingsAction extends ConnectSettingsAction
         return _('Connected applications');
     }
 
-    function isReadOnly($args)
-    {
-        return true;
-    }
-
     /**
      * Instructions for use
      *
@@ -153,6 +149,13 @@ class OauthconnectionssettingsAction extends ConnectSettingsAction
         }
     }
 
+    /**
+     * Revoke access to an authorized OAuth application
+     *
+     * @param int $appId the ID of the application
+     *
+     */
+
     function revokeAccess($appId)
     {
         $cur = common_current_user();
@@ -164,6 +167,8 @@ class OauthconnectionssettingsAction extends ConnectSettingsAction
             return false;
         }
 
+        // XXX: Transaction here?
+
         $appUser = Oauth_application_user::getByKeys($cur, $app);
 
         if (empty($appUser)) {
@@ -171,12 +176,13 @@ class OauthconnectionssettingsAction extends ConnectSettingsAction
             return false;
         }
 
-        $orig = clone($appUser);
+        $datastore = new ApiStatusNetOAuthDataStore();
-        $appUser->access_type = 0;  // No access
+        $datastore->revoke_token($appUser->token, 1);
-        $result = $appUser->update();
+
+        $result = $appUser->delete();
 
         if (!$result) {
-            common_log_db_error($orig, 'UPDATE', __FILE__);
+            common_log_db_error($orig, 'DELETE', __FILE__);
             $this->clientError(_('Unable to revoke access for app: ' . $app->id));
             return false;
         }

db/statusnet.sql

@@ -230,7 +230,7 @@ create table oauth_application (
 create table oauth_application_user (
     profile_id integer not null comment 'user of the application' references profile (id),
     application_id integer not null comment 'id of the application' references oauth_application (id),
-    access_type tinyint default 0 comment 'access type, bit 1 = read, bit 2 = write, bit 3 = revoked',
+    access_type tinyint default 0 comment 'access type, bit 1 = read, bit 2 = write',
     token varchar(255) comment 'request or access token',
     created datetime not null comment 'date this record was created',
     modified timestamp comment 'date this record was modified',

lib/apioauthstore.php

@@ -159,5 +159,32 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
         }
     }
 
+    /**
+     * Revoke specified access token
+     *
+     * Revokes the token specified by $token_key.
+     * Throws exceptions in case of error.
+     *
+     * @param string $token_key the token to be revoked
+     * @param int    $type      type of token (0 = req, 1 = access)
+     *
+     * @access public
+     *
+     * @return void
+     */
+
+    public function revoke_token($token_key, $type = 0) {
+        $rt = new Token();
+        $rt->tok = $token_key;
+        $rt->type = $type;
+        $rt->state = 0;
+        if (!$rt->find(true)) {
+            throw new Exception('Tried to revoke unknown token');
+        }
+        if (!$rt->delete()) {
+            throw new Exception('Failed to delete revoked token');
+        }
+    }
+
 }