From 923b7de3c661098cfe3d5bdc0878d9329e1ee2bf Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 27 Jan 2010 08:41:26 +0000 Subject: [PATCH] - Check for read-only vs. read-write access to protected API resources (OAuth) - Some cleanup --- actions/apistatusesupdate.php | 18 +++-- lib/apiauth.php | 126 ++++++++++++++++++---------------- 2 files changed, 75 insertions(+), 69 deletions(-) diff --git a/actions/apistatusesupdate.php b/actions/apistatusesupdate.php index 31c9b20ce2..bf367e1e18 100644 --- a/actions/apistatusesupdate.php +++ b/actions/apistatusesupdate.php @@ -28,7 +28,7 @@ * @author Mike Cochrane * @author Robin Millette * @author Zach Copley - * @copyright 2009 StatusNet, Inc. + * @copyright 2009-2010 StatusNet, Inc. * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @link http://status.net/ */ @@ -79,7 +79,6 @@ class ApiStatusesUpdateAction extends ApiAuthAction { parent::prepare($args); - $this->user = $this->auth_user; $this->status = $this->trimmed('status'); $this->source = $this->trimmed('source'); $this->lat = $this->trimmed('lat'); @@ -145,7 +144,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction return; } - if (empty($this->user)) { + if (empty($this->auth_user)) { $this->clientError(_('No such user.'), 404, $this->format); return; } @@ -172,7 +171,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction // Check for commands $inter = new CommandInterpreter(); - $cmd = $inter->handle_command($this->user, $status_shortened); + $cmd = $inter->handle_command($this->auth_user, $status_shortened); if ($cmd) { @@ -184,7 +183,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction // And, it returns your last status whether the cmd was successful // or not! - $this->notice = $this->user->getCurrentNotice(); + $this->notice = $this->auth_user->getCurrentNotice(); } else { @@ -211,7 +210,7 @@ class ApiStatusesUpdateAction extends ApiAuthAction $upload = null; try { - $upload = MediaFile::fromUpload('media', $this->user); + $upload = MediaFile::fromUpload('media', $this->auth_user); } catch (ClientException $ce) { $this->clientError($ce->getMessage()); return; @@ -234,19 +233,19 @@ class ApiStatusesUpdateAction extends ApiAuthAction $options = array('reply_to' => $reply_to); - if ($this->user->shareLocation()) { + if ($this->auth_user->shareLocation()) { $locOptions = Notice::locationOptions($this->lat, $this->lon, null, null, - $this->user->getProfile()); + $this->auth_user->getProfile()); $options = array_merge($options, $locOptions); } $this->notice = - Notice::saveNew($this->user->id, + Notice::saveNew($this->auth_user->id, $content, $this->source, $options); @@ -255,7 +254,6 @@ class ApiStatusesUpdateAction extends ApiAuthAction $upload->attachToNotice($this->notice); } - } $this->showNotice(); diff --git a/lib/apiauth.php b/lib/apiauth.php index 37070d212f..ad9651ff26 100644 --- a/lib/apiauth.php +++ b/lib/apiauth.php @@ -29,7 +29,7 @@ * @author mEDI * @author Sarven Capadisli * @author Zach Copley - * @copyright 2009 StatusNet, Inc. + * @copyright 2009-2010 StatusNet, Inc. * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @link http://status.net/ */ @@ -53,9 +53,11 @@ require_once INSTALLDIR . '/lib/apioauth.php'; class ApiAuthAction extends ApiAction { - var $access_token; - var $oauth_access_type; - var $oauth_source; + var $auth_user_nickname = null; + var $auth_user_password = null; + var $access_token = null; + var $oauth_source = null; + var $auth_user = null; /** * Take arguments for running, and output basic auth header if needed @@ -70,18 +72,28 @@ class ApiAuthAction extends ApiAction { parent::prepare($args); + $this->consumer_key = $this->arg('oauth_consumer_key'); + $this->access_token = $this->arg('oauth_token'); + + // NOTE: $this->auth_user has to get set in prepare(), not handle(), + // because subclasses do stuff with it in their prepares. + if ($this->requiresAuth()) { - - $this->consumer_key = $this->arg('oauth_consumer_key'); - $this->access_token = $this->arg('oauth_token'); - if (!empty($this->access_token)) { $this->checkOAuthRequest(); } else { $this->checkBasicAuthUser(); - // By default, all basic auth users have read and write access + } - $this->access = self::READ_WRITE; + // Reject API calls with the wrong access level + + if ($this->isReadOnly($args) == false) { + if ($this->access != self::READ_WRITE) { + $msg = 'API resource requires read-write access, ' . + 'but you only have read access.'; + $this->clientError($msg, 401, $this->format); + exit(); + } } } @@ -95,8 +107,6 @@ class ApiAuthAction extends ApiAction function checkOAuthRequest() { - common_debug("We have an OAuth request."); - $datastore = new ApiStatusNetOAuthDataStore(); $server = new OAuthServer($datastore); $hmac_method = new OAuthSignatureMethod_HMAC_SHA1(); @@ -114,9 +124,10 @@ class ApiAuthAction extends ApiAction if (empty($app)) { - // this should really not happen - common_log(LOG_WARN, - "Couldn't find the OAuth app for consumer key: $this->consumer_key"); + // this should probably not happen + common_log(LOG_WARNING, + 'Couldn\'t find the OAuth app for consumer key: ' . + $this->consumer_key); throw new OAuthException('No application for that consumer key.'); } @@ -128,20 +139,18 @@ class ApiAuthAction extends ApiAction $appUser = Oauth_application_user::staticGet('token', $this->access_token); - // XXX: check that app->id and appUser->application_id and consumer all + // XXX: Check that app->id and appUser->application_id and consumer all // match? if (!empty($appUser)) { - // read or read-write - $this->oauth_access_type = $appUser->access_type; - // If access_type == 0 we have either a request token // or a bad / revoked access token - if ($this->oauth_access_type != 0) { + if ($appUser->access_type != 0) { + + // Set the access level for the api call - // Set the read or read-write access for the api call $this->access = ($appUser->access_type & Oauth_application::$writeAccess) ? self::READ_WRITE : self::READ_ONLY; @@ -151,38 +160,34 @@ class ApiAuthAction extends ApiAction } $msg = "API OAuth authentication for user '%s' (id: %d) on behalf of " . - "application '%s' (id: %d)."; + "application '%s' (id: %d) with %s access."; common_log(LOG_INFO, sprintf($msg, $this->auth_user->nickname, $this->auth_user->id, $app->name, - $app->id)); + $app->id, + ($this->access = self::READ_WRITE) ? + 'read-write' : 'read-only' + )); return true; } else { throw new OAuthException('Bad access token.'); } } else { - // also should not happen + // Also should not happen + throw new OAuthException('No user for that token.'); - } + } } catch (OAuthException $e) { - common_log(LOG_WARN, 'API OAuthException - ' . $e->getMessage()); - common_debug(var_export($req, true)); - $this->showOAuthError($e->getMessage()); - exit(); + common_log(LOG_WARNING, 'API OAuthException - ' . $e->getMessage()); + $this->showAuthError(); + exit; } } - function showOAuthError($msg) - { - header('HTTP/1.1 401 Unauthorized'); - header('Content-Type: text/html; charset=utf-8'); - print $msg . "\n"; - } - /** * Does this API resource require authentication? * @@ -207,39 +212,44 @@ class ApiAuthAction extends ApiAction $realm = common_config('site', 'name') . ' API'; - if (!isset($this->auth_user)) { + if (!isset($this->auth_user_nickname)) { header('WWW-Authenticate: Basic realm="' . $realm . '"'); // show error if the user clicks 'cancel' - $this->showBasicAuthError(); + $this->showAuthError(); exit; } else { - $nickname = $this->auth_user; - $password = $this->auth_pw; - $user = common_check_user($nickname, $password); + + $user = common_check_user($this->auth_user_nickname, + $this->auth_user_password); + if (Event::handle('StartSetApiUser', array(&$user))) { $this->auth_user = $user; Event::handle('EndSetApiUser', array($user)); } + // By default, basic auth users have rw access + + $this->access = self::READ_WRITE; + if (empty($this->auth_user)) { // basic authentication failed list($proxy, $ip) = common_client_ip(); + common_log( LOG_WARNING, 'Failed API auth attempt, nickname = ' . "$nickname, proxy = $proxy, ip = $ip." ); - $this->showBasicAuthError(); + + $this->showAuthError(); exit; } } - - return true; } /** @@ -253,32 +263,30 @@ class ApiAuthAction extends ApiAction { if (isset($_SERVER['AUTHORIZATION']) || isset($_SERVER['HTTP_AUTHORIZATION']) - ) { - $authorization_header = isset($_SERVER['HTTP_AUTHORIZATION']) - ? $_SERVER['HTTP_AUTHORIZATION'] : $_SERVER['AUTHORIZATION']; + ) { + $authorization_header = isset($_SERVER['HTTP_AUTHORIZATION']) + ? $_SERVER['HTTP_AUTHORIZATION'] : $_SERVER['AUTHORIZATION']; } if (isset($_SERVER['PHP_AUTH_USER'])) { - $this->auth_user = $_SERVER['PHP_AUTH_USER']; - $this->auth_pw = $_SERVER['PHP_AUTH_PW']; + $this->auth_user_nickname = $_SERVER['PHP_AUTH_USER']; + $this->auth_user_password = $_SERVER['PHP_AUTH_PW']; } elseif (isset($authorization_header) && strstr(substr($authorization_header, 0, 5), 'Basic')) { - // decode the HTTP_AUTHORIZATION header on php-cgi server self + // Decode the HTTP_AUTHORIZATION header on php-cgi server self // on fcgid server the header name is AUTHORIZATION $auth_hash = base64_decode(substr($authorization_header, 6)); - list($this->auth_user, $this->auth_pw) = explode(':', $auth_hash); + list($this->auth_user_nickname, + $this->auth_user_password) = explode(':', $auth_hash); - // set all to null on a empty basic auth request + // Set all to null on a empty basic auth request - if ($this->auth_user == "") { - $this->auth_user = null; - $this->auth_pw = null; + if (empty($this->auth_user_nickname)) { + $this->auth_user_nickname = null; + $this->auth_password = null; } - } else { - $this->auth_user = null; - $this->auth_pw = null; } } @@ -289,7 +297,7 @@ class ApiAuthAction extends ApiAction * @return void */ - function showBasicAuthError() + function showAuthError() { header('HTTP/1.1 401 Unauthorized'); $msg = 'Could not authenticate you.';