From 6dc94a538993580b27813004915edc4cee957e60 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 20 Jan 2011 10:43:27 -0800 Subject: [PATCH 1/7] Move getConnectedApps() from Profile to User, where it belongs Conflicts: classes/User.php --- actions/oauthconnectionssettings.php | 2 +- classes/Profile.php | 25 ------------ classes/User.php | 59 ++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/actions/oauthconnectionssettings.php b/actions/oauthconnectionssettings.php index 9a7cda924a..cdb73203f0 100644 --- a/actions/oauthconnectionssettings.php +++ b/actions/oauthconnectionssettings.php @@ -97,7 +97,7 @@ class OauthconnectionssettingsAction extends ConnectSettingsAction $offset = ($this->page - 1) * APPS_PER_PAGE; $limit = APPS_PER_PAGE + 1; - $connection = $profile->getConnectedApps($offset, $limit); + $connection = $user->getConnectedApps($offset, $limit); $cnt = 0; diff --git a/classes/Profile.php b/classes/Profile.php index 00e076a624..e29e6db7b7 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -409,31 +409,6 @@ class Profile extends Memcached_DataObject return $profile; } - function getConnectedApps($offset = 0, $limit = null) - { - $qry = - 'SELECT u.* ' . - 'FROM oauth_application_user u, oauth_application a ' . - 'WHERE u.profile_id = %d ' . - 'AND a.id = u.application_id ' . - 'AND u.access_type > 0 ' . - 'ORDER BY u.created DESC '; - - if ($offset > 0) { - if (common_config('db','type') == 'pgsql') { - $qry .= ' LIMIT ' . $limit . ' OFFSET ' . $offset; - } else { - $qry .= ' LIMIT ' . $offset . ', ' . $limit; - } - } - - $apps = new Oauth_application_user(); - - $cnt = $apps->query(sprintf($qry, $this->id)); - - return $apps; - } - function subscriptionCount() { $c = common_memcache(); diff --git a/classes/User.php b/classes/User.php index c824ddb0c2..1b1b971ec7 100644 --- a/classes/User.php +++ b/classes/User.php @@ -116,6 +116,16 @@ class User extends Memcached_DataObject return $result; } + /** + * Check whether the given nickname is potentially usable, or if it's + * excluded by any blacklists on this system. + * + * WARNING: INPUT IS NOT VALIDATED OR NORMALIZED. NON-NORMALIZED INPUT + * OR INVALID INPUT MAY LEAD TO FALSE RESULTS. + * + * @param string $nickname + * @return boolean true if clear, false if blacklisted + */ static function allowed_nickname($nickname) { // XXX: should already be validated for size, content, etc. @@ -949,4 +959,53 @@ class User extends Memcached_DataObject throw $e; } } + + /** + * Find and shorten links in the given text using this user's URL shortening + * settings. + * + * By default, links will be left untouched if the text is shorter than the + * configured maximum notice length. Pass true for the $always parameter + * to force all links to be shortened regardless. + * + * Side effects: may save file and file_redirection records for referenced URLs. + * + * @param string $text + * @param boolean $always + * @return string + */ + public function shortenLinks($text, $always=false) + { + return common_shorten_links($text, $always, $this); + } + + /* + * Get a list of OAuth client application that have access to this + * user's account. + */ + function getConnectedApps($offset = 0, $limit = null) + { + $qry = + 'SELECT u.* ' . + 'FROM oauth_application_user u, oauth_application a ' . + 'WHERE u.profile_id = %d ' . + 'AND a.id = u.application_id ' . + 'AND u.access_type > 0 ' . + 'ORDER BY u.created DESC '; + + if ($offset > 0) { + if (common_config('db','type') == 'pgsql') { + $qry .= ' LIMIT ' . $limit . ' OFFSET ' . $offset; + } else { + $qry .= ' LIMIT ' . $offset . ', ' . $limit; + } + } + + $apps = new Oauth_application_user(); + + $cnt = $apps->query(sprintf($qry, $this->id)); + + return $apps; + } + } From 3a24b95edb87a20302f5d9911763867a0fcb8277 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 19 Jan 2011 15:52:18 -0800 Subject: [PATCH 2/7] Fix a couple spelling mistakes in comments and remove redundant statement terminator --- classes/User.php | 2 +- lib/apiauth.php | 2 +- lib/apioauthstore.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/classes/User.php b/classes/User.php index 1b1b971ec7..b339d61702 100644 --- a/classes/User.php +++ b/classes/User.php @@ -980,7 +980,7 @@ class User extends Memcached_DataObject } /* - * Get a list of OAuth client application that have access to this + * Get a list of OAuth client applications that have access to this * user's account. */ function getConnectedApps($offset = 0, $limit = null) diff --git a/lib/apiauth.php b/lib/apiauth.php index 1dacf1409b..0cc184c04c 100644 --- a/lib/apiauth.php +++ b/lib/apiauth.php @@ -337,7 +337,7 @@ class ApiAuthAction extends ApiAction } /** - * Log an API authentication failer. Collect the proxy and IP + * Log an API authentication failure. Collect the proxy and IP * and log them * * @param string $logMsg additional log message diff --git a/lib/apioauthstore.php b/lib/apioauthstore.php index 2a65fffc4b..6aabde047c 100644 --- a/lib/apioauthstore.php +++ b/lib/apioauthstore.php @@ -229,7 +229,7 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore // insert a new Oauth_application_user record w/access token $appUser = new Oauth_application_user(); - $appUser->profile_id = $tokenAssoc->profile_id;; + $appUser->profile_id = $tokenAssoc->profile_id; $appUser->application_id = $app->id; $appUser->access_type = $app->access_type; $appUser->token = $at->tok; From 28d40e5eed82626be7543a0998cf4956b95034cc Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 19 Jan 2011 16:13:42 -0800 Subject: [PATCH 3/7] Fix syntax error in error msg --- actions/apioauthaccesstoken.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/apioauthaccesstoken.php b/actions/apioauthaccesstoken.php index 064d05120f..a3ce9edbf6 100644 --- a/actions/apioauthaccesstoken.php +++ b/actions/apioauthaccesstoken.php @@ -98,7 +98,7 @@ class ApiOauthAccessTokenAction extends ApiOauthAction common_log(LOG_WARNING, $msg); // TRANS: Client error given from the OAuth API when the request token or verifier is invalid. - $this->clientError(_("Invalid request token or verifier.", 400, 'text')); + $this->clientError(_("Invalid request token or verifier."), 400, 'text'); } else { common_log( LOG_INFO, From 05361bb6868dac96566ea477530c9ac148c37ae6 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 19 Jan 2011 22:55:00 -0800 Subject: [PATCH 4/7] OAuth: Fix rare problem in which request tokens were sometimes being returned as access tokens. --- actions/apioauthaccesstoken.php | 1 + lib/apioauthstore.php | 194 +++++++++++++++++++++++--------- 2 files changed, 141 insertions(+), 54 deletions(-) diff --git a/actions/apioauthaccesstoken.php b/actions/apioauthaccesstoken.php index a3ce9edbf6..6c3819c3bd 100644 --- a/actions/apioauthaccesstoken.php +++ b/actions/apioauthaccesstoken.php @@ -84,6 +84,7 @@ class ApiOauthAccessTokenAction extends ApiOauthAction common_debug(var_export($req, true)); $code = $e->getCode(); $this->clientError($e->getMessage(), empty($code) ? 401 : $code, 'text'); + return; } if (empty($atok)) { diff --git a/lib/apioauthstore.php b/lib/apioauthstore.php index 6aabde047c..f1f88b13a1 100644 --- a/lib/apioauthstore.php +++ b/lib/apioauthstore.php @@ -152,8 +152,11 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore ); } - // check to see if we have previously issued an access token for this application - // and profile + // Check to see if we have previously issued an access token for + // this application and profile; if so we can just return the + // existing access token. That seems to be the best practice. It + // makes it so users only have to authorize the app once per + // machine. $appUser = new Oauth_application_user(); @@ -172,19 +175,40 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore ) ); - $at = new Token(); + $at = null; - // fetch the full access token - $at->consumer_key = $consumer->key; - $at->tok = $appUser->token; + // Special case: we used to store request tokens in the + // Oauth_application_user record, and the access_type would + // always be 0 (no access) as a failsafe until an access + // token was issued and replaced the request token. There could + // be a few old Oauth_application_user records storing request + // tokens still around, and we don't want to accidentally + // return a useless request token instead of a new access + // token. So if we find one, we generate a new access token + // and update the existing Oauth_application_user record before + // returning the new access token. This should be rare. - $result = $at->find(true); + if ($appUser->access_type == 0) { - if (!$result) { - throw new Exception( - // TRANS: Exception thrown when no access token can be issued. - _('Could not issue access token.') - ); + $at = $this->generateNewAccessToken($consumer, $rt, $verifier); + $this->updateAppUser($appUser, $app, $at); + + } else { + + $at = new Token(); + + // fetch the full access token + $at->consumer_key = $consumer->key; + $at->tok = $appUser->token; + + $result = $at->find(true); + + if (!$result) { + throw new Exception( + // TRANS: Exception thrown when no access token can be issued. + _('Could not issue access token.') + ); + } } // Yay, we can re-issue the access token @@ -200,48 +224,8 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore ) ); - // make a brand new access token - $at = new Token(); - - $at->consumer_key = $consumer->key; - $at->tok = common_good_rand(16); - $at->secret = common_good_rand(16); - $at->type = 1; // access - $at->verifier = $verifier; - $at->verified_callback = $rt->verified_callback; // 1.0a - $at->created = common_sql_now(); - - if (!$at->insert()) { - $e = $at->_lastError; - common_debug('access token "' . $at->tok . '" not inserted: "' . $e->message . '"', __FILE__); - return null; - } else { - common_debug('access token "' . $at->tok . '" inserted', __FILE__); - // burn the old one - $orig_rt = clone($rt); - $rt->state = 2; // used - if (!$rt->update($orig_rt)) { - return null; - } - common_debug('request token "' . $rt->tok . '" updated', __FILE__); - } - - // insert a new Oauth_application_user record w/access token - $appUser = new Oauth_application_user(); - - $appUser->profile_id = $tokenAssoc->profile_id; - $appUser->application_id = $app->id; - $appUser->access_type = $app->access_type; - $appUser->token = $at->tok; - $appUser->created = common_sql_now(); - - $result = $appUser->insert(); - - if (!$result) { - common_log_db_error($appUser, 'INSERT', __FILE__); - // TRANS: Server error displayed when a database error occurs. - $this->serverError(_('Database error inserting OAuth application user.')); - } + $at = $this->generateNewAccessToken($consumer, $rt, $verifier); + $this->newAppUser($tokenAssoc, $app, $at); // Okay, good return new OAuthToken($at->tok, $at->secret); @@ -261,6 +245,108 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore } } + /* + * Generate a new access token and save it to the database + * + * @param Consumer $consumer the OAuth consumer + * @param Token $rt the authorized request token + * @param string $verifier the OAuth 1.0a verifier + * + * @access private + * + * @return Token $at the new access token + */ + private function generateNewAccessToken($consumer, $rt, $verifier) + { + $at = new Token(); + + $at->consumer_key = $consumer->key; + $at->tok = common_good_rand(16); + $at->secret = common_good_rand(16); + $at->type = 1; // access + $at->verifier = $verifier; + $at->verified_callback = $rt->verified_callback; // 1.0a + $at->created = common_sql_now(); + + if (!$at->insert()) { + $e = $at->_lastError; + common_debug('access token "' . $at->tok . '" not inserted: "' . $e->message . '"', __FILE__); + return null; + } else { + common_debug('access token "' . $at->tok . '" inserted', __FILE__); + // burn the old one + $orig_rt = clone($rt); + $rt->state = 2; // used + if (!$rt->update($orig_rt)) { + return null; + } + common_debug('request token "' . $rt->tok . '" updated', __FILE__); + } + + return $at; + } + + /* + * Add a new app user (Oauth_application_user) record + * + * @param Oauth_token_association $tokenAssoc token-to-app association + * @param Oauth_application $app the OAuth client app + * @param Token $at the access token + * + * @access private + * + * @return void + */ + private function newAppUser($tokenAssoc, $app, $at) + { + $appUser = new Oauth_application_user(); + + $appUser->profile_id = $tokenAssoc->profile_id; + $appUser->application_id = $app->id; + $appUser->access_type = $app->access_type; + $appUser->token = $at->tok; + $appUser->created = common_sql_now(); + + $result = $appUser->insert(); + + if (!$result) { + common_log_db_error($appUser, 'INSERT', __FILE__); + + // TRANS: Server error displayed when a database error occurs. + throw new Exception( + _('Database error inserting OAuth application user.') + ); + } + } + + /* + * Update an existing app user (Oauth_application_user) record + * + * @param Oauth_application_user $appUser existing app user rec + * @param Oauth_application $app the OAuth client app + * @param Token $at the access token + * + * @access private + * + * @return void + */ + private function updateAppUser($appUser, $app, $at) + { + $original = clone($appUser); + $appUser->access_type = $app->access_type; + $appUser->token = $at->tok; + + $result = $appUser->update($original); + + if (!$result) { + common_log_db_error($appUser, 'UPDATE', __FILE__); + // TRANS: Server error displayed when a database error occurs. + throw new Exception( + _('Database error updating OAuth application user.') + ); + } + } + /** * Revoke specified access token * From 73011e6344efa0bd73c0a11cef10c93b2764eb26 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 26 Jan 2011 10:49:14 -0800 Subject: [PATCH 5/7] Add IdentiCurse to notice sources --- db/notice_source.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/db/notice_source.sql b/db/notice_source.sql index 82074077b4..06b1348898 100644 --- a/db/notice_source.sql +++ b/db/notice_source.sql @@ -24,6 +24,7 @@ VALUES ('HelloTxt','HelloTxt','http://hellotxt.com/', now()), ('identicatools','Laconica Tools','http://bitbucketlabs.net/laconica-tools/', now()), ('identichat','identichat','http://identichat.prosody.im/', now()), + ('IdentiCurse','IdentiCurse','http://identicurse.net/', now()), ('IdentiFox','IdentiFox','http://www.bitbucket.org/uncryptic/identifox/', now()), ('identitwitch','IdentiTwitch','http://richfish.org/identitwitch/', now()), ('Jiminy','Jiminy','http://code.google.com/p/jiminy/', now()), From 433ec211199aedc98e7d6949a17d6b5c2d0932f3 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 27 Jan 2011 12:07:29 -0800 Subject: [PATCH 6/7] Add $config['sessions']['gc_limit'] to limit how much work we do in each session GC; defaulting to killing 1000 sessions at a time. --- classes/Session.php | 7 +++++++ lib/default.php | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/classes/Session.php b/classes/Session.php index e1c83ad4dc..166b89815a 100644 --- a/classes/Session.php +++ b/classes/Session.php @@ -156,6 +156,13 @@ class Session extends Memcached_DataObject $session->selectAdd(); $session->selectAdd('id'); + $limit = common_config('sessions', 'gc_limit'); + if ($limit > 0) { + // On large sites, too many sessions to expire + // at once will just result in failure. + $session->limit($limit); + } + $session->find(); while ($session->fetch()) { diff --git a/lib/default.php b/lib/default.php index 85d27f5220..4b28e3238b 100644 --- a/lib/default.php +++ b/lib/default.php @@ -261,8 +261,9 @@ $default = 'search' => array('type' => 'fulltext'), 'sessions' => - array('handle' => false, // whether to handle sessions ourselves - 'debug' => false), // debugging output for sessions + array('handle' => false, // whether to handle sessions ourselves + 'debug' => false, // debugging output for sessions + 'gc_limit' => 1000), // max sessions to expire at a time 'design' => array('backgroundcolor' => null, // null -> 'use theme default' 'contentcolor' => null, From a7abb2323e7d57125b9fbc903a1cecc06c27944e Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 27 Jan 2011 12:27:31 -0800 Subject: [PATCH 7/7] Session GC fix: save session.modified field as UTC so our comparisons work. Had to tweak statusnet.ini to remove the DB_DATAOBJECT_MYSQLTIMESTAMP bitfield constant on session.modified; while it sounds like a useful and legit setting, it actually just means that DB_DataObject silently fails to pass through any attempts to explicitly set the value. As a result, MySQL does its default behavior which is to insert the current *LOCAL* time, which is useless. This was leading to early GC west of GMT, or late GC east of it. Early GC could at worst destroy all live sessions (whoever's session *triggered* GC is fine, as the session then gets saved right back.) --- classes/Session.php | 2 ++ classes/statusnet.ini | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/classes/Session.php b/classes/Session.php index 166b89815a..b9daf364db 100644 --- a/classes/Session.php +++ b/classes/Session.php @@ -87,6 +87,7 @@ class Session extends Memcached_DataObject $session->id = $id; $session->session_data = $session_data; $session->created = common_sql_now(); + $session->modified = common_sql_now(); $result = $session->insert(); @@ -108,6 +109,7 @@ class Session extends Memcached_DataObject $orig = clone($session); $session->session_data = $session_data; + $session->modified = common_sql_now(); $result = $session->update($orig); diff --git a/classes/statusnet.ini b/classes/statusnet.ini index ef631e28d3..29fde93b5d 100644 --- a/classes/statusnet.ini +++ b/classes/statusnet.ini @@ -513,7 +513,20 @@ profile_id = K id = 130 session_data = 34 created = 142 -modified = 384 +modified = 142 +; Warning: using DB_DATAOBJECT_MYSQLTIMESTAMP (256) causes DB_DataObject +; to SILENTLY REMOVE ATTEMPTS TO SET THIS FIELD DIRECTLY, which is pretty +; bad because the default behavior for auto-updated TIMESTAMP fields is +; to use local time. Local time can't be compared to UTC in any useful +; way, so doing that breaks session GC. +; +; Instead we'll use the plain datetime settings so it'll actually save the +; UTC value we provide when updating. +; +; Long-term fix: punch MySQL in the face until it understands that local +; time is a tool of the cyber-devil. +; +;modified = 384 [session__keys] id = K