From c5a5eaf288a55539f91bd51e980770b9a2d82087 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 11 Jul 2015 19:46:01 +0200 Subject: [PATCH 01/40] Do we update feeduri and salmonuri for Ostatus_profile now? When changing from HTTP to HTTPS, following up on commit 59763ceecb33ebf842829cac15f922fa19047de2 where http to https Ostatus_profile URI changing was first introduced. --- plugins/OStatus/classes/Ostatus_profile.php | 32 +++++++++++++++++++++ plugins/OStatus/lib/salmonaction.php | 7 +---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index cb961dc96b..1c4428b16d 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -1977,6 +1977,38 @@ class Ostatus_profile extends Managed_DataObject return $oprofile->localProfile(); } + + public function updateUriKeys($profile_uri, array $hints=array()) + { + $orig = clone($this); + + common_debug('URIFIX These identities both say they are each other: "'.$orig->uri.'" and "'.$profile_uri.'"'); + $this->uri = $profile_uri; + + if (array_key_exists('feedurl', $hints)) { + if (!empty($this->feeduri)) { + common_debug('URIFIX Changing FeedSub ['.$feedsub->id.'] feeduri "'.$feedsub->uri.'" to "'.$hints['feedurl']); + $feedsub = FeedSub::getKV('uri', $this->feeduri); + $feedorig = clone($feedsub); + $feedsub->uri = $hints['feedurl']; + $feedsub->updateWithKeys($feedorig); + } else { + common_debug('URIFIX Old Ostatus_profile did not have feedurl set, ensuring feed: '.$hints['feedurl']); + FeedSub::ensureFeed($hints['feedurl']); + } + $this->feeduri = $hints['feedurl']; + } + if (array_key_exists('salmon', $hints)) { + common_debug('URIFIX Changing Ostatus_profile salmonuri from "'.$this->salmonuri.'" to "'.$hints['salmon'].'"'); + $this->salmonuri = $hints['salmon']; + } + + common_debug('URIFIX Updating Ostatus_profile URI for '.$orig->uri.' to '.$this->uri); + $this->updateWithKeys($orig, 'uri'); // 'uri' is the primary key column + + common_debug('URIFIX Subscribing/renewing feedsub for Ostatus_profile '.$this->uri); + $this->subscribe(); + } } /** diff --git a/plugins/OStatus/lib/salmonaction.php b/plugins/OStatus/lib/salmonaction.php index 5193d302f1..320ea6cdfa 100644 --- a/plugins/OStatus/lib/salmonaction.php +++ b/plugins/OStatus/lib/salmonaction.php @@ -259,12 +259,7 @@ class SalmonAction extends Action // Step 4: Is the newly introduced https://example.com/user/1 URI in the list of aliases // presented by http://example.com/user/1 (i.e. do they both say they are the same identity?) if (in_array($e->object_uri, $doublecheck_aliases)) { - common_debug('URIFIX These identities both say they are each other: "'.$aliased_uri.'" and "'.$e->object_uri.'"'); - $orig = clone($oprofile); - $oprofile->uri = $e->object_uri; - common_debug('URIFIX Updating Ostatus_profile URI for '.$aliased_uri.' to '.$oprofile->uri); - $oprofile->updateWithKeys($orig, 'uri'); // 'uri' is the primary key column - unset($orig); + $oprofile->updateUriKeys($e->object_uri, DiscoveryHints::fromXRD($xrd)); $this->oprofile = $oprofile; break; // don't iterate through aliases anymore } From e868ac41cd8221c45dfed56a43d155fb2307a7e4 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 11 Jul 2015 19:48:18 +0200 Subject: [PATCH 02/40] userrss action didn't call parent preparation method --- actions/userrss.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/userrss.php b/actions/userrss.php index fd49a0e899..7bed1dd256 100644 --- a/actions/userrss.php +++ b/actions/userrss.php @@ -27,18 +27,20 @@ class UserrssAction extends TargetedRss10Action protected function doStreamPreparation() { + parent::doStreamPreparation(); + $this->tag = $this->trimmed('tag'); } protected function getNotices() { if (!empty($this->tag)) { - $stream = $this->target->getTaggedNotices($this->tag, 0, $this->limit); + $stream = $this->getTarget()->getTaggedNotices($this->tag, 0, $this->limit); return $stream->fetchAll(); } // otherwise we fetch a normal user stream - $stream = $this->target->getNotices(0, $this->limit); + $stream = $this->getTarget()->getNotices(0, $this->limit); return $stream->fetchAll(); } From 01a4ab30dc1dfbf4d6ee6a42a7f8f45a1a42894d Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 14 Jul 2015 16:52:20 +0200 Subject: [PATCH 03/40] Removing MicroID as well as simplifying profileaction sorry, forgot to commit in between --- actions/emailsettings.php | 8 --- actions/groupbyid.php | 7 +-- actions/imsettings.php | 4 +- actions/shownotice.php | 15 +----- actions/showstream.php | 39 --------------- classes/User.php | 4 -- classes/User_im_prefs.php | 2 - lib/galleryaction.php | 29 ----------- lib/implugin.php | 24 --------- lib/microid.php | 97 ------------------------------------- lib/profileaction.php | 35 +++++++++++-- plugins/Xmpp/XmppPlugin.php | 5 -- 12 files changed, 33 insertions(+), 236 deletions(-) delete mode 100644 lib/microid.php diff --git a/actions/emailsettings.php b/actions/emailsettings.php index a0f111c0d5..cffc5da247 100644 --- a/actions/emailsettings.php +++ b/actions/emailsettings.php @@ -231,12 +231,6 @@ class EmailsettingsAction extends SettingsAction _('Allow friends to nudge me and send me an email.'), $user->emailnotifynudge); $this->elementEnd('li'); - $this->elementStart('li'); - $this->checkbox('emailmicroid', - // TRANS: Checkbox label in e-mail preferences form. - _('Publish a MicroID for my email address.'), - $user->emailmicroid); - $this->elementEnd('li'); Event::handle('EndEmailFormData', array($this, $this->scoped)); } $this->elementEnd('ul'); @@ -320,7 +314,6 @@ class EmailsettingsAction extends SettingsAction $emailnotifymsg = $this->booleanintstring('emailnotifymsg'); $emailnotifynudge = $this->booleanintstring('emailnotifynudge'); $emailnotifyattn = $this->booleanintstring('emailnotifyattn'); - $emailmicroid = $this->booleanintstring('emailmicroid'); $emailpost = $this->booleanintstring('emailpost'); $user->query('BEGIN'); @@ -331,7 +324,6 @@ class EmailsettingsAction extends SettingsAction $user->emailnotifymsg = $emailnotifymsg; $user->emailnotifynudge = $emailnotifynudge; $user->emailnotifyattn = $emailnotifyattn; - $user->emailmicroid = $emailmicroid; $user->emailpost = $emailpost; $result = $user->update($original); diff --git a/actions/groupbyid.php b/actions/groupbyid.php index b82a861e97..8556675155 100644 --- a/actions/groupbyid.php +++ b/actions/groupbyid.php @@ -28,12 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - -require_once INSTALLDIR.'/lib/noticelist.php'; -require_once INSTALLDIR.'/lib/feedlist.php'; +if (!defined('GNUSOCIAL')) { exit(1); } /** * Permalink for a group diff --git a/actions/imsettings.php b/actions/imsettings.php index 92fff45a7d..feb5d32fc1 100644 --- a/actions/imsettings.php +++ b/actions/imsettings.php @@ -179,8 +179,6 @@ class ImsettingsAction extends SettingsAction // TRANS: Checkbox label in IM preferences form. array('name'=>'replies', 'description'=>_('Send me replies '. 'from people I\'m not subscribed to.')), - // TRANS: Checkbox label in IM preferences form. - array('name'=>'microid', 'description'=>_('Publish a MicroID')) ); foreach($preferences as $preference) { @@ -277,7 +275,7 @@ class ImsettingsAction extends SettingsAction $user_im_prefs->user_id = $user->id; if($user_im_prefs->find() && $user_im_prefs->fetch()) { - $preferences = array('notify', 'updatefrompresence', 'replies', 'microid'); + $preferences = array('notify', 'updatefrompresence', 'replies'); do { $original = clone($user_im_prefs); diff --git a/actions/shownotice.php b/actions/shownotice.php index 4a1adfd7de..23386868dd 100644 --- a/actions/shownotice.php +++ b/actions/shownotice.php @@ -222,25 +222,12 @@ class ShownoticeAction extends ManagedAction /** * Extra content * - * We show the microid(s) for the author, if any. + * Facebook OpenGraph metadata. * * @return void */ function extraHead() { - $user = User::getKV($this->profile->id); - - if (!$user instanceof User) { - return; - } - - if ($user->emailmicroid && $user->email && $this->notice->uri) { - $id = new Microid('mailto:'. $user->email, - $this->notice->uri); - $this->element('meta', array('name' => 'microid', - 'content' => $id->toString())); - } - // Extras to aid in sharing notices to Facebook $avatarUrl = $this->profile->avatarUrl(AVATAR_PROFILE_SIZE); $this->element('meta', array('property' => 'og:image', diff --git a/actions/showstream.php b/actions/showstream.php index 51384eb487..890c1e711b 100644 --- a/actions/showstream.php +++ b/actions/showstream.php @@ -47,38 +47,6 @@ if (!defined('GNUSOCIAL')) { exit(1); } */ class ShowstreamAction extends NoticestreamAction { - protected $target = null; - - protected function doPreparation() - { - // showstream requires a nickname - $nickname_arg = $this->trimmed('nickname'); - $nickname = common_canonical_nickname($nickname_arg); - - // Permanent redirect on non-canonical nickname - if ($nickname_arg != $nickname) { - $args = array('nickname' => $nickname); - if ($this->arg('page') && $this->arg('page') != 1) { - $args['page'] = $this->arg['page']; - } - common_redirect(common_local_url($this->getActionName(), $args), 301); - } - - try { - $user = User::getByNickname($nickname); - } catch (NoSuchUserException $e) { - $group = Local_group::getKV('nickname', $nickname); - if ($group instanceof Local_group) { - common_redirect($group->getProfile()->getUrl()); - } - - // No user nor group found, throw the NoSuchUserException again - throw $e; - } - - $this->target = $user->getProfile(); - } - public function getStream() { if (empty($this->tag)) { @@ -195,13 +163,6 @@ class ShowstreamAction extends NoticestreamAction 'content' => $this->target->getDescription())); } - if ($this->target->isLocal() && $this->target->getUser()->emailmicroid && $this->target->getUser()->email && $this->target->getUrl()) { - $id = new Microid('mailto:'.$this->target->getUser()->email, - $this->selfUrl()); - $this->element('meta', array('name' => 'microid', - 'content' => $id->toString())); - } - // See https://wiki.mozilla.org/Microsummaries $this->element('link', array('rel' => 'microsummary', diff --git a/classes/User.php b/classes/User.php index ec6ceccf12..e1ce33be18 100644 --- a/classes/User.php +++ b/classes/User.php @@ -42,7 +42,6 @@ class User extends Managed_DataObject public $emailnotifynudge; // tinyint(1) default_1 public $emailnotifymsg; // tinyint(1) default_1 public $emailnotifyattn; // tinyint(1) default_1 - public $emailmicroid; // tinyint(1) default_1 public $language; // varchar(50) public $timezone; // varchar(50) public $emailpost; // tinyint(1) default_1 @@ -77,7 +76,6 @@ class User extends Managed_DataObject 'emailnotifynudge' => array('type' => 'int', 'size' => 'tiny', 'default' => 1, 'description' => 'Notify by email of nudges'), 'emailnotifymsg' => array('type' => 'int', 'size' => 'tiny', 'default' => 1, 'description' => 'Notify by email of direct messages'), 'emailnotifyattn' => array('type' => 'int', 'size' => 'tiny', 'default' => 1, 'description' => 'Notify by email of @-replies'), - 'emailmicroid' => array('type' => 'int', 'size' => 'tiny', 'default' => 1, 'description' => 'whether to publish email microid'), 'language' => array('type' => 'varchar', 'length' => 50, 'description' => 'preferred language'), 'timezone' => array('type' => 'varchar', 'length' => 50, 'description' => 'timezone'), 'emailpost' => array('type' => 'int', 'size' => 'tiny', 'default' => 1, 'description' => 'Post by email'), @@ -276,9 +274,7 @@ class User extends Managed_DataObject $user->emailnotifynudge = 1; $user->emailnotifymsg = 1; $user->emailnotifyattn = 1; - $user->emailmicroid = 1; $user->emailpost = 1; - $user->jabbermicroid = 1; $user->created = common_sql_now(); diff --git a/classes/User_im_prefs.php b/classes/User_im_prefs.php index 16fd030bb4..6a5fc7f3ab 100644 --- a/classes/User_im_prefs.php +++ b/classes/User_im_prefs.php @@ -40,7 +40,6 @@ class User_im_prefs extends Managed_DataObject public $transport; // varchar(191) not_null not 255 because utf8mb4 takes more space public $notify; // tinyint(1) public $replies; // tinyint(1) - public $microid; // tinyint(1) public $updatefrompresence; // tinyint(1) public $created; // datetime not_null default_0000-00-00%2000%3A00%3A00 public $modified; // timestamp not_null default_CURRENT_TIMESTAMP @@ -57,7 +56,6 @@ class User_im_prefs extends Managed_DataObject 'transport' => array('type' => 'varchar', 'length' => 191, 'not null' => true, 'description' => 'transport (ex xmpp, aim)'), 'notify' => array('type' => 'int', 'size' => 'tiny', 'not null' => true, 'default' => 0, 'description' => 'Notify when a new notice is sent'), 'replies' => array('type' => 'int', 'size' => 'tiny', 'not null' => true, 'default' => 0, 'description' => 'Send replies from people not subscribed to'), - 'microid' => array('type' => 'int', 'size' => 'tiny', 'not null' => true, 'default' => 1, 'description' => 'Publish a MicroID'), 'updatefrompresence' => array('type' => 'int', 'size' => 'tiny', 'not null' => true, 'default' => 0, 'description' => 'Send replies from people not subscribed to.'), 'created' => array('type' => 'datetime', 'not null' => true, 'description' => 'date this record was created'), 'modified' => array('type' => 'timestamp', 'not null' => true, 'description' => 'date this record was modified'), diff --git a/lib/galleryaction.php b/lib/galleryaction.php index f87043ac06..5dd0cfcfa7 100644 --- a/lib/galleryaction.php +++ b/lib/galleryaction.php @@ -36,35 +36,6 @@ class GalleryAction extends ProfileAction parent::handle(); } - protected function doPreparation() - { - // showstream requires a nickname - $nickname_arg = $this->arg('nickname'); - $nickname = common_canonical_nickname($nickname_arg); - - // Permanent redirect on non-canonical nickname - - if ($nickname_arg != $nickname) { - $args = array('nickname' => $nickname); - if ($this->arg('page') && $this->arg('page') != 1) { - $args['page'] = $this->arg['page']; - } - common_redirect(common_local_url($this->getActionName(), $args), 301); - } - $this->user = User::getKV('nickname', $nickname); - - if (!$this->user) { - $group = Local_group::getKV('nickname', $nickname); - if ($group instanceof Local_group) { - common_redirect($group->getProfile()->getUrl()); - } - // TRANS: Client error displayed when calling a profile action without specifying a user. - $this->clientError(_('No such user.'), 404); - } - - $this->target = $this->user->getProfile(); - } - function showContent() { $this->showTagsDropdown(); diff --git a/lib/implugin.php b/lib/implugin.php index 87ca037160..6395ecbdb7 100644 --- a/lib/implugin.php +++ b/lib/implugin.php @@ -126,17 +126,6 @@ abstract class ImPlugin extends Plugin */ abstract function daemonScreenname(); - /** - * get the microid uri of a given screenname - * - * @param string $screenname screenname - * - * @return string microid uri - */ - function microiduri($screenname) - { - return $this->transport . ':' . $screenname; - } //========================UTILITY FUNCTIONS USEFUL TO IMPLEMENTATIONS - MISC ========================\ /** @@ -571,25 +560,12 @@ abstract class ImPlugin extends Plugin $user_im_prefs->user_id = $action->notice->getProfile()->getID(); $user_im_prefs->transport = $this->transport; - if ($user_im_prefs->find() && $user_im_prefs->fetch() && $user_im_prefs->microid && $action->notice->uri) { - $id = new Microid($this->microiduri($user_im_prefs->screenname), - $action->notice->uri); - $action->element('meta', array('name' => 'microid', - 'content' => $id->toString())); - } - } elseif ($action instanceof ShowstreamAction) { $user_im_prefs = new User_im_prefs(); $user_im_prefs->user_id = $action->getTarget()->getID(); $user_im_prefs->transport = $this->transport; - if ($user_im_prefs->find() && $user_im_prefs->fetch() && $user_im_prefs->microid && $action->getTarget()->getUrl()) { - $id = new Microid($this->microiduri($user_im_prefs->screenname), - $action->selfUrl()); - $action->element('meta', array('name' => 'microid', - 'content' => $id->toString())); - } } } diff --git a/lib/microid.php b/lib/microid.php deleted file mode 100644 index e2e7d7607f..0000000000 --- a/lib/microid.php +++ /dev/null @@ -1,97 +0,0 @@ -. - * - * @category ID - * @package StatusNet - * @author Evan Prodromou - * @copyright 2008 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/ - */ - -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - -/** - * A class for microids - * - * @category ID - * @package StatusNet - * @author Evan Prodromou - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ - * @see http://microid.org/ - */ - -class Microid -{ - /** Agent part of the ID. */ - - var $agent = null; - - /** Resource part of the ID. */ - - var $resource = null; - - /** - * Constructor - * - * @param string $agent Agent of the ID - * @param string $resource Resource part - */ - - function __construct($agent, $resource) - { - $this->agent = $agent; - $this->resource = $resource; - - } - - /** - * Generate a MicroID string - * - * @return string MicroID for agent and resource - */ - - function toString() - { - $agent_proto = $this->_getProto($this->agent); - $resource_proto = $this->_getProto($this->resource); - - return $agent_proto.'+'.$resource_proto.':sha1:'. - sha1(sha1($this->agent).sha1($this->resource)); - } - - /** - * Utility for getting the protocol part of a URI - * - * @param string $uri URI to parse - * - * @return string scheme part of the URI - */ - - function _getProto($uri) - { - $colon = strpos($uri, ':'); - return substr($uri, 0, $colon); - } -} diff --git a/lib/profileaction.php b/lib/profileaction.php index 5a5d526e42..3dc28a7cc4 100644 --- a/lib/profileaction.php +++ b/lib/profileaction.php @@ -48,6 +48,36 @@ abstract class ProfileAction extends ManagedAction protected $target = null; // Profile that we're showing + protected function doPreparation() + { + // showstream requires a nickname + $nickname_arg = $this->trimmed('nickname'); + $nickname = common_canonical_nickname($nickname_arg); + + // Permanent redirect on non-canonical nickname + if ($nickname_arg != $nickname) { + $args = array('nickname' => $nickname); + if ($this->arg('page') && $this->arg('page') != 1) { + $args['page'] = $this->arg['page']; + } + common_redirect(common_local_url($this->getActionName(), $args), 301); + } + + try { + $user = User::getByNickname($nickname); + } catch (NoSuchUserException $e) { + $group = Local_group::getKV('nickname', $nickname); + if ($group instanceof Local_group) { + common_redirect($group->getProfile()->getUrl()); + } + + // No user nor group found, throw the NoSuchUserException again + throw $e; + } + + $this->target = $user->getProfile(); + } + protected function prepare(array $args=array()) { // this will call ->doPreparation() which child classes use to set $this->target @@ -65,11 +95,6 @@ abstract class ProfileAction extends ManagedAction return true; } - protected function profileActionPreparation() - { - // Nothing to do by default. - } - public function getTarget() { if (!$this->target instanceof Profile) { diff --git a/plugins/Xmpp/XmppPlugin.php b/plugins/Xmpp/XmppPlugin.php index 0a70268735..04101a8e2b 100644 --- a/plugins/Xmpp/XmppPlugin.php +++ b/plugins/Xmpp/XmppPlugin.php @@ -308,11 +308,6 @@ class XmppPlugin extends ImPlugin return true; } - function microiduri($screenname) - { - return 'xmpp:' . $screenname; - } - function sendMessage($screenname, $body) { $this->queuedConnection()->message($screenname, $body, 'chat'); From cd23c78800b0e2b80200f8e4b1156190330f39c9 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Wed, 15 Jul 2015 19:21:21 +0200 Subject: [PATCH 04/40] Less redundant code. --- classes/Memcached_DataObject.php | 2 +- classes/Notice.php | 22 ++++++---------------- lib/util.php | 28 ++++++++++------------------ 3 files changed, 17 insertions(+), 35 deletions(-) diff --git a/classes/Memcached_DataObject.php b/classes/Memcached_DataObject.php index c1f6f644db..41ce715210 100644 --- a/classes/Memcached_DataObject.php +++ b/classes/Memcached_DataObject.php @@ -74,7 +74,7 @@ class Memcached_DataObject extends Safe_DataObject { $obj = new $cls; - // php-compatible, for settype(), datatype + // PHP compatible datatype for settype() below $colType = $obj->columnType($keyCol); if (!in_array($colType, array('integer', 'int'))) { diff --git a/classes/Notice.php b/classes/Notice.php index ae722138b3..6301f9ce62 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -1656,32 +1656,22 @@ class Notice extends Managed_DataObject protected $_replies = array(); /** - * Pull the complete list of @-reply targets for this notice. + * Pull the complete list of @-mentioned profile IDs for this notice. * * @return array of integer profile ids */ function getReplies() { - if (isset($this->_replies[$this->id])) { - return $this->_replies[$this->id]; + if (!isset($this->_replies[$this->getID()])) { + $mentions = Reply::multiGet('notice_id', array($this->getID())); + $this->_replies[$this->getID()] = $mentions->fetchAll('profile_id'); } - - $replyMap = Reply::listGet('notice_id', array($this->id)); - - $ids = array(); - - foreach ($replyMap[$this->id] as $reply) { - $ids[] = $reply->profile_id; - } - - $this->_replies[$this->id] = $ids; - - return $ids; + return $this->_replies[$this->getID()]; } function _setReplies($replies) { - $this->_replies[$this->id] = $replies; + $this->_replies[$this->getID()] = $replies; } /** diff --git a/lib/util.php b/lib/util.php index f29d9559b9..4d69651089 100644 --- a/lib/util.php +++ b/lib/util.php @@ -710,25 +710,17 @@ function common_find_mentions($text, Notice $notice) // Is it a reply? - if ($notice instanceof Notice) { + $origNotice = $notice->getParent(); + $origAuthor = $origNotice->getProfile(); + + $ids = $origNotice->getReplies(); + + foreach ($ids as $id) { try { - $origNotice = $notice->getParent(); - $origAuthor = $origNotice->getProfile(); - - $ids = $origNotice->getReplies(); - - foreach ($ids as $id) { - $repliedTo = Profile::getKV('id', $id); - if ($repliedTo instanceof Profile) { - $origMentions[$repliedTo->nickname] = $repliedTo; - } - } - } catch (NoProfileException $e) { - common_log(LOG_WARNING, sprintf('Notice %d author profile id %d does not exist', $origNotice->id, $origNotice->profile_id)); - } catch (NoParentNoticeException $e) { - // This notice is not in reply to anything - } catch (Exception $e) { - common_log(LOG_WARNING, __METHOD__ . ' got exception ' . get_class($e) . ' : ' . $e->getMessage()); + $repliedTo = Profile::getByID($id); + $origMentions[$repliedTo->nickname] = $repliedTo; + } catch (NoResultException $e) { + // continue foreach } } From 44dc00a58c4d0f3d78e7b6dc2cee13959e480aad Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 12:53:10 +0200 Subject: [PATCH 05/40] Non-replies cannot harvest parent notice nicknames A feature we use of parent notices is that if you use the same @user as the parent notice, the same @user will be notified, regardless if there might be @user@site.com as well as @user@example.com and you're subscribed to just one of them (or both, or none of them!). But this threw an exception since we tested this on new notice threads. --- lib/util.php | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/util.php b/lib/util.php index 4d69651089..4949231c52 100644 --- a/lib/util.php +++ b/lib/util.php @@ -710,18 +710,22 @@ function common_find_mentions($text, Notice $notice) // Is it a reply? - $origNotice = $notice->getParent(); - $origAuthor = $origNotice->getProfile(); + try { + $origNotice = $notice->getParent(); + $origAuthor = $origNotice->getProfile(); - $ids = $origNotice->getReplies(); + $ids = $origNotice->getReplies(); - foreach ($ids as $id) { - try { - $repliedTo = Profile::getByID($id); - $origMentions[$repliedTo->nickname] = $repliedTo; - } catch (NoResultException $e) { - // continue foreach + foreach ($ids as $id) { + try { + $repliedTo = Profile::getByID($id); + $origMentions[$repliedTo->getNickname()] = $repliedTo; + } catch (NoResultException $e) { + // continue foreach + } } + } catch (NoParentNoticeException $e) { + // It wasn't a reply to anything, so we can't harvest nickname-relations. } $matches = common_find_mentions_raw($text); From 94d54ebc29ea9f7dddc9963e47a7cb68fe053048 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 18:45:59 +0200 Subject: [PATCH 06/40] Function declarations to match parent class --- lib/apignusocialoauthdatastore.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/apignusocialoauthdatastore.php b/lib/apignusocialoauthdatastore.php index da412df8d2..6f4403dec7 100644 --- a/lib/apignusocialoauthdatastore.php +++ b/lib/apignusocialoauthdatastore.php @@ -26,20 +26,20 @@ require_once 'OAuth.php'; */ class ApiGNUsocialOAuthDataStore extends OAuthDataStore { - function lookup_consumer($consumerKey) + function lookup_consumer($consumer_key) { - $con = Consumer::getKV('consumer_key', $consumerKey); + $con = Consumer::getKV('consumer_key', $consumer_key); if (!$con instanceof Consumer) { // Create an anon consumer and anon application if one // doesn't exist already - if ($consumerKey == 'anonymous') { + if ($consumer_key == 'anonymous') { common_debug("API OAuth - creating anonymous consumer"); $con = new Consumer(); - $con->consumer_key = $consumerKey; - $con->consumer_secret = $consumerKey; + $con->consumer_key = $consumer_key; + $con->consumer_secret = $consumer_key; $con->created = common_sql_now(); $result = $con->insert(); @@ -388,7 +388,7 @@ class ApiGNUsocialOAuthDataStore extends OAuthDataStore * * @return OAuthToken $token a new unauthorized OAuth request token */ - function new_request_token($consumer, $callback) + function new_request_token($consumer, $callback = null) { $t = new Token(); $t->consumer_key = $consumer->key; @@ -473,13 +473,13 @@ class ApiGNUsocialOAuthDataStore extends OAuthDataStore * @param type $token_key * @return OAuthToken */ - function lookup_token($consumer, $token_type, $token_key) + function lookup_token($consumer, $token_type, $token) { $t = new Token(); if (!is_null($consumer)) { $t->consumer_key = $consumer->key; } - $t->tok = $token_key; + $t->tok = $token; $t->type = ($token_type == 'access') ? 1 : 0; if ($t->find(true)) { return new OAuthToken($t->tok, $t->secret); From 673bef2fdae9db750f4daf5bfe7d639eb2ec2acc Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 18:52:43 +0200 Subject: [PATCH 07/40] OAuth widgets separated into their own files --- actions/oauthappssettings.php | 6 +- actions/oauthconnectionssettings.php | 6 +- lib/applicationlist.php | 145 +---------------------- lib/connectedappslist.php | 167 +++++++++++++++++++++++++++ lib/framework.php | 1 + 5 files changed, 171 insertions(+), 154 deletions(-) create mode 100644 lib/connectedappslist.php diff --git a/actions/oauthappssettings.php b/actions/oauthappssettings.php index 29e6d56073..e9b6280feb 100644 --- a/actions/oauthappssettings.php +++ b/actions/oauthappssettings.php @@ -27,11 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - -require_once INSTALLDIR . '/lib/applicationlist.php'; +if (!defined('GNUSOCIAL')) { exit(1); } /** * Show a user's registered OAuth applications diff --git a/actions/oauthconnectionssettings.php b/actions/oauthconnectionssettings.php index 9aa3ad434f..a3ba7eda39 100644 --- a/actions/oauthconnectionssettings.php +++ b/actions/oauthconnectionssettings.php @@ -27,11 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - -require_once INSTALLDIR . '/lib/applicationlist.php'; +if (!defined('GNUSOCIAL')) { exit(1); } /** * Show connected OAuth applications diff --git a/lib/applicationlist.php b/lib/applicationlist.php index d0c9256df7..b2cc572e3e 100644 --- a/lib/applicationlist.php +++ b/lib/applicationlist.php @@ -27,13 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - -require_once INSTALLDIR . '/lib/widget.php'; - -define('APPS_PER_PAGE', 20); +if (!defined('GNUSOCIAL')) { exit(1); } /** * Widget to show a list of OAuth applications @@ -119,140 +113,3 @@ class ApplicationList extends Widget return; } } - -/** - * Widget to show a list of connected OAuth clients - * - * @category Application - * @package StatusNet - * @author Zach Copley - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ - */ -class ConnectedAppsList extends Widget -{ - /** Current connected application query */ - var $connection = null; - - /** Owner of this list */ - var $owner = null; - - /** Action object using us. */ - var $action = null; - - function __construct($connection, $owner=null, $action=null) - { - parent::__construct($action); - - common_debug("ConnectedAppsList constructor"); - - $this->connection = $connection; - $this->owner = $owner; - $this->action = $action; - } - - /* Override this in subclasses. */ - function showOwnerControls() - { - return; - } - - function show() - { - $this->out->elementStart('ul', 'applications'); - - $cnt = 0; - - while ($this->connection->fetch()) { - $cnt++; - if($cnt > APPS_PER_PAGE) { - break; - } - $this->showConnection(); - } - - $this->out->elementEnd('ul'); - - return $cnt; - } - - function showConnection() - { - $app = Oauth_application::getKV('id', $this->connection->application_id); - - $this->out->elementStart('li', array('class' => 'application h-entry', - 'id' => 'oauthclient-' . $app->id)); - - $this->out->elementStart('a', array('href' => $app->source_url, - 'class' => 'h-card p-name')); - - if (!empty($app->icon)) { - $this->out->element('img', array('src' => $app->icon, - 'class' => 'avatar u-photo')); - } - if ($app->name != 'anonymous') { - $this->out->text($app->name); - } else { - // TRANS: Name for an anonymous application in application list. - $this->out->element('span', 'p-name', _('Unknown application')); - } - $this->out->elementEnd('a'); - - if ($app->name != 'anonymous') { - // @todo FIXME: i18n trouble. - // TRANS: Message has a leading space and a trailing space. Used in application list. - // TRANS: Before this message the application name is put, behind it the organisation that manages it. - $this->out->raw(_(' by ')); - - $this->out->element('a', array('href' => $app->homepage, - 'class' => 'h-card'), - $app->organization); - } - - // TRANS: Application access type - $readWriteText = _('read-write'); - // TRANS: Application access type - $readOnlyText = _('read-only'); - - $access = ($this->connection->access_type & Oauth_application::$writeAccess) - ? $readWriteText : $readOnlyText; - $modifiedDate = common_date_string($this->connection->modified); - // TRANS: Used in application list. %1$s is a modified date, %2$s is access type ("read-write" or "read-only") - $txt = sprintf(_('Approved %1$s - "%2$s" access.'), $modifiedDate, $access); - - // @todo FIXME: i18n trouble. - $this->out->raw(" - $txt"); - if (!empty($app->description)) { - $this->out->element( - 'p', array('class' => 'application_description'), - $app->description - ); - } - $this->out->element( - 'p', array( - 'class' => 'access_token'), - // TRANS: Access token in the application list. - // TRANS: %s are the first 7 characters of the access token. - sprintf(_('Access token starting with: %s'), substr($this->connection->token, 0, 7)) - ); - - $this->out->elementStart( - 'form', - array( - 'id' => 'form_revoke_app', - 'class' => 'form_revoke_app', - 'method' => 'POST', - 'action' => common_local_url('oauthconnectionssettings') - ) - ); - $this->out->elementStart('fieldset'); - $this->out->hidden('oauth_token', $this->connection->token); - $this->out->hidden('token', common_session_token()); - // TRANS: Button label in application list to revoke access to user data. - $this->out->submit('revoke', _m('BUTTON','Revoke')); - $this->out->elementEnd('fieldset'); - $this->out->elementEnd('form'); - - $this->out->elementEnd('li'); - } -} diff --git a/lib/connectedappslist.php b/lib/connectedappslist.php new file mode 100644 index 0000000000..7e5eb7482c --- /dev/null +++ b/lib/connectedappslist.php @@ -0,0 +1,167 @@ +. + * + * @category Application + * @package StatusNet + * @author Zach Copley + * @copyright 2008-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/ + */ + +if (!defined('GNUSOCIAL')) { exit(1); } + +/** + * Widget to show a list of connected OAuth clients + * + * @category Application + * @package StatusNet + * @author Zach Copley + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 + * @link http://status.net/ + */ +class ConnectedAppsList extends Widget +{ + /** Current connected application query */ + var $connection = null; + + /** Owner of this list */ + var $owner = null; + + /** Action object using us. */ + var $action = null; + + function __construct($connection, $owner=null, $action=null) + { + parent::__construct($action); + + common_debug("ConnectedAppsList constructor"); + + $this->connection = $connection; + $this->owner = $owner; + $this->action = $action; + } + + /* Override this in subclasses. */ + function showOwnerControls() + { + return; + } + + function show() + { + $this->out->elementStart('ul', 'applications'); + + $cnt = 0; + + while ($this->connection->fetch()) { + $cnt++; + if($cnt > APPS_PER_PAGE) { + break; + } + $this->showConnection(); + } + + $this->out->elementEnd('ul'); + + return $cnt; + } + + function showConnection() + { + $app = Oauth_application::getKV('id', $this->connection->application_id); + + $this->out->elementStart('li', array('class' => 'application h-entry', + 'id' => 'oauthclient-' . $app->id)); + + $this->out->elementStart('a', array('href' => $app->source_url, + 'class' => 'h-card p-name')); + + if (!empty($app->icon)) { + $this->out->element('img', array('src' => $app->icon, + 'class' => 'avatar u-photo')); + } + if ($app->name != 'anonymous') { + $this->out->text($app->name); + } else { + // TRANS: Name for an anonymous application in application list. + $this->out->element('span', 'p-name', _('Unknown application')); + } + $this->out->elementEnd('a'); + + if ($app->name != 'anonymous') { + // @todo FIXME: i18n trouble. + // TRANS: Message has a leading space and a trailing space. Used in application list. + // TRANS: Before this message the application name is put, behind it the organisation that manages it. + $this->out->raw(_(' by ')); + + $this->out->element('a', array('href' => $app->homepage, + 'class' => 'h-card'), + $app->organization); + } + + // TRANS: Application access type + $readWriteText = _('read-write'); + // TRANS: Application access type + $readOnlyText = _('read-only'); + + $access = ($this->connection->access_type & Oauth_application::$writeAccess) + ? $readWriteText : $readOnlyText; + $modifiedDate = common_date_string($this->connection->modified); + // TRANS: Used in application list. %1$s is a modified date, %2$s is access type ("read-write" or "read-only") + $txt = sprintf(_('Approved %1$s - "%2$s" access.'), $modifiedDate, $access); + + // @todo FIXME: i18n trouble. + $this->out->raw(" - $txt"); + if (!empty($app->description)) { + $this->out->element( + 'p', array('class' => 'application_description'), + $app->description + ); + } + $this->out->element( + 'p', array( + 'class' => 'access_token'), + // TRANS: Access token in the application list. + // TRANS: %s are the first 7 characters of the access token. + sprintf(_('Access token starting with: %s'), substr($this->connection->token, 0, 7)) + ); + + $this->out->elementStart( + 'form', + array( + 'id' => 'form_revoke_app', + 'class' => 'form_revoke_app', + 'method' => 'POST', + 'action' => common_local_url('oauthconnectionssettings') + ) + ); + $this->out->elementStart('fieldset'); + $this->out->hidden('oauth_token', $this->connection->token); + $this->out->hidden('token', common_session_token()); + // TRANS: Button label in application list to revoke access to user data. + $this->out->submit('revoke', _m('BUTTON','Revoke')); + $this->out->elementEnd('fieldset'); + $this->out->elementEnd('form'); + + $this->out->elementEnd('li'); + } +} diff --git a/lib/framework.php b/lib/framework.php index fec265fd1e..1834c3e786 100644 --- a/lib/framework.php +++ b/lib/framework.php @@ -37,6 +37,7 @@ define('NOTICES_PER_PAGE', 20); define('PROFILES_PER_PAGE', 20); define('MESSAGES_PER_PAGE', 20); define('GROUPS_PER_PAGE', 20); +define('APPS_PER_PAGE', 20); define('GROUPS_PER_MINILIST', 8); define('PROFILES_PER_MINILIST', 8); From 2d44400cfc720e212f506e86c29eec35d42a985d Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 19:03:53 +0200 Subject: [PATCH 08/40] SettingsAction now extends FormAction (and thus ManagedAction) --- lib/settingsaction.php | 111 +---------------------------------------- 1 file changed, 2 insertions(+), 109 deletions(-) diff --git a/lib/settingsaction.php b/lib/settingsaction.php index fd4885c830..a98d002ca4 100644 --- a/lib/settingsaction.php +++ b/lib/settingsaction.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Base class for settings group of actions @@ -43,113 +41,8 @@ if (!defined('STATUSNET') && !defined('LACONICA')) { * @see Widget */ -class SettingsAction extends Action +class SettingsAction extends FormAction { - /** - * A message for the user. - */ - - var $msg = null; - - /** - * Whether the message is a good one or a bad one. - */ - - var $success = false; - - /** - * Handle input and output a page - * - * @param array $args $_REQUEST arguments - * - * @return void - */ - - function handle($args) - { - parent::handle($args); - if (!common_logged_in()) { - // TRANS: Error message displayed when trying to perform an action that requires a logged in user. - $this->clientError(_('Not logged in.')); - } else if (!common_is_real_login()) { - // Cookie theft means that automatic logins can't - // change important settings or see private info, and - // _all_ our settings are important - common_set_returnto($this->selfUrl()); - $user = common_current_user(); - if (Event::handle('RedirectToLogin', array($this, $user))) { - common_redirect(common_local_url('login'), 303); - } - } else if ($_SERVER['REQUEST_METHOD'] == 'POST') { - $this->handlePost(); - } else { - $this->showForm(); - } - } - - /** - * Handle a POST request - * - * @return boolean success flag - */ - - function handlePost() - { - return false; - } - - /** - * show the settings form - * - * @param string $msg an extra message for the user - * @param string $success good message or bad message? - * - * @return void - */ - - function showForm($msg=null, $success=false) - { - $this->msg = $msg; - $this->success = $success; - - $this->showPage(); - } - - /** - * show human-readable instructions for the page - * - * @return void - */ - - function showPageNotice() - { - if ($this->msg) { - $this->element('div', ($this->success) ? 'success' : 'error', - $this->msg); - } else { - $inst = $this->getInstructions(); - $output = common_markup_to_html($inst); - - $this->elementStart('div', 'instructions'); - $this->raw($output); - $this->elementEnd('div'); - } - } - - /** - * instructions recipe for sub-classes - * - * Subclasses should override this to return readable instructions. They'll - * be processed by common_markup_to_html(). - * - * @return string instructions text - */ - - function getInstructions() - { - return ''; - } - /** * Show the local navigation menu * From fd2efbc6f81aba67ff3776712af65b1a8fa73d06 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 19:21:12 +0200 Subject: [PATCH 09/40] AvatarSettings more aligned to FormAction TODO: Make classes called AvatarCropForm and AvatarUploadForm --- actions/avatarsettings.php | 108 ++++++++++--------------------------- lib/imagefile.php | 2 + 2 files changed, 31 insertions(+), 79 deletions(-) diff --git a/actions/avatarsettings.php b/actions/avatarsettings.php index 1f31cbdafe..4b618eb9be 100644 --- a/actions/avatarsettings.php +++ b/actions/avatarsettings.php @@ -76,11 +76,11 @@ class AvatarsettingsAction extends SettingsAction /** * Content area of the page * - * Shows a form for uploading an avatar. + * Shows a form for uploading an avatar. Currently overrides FormAction's showContent + * since we haven't made classes out of AvatarCropForm and AvatarUploadForm. * * @return void */ - function showContent() { if ($this->mode == 'crop') { @@ -243,52 +243,19 @@ class AvatarsettingsAction extends SettingsAction $this->elementEnd('form'); } - /** - * Handle a post - * - * We mux on the button name to figure out what the user actually wanted. - * - * @return void - */ - function handlePost() + protected function doPost() { - // Workaround for PHP returning empty $_POST and $_FILES when POST - // length > post_max_size in php.ini - - if (empty($_FILES) - && empty($_POST) - && ($_SERVER['CONTENT_LENGTH'] > 0) - ) { - // TRANS: Client error displayed when the number of bytes in a POST request exceeds a limit. - // TRANS: %s is the number of bytes of the CONTENT_LENGTH. - $msg = _m('The server was unable to handle that much POST data (%s byte) due to its current configuration.', - 'The server was unable to handle that much POST data (%s bytes) due to its current configuration.', - intval($_SERVER['CONTENT_LENGTH'])); - $this->showForm(sprintf($msg, $_SERVER['CONTENT_LENGTH'])); - return; - } - - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if (Event::handle('StartAvatarSaveForm', array($this))) { - if ($this->arg('upload')) { - $this->uploadAvatar(); - } else if ($this->arg('crop')) { - $this->cropAvatar(); - } else if ($this->arg('delete')) { - $this->deleteAvatar(); - } else { - // TRANS: Unexpected validation error on avatar upload form. - $this->showForm(_('Unexpected form submission.')); - } + if ($this->trimmed('upload')) { + return $this->uploadAvatar(); + } else if ($this->trimmed('crop')) { + return $this->cropAvatar(); + } else if ($this->trimmed('delete')) { + return $this->deleteAvatar(); + } else { + // TRANS: Unexpected validation error on avatar upload form. + throw new ClientException(_('Unexpected form submission.')); + } Event::handle('EndAvatarSaveForm', array($this)); } } @@ -303,21 +270,12 @@ class AvatarsettingsAction extends SettingsAction */ function uploadAvatar() { - try { - $imagefile = ImageFile::fromUpload('avatarfile'); - } catch (Exception $e) { - $this->showForm($e->getMessage()); - return; - } - if ($imagefile === null) { - // TRANS: Validation error on avatar upload form when no file was uploaded. - $this->showForm(_('No file uploaded.')); - return; - } + // ImageFile throws exception if something goes wrong, which we'll + // pick up and show as an error message above the form. + $imagefile = ImageFile::fromUpload('avatarfile'); - $cur = common_current_user(); $type = $imagefile->preferredType(); - $filename = Avatar::filename($cur->id, + $filename = Avatar::filename($this->scoped->getID(), image_type_to_extension($type), null, 'tmp'.common_timestamp()); @@ -338,8 +296,7 @@ class AvatarsettingsAction extends SettingsAction $this->mode = 'crop'; // TRANS: Avatar upload form instruction after uploading a file. - $this->showForm(_('Pick a square area of the image to be your avatar.'), - true); + return _('Pick a square area of the image to be your avatar.'); } /** @@ -351,13 +308,12 @@ class AvatarsettingsAction extends SettingsAction { $filedata = $_SESSION['FILEDATA']; - if (!$filedata) { + if (empty($filedata)) { // TRANS: Server error displayed if an avatar upload went wrong somehow server side. - $this->serverError(_('Lost our file data.')); + throw new ServerException(_('Lost our file data.')); } - $file_d = ($filedata['width'] > $filedata['height']) - ? $filedata['height'] : $filedata['width']; + $file_d = min($filedata['width'], $filedata['height']); $dest_x = $this->arg('avatar_crop_x') ? $this->arg('avatar_crop_x'):0; $dest_y = $this->arg('avatar_crop_y') ? $this->arg('avatar_crop_y'):0; @@ -369,11 +325,8 @@ class AvatarsettingsAction extends SettingsAction 'x' => $dest_x, 'y' => $dest_y, 'w' => $dest_w, 'h' => $dest_h); - $user = common_current_user(); - $profile = $user->getProfile(); - $imagefile = new ImageFile(null, $filedata['filepath']); - $filename = Avatar::filename($profile->getID(), image_type_to_extension($imagefile->preferredType()), + $filename = Avatar::filename($this->scoped->getID(), image_type_to_extension($imagefile->preferredType()), $size, common_timestamp()); try { $imagefile->resizeTo(Avatar::path($filename), $box); @@ -385,16 +338,16 @@ class AvatarsettingsAction extends SettingsAction } } - if ($profile->setOriginal($filename)) { + if ($this->scoped->setOriginal($filename)) { @unlink($filedata['filepath']); unset($_SESSION['FILEDATA']); $this->mode = 'upload'; // TRANS: Success message for having updated a user avatar. - $this->showForm(_('Avatar updated.'), true); - } else { - // TRANS: Error displayed on the avatar upload page if the avatar could not be updated for an unknown reason. - $this->showForm(_('Failed updating avatar.')); + return _('Avatar updated.'); } + + // TRANS: Error displayed on the avatar upload page if the avatar could not be updated for an unknown reason. + throw new ServerException(_('Failed updating avatar.')); } /** @@ -404,13 +357,10 @@ class AvatarsettingsAction extends SettingsAction */ function deleteAvatar() { - $user = common_current_user(); - $profile = $user->getProfile(); - - Avatar::deleteFromProfile($profile); + Avatar::deleteFromProfile($this->scoped); // TRANS: Success message for deleting a user avatar. - $this->showForm(_('Avatar deleted.'), true); + return _('Avatar deleted.'); } /** diff --git a/lib/imagefile.php b/lib/imagefile.php index 2d1a3af02e..68cfea48e7 100644 --- a/lib/imagefile.php +++ b/lib/imagefile.php @@ -189,6 +189,8 @@ class ImageFile case UPLOAD_ERR_NO_FILE: // No file; probably just a non-AJAX submission. + throw new ClientException(_('No file uploaded.')); + default: common_log(LOG_ERR, __METHOD__ . ": Unknown upload error " . $_FILES[$param]['error']); // TRANS: Exception thrown when uploading an image fails for an unknown reason. From 647171e089d49eee770c77c7d1a5b9bbbdcdd55a Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 19:42:37 +0200 Subject: [PATCH 10/40] EmailSettingsAction adapted to FormAction TODO: EmailSettingsForm as a separate class would be good! --- actions/emailsettings.php | 173 ++++++++++++++------------------------ 1 file changed, 63 insertions(+), 110 deletions(-) diff --git a/actions/emailsettings.php b/actions/emailsettings.php index cffc5da247..c02f1cdfad 100644 --- a/actions/emailsettings.php +++ b/actions/emailsettings.php @@ -28,11 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - - +if (!defined('GNUSOCIAL')) { exit(1); } /** * Settings for email @@ -112,8 +108,8 @@ class EmailsettingsAction extends SettingsAction // TRANS: Button label to remove a confirmed e-mail address. $this->submit('remove', _m('BUTTON','Remove')); } else { - $confirm = $this->getConfirmation(); - if ($confirm) { + try { + $confirm = $this->getConfirmation(); $this->element('p', array('id' => 'form_unconfirmed'), $confirm->address); $this->element('p', array('class' => 'form_note'), // TRANS: Form note in e-mail settings form. @@ -123,12 +119,12 @@ class EmailsettingsAction extends SettingsAction $this->hidden('email', $confirm->address); // TRANS: Button label to cancel an e-mail address confirmation procedure. $this->submit('cancel', _m('BUTTON','Cancel')); - } else { + } catch (NoResultException $e) { $this->elementStart('ul', 'form_data'); $this->elementStart('li'); // TRANS: Field label for e-mail address input in e-mail settings form. $this->input('email', _('Email address'), - ($this->arg('email')) ? $this->arg('email') : null, + $this->trimmed('email') ?: null, // TRANS: Instructions for e-mail address input form. Do not translate // TRANS: "example.org". It is one of the domain names reserved for // TRANS: use in examples by http://www.rfc-editor.org/rfc/rfc2606.txt. @@ -248,56 +244,36 @@ class EmailsettingsAction extends SettingsAction */ function getConfirmation() { - $user = common_current_user(); - $confirm = new Confirm_address(); - $confirm->user_id = $user->id; + $confirm->user_id = $this->scoped->getID(); $confirm->address_type = 'email'; if ($confirm->find(true)) { return $confirm; - } else { - return null; } + + throw new NoResultException($confirm); } - /** - * Handle posts - * - * Since there are a lot of different options on the page, we - * figure out what we're supposed to do based on which button was - * pushed - * - * @return void - */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->show_form(_('There was a problem with your session token. '. - 'Try again, please.')); - return; + if ($this->arg('save')) { + return $this->savePreferences(); + } else if ($this->arg('add')) { + return $this->addAddress(); + } else if ($this->arg('cancel')) { + return $this->cancelConfirmation(); + } else if ($this->arg('remove')) { + return $this->removeAddress(); + } else if ($this->arg('removeincoming')) { + return $this->removeIncoming(); + } else if ($this->arg('newincoming')) { + return $this->newIncoming(); } - if ($this->arg('save')) { - $this->savePreferences(); - } else if ($this->arg('add')) { - $this->addAddress(); - } else if ($this->arg('cancel')) { - $this->cancelConfirmation(); - } else if ($this->arg('remove')) { - $this->removeAddress(); - } else if ($this->arg('removeincoming')) { - $this->removeIncoming(); - } else if ($this->arg('newincoming')) { - $this->newIncoming(); - } else { - // TRANS: Message given submitting a form with an unknown action in e-mail settings. - $this->showForm(_('Unexpected form submission.')); - } + // TRANS: Message given submitting a form with an unknown action in e-mail settings. + throw new ClientException(_('Unexpected form submission.')); } /** @@ -307,8 +283,6 @@ class EmailsettingsAction extends SettingsAction */ function savePreferences() { - $user = $this->scoped->getUser(); - if (Event::handle('StartEmailSaveForm', array($this, $this->scoped))) { $emailnotifysub = $this->booleanintstring('emailnotifysub'); $emailnotifymsg = $this->booleanintstring('emailnotifymsg'); @@ -316,8 +290,8 @@ class EmailsettingsAction extends SettingsAction $emailnotifyattn = $this->booleanintstring('emailnotifyattn'); $emailpost = $this->booleanintstring('emailpost'); + $user = $this->scoped->getUser(); $user->query('BEGIN'); - $original = clone($user); $user->emailnotifysub = $emailnotifysub; @@ -332,16 +306,15 @@ class EmailsettingsAction extends SettingsAction common_log_db_error($user, 'UPDATE', __FILE__); $user->query('ROLLBACK'); // TRANS: Server error thrown on database error updating e-mail preferences. - $this->serverError(_('Could not update user.')); + throw new ServerException(_('Could not update user.')); } $user->query('COMMIT'); Event::handle('EndEmailSaveForm', array($this, $this->scoped)); - - // TRANS: Confirmation message for successful e-mail preferences save. - $this->showForm(_('Email preferences saved.'), true); } + // TRANS: Confirmation message for successful e-mail preferences save. + return _('Email preferences saved.'); } /** @@ -351,38 +324,32 @@ class EmailsettingsAction extends SettingsAction */ function addAddress() { - $user = common_current_user(); + $user = $this->scoped->getUser(); $email = $this->trimmed('email'); // Some validation - if (!$email) { + if (empty($email)) { // TRANS: Message given saving e-mail address without having provided one. - $this->showForm(_('No email address.')); - return; + throw new ClientException(_('No email address.')); } $email = common_canonical_email($email); - if (!$email) { + if (empty($email)) { // TRANS: Message given saving e-mail address that cannot be normalised. - $this->showForm(_('Cannot normalize that email address.')); - return; + throw new ClientException(_('Cannot normalize that email address.')); } if (!Validate::email($email, common_config('email', 'check_domain'))) { // TRANS: Message given saving e-mail address that not valid. - $this->showForm(_('Not a valid email address.')); - return; + throw new ClientException(_('Not a valid email address.')); } else if ($user->email == $email) { // TRANS: Message given saving e-mail address that is already set. - $this->showForm(_('That is already your email address.')); - return; + throw new ClientException(_('That is already your email address.')); } else if ($this->emailExists($email)) { // TRANS: Message given saving e-mail address that is already set for another user. - $this->showForm(_('That email address already belongs '. - 'to another user.')); - return; + throw new ClientException(_('That email address already belongs to another user.')); } if (Event::handle('StartAddEmailAddress', array($user, $email))) { @@ -391,7 +358,7 @@ class EmailsettingsAction extends SettingsAction $confirm->address = $email; $confirm->address_type = 'email'; - $confirm->user_id = $user->id; + $confirm->user_id = $user->getID(); $confirm->code = common_confirmation_code(64); $result = $confirm->insert(); @@ -399,21 +366,19 @@ class EmailsettingsAction extends SettingsAction if ($result === false) { common_log_db_error($confirm, 'INSERT', __FILE__); // TRANS: Server error thrown on database error adding e-mail confirmation code. - $this->serverError(_('Could not insert confirmation code.')); + throw new ServerException(_('Could not insert confirmation code.')); } - common_debug('Sending confirmation address for user '.$user->id.' to email '.$email); - mail_confirm_address($user, $confirm->code, $user->nickname, $email); + common_debug('Sending confirmation address for user '.$user->getID().' to email '.$email); + mail_confirm_address($user, $confirm->code, $user->getNickname(), $email); Event::handle('EndAddEmailAddress', array($user, $email)); } // TRANS: Message given saving valid e-mail address that is to be confirmed. - $msg = _('A confirmation code was sent to the email address you added. '. + return _('A confirmation code was sent to the email address you added. '. 'Check your inbox (and spam box!) for the code and instructions '. 'on how to use it.'); - - $this->showForm($msg, true); } /** @@ -423,31 +388,29 @@ class EmailsettingsAction extends SettingsAction */ function cancelConfirmation() { - $email = $this->arg('email'); + $email = $this->trimmed('email'); - $confirm = $this->getConfirmation(); - - if (!$confirm) { + try { + $confirm = $this->getConfirmation(); + if ($confirm->address !== $email) { + // TRANS: Message given canceling e-mail address confirmation for the wrong e-mail address. + throw new ClientException(_('That is the wrong email address.')); + } + } catch (NoResultException $e) { // TRANS: Message given canceling e-mail address confirmation that is not pending. - $this->showForm(_('No pending confirmation to cancel.')); - return; - } - if ($confirm->address != $email) { - // TRANS: Message given canceling e-mail address confirmation for the wrong e-mail address. - $this->showForm(_('That is the wrong email address.')); - return; + throw new AlreadyFulfilledException(_('No pending confirmation to cancel.')); } $result = $confirm->delete(); - if (!$result) { + if ($result === false) { common_log_db_error($confirm, 'DELETE', __FILE__); // TRANS: Server error thrown on database error canceling e-mail address confirmation. - $this->serverError(_('Could not delete email confirmation.')); + throw new ServerException(_('Could not delete email confirmation.')); } // TRANS: Message given after successfully canceling e-mail address confirmation. - $this->showForm(_('Email confirmation cancelled.'), true); + return _('Email confirmation cancelled.'); } /** @@ -459,26 +422,22 @@ class EmailsettingsAction extends SettingsAction { $user = common_current_user(); - $email = $this->arg('email'); + $email = $this->trimmed('email'); // Maybe an old tab open...? - - if ($user->email != $email) { + if ($user->email !== $email) { // TRANS: Message given trying to remove an e-mail address that is not // TRANS: registered for the active user. - $this->showForm(_('That is not your email address.')); - return; + throw new ClientException(_('That is not your email address.')); } $original = clone($user); - $user->email = null; - // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($original); // TRANS: Message given after successfully removing a registered e-mail address. - $this->showForm(_('The email address was removed.'), true); + return _('The email address was removed.'); } /** @@ -490,22 +449,19 @@ class EmailsettingsAction extends SettingsAction { $user = common_current_user(); - if (!$user->incomingemail) { + if (empty($user->incomingemail)) { // TRANS: Form validation error displayed when trying to remove an incoming e-mail address while no address has been set. - $this->showForm(_('No incoming email address.')); - return; + throw new AlreadyFulfilledException(_('No incoming email address.')); } $orig = clone($user); - $user->incomingemail = null; $user->emailpost = 0; - // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($orig); // TRANS: Message given after successfully removing an incoming e-mail address. - $this->showForm(_('Incoming email address removed.'), true); + return _('Incoming email address removed.'); } /** @@ -516,17 +472,14 @@ class EmailsettingsAction extends SettingsAction function newIncoming() { $user = common_current_user(); - $orig = clone($user); - $user->incomingemail = mail_new_incoming_address(); $user->emailpost = 1; - // Throws exception on failure. Also performs it within a transaction. $user->updateWithKeys($orig); // TRANS: Message given after successfully adding an incoming e-mail address. - $this->showForm(_('New incoming email address added.'), true); + return _('New incoming email address added.'); } /** @@ -545,10 +498,10 @@ class EmailsettingsAction extends SettingsAction $other = User::getKV('email', $email); - if (!$other) { + if (!$other instanceof User) { return false; - } else { - return $other->id != $user->id; } + + return $other->id != $user->id; } } From f1d9d8a6eded2d597d5a3157febc2de09c93d3a1 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 21:18:50 +0200 Subject: [PATCH 11/40] ImSettings adapted to FormAction inheritance TODO: Get separate Form classes and move User_im_prefs to Profile_prefs --- actions/imsettings.php | 146 ++++++++++++++--------------------------- lib/implugin.php | 10 +-- 2 files changed, 55 insertions(+), 101 deletions(-) diff --git a/actions/imsettings.php b/actions/imsettings.php index feb5d32fc1..40bea10e68 100644 --- a/actions/imsettings.php +++ b/actions/imsettings.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Settings for Jabber/XMPP integration @@ -118,8 +116,8 @@ class ImsettingsAction extends SettingsAction // TRANS: Button label to remove a confirmed IM address. $this->submit('remove', _m('BUTTON','Remove')); } else { - $confirm = $this->getConfirmation($transport); - if ($confirm) { + try { + $confirm = $this->getConfirmation($transport); $this->element('p', 'form_unconfirmed', $confirm->address); // TRANS: Form note in IM settings form. $this->element('p', 'form_note', @@ -134,7 +132,7 @@ class ImsettingsAction extends SettingsAction $this->hidden('screenname', $confirm->address); // TRANS: Button label to cancel an IM address confirmation procedure. $this->submit('cancel', _m('BUTTON','Cancel')); - } else { + } catch (NoResultException $e) { $this->elementStart('ul', 'form_data'); $this->elementStart('li'); // TRANS: Field label for IM address. @@ -209,57 +207,35 @@ class ImsettingsAction extends SettingsAction */ function getConfirmation($transport) { - $user = common_current_user(); - $confirm = new Confirm_address(); - $confirm->user_id = $user->id; + $confirm->user_id = $this->scoped->getID(); $confirm->address_type = $transport; if ($confirm->find(true)) { return $confirm; - } else { - return null; } + + throw new NoResultException($confirm); } - /** - * Handle posts to this form - * - * Based on the button that was pressed, muxes out to other functions - * to do the actual task requested. - * - * All sub-functions reload the form with a message -- success or failure. - * - * @return void - */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if ($this->arg('save')) { - $this->savePreferences(); + return $this->savePreferences(); } else if ($this->arg('add')) { - $this->addAddress(); + return $this->addAddress(); } else if ($this->arg('cancel')) { - $this->cancelConfirmation(); + return $this->cancelConfirmation(); } else if ($this->arg('remove')) { - $this->removeAddress(); - } else { - // TRANS: Message given submitting a form with an unknown action in Instant Messaging settings. - $this->showForm(_('Unexpected form submission.')); + return $this->removeAddress(); } + // TRANS: Message given submitting a form with an unknown action in Instant Messaging settings. + throw new ClientException(_('Unexpected form submission.')); } /** - * Save user's Jabber preferences + * Save user's XMPP preferences * * These are the checkboxes at the bottom of the page. They're used to * set different settings @@ -268,11 +244,9 @@ class ImsettingsAction extends SettingsAction */ function savePreferences() { - $user = common_current_user(); - $user_im_prefs = new User_im_prefs(); $user_im_prefs->query('BEGIN'); - $user_im_prefs->user_id = $user->id; + $user_im_prefs->user_id = $this->scoped->getID(); if($user_im_prefs->find() && $user_im_prefs->fetch()) { $preferences = array('notify', 'updatefrompresence', 'replies'); @@ -287,15 +261,15 @@ class ImsettingsAction extends SettingsAction $result = $new->update($original); if ($result === false) { - common_log_db_error($user, 'UPDATE', __FILE__); + common_log_db_error($user_im_prefs, 'UPDATE', __FILE__); // TRANS: Server error thrown on database error updating IM preferences. - $this->serverError(_('Could not update IM preferences.')); + throw new ServerException(_('Could not update IM preferences.')); } }while($user_im_prefs->fetch()); } $user_im_prefs->query('COMMIT'); // TRANS: Confirmation message for successful IM preferences save. - $this->showForm(_('Preferences saved.'), true); + return _('Preferences saved.'); } /** @@ -308,49 +282,42 @@ class ImsettingsAction extends SettingsAction */ function addAddress() { - $user = common_current_user(); - $screenname = $this->trimmed('screenname'); $transport = $this->trimmed('transport'); // Some validation - if (!$screenname) { + if (empty($screenname)) { // TRANS: Message given saving IM address without having provided one. - $this->showForm(_('No screenname.')); - return; + throw new ClientException(_('No screenname.')); } - if (!$transport) { + if (empty($transport)) { // TRANS: Form validation error when no transport is available setting an IM address. - $this->showForm(_('No transport.')); - return; + throw new ClientException(_('No transport.')); } Event::handle('NormalizeImScreenname', array($transport, &$screenname)); - if (!$screenname) { + if (empty($screenname)) { // TRANS: Message given saving IM address that cannot be normalised. - $this->showForm(_('Cannot normalize that screenname.')); - return; + throw new ClientException(_('Cannot normalize that screenname.')); } $valid = false; Event::handle('ValidateImScreenname', array($transport, $screenname, &$valid)); if (!$valid) { // TRANS: Message given saving IM address that not valid. - $this->showForm(_('Not a valid screenname.')); - return; + throw new ClientException(_('Not a valid screenname.')); } else if ($this->screennameExists($transport, $screenname)) { // TRANS: Message given saving IM address that is already set for another user. - $this->showForm(_('Screenname already belongs to another user.')); - return; + throw new ClientException(_('Screenname already belongs to another user.')); } $confirm = new Confirm_address(); $confirm->address = $screenname; $confirm->address_type = $transport; - $confirm->user_id = $user->id; + $confirm->user_id = $this->scoped->getID(); $confirm->code = common_confirmation_code(64); $confirm->sent = common_sql_now(); $confirm->claimed = common_sql_now(); @@ -363,13 +330,10 @@ class ImsettingsAction extends SettingsAction $this->serverError(_('Could not insert confirmation code.')); } - Event::handle('SendImConfirmationCode', array($transport, $screenname, $confirm->code, $user)); + Event::handle('SendImConfirmationCode', array($transport, $screenname, $confirm->code, $this->scoped)); // TRANS: Message given saving valid IM address that is to be confirmed. - $msg = _('A confirmation code was sent '. - 'to the IM address you added.'); - - $this->showForm($msg, true); + return _('A confirmation code was sent to the IM address you added.'); } /** @@ -384,29 +348,27 @@ class ImsettingsAction extends SettingsAction $screenname = $this->trimmed('screenname'); $transport = $this->trimmed('transport'); - $confirm = $this->getConfirmation($transport); - - if (!$confirm) { + try { + $confirm = $this->getConfirmation($transport); + if ($confirm->address != $screenname) { + // TRANS: Message given canceling IM address confirmation for the wrong IM address. + throw new ClientException(_('That is the wrong IM address.')); + } + } catch (NoResultException $e) { // TRANS: Message given canceling Instant Messaging address confirmation that is not pending. - $this->showForm(_('No pending confirmation to cancel.')); - return; - } - if ($confirm->address != $screenname) { - // TRANS: Message given canceling IM address confirmation for the wrong IM address. - $this->showForm(_('That is the wrong IM address.')); - return; + throw new AlreadyFulfilledException(_('No pending confirmation to cancel.')); } $result = $confirm->delete(); - if (!$result) { + if ($result === false) { common_log_db_error($confirm, 'DELETE', __FILE__); // TRANS: Server error thrown on database error canceling IM address confirmation. - $this->serverError(_('Could not delete confirmation.')); + throw new ServerException(_('Could not delete confirmation.')); } // TRANS: Message given after successfully canceling IM address confirmation. - $this->showForm(_('IM confirmation cancelled.'), true); + return _('IM confirmation cancelled.'); } /** @@ -418,34 +380,32 @@ class ImsettingsAction extends SettingsAction */ function removeAddress() { - $user = common_current_user(); - $screenname = $this->trimmed('screenname'); $transport = $this->trimmed('transport'); // Maybe an old tab open...? $user_im_prefs = new User_im_prefs(); - $user_im_prefs->user_id = $user->id; - if(! ($user_im_prefs->find() && $user_im_prefs->fetch())) { + $user_im_prefs->user_id = $this->scoped->getID(); + $user_im_prefs->transport = $transport; + if (!$user_im_prefs->find(true)) { // TRANS: Message given trying to remove an IM address that is not // TRANS: registered for the active user. - $this->showForm(_('That is not your screenname.')); - return; + throw new AlreadyFulfilledException(_('There were no preferences stored for this transport.')); } $result = $user_im_prefs->delete(); - if (!$result) { - common_log_db_error($user, 'UPDATE', __FILE__); + if ($result === false) { + common_log_db_error($user_im_prefs, 'UPDATE', __FILE__); // TRANS: Server error thrown on database error removing a registered IM address. - $this->serverError(_('Could not update user IM preferences.')); + throw new ServerException(_('Could not update user IM preferences.')); } // XXX: unsubscribe to the old address // TRANS: Message given after successfully removing a registered Instant Messaging address. - $this->showForm(_('The IM address was removed.'), true); + return _('The IM address was removed.'); } /** @@ -461,15 +421,9 @@ class ImsettingsAction extends SettingsAction function screennameExists($transport, $screenname) { - $user = common_current_user(); - $user_im_prefs = new User_im_prefs(); $user_im_prefs->transport = $transport; $user_im_prefs->screenname = $screenname; - if($user_im_prefs->find() && $user_im_prefs->fetch()){ - return true; - }else{ - return false; - } + return $user_im_prefs->find(true) ? true : false; } } diff --git a/lib/implugin.php b/lib/implugin.php index 6395ecbdb7..742147dbbd 100644 --- a/lib/implugin.php +++ b/lib/implugin.php @@ -243,11 +243,11 @@ abstract class ImPlugin extends Plugin * * @param string $screenname screenname sending to * @param string $code the confirmation code - * @param User $user user sending to + * @param Profile $target For whom the code is valid for * * @return boolean success value */ - function sendConfirmationCode($screenname, $code, $user) + function sendConfirmationCode($screenname, $code, Profile $target) { // TRANS: Body text for confirmation code e-mail. // TRANS: %1$s is a user nickname, %2$s is the StatusNet sitename, @@ -258,7 +258,7 @@ abstract class ImPlugin extends Plugin ' . (If you cannot click it, copy-and-paste it into the ' . 'address bar of your browser). If that user is not you, ' . 'or if you did not request this confirmation, just ignore this message.'), - $user->nickname, common_config('site', 'name'), $this->getDisplayName(), common_local_url('confirmaddress', null, array('code' => $code))); + $target->getNickname(), common_config('site', 'name'), $this->getDisplayName(), common_local_url('confirmaddress', null, array('code' => $code))); return $this->sendMessage($screenname, $body); } @@ -594,11 +594,11 @@ abstract class ImPlugin extends Plugin 'daemonScreenname' => $this->daemonScreenname()); } - function onSendImConfirmationCode($transport, $screenname, $code, $user) + function onSendImConfirmationCode($transport, $screenname, $code, Profile $target) { if($transport == $this->transport) { - $this->sendConfirmationCode($screenname, $code, $user); + $this->sendConfirmationCode($screenname, $code, $target); return false; } } From ba5a43f2f9c712cfb4d5dac4112e8cab25b4260c Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 16 Jul 2015 23:58:04 +0200 Subject: [PATCH 12/40] If XMLOutputter $output arg is null, use php://output Since pushing a null value to the argument actually sets it to null and not the default fallback (previously $output='php://output'); --- lib/xmloutputter.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/xmloutputter.php b/lib/xmloutputter.php index 71d57e2d56..463f91be30 100644 --- a/lib/xmloutputter.php +++ b/lib/xmloutputter.php @@ -63,12 +63,15 @@ class XMLOutputter * * Initializes the wrapped XMLWriter. * - * @param string $output URL for outputting, defaults to stdout + * @param string $output URL for outputting, if null it defaults to stdout ('php://output') * @param boolean $indent Whether to indent output, default true */ - function __construct($output='php://output', $indent=null) + function __construct($output=null, $indent=null) { + if (is_null($output)) { + $output = 'php://output'; + } $this->xw = new XMLWriter(); $this->xw->openURI($output); if(is_null($indent)) { From a6e299a2fc83d7b4d0e82082f7622279f5d78aba Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 00:20:46 +0200 Subject: [PATCH 13/40] OAuth stuff adapted for FormAction TODO: Break OAuth out into a plugin. --- actions/newapplication.php | 7 ++-- actions/oauthappssettings.php | 51 ++++------------------------ actions/oauthconnectionssettings.php | 40 +++++++--------------- classes/Profile.php | 5 +++ lib/applicationlist.php | 12 ++----- lib/connectedappslist.php | 10 ++---- 6 files changed, 34 insertions(+), 91 deletions(-) diff --git a/actions/newapplication.php b/actions/newapplication.php index 37bede0d72..5032bb7495 100644 --- a/actions/newapplication.php +++ b/actions/newapplication.php @@ -41,7 +41,7 @@ if (!defined('GNUSOCIAL')) { exit(1); } * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @link http://status.net/ */ -class NewApplicationAction extends FormAction +class NewApplicationAction extends SettingsAction { function title() { @@ -54,6 +54,7 @@ class NewApplicationAction extends FormAction if ($this->arg('cancel')) { common_redirect(common_local_url('oauthappssettings'), 303); } elseif ($this->arg('save')) { + //trySave will never return, just throw exception or redirect $this->trySave(); } @@ -72,7 +73,7 @@ class NewApplicationAction extends FormAction return _('Use this form to register a new application.'); } - private function trySave() + protected function trySave() { $name = $this->trimmed('name'); $description = $this->trimmed('description'); @@ -137,7 +138,7 @@ class NewApplicationAction extends FormAction $app->query('BEGIN'); $app->name = $name; - $app->owner = $this->scoped->id; + $app->owner = $this->scoped->getID(); $app->description = $description; $app->source_url = $source_url; $app->organization = $organization; diff --git a/actions/oauthappssettings.php b/actions/oauthappssettings.php index e9b6280feb..43e9b33663 100644 --- a/actions/oauthappssettings.php +++ b/actions/oauthappssettings.php @@ -43,19 +43,11 @@ if (!defined('GNUSOCIAL')) { exit(1); } class OauthappssettingsAction extends SettingsAction { - var $page = 0; + protected $page = null; - function prepare($args) + protected function doPreparation() { - parent::prepare($args); - $this->page = ($this->arg('page')) ? ($this->arg('page') + 0) : 1; - - if (!common_logged_in()) { - // TRANS: Message displayed to an anonymous user trying to view OAuth application list. - $this->clientError(_('You must be logged in to list your applications.')); - } - - return true; + $this->page = $this->int('page') ?: 1; } /** @@ -82,21 +74,13 @@ class OauthappssettingsAction extends SettingsAction return _('Applications you have registered'); } - /** - * Content area of the page - * - * @return void - */ - function showContent() { - $user = common_current_user(); - $offset = ($this->page - 1) * APPS_PER_PAGE; $limit = APPS_PER_PAGE + 1; $application = new Oauth_application(); - $application->owner = $user->id; + $application->owner = $this->scoped->getID(); $application->whereAdd("name != 'anonymous'"); $application->limit($offset, $limit); $application->orderBy('created DESC'); @@ -105,7 +89,7 @@ class OauthappssettingsAction extends SettingsAction $cnt = 0; if ($application) { - $al = new ApplicationList($application, $user, $this); + $al = new ApplicationList($application, $this->scoped, $this); $cnt = $al->show(); if (0 == $cnt) { $this->showEmptyListMessage(); @@ -131,34 +115,11 @@ class OauthappssettingsAction extends SettingsAction function showEmptyListMessage() { - // TRANS: Empty list message on page with OAuth applications. + // TRANS: Empty list message on page with OAuth applications. Markup allowed $message = sprintf(_('You have not registered any applications yet.')); $this->elementStart('div', 'guide'); $this->raw(common_markup_to_html($message)); $this->elementEnd('div'); } - - /** - * Handle posts to this form - * - * Based on the button that was pressed, muxes out to other functions - * to do the actual task requested. - * - * All sub-functions reload the form with a message -- success or failure. - * - * @return void - */ - - function handlePost() - { - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - } } diff --git a/actions/oauthconnectionssettings.php b/actions/oauthconnectionssettings.php index a3ba7eda39..0c5a143443 100644 --- a/actions/oauthconnectionssettings.php +++ b/actions/oauthconnectionssettings.php @@ -42,15 +42,14 @@ if (!defined('GNUSOCIAL')) { exit(1); } */ class OauthconnectionssettingsAction extends SettingsAction { - var $page = null; - var $oauth_token = null; + var $page = null; - function prepare($args) + protected $oauth_token = null; + + protected function doPreparation() { - parent::prepare($args); $this->oauth_token = $this->arg('oauth_token'); - $this->page = ($this->arg('page')) ? ($this->arg('page') + 0) : 1; - return true; + $this->page = $this->int('page') ?: 1; } /** @@ -83,18 +82,15 @@ class OauthconnectionssettingsAction extends SettingsAction function showContent() { - $user = common_current_user(); - $profile = $user->getProfile(); - $offset = ($this->page - 1) * APPS_PER_PAGE; $limit = APPS_PER_PAGE + 1; - $connection = $user->getConnectedApps($offset, $limit); + $connection = $this->scoped->getConnectedApps($offset, $limit); $cnt = 0; if (!empty($connection)) { - $cal = new ConnectedAppsList($connection, $user, $this); + $cal = new ConnectedAppsList($connection, $this->scoped, $this); $cnt = $cal->show(); } @@ -107,7 +103,7 @@ class OauthconnectionssettingsAction extends SettingsAction $cnt > APPS_PER_PAGE, $this->page, 'connectionssettings', - array('nickname' => $user->nickname) + array('nickname' => $this->scoped->getNickname()) ); } @@ -121,24 +117,14 @@ class OauthconnectionssettingsAction extends SettingsAction * * @return void */ - function handlePost() + protected function doPost() { - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if ($this->arg('revoke')) { - $this->revokeAccess($this->oauth_token); - } else { - // TRANS: Client error when submitting a form with unexpected information. - $this->clientError(_('Unexpected form submission.'), 401); + return $this->revokeAccess($this->oauth_token); } + + // TRANS: Client error when submitting a form with unexpected information. + throw new ClientException(_('Unexpected form submission.'), 401); } /** diff --git a/classes/Profile.php b/classes/Profile.php index f628965a74..384eaa0726 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -1623,4 +1623,9 @@ class Profile extends Managed_DataObject public function setPref($namespace, $topic, $data) { return Profile_prefs::setData($this, $namespace, $topic, $data); } + + public function getConnectedApps($offset=0, $limit=null) + { + return $this->getUser()->getConnectedApps($offset, $limit); + } } diff --git a/lib/applicationlist.php b/lib/applicationlist.php index b2cc572e3e..ab51a73096 100644 --- a/lib/applicationlist.php +++ b/lib/applicationlist.php @@ -46,16 +46,12 @@ class ApplicationList extends Widget /** Owner of this list */ var $owner = null; - /** Action object using us. */ - var $action = null; - - function __construct($application, $owner=null, $action=null) + function __construct($application, Profile $owner, Action $out=null) { - parent::__construct($action); + parent::__construct($out); $this->application = $application; $this->owner = $owner; - $this->action = $action; } function show() @@ -69,7 +65,7 @@ class ApplicationList extends Widget if($cnt > APPS_PER_PAGE) { break; } - $this->showapplication(); + $this->showApplication(); } $this->out->elementEnd('ul'); @@ -79,8 +75,6 @@ class ApplicationList extends Widget function showApplication() { - $user = common_current_user(); - $this->out->elementStart('li', array('class' => 'application h-entry', 'id' => 'oauthclient-' . $this->application->id)); diff --git a/lib/connectedappslist.php b/lib/connectedappslist.php index 7e5eb7482c..c2a27e75a8 100644 --- a/lib/connectedappslist.php +++ b/lib/connectedappslist.php @@ -46,18 +46,14 @@ class ConnectedAppsList extends Widget /** Owner of this list */ var $owner = null; - /** Action object using us. */ - var $action = null; - - function __construct($connection, $owner=null, $action=null) + function __construct($connection, Profile $owner, Action $out=null) { - parent::__construct($action); + parent::__construct($out); common_debug("ConnectedAppsList constructor"); $this->connection = $connection; - $this->owner = $owner; - $this->action = $action; + $this->owner = $owner; } /* Override this in subclasses. */ From 47ef917f62c445d29b2690d73c1a7ae991fcc4dd Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 01:18:28 +0200 Subject: [PATCH 14/40] oldschool settings adapted to FormAction TODO: Rename this "Layout", "Display" or something, since it might actually be interesting to have non-threaded views for some users. --- actions/oldschoolsettings.php | 45 ++++++++--------------------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/actions/oldschoolsettings.php b/actions/oldschoolsettings.php index 25ec13e481..5c07195478 100644 --- a/actions/oldschoolsettings.php +++ b/actions/oldschoolsettings.php @@ -28,11 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET')) { - // This check helps protect against security problems; - // your code file can't be executed directly from the web. - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Old-school settings @@ -77,35 +73,23 @@ class OldschoolsettingsAction extends SettingsAction * @return boolean true */ - function prepare($argarray) + protected function doPreparation() { if (!common_config('oldschool', 'enabled')) { throw new ClientException("Old-school settings not enabled."); } - parent::prepare($argarray); - return true; } - /** - * Handler method - * - * @param array $argarray is ignored since it's now passed in in prepare() - * - * @return void - */ - - function handlePost() + function doPost() { - $user = common_current_user(); - - $osp = Old_school_prefs::getKV('user_id', $user->id); + $osp = Old_school_prefs::getKV('user_id', $this->scoped->getID()); $orig = null; if (!empty($osp)) { $orig = clone($osp); } else { $osp = new Old_school_prefs(); - $osp->user_id = $user->id; + $osp->user_id = $this->scoped->getID(); $osp->created = common_sql_now(); } @@ -113,34 +97,25 @@ class OldschoolsettingsAction extends SettingsAction $osp->stream_nicknames = $this->boolean('stream_nicknames'); $osp->modified = common_sql_now(); - if (!empty($orig)) { + if ($orig instanceof Old_school_prefs) { $osp->update($orig); } else { $osp->insert(); } // TRANS: Confirmation shown when user profile settings are saved. - $this->showForm(_('Settings saved.'), true); - - return; - } - - function showContent() - { - $user = common_current_user(); - $form = new OldSchoolForm($this, $user); - $form->show(); + return _('Settings saved.'); } } -class OldSchoolForm extends Form +class OldSchoolSettingsForm extends Form { var $user; - function __construct($out, $user) + function __construct(Action $out) { parent::__construct($out); - $this->user = $user; + $this->user = $out->getScoped()->getUser(); } /** From cfaaf3c13cd97b73c4a7033f7714e36e294e3d9b Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 01:47:43 +0200 Subject: [PATCH 15/40] PasswordsettingsAction aligned with FormAction Also made some changes in the password "munging" function call common_munge_password to accept a profile instead of user ID (which was only there because stoneage StatusNet used the ID to generate a not-very-random salt, but nowadays we primarily use AuthCrypt plugin). --- EVENTS.txt | 4 +- actions/passwordsettings.php | 72 +++++-------------- actions/recoverpassword.php | 2 +- classes/Profile.php | 12 ++++ classes/User.php | 7 +- lib/authenticationplugin.php | 6 +- lib/util.php | 5 +- plugins/AuthCrypt/AuthCryptPlugin.php | 8 +-- .../actions/confirmfirstemail.php | 2 +- 9 files changed, 47 insertions(+), 71 deletions(-) diff --git a/EVENTS.txt b/EVENTS.txt index cfba97403b..2f91a305f0 100644 --- a/EVENTS.txt +++ b/EVENTS.txt @@ -615,12 +615,12 @@ EndCheckPassword: After checking a username/password pair - $authenticatedUser: User object if credentials match a user, else null. StartChangePassword: Before changing a password -- $user: user +- Profile $target: The profile of the User that is changing password - $oldpassword: the user's old password - $newpassword: the desired new password EndChangePassword: After changing a password -- $user: user +- Profile $target: The profile of the User that just changed its password StartHashPassword: Generate a hashed version of the password (like a salted crypt) - &$hashed: Hashed version of the password, later put in the database diff --git a/actions/passwordsettings.php b/actions/passwordsettings.php index db36b612a2..cfdb6c7817 100644 --- a/actions/passwordsettings.php +++ b/actions/passwordsettings.php @@ -28,11 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} - - +if (!defined('STATUSNET')) { exit(1); } /** * Change password @@ -77,18 +73,8 @@ class PasswordsettingsAction extends SettingsAction $this->autofocus('oldpassword'); } - /** - * Content area of the page - * - * Shows a form for changing the password - * - * @return void - */ - function showContent() { - $user = common_current_user(); - $this->elementStart('form', array('method' => 'POST', 'id' => 'form_password', 'class' => 'form_settings', @@ -102,7 +88,7 @@ class PasswordsettingsAction extends SettingsAction $this->elementStart('ul', 'form_data'); // Users who logged in with OpenID won't have a pwd - if ($user->password) { + if ($this->scoped->hasPassword()) { $this->elementStart('li'); // TRANS: Field label on page where to change password. $this->password('oldpassword', _('Old password')); @@ -129,29 +115,8 @@ class PasswordsettingsAction extends SettingsAction $this->elementEnd('form'); } - /** - * Handle a post - * - * Validate input and save changes. Reload the form with a success - * or error message. - * - * @return void - */ - function handlePost() + protected function doPost() { - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - - $user = common_current_user(); - assert(!is_null($user)); // should already be checked - // FIXME: scrub input $newpassword = $this->arg('newpassword'); @@ -161,49 +126,44 @@ class PasswordsettingsAction extends SettingsAction if (strlen($newpassword) < 6) { // TRANS: Form validation error on page where to change password. - $this->showForm(_('Password must be 6 or more characters.')); - return; + throw new ClientException(_('Password must be 6 or more characters.')); } else if (0 != strcmp($newpassword, $confirm)) { // TRANS: Form validation error on password change when password confirmation does not match. - $this->showForm(_('Passwords do not match.')); - return; + throw new ClientException(_('Passwords do not match.')); } - if ($user->password) { + $oldpassword = null; + if ($this->scoped->hasPassword()) { $oldpassword = $this->arg('oldpassword'); - if (!common_check_user($user->nickname, $oldpassword)) { + if (!common_check_user($this->scoped->getNickname(), $oldpassword)) { // TRANS: Form validation error on page where to change password. - $this->showForm(_('Incorrect old password.')); - return; + throw new ClientException(_('Incorrect old password.')); } - }else{ - $oldpassword = null; } - $success = false; - if(Event::handle('StartChangePassword', array($user, $oldpassword, $newpassword))){ + if (Event::handle('StartChangePassword', array($this->scoped, $oldpassword, $newpassword))) { //no handler changed the password, so change the password internally + $user = $this->scoped->getUser(); $original = clone($user); - $user->password = common_munge_password($newpassword, $user->id); + $user->password = common_munge_password($newpassword, $this->scoped); $val = $user->validate(); if ($val !== true) { // TRANS: Form validation error on page where to change password. - $this->showForm(_('Error saving user; invalid.')); - return; + throw new ServerException(_('Error saving user; invalid.')); } if (!$user->update($original)) { // TRANS: Server error displayed on page where to change password when password change // TRANS: could not be made because of a server error. - $this->serverError(_('Cannot save new password.')); + throw new ServerException(_('Cannot save new password.')); } - Event::handle('EndChangePassword', array($user)); + Event::handle('EndChangePassword', array($this->scoped)); } // TRANS: Form validation notice on page where to change password. - $this->showForm(_('Password saved.'), true); + return _('Password saved.'); } } diff --git a/actions/recoverpassword.php b/actions/recoverpassword.php index 060ba83510..1cbb73fd4a 100644 --- a/actions/recoverpassword.php +++ b/actions/recoverpassword.php @@ -325,7 +325,7 @@ class RecoverpasswordAction extends Action $original = clone($user); - $user->password = common_munge_password($newpassword, $user->id); + $user->password = common_munge_password($newpassword, $user->getProfile()); if (!$user->update($original)) { common_log_db_error($user, 'UPDATE', __FILE__); diff --git a/classes/Profile.php b/classes/Profile.php index 384eaa0726..5709a15d01 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -138,6 +138,18 @@ class Profile extends Managed_DataObject return true; } + // Returns false if the user has no password (which will always + // be the case for remote users). This can be the case for OpenID + // logins or other mechanisms which don't store a password hash. + public function hasPassword() + { + try { + return !empty($this->getUser()->hasPassword()); + } catch (NoSuchUserException $e) { + return false; + } + } + public function getObjectType() { // FIXME: More types... like peopletags and whatever diff --git a/classes/User.php b/classes/User.php index e1ce33be18..175d945401 100644 --- a/classes/User.php +++ b/classes/User.php @@ -299,7 +299,7 @@ class User extends Managed_DataObject } if (!empty($password)) { // may not have a password for OpenID users - $user->password = common_munge_password($password, $id); + $user->password = common_munge_password($password); } $result = $user->insert(); @@ -1015,6 +1015,11 @@ class User extends Managed_DataObject return $this->getProfile()->isPrivateStream(); } + public function hasPassword() + { + return !empty($this->password); + } + public function delPref($namespace, $topic) { return $this->getProfile()->delPref($namespace, $topic); diff --git a/lib/authenticationplugin.php b/lib/authenticationplugin.php index 66f11ca1a9..c33b0ef8a8 100644 --- a/lib/authenticationplugin.php +++ b/lib/authenticationplugin.php @@ -201,13 +201,13 @@ abstract class AuthenticationPlugin extends Plugin } } - function onStartChangePassword($user,$oldpassword,$newpassword) + function onStartChangePassword(Profile $target ,$oldpassword, $newpassword) { if($this->password_changeable){ $user_username = new User_username(); - $user_username->user_id=$user->id; + $user_username->user_id = $target->getID(); $user_username->provider_name=$this->provider_name; - if($user_username->find() && $user_username->fetch()){ + if ($user_username->find(true)) { $authenticated = $this->checkPassword($user_username->username, $oldpassword); if($authenticated){ $result = $this->changePassword($user_username->username,$oldpassword,$newpassword); diff --git a/lib/util.php b/lib/util.php index 4949231c52..1ff5b13b93 100644 --- a/lib/util.php +++ b/lib/util.php @@ -210,7 +210,7 @@ function common_language() /** * Salted, hashed passwords are stored in the DB. */ -function common_munge_password($password, $id, Profile $profile=null) +function common_munge_password($password, Profile $profile=null) { $hashed = null; @@ -245,8 +245,7 @@ function common_check_user($nickname, $password) } if ($user instanceof User && !empty($password)) { - if (0 == strcmp(common_munge_password($password, $user->id), - $user->password)) { + if (0 == strcmp(common_munge_password($password, $user->getProfile()), $user->password)) { //internal checking passed $authenticatedUser = $user; } diff --git a/plugins/AuthCrypt/AuthCryptPlugin.php b/plugins/AuthCrypt/AuthCryptPlugin.php index c669566772..540019f9c2 100644 --- a/plugins/AuthCrypt/AuthCryptPlugin.php +++ b/plugins/AuthCrypt/AuthCryptPlugin.php @@ -110,17 +110,17 @@ class AuthCryptPlugin extends AuthenticationPlugin * EVENTS */ - public function onStartChangePassword($user, $oldpassword, $newpassword) + public function onStartChangePassword(Profile $target, $oldpassword, $newpassword) { - if (!$this->checkPassword($user->nickname, $oldpassword)) { + if (!$this->checkPassword($target->getNickname(), $oldpassword)) { // if we ARE in overwrite mode, test password with common_check_user - if (!$this->overwrite || !common_check_user($user->nickname, $oldpassword)) { + if (!$this->overwrite || !common_check_user($target->getNickname(), $oldpassword)) { // either we're not in overwrite mode, or the password was incorrect return !$this->authoritative; } // oldpassword was apparently ok } - $changed = $this->changePassword($user->nickname, $oldpassword, $newpassword); + $changed = $this->changePassword($target->getNickname(), $oldpassword, $newpassword); return (!$changed && empty($this->authoritative)); } diff --git a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php index cc4b646f6b..d0d1893739 100644 --- a/plugins/RequireValidatedEmail/actions/confirmfirstemail.php +++ b/plugins/RequireValidatedEmail/actions/confirmfirstemail.php @@ -157,7 +157,7 @@ class ConfirmfirstemailAction extends Action $orig = clone($this->user); - $this->user->password = common_munge_password($this->password, $this->user->id); + $this->user->password = common_munge_password($this->password, $this->user->getProfile()); $this->user->update($orig); From 53e820b46667c2f0e22ffa0a2f91e847a02f4cb8 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 11:22:25 +0200 Subject: [PATCH 16/40] Maximum character limit with utf8mb4 is 191 in varchar --- actions/profilesettings.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/actions/profilesettings.php b/actions/profilesettings.php index 7d3143d4b1..3bdad42d9a 100644 --- a/actions/profilesettings.php +++ b/actions/profilesettings.php @@ -275,9 +275,9 @@ class ProfilesettingsAction extends SettingsAction // TRANS: Validation error in form for profile settings. $this->showForm(_('Homepage is not a valid URL.')); return; - } else if (!is_null($fullname) && mb_strlen($fullname) > 255) { + } else if (!is_null($fullname) && mb_strlen($fullname) > 191) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Full name is too long (maximum 255 characters).')); + $this->showForm(_('Full name is too long (maximum 191 characters).')); return; } else if (Profile::bioTooLong($bio)) { // TRANS: Validation error in form for profile settings. @@ -288,9 +288,9 @@ class ProfilesettingsAction extends SettingsAction Profile::maxBio()), Profile::maxBio())); return; - } else if (!is_null($location) && mb_strlen($location) > 255) { + } else if (!is_null($location) && mb_strlen($location) > 191) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Location is too long (maximum 255 characters).')); + $this->showForm(_('Location is too long (maximum 191 characters).')); return; } else if (is_null($timezone) || !in_array($timezone, DateTimeZone::listIdentifiers())) { // TRANS: Validation error in form for profile settings. From 9f82da07f105792403807ab444de873bfcd7a45f Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 12:09:24 +0200 Subject: [PATCH 17/40] ProfilesettingsAction and related stuff modernised --- actions/profilesettings.php | 137 +++++++++++++++--------------------- classes/Profile.php | 5 ++ classes/Profile_tag.php | 52 +++++++------- classes/User.php | 10 --- 4 files changed, 85 insertions(+), 119 deletions(-) diff --git a/actions/profilesettings.php b/actions/profilesettings.php index 3bdad42d9a..5804f21ca5 100644 --- a/actions/profilesettings.php +++ b/actions/profilesettings.php @@ -82,7 +82,6 @@ class ProfilesettingsAction extends SettingsAction */ function showContent() { - $profile = $this->scoped; $user = $this->scoped->getUser(); $this->elementStart('form', array('method' => 'post', @@ -100,7 +99,7 @@ class ProfilesettingsAction extends SettingsAction $this->elementStart('li'); // TRANS: Field label in form for profile settings. $this->input('nickname', _('Nickname'), - $this->arg('nickname') ?: $profile->nickname, + $this->trimmed('nickname') ?: $this->scoped->getNickname(), // TRANS: Tooltip for field label in form for profile settings. _('1-64 lowercase letters or numbers, no punctuation or spaces.'), null, false, // "name" (will be set to id), then "required" @@ -111,12 +110,12 @@ class ProfilesettingsAction extends SettingsAction $this->elementStart('li'); // TRANS: Field label in form for profile settings. $this->input('fullname', _('Full name'), - ($this->arg('fullname')) ? $this->arg('fullname') : $profile->fullname); + $this->trimmed('fullname') ?: $this->scoped->getFullname()); $this->elementEnd('li'); $this->elementStart('li'); // TRANS: Field label in form for profile settings. $this->input('homepage', _('Homepage'), - ($this->arg('homepage')) ? $this->arg('homepage') : $profile->homepage, + $this->trimmed('homepage') ?: $this->scoped->getHomepage(), // TRANS: Tooltip for field label in form for profile settings. _('URL of your homepage, blog, or profile on another site.')); $this->elementEnd('li'); @@ -137,13 +136,13 @@ class ProfilesettingsAction extends SettingsAction // TRANS: Text area label in form for profile settings where users can provide // TRANS: their biography. $this->textarea('bio', _('Bio'), - ($this->arg('bio')) ? $this->arg('bio') : $profile->bio, + $this->trimmed('bio') ?: $this->scoped->getDescription(), $bioInstr); $this->elementEnd('li'); $this->elementStart('li'); // TRANS: Field label in form for profile settings. $this->input('location', _('Location'), - ($this->arg('location')) ? $this->arg('location') : $profile->location, + $this->trimmed('location') ?: $this->scoped->location, // TRANS: Tooltip for field label in form for profile settings. _('Where you are, like "City, State (or Region), Country".')); $this->elementEnd('li'); @@ -152,14 +151,14 @@ class ProfilesettingsAction extends SettingsAction // TRANS: Checkbox label in form for profile settings. $this->checkbox('sharelocation', _('Share my current location when posting notices'), ($this->arg('sharelocation')) ? - $this->arg('sharelocation') : $this->scoped->shareLocation()); + $this->boolean('sharelocation') : $this->scoped->shareLocation()); $this->elementEnd('li'); } Event::handle('EndProfileFormData', array($this)); $this->elementStart('li'); // TRANS: Field label in form for profile settings. $this->input('tags', _('Tags'), - ($this->arg('tags')) ? $this->arg('tags') : implode(' ', $user->getSelfTags()), + $this->trimmed('tags') ?: implode(' ', Profile_tag::getSelfTagsArray($this->scoped)), // TRANS: Tooltip for field label in form for profile settings. _('Tags for yourself (letters, numbers, -, ., and _), comma- or space- separated.')); $this->elementEnd('li'); @@ -228,17 +227,8 @@ class ProfilesettingsAction extends SettingsAction * * @return void */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Form validation error. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if (Event::handle('StartProfileSaveForm', array($this))) { // $nickname will only be set if this changenick value is true. @@ -246,15 +236,13 @@ class ProfilesettingsAction extends SettingsAction try { $nickname = Nickname::normalize($this->trimmed('nickname'), true); } catch (NicknameTakenException $e) { - // Abort only if the nickname is occupied by another local profile - if ($e->profile->id != $this->scoped->id) { - $this->showForm($e->getMessage()); - return; + // Abort only if the nickname is occupied by _another_ local user profile + if (!$this->scoped->sameAs($e->profile)) { + throw $e; } - $nickname = Nickname::normalize($this->trimmed('nickname')); // without in-use check this time - } catch (NicknameException $e) { - $this->showForm($e->getMessage()); - return; + // Since the variable wasn't set before the exception was thrown, let's run + // the normalize sequence again, but without in-use check this time. + $nickname = Nickname::normalize($this->trimmed('nickname')); } } @@ -273,33 +261,27 @@ class ProfilesettingsAction extends SettingsAction if (!is_null($homepage) && (strlen($homepage) > 0) && !common_valid_http_url($homepage)) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Homepage is not a valid URL.')); - return; + throw new ClientException(_('Homepage is not a valid URL.')); } else if (!is_null($fullname) && mb_strlen($fullname) > 191) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Full name is too long (maximum 191 characters).')); - return; + throw new ClientException(_('Full name is too long (maximum 191 characters).')); } else if (Profile::bioTooLong($bio)) { // TRANS: Validation error in form for profile settings. // TRANS: Plural form is used based on the maximum number of allowed // TRANS: characters for the biography (%d). - $this->showForm(sprintf(_m('Bio is too long (maximum %d character).', + throw new ClientException(sprintf(_m('Bio is too long (maximum %d character).', 'Bio is too long (maximum %d characters).', Profile::maxBio()), Profile::maxBio())); - return; } else if (!is_null($location) && mb_strlen($location) > 191) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Location is too long (maximum 191 characters).')); - return; + throw new ClientException(_('Location is too long (maximum 191 characters).')); } else if (is_null($timezone) || !in_array($timezone, DateTimeZone::listIdentifiers())) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Timezone not selected.')); - return; + throw new ClientException(_('Timezone not selected.')); } else if (!is_null($language) && strlen($language) > 50) { // TRANS: Validation error in form for profile settings. - $this->showForm(_('Language is too long (maximum 50 characters).')); - return; + throw new ClientException(_('Language is too long (maximum 50 characters).')); } $tags = array(); @@ -315,15 +297,14 @@ class ProfilesettingsAction extends SettingsAction if (!common_valid_profile_tag($tag)) { // TRANS: Validation error in form for profile settings. // TRANS: %s is an invalid tag. - $this->showForm(sprintf(_('Invalid tag: "%s".'), $tag)); - return; + throw new ClientException(sprintf(_('Invalid tag: "%s".'), $tag)); } $tag_priv[$tag] = $private; } } - $user = common_current_user(); + $user = $this->scoped->getUser(); $user->query('BEGIN'); // $user->nickname is updated through Profile->update(); @@ -346,54 +327,53 @@ class ProfilesettingsAction extends SettingsAction $result = $user->update($original); if ($result === false) { common_log_db_error($user, 'UPDATE', __FILE__); + $user->query('ROLLBACK'); // TRANS: Server error thrown when user profile settings could not be updated to // TRANS: automatically subscribe to any subscriber. - $this->serverError(_('Could not update user for autosubscribe or subscribe_policy.')); + throw new ServerException(_('Could not update user for autosubscribe or subscribe_policy.')); } // Re-initialize language environment if it changed common_init_language(); } - $profile = $user->getProfile(); + $original = clone($this->scoped); - $orig_profile = clone($profile); - - if (common_config('profile', 'changenick') == true && $profile->nickname !== $nickname) { + if (common_config('profile', 'changenick') == true && $this->scoped->getNickname() !== $nickname) { assert(Nickname::normalize($nickname)===$nickname); - common_debug("Changing user nickname from '{$profile->nickname}' to '{$nickname}'."); - $profile->nickname = $nickname; - $profile->profileurl = common_profile_url($profile->nickname); + common_debug("Changing user nickname from '{$this->scoped->getNickname()}' to '{$nickname}'."); + $this->scoped->nickname = $nickname; + $this->scoped->profileurl = common_profile_url($this->scoped->getNickname()); } - $profile->fullname = $fullname; - $profile->homepage = $homepage; - $profile->bio = $bio; - $profile->location = $location; + $this->scoped->fullname = $fullname; + $this->scoped->homepage = $homepage; + $this->scoped->bio = $bio; + $this->scoped->location = $location; $loc = Location::fromName($location); if (empty($loc)) { - $profile->lat = null; - $profile->lon = null; - $profile->location_id = null; - $profile->location_ns = null; + $this->scoped->lat = null; + $this->scoped->lon = null; + $this->scoped->location_id = null; + $this->scoped->location_ns = null; } else { - $profile->lat = $loc->lat; - $profile->lon = $loc->lon; - $profile->location_id = $loc->location_id; - $profile->location_ns = $loc->location_ns; + $this->scoped->lat = $loc->lat; + $this->scoped->lon = $loc->lon; + $this->scoped->location_id = $loc->location_id; + $this->scoped->location_ns = $loc->location_ns; } if (common_config('location', 'share') == 'user') { $exists = false; - $prefs = User_location_prefs::getKV('user_id', $user->id); + $prefs = User_location_prefs::getKV('user_id', $this->scoped->getID()); if (empty($prefs)) { $prefs = new User_location_prefs(); - $prefs->user_id = $user->id; + $prefs->user_id = $this->scoped->getID(); $prefs->created = common_sql_now(); } else { $exists = true; @@ -410,42 +390,37 @@ class ProfilesettingsAction extends SettingsAction if ($result === false) { common_log_db_error($prefs, ($exists) ? 'UPDATE' : 'INSERT', __FILE__); + $user->query('ROLLBACK'); // TRANS: Server error thrown when user profile location preference settings could not be updated. - $this->serverError(_('Could not save location prefs.')); + throw new ServerException(_('Could not save location prefs.')); } } - common_debug('Old profile: ' . common_log_objstring($orig_profile), __FILE__); - common_debug('New profile: ' . common_log_objstring($profile), __FILE__); + common_debug('Old profile: ' . common_log_objstring($original), __FILE__); + common_debug('New profile: ' . common_log_objstring($this->scoped), __FILE__); - $result = $profile->update($orig_profile); + $result = $this->scoped->update($original); if ($result === false) { - common_log_db_error($profile, 'UPDATE', __FILE__); + common_log_db_error($this->scoped, 'UPDATE', __FILE__); + $user->query('ROLLBACK'); // TRANS: Server error thrown when user profile settings could not be saved. - $this->serverError(_('Could not save profile.')); + throw new ServerException(_('Could not save profile.')); } // Set the user tags - $result = $user->setSelfTags($tags, $tag_priv); - - if (!$result) { - // TRANS: Server error thrown when user profile settings tags could not be saved. - $this->serverError(_('Could not save tags.')); - } + $result = Profile_tag::setSelfTags($this->scoped, $tags, $tag_priv); $user->query('COMMIT'); Event::handle('EndProfileSaveForm', array($this)); // TRANS: Confirmation shown when user profile settings are saved. - $this->showForm(_('Settings saved.'), true); + return _('Settings saved.'); } } function showAside() { - $user = common_current_user(); - $this->elementStart('div', array('id' => 'aside_primary', 'class' => 'aside')); @@ -453,7 +428,7 @@ class ProfilesettingsAction extends SettingsAction 'class' => 'section')); $this->elementStart('ul'); if (Event::handle('StartProfileSettingsActions', array($this))) { - if ($user->hasRight(Right::BACKUPACCOUNT)) { + if ($this->scoped->hasRight(Right::BACKUPACCOUNT)) { $this->elementStart('li'); $this->element('a', array('href' => common_local_url('backupaccount')), @@ -461,7 +436,7 @@ class ProfilesettingsAction extends SettingsAction _('Backup account')); $this->elementEnd('li'); } - if ($user->hasRight(Right::DELETEACCOUNT)) { + if ($this->scoped->hasRight(Right::DELETEACCOUNT)) { $this->elementStart('li'); $this->element('a', array('href' => common_local_url('deleteaccount')), @@ -469,7 +444,7 @@ class ProfilesettingsAction extends SettingsAction _('Delete account')); $this->elementEnd('li'); } - if ($user->hasRight(Right::RESTOREACCOUNT)) { + if ($this->scoped->hasRight(Right::RESTOREACCOUNT)) { $this->elementStart('li'); $this->element('a', array('href' => common_local_url('restoreaccount')), diff --git a/classes/Profile.php b/classes/Profile.php index 5709a15d01..5359ffb58b 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -1407,6 +1407,11 @@ class Profile extends Managed_DataObject return $this->fullname; } + public function getHomepage() + { + return $this->homepage; + } + public function getDescription() { return $this->bio; diff --git a/classes/Profile_tag.php b/classes/Profile_tag.php index 6d6f49bedc..8034e68ccf 100644 --- a/classes/Profile_tag.php +++ b/classes/Profile_tag.php @@ -2,22 +2,15 @@ /** * Table Definition for profile_tag */ -require_once INSTALLDIR.'/classes/Memcached_DataObject.php'; class Profile_tag extends Managed_DataObject { - ###START_AUTOCODE - /* the code below is auto generated do not remove the above tag */ - public $__table = 'profile_tag'; // table name public $tagger; // int(4) primary_key not_null public $tagged; // int(4) primary_key not_null public $tag; // varchar(64) primary_key not_null public $modified; // timestamp() not_null default_CURRENT_TIMESTAMP - /* the code above is auto generated do not remove the tag below */ - ###END_AUTOCODE - public static function schemaDef() { return array( @@ -52,6 +45,16 @@ class Profile_tag extends Managed_DataObject return Profile_list::pkeyGet(array('tagger' => $this->tagger, 'tag' => $this->tag)); } + static function getSelfTagsArray(Profile $target) + { + return self::getTagsArray($target->getID(), $target->getID(), $target); + } + + static function setSelfTags(Profile $target, array $newtags, array $privacy=array()) + { + return self::setTags($target->getID(), $target->getID(), $newtags, $privacy); + } + static function getTags($tagger, $tagged, $auth_user=null) { $profile_list = new Profile_list(); @@ -88,7 +91,7 @@ class Profile_tag extends Managed_DataObject return $profile_list; } - static function getTagsArray($tagger, $tagged, $auth_user_id=null) + static function getTagsArray($tagger, $tagged, Profile $scoped=null) { $ptag = new Profile_tag(); @@ -100,7 +103,7 @@ class Profile_tag extends Managed_DataObject 'and profile_tag.tagged = %d ', $tagger, $tagged); - if ($auth_user_id != $tagger) { + if (!$scoped instanceof Profile || $scoped->getID() !== $tagger) { $qry .= 'and profile_list.private = 0'; } @@ -115,10 +118,10 @@ class Profile_tag extends Managed_DataObject return $tags; } - static function setTags($tagger, $tagged, $newtags, $privacy=array()) { + static function setTags($tagger, $tagged, array $newtags, array $privacy=array()) { $newtags = array_unique($newtags); - $oldtags = self::getTagsArray($tagger, $tagged, $tagger); + $oldtags = self::getTagsArray($tagger, $tagged, Profile::getByID($tagger)); $ptag = new Profile_tag(); @@ -149,19 +152,18 @@ class Profile_tag extends Managed_DataObject 'tag' => $tag)); # if tag already exists, return it - if(!empty($ptag)) { + if ($ptag instanceof Profile_tag) { return $ptag; } - $tagger_profile = Profile::getKV('id', $tagger); - $tagged_profile = Profile::getKV('id', $tagged); + $tagger_profile = Profile::getByID($tagger); + $tagged_profile = Profile::getByID($tagged); if (Event::handle('StartTagProfile', array($tagger_profile, $tagged_profile, $tag))) { if (!$tagger_profile->canTag($tagged_profile)) { // TRANS: Client exception thrown trying to set a tag for a user that cannot be tagged. throw new ClientException(_('You cannot tag this user.')); - return false; } $tags = new Profile_list(); @@ -174,7 +176,6 @@ class Profile_tag extends Managed_DataObject 'which is the maximum allowed number of tags. ' . 'Try using or deleting some existing tags.'), common_config('peopletag', 'maxtags'))); - return false; } $plist = new Profile_list(); @@ -188,7 +189,6 @@ class Profile_tag extends Managed_DataObject 'which is the maximum allowed number. ' . 'Try unlisting others first.'), common_config('peopletag', 'maxpeople'), $tag)); - return false; } $newtag = new Profile_tag(); @@ -199,9 +199,9 @@ class Profile_tag extends Managed_DataObject $result = $newtag->insert(); - if (!$result) { common_log_db_error($newtag, 'INSERT', __FILE__); + $plist->query('ROLLBACK'); return false; } @@ -212,7 +212,6 @@ class Profile_tag extends Managed_DataObject $newtag->delete(); $profile_list->delete(); throw $e; - return false; } $profile_list->taggedCount(true); @@ -233,20 +232,17 @@ class Profile_tag extends Managed_DataObject if (Event::handle('StartUntagProfile', array($ptag))) { $orig = clone($ptag); $result = $ptag->delete(); - if (!$result) { + if ($result === false) { common_log_db_error($this, 'DELETE', __FILE__); return false; } Event::handle('EndUntagProfile', array($orig)); - if ($result) { - $profile_list = Profile_list::pkeyGet(array('tag' => $tag, 'tagger' => $tagger)); - if (!empty($profile_list)) { - $profile_list->taggedCount(true); - } - self::blowCaches($tagger, $tagged); - return true; + $profile_list = Profile_list::pkeyGet(array('tag' => $tag, 'tagger' => $tagger)); + if (!empty($profile_list)) { + $profile_list->taggedCount(true); } - return false; + self::blowCaches($tagger, $tagged); + return true; } } diff --git a/classes/User.php b/classes/User.php index 175d945401..5b9d7b51fe 100644 --- a/classes/User.php +++ b/classes/User.php @@ -462,16 +462,6 @@ class User extends Managed_DataObject return $this->getProfile()->getNotices($offset, $limit, $since_id, $before_id); } - function getSelfTags() - { - return Profile_tag::getTagsArray($this->id, $this->id, $this->id); - } - - function setSelfTags($newtags, $privacy) - { - return Profile_tag::setTags($this->id, $this->id, $newtags, $privacy); - } - function block(Profile $other) { // Add a new block record From 9045575e62f88644e7396a81883e898d808f5d3f Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 12:46:09 +0200 Subject: [PATCH 18/40] ExtendedProfile now works better as extended ProfilesettingsAction --- .../actions/profiledetailsettings.php | 230 +++++++----------- .../ExtendedProfile/lib/extendedprofile.php | 4 +- 2 files changed, 85 insertions(+), 149 deletions(-) diff --git a/plugins/ExtendedProfile/actions/profiledetailsettings.php b/plugins/ExtendedProfile/actions/profiledetailsettings.php index 8d8a33ec44..73bf364c7b 100644 --- a/plugins/ExtendedProfile/actions/profiledetailsettings.php +++ b/plugins/ExtendedProfile/actions/profiledetailsettings.php @@ -17,9 +17,7 @@ * along with this program. If not, see . */ -if (!defined('STATUSNET')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } class ProfileDetailSettingsAction extends ProfileSettingsAction { @@ -29,18 +27,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return _m('Extended profile settings'); } - /** - * Instructions for use - * - * @return instructions for use - */ - function getInstructions() - { - // TRANS: Usage instructions for profile settings. - return _m('You can update your personal profile info here '. - 'so people know more about you.'); - } - function showStylesheets() { parent::showStylesheets(); $this->cssLink('plugins/ExtendedProfile/css/profiledetail.css'); @@ -53,36 +39,21 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return true; } - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - $this->showForm( - // TRANS: Client error displayed when the session token does not match or is not given. - _m('There was a problem with your session token. ' - . 'Try again, please.' - ) - ); - return; + if ($this->arg('save')) { + return $this->saveDetails(); } - if ($this->arg('save')) { - $this->saveDetails(); - } else { - // TRANS: Message given submitting a form with an unknown action. - $this->showForm(_m('Unexpected form submission.')); - } + // TRANS: Message given submitting a form with an unknown action. + throw new ClientException(_m('Unexpected form submission.')); } function showContent() { - $cur = common_current_user(); - $profile = $cur->getProfile(); - $widget = new ExtendedProfileWidget( $this, - $profile, + $this->scoped, ExtendedProfileWidget::EDITABLE ); $widget->show(); @@ -92,49 +63,38 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction { common_debug(var_export($_POST, true)); - $user = common_current_user(); + $this->saveStandardProfileDetails(); - try { - $this->saveStandardProfileDetails($user); + $simpleFieldNames = array('title', 'spouse', 'kids', 'manager'); + $dateFieldNames = array('birthday'); - $profile = $user->getProfile(); - - $simpleFieldNames = array('title', 'spouse', 'kids', 'manager'); - $dateFieldNames = array('birthday'); - - foreach ($simpleFieldNames as $name) { - $value = $this->trimmed('extprofile-' . $name); - if (!empty($value)) { - $this->saveField($user, $name, $value); - } + foreach ($simpleFieldNames as $name) { + $value = $this->trimmed('extprofile-' . $name); + if (!empty($value)) { + $this->saveField($name, $value); } - - foreach ($dateFieldNames as $name) { - $value = $this->trimmed('extprofile-' . $name); - $dateVal = $this->parseDate($name, $value); - $this->saveField( - $user, - $name, - null, - null, - null, - $dateVal - ); - } - - $this->savePhoneNumbers($user); - $this->saveIms($user); - $this->saveWebsites($user); - $this->saveExperiences($user); - $this->saveEducations($user); - - } catch (Exception $e) { - $this->showForm($e->getMessage(), false); - return; } + foreach ($dateFieldNames as $name) { + $value = $this->trimmed('extprofile-' . $name); + $dateVal = $this->parseDate($name, $value); + $this->saveField( + $name, + null, + null, + null, + $dateVal + ); + } + + $this->savePhoneNumbers(); + $this->saveIms(); + $this->saveWebsites(); + $this->saveExperiences(); + $this->saveEducations(); + // TRANS: Success message after saving extended profile details. - $this->showForm(_m('Details saved.'), true); + return _m('Details saved.'); } @@ -168,15 +128,14 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return null; } - function savePhoneNumbers($user) { + function savePhoneNumbers() { $phones = $this->findPhoneNumbers(); - $this->removeAll($user, 'phone'); + $this->removeAll('phone'); $i = 0; foreach($phones as $phone) { if (!empty($phone['value'])) { ++$i; $this->saveField( - $user, 'phone', $phone['value'], $phone['rel'], @@ -226,15 +185,14 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return $imArray; } - function saveIms($user) { + function saveIms() { $ims = $this->findIms(); - $this->removeAll($user, 'im'); + $this->removeAll('im'); $i = 0; foreach($ims as $im) { if (!empty($im['value'])) { ++$i; $this->saveField( - $user, 'im', $im['value'], $im['rel'], @@ -262,9 +220,9 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return $wsArray; } - function saveWebsites($user) { + function saveWebsites() { $sites = $this->findWebsites(); - $this->removeAll($user, 'website'); + $this->removeAll('website'); $i = 0; foreach($sites as $site) { if (!empty($site['value']) && !common_valid_http_url($site['value'])) { @@ -276,7 +234,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction if (!empty($site['value'])) { ++$i; $this->saveField( - $user, 'website', $site['value'], $site['rel'], @@ -317,20 +274,19 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction return $expArray; } - function saveExperiences($user) { + function saveExperiences() { common_debug('save experiences'); $experiences = $this->findExperiences(); - $this->removeAll($user, 'company'); - $this->removeAll($user, 'start'); - $this->removeAll($user, 'end'); // also stores 'current' + $this->removeAll('company'); + $this->removeAll('start'); + $this->removeAll('end'); // also stores 'current' $i = 0; foreach($experiences as $experience) { if (!empty($experience['company'])) { ++$i; $this->saveField( - $user, 'company', $experience['company'], null, @@ -338,7 +294,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction ); $this->saveField( - $user, 'start', null, null, @@ -349,7 +304,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction // Save "current" employer indicator in rel if ($experience['current']) { $this->saveField( - $user, 'end', null, 'current', // rel @@ -357,7 +311,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction ); } else { $this->saveField( - $user, 'end', null, null, @@ -399,44 +352,40 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction } - function saveEducations($user) { + function saveEducations() { common_debug('save education'); $edus = $this->findEducations(); common_debug(var_export($edus, true)); - $this->removeAll($user, 'school'); - $this->removeAll($user, 'degree'); - $this->removeAll($user, 'degree_descr'); - $this->removeAll($user, 'school_start'); - $this->removeAll($user, 'school_end'); + $this->removeAll('school'); + $this->removeAll('degree'); + $this->removeAll('degree_descr'); + $this->removeAll('school_start'); + $this->removeAll('school_end'); $i = 0; foreach($edus as $edu) { if (!empty($edu['school'])) { ++$i; $this->saveField( - $user, 'school', $edu['school'], null, $i ); $this->saveField( - $user, 'degree', $edu['degree'], null, $i ); $this->saveField( - $user, 'degree_descr', $edu['description'], null, $i ); $this->saveField( - $user, 'school_start', null, null, @@ -445,7 +394,6 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction ); $this->saveField( - $user, 'school_end', null, null, @@ -491,35 +439,33 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction /** * Save an extended profile field as a Profile_detail * - * @param User $user the current user * @param string $name field name * @param string $value field value * @param string $rel field rel (type) * @param int $index index (fields can have multiple values) * @param date $date related date */ - function saveField($user, $name, $value, $rel = null, $index = null, $date = null) + function saveField($name, $value, $rel = null, $index = null, $date = null) { - $profile = $user->getProfile(); $detail = new Profile_detail(); - $detail->profile_id = $profile->id; + $detail->profile_id = $this->scoped->getID(); $detail->field_name = $name; $detail->value_index = $index; $result = $detail->find(true); - if (empty($result)) { - $detial->value_index = $index; + if (!$result instanceof Profile_detail) { + $detail->value_index = $index; $detail->rel = $rel; $detail->field_value = $value; $detail->date = $date; $detail->created = common_sql_now(); $result = $detail->insert(); - if (empty($result)) { + if ($result === false) { common_log_db_error($detail, 'INSERT', __FILE__); // TRANS: Server error displayed when a field could not be saved in the database. - $this->serverError(_m('Could not save profile details.')); + throw new ServerException(_m('Could not save profile details.')); } } else { $orig = clone($detail); @@ -529,21 +475,20 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction $detail->date = $date; $result = $detail->update($orig); - if (empty($result)) { + if ($result === false) { common_log_db_error($detail, 'UPDATE', __FILE__); // TRANS: Server error displayed when a field could not be saved in the database. - $this->serverError(_m('Could not save profile details.')); + throw new ServerException(_m('Could not save profile details.')); } } $detail->free(); } - function removeAll($user, $name) + function removeAll($name) { - $profile = $user->getProfile(); $detail = new Profile_detail(); - $detail->profile_id = $profile->id; + $detail->profile_id = $this->scoped->getID(); $detail->field_name = $name; $detail->delete(); $detail->free(); @@ -554,10 +499,8 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction * * XXX: There's a lot of dupe code here from ProfileSettingsAction. * Do not want. - * - * @param User $user the current user */ - function saveStandardProfileDetails($user) + function saveStandardProfileDetails() { $fullname = $this->trimmed('extprofile-fullname'); $location = $this->trimmed('extprofile-location'); @@ -581,54 +524,47 @@ class ProfileDetailSettingsAction extends ProfileSettingsAction } } - $profile = $user->getProfile(); - - $oldTags = $user->getSelfTags(); + $oldTags = Profile_tag::getSelfTagsArray($this->scoped); $newTags = array_diff($tags, $oldTags); - if ($fullname != $profile->fullname - || $location != $profile->location + if ($fullname != $this->scoped->getFullname() + || $location != $this->scoped->location || !empty($newTags) - || $bio != $profile->bio) { + || $bio != $this->scoped->getDescription()) { - $orig = clone($profile); + $orig = clone($this->scoped); - $profile->nickname = $user->nickname; - $profile->fullname = $fullname; - $profile->bio = $bio; - $profile->location = $location; + // Skipping nickname change here until we add logic for when the site allows it or not + // old Profilesettings will still let us do that. + + $this->scoped->fullname = $fullname; + $this->scoped->bio = $bio; + $this->scoped->location = $location; $loc = Location::fromName($location); if (empty($loc)) { - $profile->lat = null; - $profile->lon = null; - $profile->location_id = null; - $profile->location_ns = null; + $this->scoped->lat = null; + $this->scoped->lon = null; + $this->scoped->location_id = null; + $this->scoped->location_ns = null; } else { - $profile->lat = $loc->lat; - $profile->lon = $loc->lon; - $profile->location_id = $loc->location_id; - $profile->location_ns = $loc->location_ns; + $this->scoped->lat = $loc->lat; + $this->scoped->lon = $loc->lon; + $this->scoped->location_id = $loc->location_id; + $this->scoped->location_ns = $loc->location_ns; } - $profile->profileurl = common_profile_url($user->nickname); - - $result = $profile->update($orig); + $result = $this->scoped->update($orig); if ($result === false) { - common_log_db_error($profile, 'UPDATE', __FILE__); + common_log_db_error($this->scoped, 'UPDATE', __FILE__); // TRANS: Server error thrown when user profile settings could not be saved. - $this->serverError(_m('Could not save profile.')); + throw new ServerException(_m('Could not save profile.')); } // Set the user tags - $result = $user->setSelfTags($tags); - - if (!$result) { - // TRANS: Server error thrown when user profile settings tags could not be saved. - $this->serverError(_m('Could not save tags.')); - } + $result = Profile_tag::setSelfTags($this->scoped, $tags); Event::handle('EndProfileSaveForm', array($this)); } diff --git a/plugins/ExtendedProfile/lib/extendedprofile.php b/plugins/ExtendedProfile/lib/extendedprofile.php index d73170b235..3682e6fc9d 100644 --- a/plugins/ExtendedProfile/lib/extendedprofile.php +++ b/plugins/ExtendedProfile/lib/extendedprofile.php @@ -52,7 +52,7 @@ class ExtendedProfile function loadFields() { $detail = new Profile_detail(); - $detail->profile_id = $this->profile->id; + $detail->profile_id = $this->profile->getID(); $detail->find(); $fields = array(); @@ -71,7 +71,7 @@ class ExtendedProfile */ function getTags() { - return implode(' ', $this->user->getSelfTags()); + return implode(' ', Profile_tag::getSelfTagsArray($this->profile)); } /** From a093dea38c9a6b5af6d169ccfb5ea14eec76ada1 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 12:46:24 +0200 Subject: [PATCH 19/40] ExtendedProfile is not something we want by default. --- lib/siteprofile.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/siteprofile.php b/lib/siteprofile.php index 4bbffea754..a2c09efb2f 100644 --- a/lib/siteprofile.php +++ b/lib/siteprofile.php @@ -110,7 +110,6 @@ class PublicSite extends SiteProfileSettings 'plugins' => array( 'core' => self::corePlugins(), 'default' => array_merge(self::defaultPlugins(), array( - 'ExtendedProfile' => array(), 'RegisterThrottle' => array(), )) ), From 2dd979d3f83e6e2a314308fdd2a205b02fa406a8 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 14:29:07 +0200 Subject: [PATCH 20/40] SMS Settings now better adapted to FormAction --- actions/smssettings.php | 168 +++++++++++++++------------------------- 1 file changed, 63 insertions(+), 105 deletions(-) diff --git a/actions/smssettings.php b/actions/smssettings.php index ec8841281d..ca6a7d04ef 100644 --- a/actions/smssettings.php +++ b/actions/smssettings.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Settings for SMS @@ -45,6 +43,14 @@ if (!defined('STATUSNET') && !defined('LACONICA')) { class SmssettingsAction extends SettingsAction { + protected function doPreparation() + { + if (!common_config('sms', 'enabled')) { + // TRANS: Message given in the SMS settings if SMS is not enabled on the site. + throw new ServerException(_('SMS is not available.')); + } + } + /** * Title of the page * @@ -86,14 +92,7 @@ class SmssettingsAction extends SettingsAction */ function showContent() { - if (!common_config('sms', 'enabled')) { - $this->element('div', array('class' => 'error'), - // TRANS: Message given in the SMS settings if SMS is not enabled on the site. - _('SMS is not available.')); - return; - } - - $user = common_current_user(); + $user = $this->scoped->getUser(); $this->elementStart('form', array('method' => 'post', 'id' => 'form_settings_sms', @@ -118,8 +117,8 @@ class SmssettingsAction extends SettingsAction // TRANS: Button label to remove a confirmed SMS address. $this->submit('remove', _m('BUTTON','Remove')); } else { - $confirm = $this->getConfirmation(); - if ($confirm) { + try { + $confirm = $this->getConfirmation(); $carrier = Sms_carrier::getKV($confirm->address_extra); $this->element('p', 'form_unconfirmed', $confirm->address . ' (' . $carrier->name . ')'); @@ -141,7 +140,7 @@ class SmssettingsAction extends SettingsAction $this->elementEnd('ul'); // TRANS: Button label to confirm SMS confirmation code in SMS settings. $this->submit('confirm', _m('BUTTON','Confirm')); - } else { + } catch (NoResultException $e) { $this->elementStart('ul', 'form_data'); $this->elementStart('li'); // TRANS: Field label for SMS phone number input in SMS settings form. @@ -216,60 +215,38 @@ class SmssettingsAction extends SettingsAction */ function getConfirmation() { - $user = common_current_user(); - $confirm = new Confirm_address(); - $confirm->user_id = $user->id; + $confirm->user_id = $this->scoped->getID(); $confirm->address_type = 'sms'; if ($confirm->find(true)) { return $confirm; - } else { - return null; } + + throw new NoResultException($confirm); } - /** - * Handle posts to this form - * - * Based on the button that was pressed, muxes out to other functions - * to do the actual task requested. - * - * All sub-functions reload the form with a message -- success or failure. - * - * @return void - */ - function handlePost() + protected function doPost() { - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } if ($this->arg('save')) { - $this->savePreferences(); + return $this->savePreferences(); } else if ($this->arg('add')) { - $this->addAddress(); + return $this->addAddress(); } else if ($this->arg('cancel')) { - $this->cancelConfirmation(); + return $this->cancelConfirmation(); } else if ($this->arg('remove')) { - $this->removeAddress(); + return $this->removeAddress(); } else if ($this->arg('removeincoming')) { - $this->removeIncoming(); + return $this->removeIncoming(); } else if ($this->arg('newincoming')) { - $this->newIncoming(); + return $this->newIncoming(); } else if ($this->arg('confirm')) { - $this->confirmCode(); - } else { - // TRANS: Message given submitting a form with an unknown action in SMS settings. - $this->showForm(_('Unexpected form submission.')); + return $this->confirmCode(); } + // TRANS: Message given submitting a form with an unknown action in SMS settings. + throw new ClientException(_('Unexpected form submission.')); } /** @@ -281,30 +258,26 @@ class SmssettingsAction extends SettingsAction */ function savePreferences() { - $smsnotify = $this->boolean('smsnotify'); - - $user = common_current_user(); - - assert(!is_null($user)); // should already be checked + $user = $this->scoped->getUser(); $user->query('BEGIN'); $original = clone($user); - $user->smsnotify = $smsnotify; + $user->smsnotify = $this->boolean('smsnotify'); $result = $user->update($original); if ($result === false) { common_log_db_error($user, 'UPDATE', __FILE__); // TRANS: Server error thrown on database error updating SMS preferences. - $this->serverError(_('Could not update user.')); + throw new ServerException(_('Could not update user.')); } $user->query('COMMIT'); // TRANS: Confirmation message for successful SMS preferences save. - $this->showForm(_('SMS preferences saved.'), true); + return _('SMS preferences saved.'); } /** @@ -324,28 +297,24 @@ class SmssettingsAction extends SettingsAction // Some validation - if (!$sms) { + if (empty($sms)) { // TRANS: Message given saving SMS phone number without having provided one. - $this->showForm(_('No phone number.')); - return; + throw new ClientException(_('No phone number.')); } - if (!$carrier_id) { + if (empty($carrier_id)) { // TRANS: Message given saving SMS phone number without having selected a carrier. - $this->showForm(_('No carrier selected.')); - return; + throw new ClientException(_('No carrier selected.')); } $sms = common_canonical_sms($sms); - if ($user->sms == $sms) { + if ($user->sms === $sms) { // TRANS: Message given saving SMS phone number that is already set. - $this->showForm(_('That is already your phone number.')); - return; + throw new AlreadyFulfilledException(_('That is already your phone number.')); } else if ($this->smsExists($sms)) { // TRANS: Message given saving SMS phone number that is already set for another user. - $this->showForm(_('That phone number already belongs to another user.')); - return; + throw new ClientException(_('That phone number already belongs to another user.')); } $confirm = new Confirm_address(); @@ -353,7 +322,7 @@ class SmssettingsAction extends SettingsAction $confirm->address = $sms; $confirm->address_extra = $carrier_id; $confirm->address_type = 'sms'; - $confirm->user_id = $user->id; + $confirm->user_id = $this->scoped->getID(); $confirm->code = common_confirmation_code(40); $result = $confirm->insert(); @@ -371,11 +340,9 @@ class SmssettingsAction extends SettingsAction $carrier->toEmailAddress($sms)); // TRANS: Message given saving valid SMS phone number that is to be confirmed. - $msg = _('A confirmation code was sent to the phone number you added. '. + return _('A confirmation code was sent to the phone number you added. '. 'Check your phone for the code and instructions '. 'on how to use it.'); - - $this->showForm($msg, true); } /** @@ -390,29 +357,27 @@ class SmssettingsAction extends SettingsAction $sms = $this->trimmed('sms'); $carrier = $this->trimmed('carrier'); - $confirm = $this->getConfirmation(); - - if (!$confirm) { + try { + $confirm = $this->getConfirmation(); + if ($confirm->address != $sms) { + // TRANS: Message given canceling SMS phone number confirmation for the wrong phone number. + throw new ClientException(_('That is the wrong confirmation number.')); + } + } catch (NoResultException $e) { // TRANS: Message given canceling SMS phone number confirmation that is not pending. - $this->showForm(_('No pending confirmation to cancel.')); - return; - } - if ($confirm->address != $sms) { - // TRANS: Message given canceling SMS phone number confirmation for the wrong phone number. - $this->showForm(_('That is the wrong confirmation number.')); - return; + throw new AlreadyFulfilledException(_('No pending confirmation to cancel.')); } $result = $confirm->delete(); - if (!$result) { + if ($result === false) { common_log_db_error($confirm, 'DELETE', __FILE__); // TRANS: Server error thrown on database error canceling SMS phone number confirmation. - $this->serverError(_('Could not delete SMS confirmation.')); + throw new ServerException(_('Could not delete SMS confirmation.')); } // TRANS: Message given after successfully canceling SMS phone number confirmation. - $this->showForm(_('SMS confirmation cancelled.'), true); + return _('SMS confirmation cancelled.'); } /** @@ -422,7 +387,7 @@ class SmssettingsAction extends SettingsAction */ function removeAddress() { - $user = common_current_user(); + $user = $this->scoped->getUser(); $sms = $this->arg('sms'); $carrier = $this->arg('carrier'); @@ -432,8 +397,7 @@ class SmssettingsAction extends SettingsAction if ($user->sms != $sms) { // TRANS: Message given trying to remove an SMS phone number that is not // TRANS: registered for the active user. - $this->showForm(_('That is not your phone number.')); - return; + throw new ClientException(_('That is not your phone number.')); } $original = clone($user); @@ -446,7 +410,7 @@ class SmssettingsAction extends SettingsAction $user->updateWithKeys($original); // TRANS: Message given after successfully removing a registered SMS phone number. - $this->showForm(_('The SMS phone number was removed.'), true); + return _('The SMS phone number was removed.'); } /** @@ -460,15 +424,13 @@ class SmssettingsAction extends SettingsAction */ function smsExists($sms) { - $user = common_current_user(); - $other = User::getKV('sms', $sms); - if (!$other) { + if (!$other instanceof User) { return false; - } else { - return $other->id != $user->id; } + + return !$this->scoped->sameAs($other->getProfile()); } /** @@ -519,15 +481,12 @@ class SmssettingsAction extends SettingsAction { $code = $this->trimmed('code'); - if (!$code) { + if (empty($code)) { // TRANS: Message given saving SMS phone number confirmation code without having provided one. - $this->showForm(_('No code entered.')); - return; + throw new ClientException(_('No code entered.')); } - common_redirect(common_local_url('confirmaddress', - array('code' => $code)), - 303); + common_redirect(common_local_url('confirmaddress', array('code' => $code)), 303); } /** @@ -541,8 +500,7 @@ class SmssettingsAction extends SettingsAction if (!$user->incomingemail) { // TRANS: Form validation error displayed when trying to remove an incoming e-mail address while no address has been set. - $this->showForm(_('No incoming email address.')); - return; + throw new ClientException(_('No incoming email address.')); } $orig = clone($user); @@ -553,7 +511,7 @@ class SmssettingsAction extends SettingsAction $user->updateWithKeys($orig); // TRANS: Confirmation text after updating SMS settings. - $this->showForm(_('Incoming email address removed.'), true); + return _('Incoming email address removed.'); } /** @@ -565,7 +523,7 @@ class SmssettingsAction extends SettingsAction */ function newIncoming() { - $user = common_current_user(); + $user = $this->scoped->getUser(); $orig = clone($user); @@ -575,6 +533,6 @@ class SmssettingsAction extends SettingsAction $user->updateWithKeys($orig); // TRANS: Confirmation text after updating SMS settings. - $this->showForm(_('New incoming email address added.'), true); + return _('New incoming email address added.'); } } From 8d516d7f08c02f1144d2f43ffbf6536921fde29f Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 14:40:09 +0200 Subject: [PATCH 21/40] Don't allow imports by default until it works well on large instances. --- lib/default.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/default.php b/lib/default.php index cf40f678ed..5bec29cc52 100644 --- a/lib/default.php +++ b/lib/default.php @@ -129,7 +129,7 @@ $default = 'biolimit' => null, 'changenick' => false, 'backup' => true, - 'restore' => true, + 'restore' => false, 'delete' => false, 'move' => true), 'image' => From 992fe6896fd70dd868b6df01fa3f870469f27259 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 15:16:45 +0200 Subject: [PATCH 22/40] Urlsettings now adapted to FormAction --- actions/urlsettings.php | 44 +++++++++++------------------------------ 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/actions/urlsettings.php b/actions/urlsettings.php index 69ad4c8690..c3e4ed50b2 100644 --- a/actions/urlsettings.php +++ b/actions/urlsettings.php @@ -28,9 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Miscellaneous settings actions @@ -83,7 +81,7 @@ class UrlsettingsAction extends SettingsAction */ function showContent() { - $user = common_current_user(); + $user = $this->scoped->getUser(); $this->elementStart('form', array('method' => 'post', 'id' => 'form_settings_other', @@ -154,31 +152,13 @@ class UrlsettingsAction extends SettingsAction $this->elementEnd('form'); } - /** - * Handle a post - * - * Saves the changes to url-shortening prefs and shows a success or failure - * message. - * - * @return void - */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - $urlshorteningservice = $this->trimmed('urlshorteningservice'); if (!is_null($urlshorteningservice) && strlen($urlshorteningservice) > 50) { // TRANS: Form validation error for form "Other settings" in user profile. - $this->showForm(_('URL shortening service is too long (maximum 50 characters).')); - return; + throw new ClientException(_('URL shortening service is too long (maximum 50 characters).')); } $maxurllength = $this->trimmed('maxurllength'); @@ -195,9 +175,7 @@ class UrlsettingsAction extends SettingsAction throw new ClientException(_('Invalid number for maximum notice length.')); } - $user = common_current_user(); - - assert(!is_null($user)); // should already be checked + $user = $this->scoped->getUser(); $user->query('BEGIN'); @@ -209,14 +187,15 @@ class UrlsettingsAction extends SettingsAction if ($result === false) { common_log_db_error($user, 'UPDATE', __FILE__); + $user->query('ROLLBACK'); // TRANS: Server error displayed when "Other" settings in user profile could not be updated on the server. - $this->serverError(_('Could not update user.')); + throw new ServerException(_('Could not update user.')); } $prefs = User_urlshortener_prefs::getPrefs($user); $orig = null; - if (empty($prefs)) { + if (!$prefs instanceof User_urlshortener_prefs) { $prefs = new User_urlshortener_prefs(); $prefs->user_id = $user->id; @@ -229,13 +208,14 @@ class UrlsettingsAction extends SettingsAction $prefs->maxurllength = $maxurllength; $prefs->maxnoticelength = $maxnoticelength; - if (!empty($orig)) { + if ($orig instanceof User_urlshortener_prefs) { $result = $prefs->update($orig); } else { $result = $prefs->insert(); } - if (!$result) { + if ($result === null) { + $user->query('ROLLBACK'); // TRANS: Server exception thrown in profile URL settings when preferences could not be saved. throw new ServerException(_('Error saving user URL shortening preferences.')); } @@ -243,6 +223,6 @@ class UrlsettingsAction extends SettingsAction $user->query('COMMIT'); // TRANS: Confirmation message after saving preferences. - $this->showForm(_('Preferences saved.'), true); + return _('Preferences saved.'); } } From be0c10e8f6f9c60b1cb840cda9ac6f42c8289738 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 15:24:37 +0200 Subject: [PATCH 23/40] Facebooksettings adapted to FormAction --- .../actions/facebooksettings.php | 65 ++++++------------- 1 file changed, 20 insertions(+), 45 deletions(-) diff --git a/plugins/FacebookBridge/actions/facebooksettings.php b/plugins/FacebookBridge/actions/facebooksettings.php index 31e020a3ce..d6e4c14c4c 100644 --- a/plugins/FacebookBridge/actions/facebooksettings.php +++ b/plugins/FacebookBridge/actions/facebooksettings.php @@ -26,9 +26,7 @@ * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @link http://status.net/ */ -if (!defined('STATUSNET')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Edit user settings for Facebook @@ -44,18 +42,9 @@ if (!defined('STATUSNET')) { class FacebooksettingsAction extends SettingsAction { private $facebook; // Facebook PHP-SDK client obj private $flink; - private $user; - - /** - * For initializing members of the class. - * - * @param array $argarray misc. arguments - * - * @return boolean true - */ - function prepare($args) { - parent::prepare($args); + protected function doPreparation() + { $this->facebook = new Facebook( array( 'appId' => common_config('facebook', 'appid'), @@ -64,36 +53,23 @@ class FacebooksettingsAction extends SettingsAction { ) ); - $this->user = common_current_user(); - $this->flink = Foreign_link::getByUserID( - $this->user->id, + $this->scoped->getID(), FACEBOOK_SERVICE ); return true; } - /* - * Check the sessions token and dispatch - */ - function handlePost($args) { - // CSRF protection - - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - $this->showForm( - // TRANS: Client error displayed when the session token does not match or is not given. - _m('There was a problem with your session token. Try again, please.') - ); - return; - } - + protected function doPost() + { if ($this->arg('save')) { - $this->saveSettings(); + return $this->saveSettings(); } else if ($this->arg('disconnect')) { - $this->disconnect(); + return $this->disconnect(); } + + throw new ClientException(_('No action to take on POST.')); } /** @@ -166,7 +142,7 @@ class FacebooksettingsAction extends SettingsAction { 'noticesync', // TRANS: Checkbox label in Facebook settings. _m('Publish my notices to Facebook.'), - ($this->flink) ? ($this->flink->noticesync & FOREIGN_NOTICE_SEND) : true + $this->flink->noticesync & FOREIGN_NOTICE_SEND ); $this->elementEnd('li'); @@ -177,7 +153,7 @@ class FacebooksettingsAction extends SettingsAction { 'replysync', // TRANS: Checkbox label in Facebook settings. _m('Send "@" replies to Facebook.'), - ($this->flink) ? ($this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY) : true + $this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY ); $this->elementEnd('li'); @@ -196,7 +172,7 @@ class FacebooksettingsAction extends SettingsAction { // TRANS: Fieldset legend for form to disconnect from Facebook. $this->element('legend', null, _m('Disconnect my account from Facebook')); - if (empty($this->user->password)) { + if (!$this->scoped->hasPassword()) { $this->elementStart('p', array('class' => 'form_guide')); $msg = sprintf( @@ -242,11 +218,10 @@ class FacebooksettingsAction extends SettingsAction { if ($result === false) { // TRANS: Notice in case saving of synchronisation preferences fail. - $this->showForm(_m('There was a problem saving your sync preferences.')); - } else { - // TRANS: Confirmation that synchronisation settings have been saved into the system. - $this->showForm(_m('Sync preferences saved.'), true); + throw new ServerException(_m('There was a problem saving your sync preferences.')); } + // TRANS: Confirmation that synchronisation settings have been saved into the system. + return _m('Sync preferences saved.'); } /* @@ -258,12 +233,12 @@ class FacebooksettingsAction extends SettingsAction { $this->flink = null; if ($result === false) { - common_log_db_error($user, 'DELETE', __FILE__); + common_log_db_error($this->flink, 'DELETE', __FILE__); // TRANS: Server error displayed when deleting the link to a Facebook account fails. - $this->serverError(_m('Could not delete link to Facebook.')); + throw new ServerException(_m('Could not delete link to Facebook.')); } - // TRANS: Confirmation message. StatusNet account was unlinked from Facebook. - $this->showForm(_m('You have disconnected from Facebook.'), true); + // TRANS: Confirmation message. GNU social account was unlinked from Facebook. + return _m('You have disconnected this account from Facebook.'); } } From 8d2504a809605e6bef61c13fc614dc0a57207385 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 15:27:47 +0200 Subject: [PATCH 24/40] Early return in FacebookBridge settings action --- .../actions/facebooksettings.php | 178 +++++++++--------- 1 file changed, 89 insertions(+), 89 deletions(-) diff --git a/plugins/FacebookBridge/actions/facebooksettings.php b/plugins/FacebookBridge/actions/facebooksettings.php index d6e4c14c4c..67dd20e036 100644 --- a/plugins/FacebookBridge/actions/facebooksettings.php +++ b/plugins/FacebookBridge/actions/facebooksettings.php @@ -41,7 +41,8 @@ if (!defined('GNUSOCIAL')) { exit(1); } */ class FacebooksettingsAction extends SettingsAction { private $facebook; // Facebook PHP-SDK client obj - private $flink; + + protected $flink; protected function doPreparation() { @@ -57,8 +58,6 @@ class FacebooksettingsAction extends SettingsAction { $this->scoped->getID(), FACEBOOK_SERVICE ); - - return true; } protected function doPost() @@ -98,109 +97,110 @@ class FacebooksettingsAction extends SettingsAction { * @return void */ function showContent() { - if (!empty($this->flink)) { + if (!$this->flink instanceof Foreign_link) { + throw new ServerException(_m('You have not linked this account to Facebook.')); + } - $this->elementStart( - 'form', - array( - 'method' => 'post', - 'id' => 'form_settings_facebook', - 'class' => 'form_settings', - 'action' => common_local_url('facebooksettings') - ) - ); + $this->elementStart( + 'form', + array( + 'method' => 'post', + 'id' => 'form_settings_facebook', + 'class' => 'form_settings', + 'action' => common_local_url('facebooksettings') + ) + ); - $this->hidden('token', common_session_token()); + $this->hidden('token', common_session_token()); - // TRANS: Form note. User is connected to facebook. - $this->element('p', 'form_note', _m('Connected Facebook user')); + // TRANS: Form note. User is connected to facebook. + $this->element('p', 'form_note', _m('Connected Facebook user')); - $this->elementStart('p', array('class' => 'facebook-user-display')); + $this->elementStart('p', array('class' => 'facebook-user-display')); - $this->element( - 'fb:profile-pic', - array( - 'uid' => $this->flink->foreign_id, - 'size' => 'small', - 'linked' => 'true', - 'facebook-logo' => 'true' - ) - ); + $this->element( + 'fb:profile-pic', + array( + 'uid' => $this->flink->foreign_id, + 'size' => 'small', + 'linked' => 'true', + 'facebook-logo' => 'true' + ) + ); - $this->element( - 'fb:name', - array('uid' => $this->flink->foreign_id, 'useyou' => 'false') - ); + $this->element( + 'fb:name', + array('uid' => $this->flink->foreign_id, 'useyou' => 'false') + ); - $this->elementEnd('p'); + $this->elementEnd('p'); - $this->elementStart('ul', 'form_data'); + $this->elementStart('ul', 'form_data'); - $this->elementStart('li'); + $this->elementStart('li'); - $this->checkbox( - 'noticesync', + $this->checkbox( + 'noticesync', + // TRANS: Checkbox label in Facebook settings. + _m('Publish my notices to Facebook.'), + $this->flink->noticesync & FOREIGN_NOTICE_SEND + ); + + $this->elementEnd('li'); + + $this->elementStart('li'); + + $this->checkbox( + 'replysync', // TRANS: Checkbox label in Facebook settings. - _m('Publish my notices to Facebook.'), - $this->flink->noticesync & FOREIGN_NOTICE_SEND + _m('Send "@" replies to Facebook.'), + $this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY + ); + + $this->elementEnd('li'); + + $this->elementStart('li'); + + // TRANS: Submit button to save synchronisation settings. + $this->submit('save', _m('BUTTON', 'Save')); + + $this->elementEnd('li'); + + $this->elementEnd('ul'); + + $this->elementStart('fieldset'); + + // TRANS: Fieldset legend for form to disconnect from Facebook. + $this->element('legend', null, _m('Disconnect my account from Facebook')); + + if (!$this->scoped->hasPassword()) { + $this->elementStart('p', array('class' => 'form_guide')); + + $msg = sprintf( + // TRANS: Notice in disconnect from Facebook form if user has no local StatusNet password. + _m('Disconnecting your Faceboook would make it impossible to '. + 'log in! Please [set a password](%s) first.'), + common_local_url('passwordsettings') ); - $this->elementEnd('li'); - - $this->elementStart('li'); - - $this->checkbox( - 'replysync', - // TRANS: Checkbox label in Facebook settings. - _m('Send "@" replies to Facebook.'), - $this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY + $this->raw(common_markup_to_html($msg)); + $this->elementEnd('p'); + } else { + // @todo FIXME: i18n: This message is not being used. + // TRANS: Message displayed when initiating disconnect of a StatusNet user + // TRANS: from a Facebook account. %1$s is the StatusNet site name. + $msg = sprintf(_m('Keep your %1$s account but disconnect from Facebook. ' . + 'You\'ll use your %1$s password to log in.'), + common_config('site', 'name') ); - $this->elementEnd('li'); + // TRANS: Submit button. + $this->submit('disconnect', _m('BUTTON', 'Disconnect')); + } - $this->elementStart('li'); + $this->elementEnd('fieldset'); - // TRANS: Submit button to save synchronisation settings. - $this->submit('save', _m('BUTTON', 'Save')); - - $this->elementEnd('li'); - - $this->elementEnd('ul'); - - $this->elementStart('fieldset'); - - // TRANS: Fieldset legend for form to disconnect from Facebook. - $this->element('legend', null, _m('Disconnect my account from Facebook')); - - if (!$this->scoped->hasPassword()) { - $this->elementStart('p', array('class' => 'form_guide')); - - $msg = sprintf( - // TRANS: Notice in disconnect from Facebook form if user has no local StatusNet password. - _m('Disconnecting your Faceboook would make it impossible to '. - 'log in! Please [set a password](%s) first.'), - common_local_url('passwordsettings') - ); - - $this->raw(common_markup_to_html($msg)); - $this->elementEnd('p'); - } else { - // @todo FIXME: i18n: This message is not being used. - // TRANS: Message displayed when initiating disconnect of a StatusNet user - // TRANS: from a Facebook account. %1$s is the StatusNet site name. - $msg = sprintf(_m('Keep your %1$s account but disconnect from Facebook. ' . - 'You\'ll use your %1$s password to log in.'), - common_config('site', 'name') - ); - - // TRANS: Submit button. - $this->submit('disconnect', _m('BUTTON', 'Disconnect')); - } - - $this->elementEnd('fieldset'); - - $this->elementEnd('form'); - } + $this->elementEnd('form'); } /* From da168674f9bef92d700f644e3232b28de794afe7 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 16:08:22 +0200 Subject: [PATCH 25/40] OpenID settings aligned with FormAction --- plugins/OpenID/actions/openidsettings.php | 114 ++++++++-------------- plugins/OpenID/openid.php | 18 ++-- 2 files changed, 50 insertions(+), 82 deletions(-) diff --git a/plugins/OpenID/actions/openidsettings.php b/plugins/OpenID/actions/openidsettings.php index 0c20dac4cd..bf5d8886f1 100644 --- a/plugins/OpenID/actions/openidsettings.php +++ b/plugins/OpenID/actions/openidsettings.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } require_once INSTALLDIR.'/plugins/OpenID/openid.php'; @@ -86,8 +84,6 @@ class OpenidsettingsAction extends SettingsAction */ function showContent() { - $user = common_current_user(); - if (!common_config('openid', 'trusted_provider')) { $this->elementStart('form', array('method' => 'post', 'id' => 'form_settings_openid_add', @@ -115,7 +111,7 @@ class OpenidsettingsAction extends SettingsAction } $oid = new User_openid(); - $oid->user_id = $user->id; + $oid->user_id = $this->scoped->getID(); $cnt = $oid->find(); @@ -123,7 +119,7 @@ class OpenidsettingsAction extends SettingsAction // TRANS: Header on OpenID settings page. $this->element('h2', null, _m('HEADER','Remove OpenID')); - if ($cnt == 1 && !$user->password) { + if ($cnt == 1 && !$this->scoped->hasPassword()) { $this->element('p', 'form_guide', // TRANS: Form guide. @@ -184,7 +180,7 @@ class OpenidsettingsAction extends SettingsAction 'this list to deny it access to your OpenID.')); $this->elementStart('ul', 'form_data'); $user_openid_trustroot = new User_openid_trustroot(); - $user_openid_trustroot->user_id=$user->id; + $user_openid_trustroot->user_id = $this->scoped->getID(); if($user_openid_trustroot->find()) { while($user_openid_trustroot->fetch()) { $this->elementStart('li'); @@ -203,7 +199,7 @@ class OpenidsettingsAction extends SettingsAction $this->submit('settings_openid_trustroots_action-submit', _m('BUTTON','Remove'), 'submit', 'remove_trustroots'); $this->elementEnd('fieldset'); - $prefs = User_openid_prefs::getKV('user_id', $user->id); + $prefs = User_openid_prefs::getKV('user_id', $this->scoped->getID()); $this->elementStart('fieldset'); $this->element('legend', null, _m('LEGEND','Preferences')); @@ -224,38 +220,29 @@ class OpenidsettingsAction extends SettingsAction * * @return void */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_m('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if ($this->arg('add')) { if (common_config('openid', 'trusted_provider')) { // TRANS: Form validation error if no OpenID providers can be added. - $this->showForm(_m('Cannot add new providers.')); + throw new ServerException(_m('Cannot add new providers.')); } else { - $result = oid_authenticate($this->trimmed('openid_url'), - 'finishaddopenid'); + $result = oid_authenticate($this->trimmed('openid_url'), 'finishaddopenid'); if (is_string($result)) { // error message - $this->showForm($result); + throw new ServerException($result); } + return _('Added new provider.'); } } else if ($this->arg('remove')) { - $this->removeOpenid(); + return $this->removeOpenid(); } else if($this->arg('remove_trustroots')) { - $this->removeTrustroots(); + return $this->removeTrustroots(); } else if($this->arg('save_prefs')) { - $this->savePrefs(); - } else { - // TRANS: Unexpected form validation error. - $this->showForm(_m('Something weird happened.')); + return $this->savePrefs(); } + + // TRANS: Unexpected form validation error. + throw new ServerException(_m('No known action for POST.')); } /** @@ -268,26 +255,20 @@ class OpenidsettingsAction extends SettingsAction */ function removeTrustroots() { - $user = common_current_user(); - $trustroots = $this->arg('openid_trustroot'); - if($trustroots) { - foreach($trustroots as $trustroot) { - $user_openid_trustroot = User_openid_trustroot::pkeyGet( - array('user_id'=>$user->id, 'trustroot'=>$trustroot)); - if($user_openid_trustroot) { - $user_openid_trustroot->delete(); - } else { - // TRANS: Form validation error when trying to remove a non-existing trustroot. - $this->showForm(_m('No such OpenID trustroot.')); - return; - } + $trustroots = $this->arg('openid_trustroot', array()); + foreach($trustroots as $trustroot) { + $user_openid_trustroot = User_openid_trustroot::pkeyGet( + array('user_id'=>$this->scoped->getID(), 'trustroot'=>$trustroot)); + if($user_openid_trustroot) { + $user_openid_trustroot->delete(); + } else { + // TRANS: Form validation error when trying to remove a non-existing trustroot. + throw new ClientException(_m('No such OpenID trustroot.')); } - // TRANS: Success message after removing trustroots. - $this->showForm(_m('Trustroots removed.'), true); - } else { - $this->showForm(); } - return; + + // TRANS: Success message after removing trustroots. + return _m('Trustroots removed.'); } /** @@ -300,25 +281,19 @@ class OpenidsettingsAction extends SettingsAction */ function removeOpenid() { - $openid_url = $this->trimmed('openid_url'); + $oid = User_openid::getKV('canonical', $this->trimmed('openid_url')); - $oid = User_openid::getKV('canonical', $openid_url); - - if (!$oid) { + if (!$oid instanceof User_openid) { // TRANS: Form validation error for a non-existing OpenID. - $this->showForm(_m('No such OpenID.')); - return; + throw new ClientException(_m('No such OpenID.')); } - $cur = common_current_user(); - if (!$cur || $oid->user_id != $cur->id) { + if ($this->scoped->getID() !== $oid->user_id) { // TRANS: Form validation error if OpenID is connected to another user. - $this->showForm(_m('That OpenID does not belong to you.')); - return; + throw new ClientException(_m('That OpenID does not belong to you.')); } $oid->delete(); // TRANS: Success message after removing an OpenID. - $this->showForm(_m('OpenID removed.'), true); - return; + return _m('OpenID removed.'); } /** @@ -331,18 +306,12 @@ class OpenidsettingsAction extends SettingsAction */ function savePrefs() { - $cur = common_current_user(); - - if (empty($cur)) { - throw new ClientException(_("Not logged in.")); - } - $orig = null; - $prefs = User_openid_prefs::getKV('user_id', $cur->id); + $prefs = User_openid_prefs::getKV('user_id', $this->scoped->getID()); - if (empty($prefs)) { + if (!$prefs instanceof User_openid_prefs) { $prefs = new User_openid_prefs(); - $prefs->user_id = $cur->id; + $prefs->user_id = $this->scoped->getID(); $prefs->created = common_sql_now(); } else { $orig = clone($prefs); @@ -350,13 +319,12 @@ class OpenidsettingsAction extends SettingsAction $prefs->hide_profile_link = $this->booleanintstring('hide_profile_link'); - if (empty($orig)) { - $prefs->insert(); - } else { + if ($orig instanceof User_openid_prefs) { $prefs->update($orig); + } else { + $prefs->insert(); } - $this->showForm(_m('OpenID preferences saved.'), true); - return; + return _m('OpenID preferences saved.'); } } diff --git a/plugins/OpenID/openid.php b/plugins/OpenID/openid.php index 91a34bd6e3..ee854e8140 100644 --- a/plugins/OpenID/openid.php +++ b/plugins/OpenID/openid.php @@ -131,13 +131,15 @@ function oid_check_immediate($openid_url, $backto=null) function oid_authenticate($openid_url, $returnto, $immediate=false) { + if (!common_valid_http_url($openid_url)) { + throw new ClientException(_m('No valid URL provided for OpenID.')); + } $consumer = oid_consumer(); if (!$consumer) { // TRANS: OpenID plugin server error. - common_server_error(_m('Cannot instantiate OpenID consumer object.')); - return false; + throw new ServerException(_m('Cannot instantiate OpenID consumer object.')); } common_ensure_session(); @@ -148,12 +150,12 @@ function oid_authenticate($openid_url, $returnto, $immediate=false) if (!$auth_request) { common_log(LOG_ERR, __METHOD__ . ": mystery fail contacting $openid_url"); // TRANS: OpenID plugin message. Given when an OpenID is not valid. - return _m('Not a valid OpenID.'); + throw new ServerException(_m('Not a valid OpenID.')); } else if (Auth_OpenID::isFailure($auth_request)) { common_log(LOG_ERR, __METHOD__ . ": OpenID fail to $openid_url: $auth_request->message"); // TRANS: OpenID plugin server error. Given when the OpenID authentication request fails. // TRANS: %s is the failure message. - return sprintf(_m('OpenID failure: %s.'), $auth_request->message); + throw new ServerException(sprintf(_m('OpenID failure: %s.'), $auth_request->message)); } $sreg_request = Auth_OpenID_SRegRequest::build(// Required @@ -199,14 +201,12 @@ function oid_authenticate($openid_url, $returnto, $immediate=false) $redirect_url = $auth_request->redirectURL($trust_root, $process_url, $immediate); - if (!$redirect_url) { - } else if (Auth_OpenID::isFailure($redirect_url)) { + if (Auth_OpenID::isFailure($redirect_url)) { // TRANS: OpenID plugin server error. Given when the OpenID authentication request cannot be redirected. // TRANS: %s is the failure message. - return sprintf(_m('Could not redirect to server: %s.'), $redirect_url->message); - } else { - common_redirect($redirect_url, 303); + throw new ServerException(sprintf(_m('Could not redirect to server: %s.'), $redirect_url->message)); } + common_redirect($redirect_url, 303); /* } else { // Generate form markup and render it. From 712a6d49d0cb9f8a6afffd7c2b163b429b0b2666 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 16:19:02 +0200 Subject: [PATCH 26/40] Poll settings adapted to FormAction --- plugins/Poll/actions/pollsettings.php | 129 ++------------------------ plugins/Poll/forms/pollprefs.php | 87 +++++++++++++++++ 2 files changed, 97 insertions(+), 119 deletions(-) create mode 100644 plugins/Poll/forms/pollprefs.php diff --git a/plugins/Poll/actions/pollsettings.php b/plugins/Poll/actions/pollsettings.php index b390812cf4..b95465d8d6 100644 --- a/plugins/Poll/actions/pollsettings.php +++ b/plugins/Poll/actions/pollsettings.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } class PollSettingsAction extends SettingsAction { @@ -56,143 +54,36 @@ class PollSettingsAction extends SettingsAction return _m('Set your poll preferences'); } - /** - * Show the form for Poll - * - * @return void - */ - function showContent() + protected function getForm() { - $user = common_current_user(); - - $prefs = User_poll_prefs::getKV('user_id', $user->id); - + $prefs = User_poll_prefs::getKV('user_id', $this->scoped->getID()); $form = new PollPrefsForm($this, $prefs); - - $form->show(); + return $form; } - /** - * Handler method - * - * @param array $argarray is ignored since it's now passed in in prepare() - * - * @return void - */ - - function handlePost() + protected function doPost() { - $user = common_current_user(); - - $upp = User_poll_prefs::getKV('user_id', $user->id); + $upp = User_poll_prefs::getKV('user_id', $this->scoped->getID()); $orig = null; - if (!empty($upp)) { + if ($upp instanceof User_poll_prefs) { $orig = clone($upp); } else { $upp = new User_poll_prefs(); - $upp->user_id = $user->id; + $upp->user_id = $this->scoped->getID(); $upp->created = common_sql_now(); } $upp->hide_responses = $this->boolean('hide_responses'); $upp->modified = common_sql_now(); - if (!empty($orig)) { + if ($orig instanceof User_poll_prefs) { $upp->update($orig); } else { $upp->insert(); } // TRANS: Confirmation shown when user profile settings are saved. - $this->showForm(_('Settings saved.'), true); - - return; - } -} - -class PollPrefsForm extends Form -{ - var $prefs; - - function __construct($out, $prefs) - { - parent::__construct($out); - $this->prefs = $prefs; - } - - /** - * Visible or invisible data elements - * - * Display the form fields that make up the data of the form. - * Sub-classes should overload this to show their data. - * - * @return void - */ - - function formData() - { - $this->elementStart('fieldset'); - $this->elementStart('ul', 'form_data'); - $this->elementStart('li'); - $this->checkbox('hide_responses', - _('Do not deliver poll responses to my home timeline'), - (!empty($this->prefs) && $this->prefs->hide_responses)); - $this->elementEnd('li'); - $this->elementEnd('ul'); - $this->elementEnd('fieldset'); - } - - /** - * Buttons for form actions - * - * Submit and cancel buttons (or whatever) - * Sub-classes should overload this to show their own buttons. - * - * @return void - */ - - function formActions() - { - $this->submit('submit', _('Save')); - } - - /** - * ID of the form - * - * Should be unique on the page. Sub-classes should overload this - * to show their own IDs. - * - * @return int ID of the form - */ - - function id() - { - return 'form_poll_prefs'; - } - - /** - * Action of the form. - * - * URL to post to. Should be overloaded by subclasses to give - * somewhere to post to. - * - * @return string URL to post to - */ - - function action() - { - return common_local_url('pollsettings'); - } - - /** - * Class of the form. May include space-separated list of multiple classes. - * - * @return string the form's class - */ - - function formClass() - { - return 'form_settings'; + return _('Settings saved.'); } } diff --git a/plugins/Poll/forms/pollprefs.php b/plugins/Poll/forms/pollprefs.php new file mode 100644 index 0000000000..627b77d948 --- /dev/null +++ b/plugins/Poll/forms/pollprefs.php @@ -0,0 +1,87 @@ +prefs = $prefs; + } + + /** + * Visible or invisible data elements + * + * Display the form fields that make up the data of the form. + * Sub-classes should overload this to show their data. + * + * @return void + */ + + function formData() + { + $this->elementStart('fieldset'); + $this->elementStart('ul', 'form_data'); + $this->elementStart('li'); + $this->checkbox('hide_responses', + _('Do not deliver poll responses to my home timeline'), + ($this->prefs instanceof User_poll_prefs && $this->prefs->hide_responses)); + $this->elementEnd('li'); + $this->elementEnd('ul'); + $this->elementEnd('fieldset'); + } + + /** + * Buttons for form actions + * + * Submit and cancel buttons (or whatever) + * Sub-classes should overload this to show their own buttons. + * + * @return void + */ + + function formActions() + { + $this->submit('submit', _('Save')); + } + + /** + * ID of the form + * + * Should be unique on the page. Sub-classes should overload this + * to show their own IDs. + * + * @return int ID of the form + */ + + function id() + { + return 'form_poll_prefs'; + } + + /** + * Action of the form. + * + * URL to post to. Should be overloaded by subclasses to give + * somewhere to post to. + * + * @return string URL to post to + */ + + function action() + { + return common_local_url('pollsettings'); + } + + /** + * Class of the form. May include space-separated list of multiple classes. + * + * @return string the form's class + */ + + function formClass() + { + return 'form_settings'; + } +} From d6d06c8cbbb97cc5adf585ef49a0ac32e86bfef7 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 17:09:36 +0200 Subject: [PATCH 27/40] SubMirror now works properly as extended from FormAction --- plugins/SubMirror/actions/mirrorsettings.php | 52 +++----------------- 1 file changed, 7 insertions(+), 45 deletions(-) diff --git a/plugins/SubMirror/actions/mirrorsettings.php b/plugins/SubMirror/actions/mirrorsettings.php index b5a49fe4fa..1bfc848ebb 100644 --- a/plugins/SubMirror/actions/mirrorsettings.php +++ b/plugins/SubMirror/actions/mirrorsettings.php @@ -25,7 +25,7 @@ * @link http://status.net/ */ -if (!defined('GNUSOCIAL') && !defined('STATUSNET')) { exit(1); } +if (!defined('GNUSOCIAL')) { exit(1); } class MirrorSettingsAction extends SettingsAction { @@ -62,9 +62,8 @@ class MirrorSettingsAction extends SettingsAction */ function showContent() { - $user = common_current_user(); $provider = $this->trimmed('provider'); - if ($provider) { + if (!empty($provider) || GNUsocial::isAjax()) { $this->showAddFeedForm($provider); } else { $this->elementStart('div', array('id' => 'add-mirror')); @@ -72,7 +71,7 @@ class MirrorSettingsAction extends SettingsAction $this->elementEnd('div'); $mirror = new SubMirror(); - $mirror->subscriber = $user->id; + $mirror->subscriber = $this->scoped->getID(); if ($mirror->find()) { while ($mirror->fetch()) { $this->showFeedForm($mirror); @@ -87,13 +86,11 @@ class MirrorSettingsAction extends SettingsAction $form->show(); } - function showFeedForm($mirror) + function showFeedForm(SubMirror $mirror) { - $profile = Profile::getKV('id', $mirror->subscribed); - if ($profile) { - $form = new EditMirrorForm($this, $profile); - $form->show(); - } + $profile = Profile::getByID($mirror->subscribed); + $form = new EditMirrorForm($this, $profile); + $form->show(); } function showAddFeedForm() @@ -112,41 +109,6 @@ class MirrorSettingsAction extends SettingsAction $form->show(); } - /** - * - * @param array $args - * - * @todo move the ajax display handling to common code - */ - function handle($args) - { - if ($this->boolean('ajax')) { - $this->startHTML('text/xml;charset=utf-8'); - $this->elementStart('head'); - // TRANS: Title for page with form to add a mirror feed provider on. - $this->element('title', null, _m('Provider add')); - $this->elementEnd('head'); - $this->elementStart('body'); - - $this->showAddFeedForm(); - - $this->elementEnd('body'); - $this->endHTML(); - } else { - return parent::handle($args); - } - } - /** - * Handle a POST request - * - * Muxes to different sub-functions based on which button was pushed - * - * @return void - */ - function handlePost() - { - } - /** * Show the local navigation menu * From 5933056a5b9490bfbe030843cc83c5e43084e56c Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 17:50:43 +0200 Subject: [PATCH 28/40] Twittersettings now works better as Profilesettings extension --- .../TwitterBridge/actions/twittersettings.php | 100 +++++++----------- 1 file changed, 39 insertions(+), 61 deletions(-) diff --git a/plugins/TwitterBridge/actions/twittersettings.php b/plugins/TwitterBridge/actions/twittersettings.php index 37abb4d272..85d1a4bbe2 100644 --- a/plugins/TwitterBridge/actions/twittersettings.php +++ b/plugins/TwitterBridge/actions/twittersettings.php @@ -27,9 +27,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } require_once dirname(__DIR__) . '/twitter.php'; @@ -46,6 +44,16 @@ require_once dirname(__DIR__) . '/twitter.php'; */ class TwittersettingsAction extends ProfileSettingsAction { + protected $flink = null; + protected $fuser = null; + + protected function doPreparation() + { + $this->flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); + if ($this->flink instanceof Foreign_link) { + $this->fuser = $this->flink->getForeignUser(); + } + } /** * Title of the page * @@ -81,19 +89,6 @@ class TwittersettingsAction extends ProfileSettingsAction */ function showContent() { - - $user = common_current_user(); - - $profile = $user->getProfile(); - - $fuser = null; - - $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); - - if (!empty($flink)) { - $fuser = $flink->getForeignUser(); - } - $this->elementStart('form', array('method' => 'post', 'id' => 'form_settings_twitter', 'class' => 'form_settings', @@ -104,21 +99,11 @@ class TwittersettingsAction extends ProfileSettingsAction $this->elementStart('fieldset', array('id' => 'settings_twitter_account')); - if (empty($fuser)) { - $this->elementStart('ul', 'form_data'); - $this->elementStart('li', array('id' => 'settings_twitter_login_button')); - $this->element('a', array('href' => common_local_url('twitterauthorization')), - // TRANS: Link description to connect to a Twitter account. - 'Connect my Twitter account'); - $this->elementEnd('li'); - $this->elementEnd('ul'); - - $this->elementEnd('fieldset'); - } else { + if ($this->fuser instanceof Foreign_user) { // TRANS: Fieldset legend. $this->element('legend', null, _m('Twitter account')); $this->elementStart('p', array('id' => 'form_confirmed')); - $this->element('a', array('href' => $fuser->uri), $fuser->nickname); + $this->element('a', array('href' => $this->fuser->uri), $this->fuser->nickname); $this->elementEnd('p'); $this->element('p', 'form_note', // TRANS: Form note when a Twitter account has been connected. @@ -130,7 +115,7 @@ class TwittersettingsAction extends ProfileSettingsAction // TRANS: Fieldset legend. $this->element('legend', null, _m('Disconnect my account from Twitter')); - if (!$user->password) { + if (!$this->scoped->hasPassword()) { $this->elementStart('p', array('class' => 'form_guide')); // TRANS: Form guide. %s is a URL to the password settings. // TRANS: This message contains a Markdown link in the form [description](link). @@ -165,25 +150,19 @@ class TwittersettingsAction extends ProfileSettingsAction $this->checkbox('noticesend', // TRANS: Checkbox label. _m('Automatically send my notices to Twitter.'), - ($flink) ? - ($flink->noticesync & FOREIGN_NOTICE_SEND) : - true); + $this->flink->noticesync & FOREIGN_NOTICE_SEND); $this->elementEnd('li'); $this->elementStart('li'); $this->checkbox('replysync', // TRANS: Checkbox label. _m('Send local "@" replies to Twitter.'), - ($flink) ? - ($flink->noticesync & FOREIGN_NOTICE_SEND_REPLY) : - true); + $this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY); $this->elementEnd('li'); $this->elementStart('li'); $this->checkbox('friendsync', // TRANS: Checkbox label. _m('Subscribe to my Twitter friends here.'), - ($flink) ? - ($flink->friendsync & FOREIGN_FRIEND_RECV) : - false); + $this->flink->friendsync & FOREIGN_FRIEND_RECV); $this->elementEnd('li'); if (common_config('twitterimport','enabled')) { @@ -191,31 +170,37 @@ class TwittersettingsAction extends ProfileSettingsAction $this->checkbox('noticerecv', // TRANS: Checkbox label. _m('Import my friends timeline.'), - ($flink) ? - ($flink->noticesync & FOREIGN_NOTICE_RECV) : - false); + $this->flink->noticesync & FOREIGN_NOTICE_RECV); $this->elementEnd('li'); } else { // preserve setting even if bidrection bridge toggled off - if ($flink && ($flink->noticesync & FOREIGN_NOTICE_RECV)) { + if ($this->flink->noticesync & FOREIGN_NOTICE_RECV) { $this->hidden('noticerecv', true, 'noticerecv'); } } $this->elementEnd('ul'); - if ($flink) { + if ($this->flink) { // TRANS: Button text for saving Twitter integration settings. $this->submit('save', _m('BUTTON','Save')); } else { // TRANS: Button text for adding Twitter integration. $this->submit('add', _m('BUTTON','Add')); } - - $this->elementEnd('fieldset'); + } else { + $this->elementStart('ul', 'form_data'); + $this->elementStart('li', array('id' => 'settings_twitter_login_button')); + $this->element('a', array('href' => common_local_url('twitterauthorization')), + // TRANS: Link description to connect to a Twitter account. + 'Connect my Twitter account'); + $this->elementEnd('li'); + $this->elementEnd('ul'); } + $this->elementEnd('fieldset'); + $this->elementEnd('form'); } @@ -289,32 +274,25 @@ class TwittersettingsAction extends ProfileSettingsAction $friendsync = $this->boolean('friendsync'); $replysync = $this->boolean('replysync'); - $user = common_current_user(); - $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); - - if (empty($flink)) { - common_log_db_error($flink, 'SELECT', __FILE__); - // @todo FIXME: Shouldn't this be a serverError()? + if (!$this->flink instanceof Foreign_link) { + common_log_db_error($this->flink, 'SELECT', __FILE__); // TRANS: Server error displayed when saving Twitter integration preferences fails. - $this->showForm(_m('Could not save Twitter preferences.')); - return; + throw new ServerException(_m('Your account is not linked to Twitter.')); } - $original = clone($flink); + $original = clone($this->flink); $wasReceiving = (bool)($original->noticesync & FOREIGN_NOTICE_RECV); - $flink->set_flags($noticesend, $noticerecv, $replysync, $friendsync); - $result = $flink->update($original); + $this->flink->set_flags($noticesend, $noticerecv, $replysync, $friendsync); + $result = $this->flink->update($original); if ($result === false) { - common_log_db_error($flink, 'UPDATE', __FILE__); - // @todo FIXME: Shouldn't this be a serverError()? + common_log_db_error($this->flink, 'UPDATE', __FILE__); // TRANS: Server error displayed when saving Twitter integration preferences fails. - $this->showForm(_m('Could not save Twitter preferences.')); - return; + throw new ServerException(_m('Could not save Twitter preferences.')); } if ($wasReceiving xor $noticerecv) { - $this->notifyDaemon($flink->foreign_id, $noticerecv); + $this->notifyDaemon($this->flink->foreign_id, $noticerecv); } // TRANS: Success message after saving Twitter integration preferences. From 6cd7a4a400e87e6a0ff1ddb2a2ba0a26c5df9b67 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 18:44:09 +0200 Subject: [PATCH 29/40] TwitterBridge messing about, Twitter OAuth requires Authorization header now? https://dev.twitter.com/oauth/reference/post/oauth/request_token says that the request should be a GET with a specific HTTP header instead of query string parameters for OAuth? --- classes/Foreign_link.php | 6 +- classes/Foreign_user.php | 31 +-- plugins/FacebookBridge/lib/facebookclient.php | 11 +- plugins/TwitterBridge/TwitterBridgePlugin.php | 18 +- .../actions/twitterauthorization.php | 229 ++++++------------ .../TwitterBridge/actions/twitterlogin.php | 27 +-- .../TwitterBridge/lib/twitteroauthclient.php | 4 +- plugins/TwitterBridge/twitter.php | 31 +-- 8 files changed, 131 insertions(+), 226 deletions(-) diff --git a/classes/Foreign_link.php b/classes/Foreign_link.php index 6176ec43bc..0d942443f3 100644 --- a/classes/Foreign_link.php +++ b/classes/Foreign_link.php @@ -124,11 +124,11 @@ class Foreign_link extends Managed_DataObject $fuser->limit(1); - if ($fuser->find(true)) { - return $fuser; + if (!$fuser->find(true)) { + throw new NoResultException($fuser); } - return null; + return $fuser; } function getUser() diff --git a/classes/Foreign_user.php b/classes/Foreign_user.php index c1739d318a..3d23eabef9 100644 --- a/classes/Foreign_user.php +++ b/classes/Foreign_user.php @@ -42,32 +42,33 @@ class Foreign_user extends Managed_DataObject } static function getForeignUser($id, $service) { - $fuser = new Foreign_user(); - $fuser->id = $id; $fuser->service = $service; - $fuser->limit(1); - $result = $fuser->find(true); + if (!$fuser->find(true)) { + throw new NoResultException($fuser); + } - return empty($result) ? null : $fuser; + return $fuser; } static function getByNickname($nickname, $service) { if (empty($nickname) || empty($service)) { - return null; - } else { - $fuser = new Foreign_user(); - $fuser->service = $service; - $fuser->nickname = $nickname; - $fuser->limit(1); - - $result = $fuser->find(true); - - return empty($result) ? null : $fuser; + throw new ServerException('Empty nickname or service for Foreign_user::getByNickname'); } + + $fuser = new Foreign_user(); + $fuser->service = $service; + $fuser->nickname = $nickname; + $fuser->limit(1); + + if (!$fuser->find(true)) { + throw new NoResultException($fuser); + } + + return $fuser; } } diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index 5b512dfdff..e55e19d88b 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -918,12 +918,9 @@ class Facebookclient static function addFacebookUser($fbuser) { // remove any existing, possibly outdated, record - $luser = Foreign_user::getForeignUser($fbuser->id, FACEBOOK_SERVICE); - - if (!empty($luser)) { - - $result = $luser->delete(); - + try { + $fuser = Foreign_user::getForeignUser($fbuser->id, FACEBOOK_SERVICE); + $result = $fuser->delete(); if ($result != false) { common_log( LOG_INFO, @@ -935,6 +932,8 @@ class Facebookclient __FILE__ ); } + } catch (NoResultException $e) { + // no old foreign users exist for this id } $fuser = new Foreign_user(); diff --git a/plugins/TwitterBridge/TwitterBridgePlugin.php b/plugins/TwitterBridge/TwitterBridgePlugin.php index 9b129ea21b..566038f2ec 100644 --- a/plugins/TwitterBridge/TwitterBridgePlugin.php +++ b/plugins/TwitterBridge/TwitterBridgePlugin.php @@ -26,9 +26,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } require_once __DIR__ . '/twitter.php'; @@ -387,12 +385,11 @@ class TwitterBridgePlugin extends Plugin { $n2s = Notice_to_status::getKV('notice_id', $notice->id); - if (!empty($n2s)) { + if ($n2s instanceof Notice_to_status) { - $flink = Foreign_link::getByUserID($notice->profile_id, - TWITTER_SERVICE); // twitter service + $flink = Foreign_link::getByUserID($notice->profile_id, TWITTER_SERVICE); // twitter service - if (empty($flink)) { + if (!$flink instanceof Foreign_link) { return true; } @@ -424,15 +421,14 @@ class TwitterBridgePlugin extends Plugin */ function onEndFavorNotice(Profile $profile, Notice $notice) { - $flink = Foreign_link::getByUserID($profile->id, - TWITTER_SERVICE); // twitter service + $flink = Foreign_link::getByUserID($profile->getID(), TWITTER_SERVICE); // twitter service - if (empty($flink)) { + if (!$flink instanceof Foreign_link) { return true; } if (!TwitterOAuthClient::isPackedToken($flink->credentials)) { - $this->log(LOG_INFO, "Skipping fave processing for {$profile->id} since link is not OAuth."); + $this->log(LOG_INFO, "Skipping fave processing for {$profile->getID()} since link is not OAuth."); return true; } diff --git a/plugins/TwitterBridge/actions/twitterauthorization.php b/plugins/TwitterBridge/actions/twitterauthorization.php index ae293edcf7..90e4c35410 100644 --- a/plugins/TwitterBridge/actions/twitterauthorization.php +++ b/plugins/TwitterBridge/actions/twitterauthorization.php @@ -28,7 +28,7 @@ * @link http://status.net/ */ -if (!defined('GNUSOCIAL') && !defined('STATUSNET')) { exit(1); } +if (!defined('GNUSOCIAL')) { exit(1); } require_once dirname(__DIR__) . '/twitter.php'; @@ -48,7 +48,7 @@ require_once dirname(__DIR__) . '/twitter.php'; * @link http://status.net/ * */ -class TwitterauthorizationAction extends Action +class TwitterauthorizationAction extends FormAction { var $twuid = null; var $tw_fields = null; @@ -56,112 +56,80 @@ class TwitterauthorizationAction extends Action var $signin = null; var $verifier = null; - /** - * Initialize class members. Looks for 'oauth_token' parameter. - * - * @param array $args misc. arguments - * - * @return boolean true - */ - function prepare($args) - { - parent::prepare($args); + protected $needLogin = false; // authorization page can also be used to create a new user + protected function doPreparation() + { $this->signin = $this->boolean('signin'); $this->oauth_token = $this->arg('oauth_token'); $this->verifier = $this->arg('oauth_verifier'); - return true; - } - - /** - * Handler method - * - * @param array $args is ignored since it's now passed in in prepare() - * - * @return nothing - */ - function handle($args) - { - parent::handle($args); - - if (common_logged_in()) { - $user = common_current_user(); - $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); + if ($this->scoped instanceof Profile) { + $flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); // If there's already a foreign link record and a foreign user // it means the accounts are already linked, and this is unecessary. // So go back. - if (isset($flink)) { - $fuser = $flink->getForeignUser(); - if (!empty($fuser)) { + if ($flink instanceof Foreign_link) { + try { + $fuser = $flink->getForeignUser(); common_redirect(common_local_url('twittersettings')); + } catch (NoResultException $e) { + // ok, so no foreign user but there's a foreign link?? + // this logic is left since the StatusNet days } } } - if ($_SERVER['REQUEST_METHOD'] == 'POST') { - - // User was not logged in to StatusNet before - - $this->twuid = $this->trimmed('twuid'); - - $this->tw_fields = array('screen_name' => $this->trimmed('tw_fields_screen_name'), - 'fullname' => $this->trimmed('tw_fields_fullname')); - - $this->access_token = new OAuthToken($this->trimmed('access_token_key'), $this->trimmed('access_token_secret')); - - $token = $this->trimmed('token'); - - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_m('There was a problem with your session token. Try again, please.')); - return; - } - - if ($this->arg('create')) { - if (!$this->boolean('license')) { - // TRANS: Form validation error displayed when the checkbox to agree to the license has not been checked. - $this->showForm(_m('You cannot register if you do not agree to the license.'), - $this->trimmed('newname')); - return; - } - $this->createNewUser(); - } else if ($this->arg('connect')) { - $this->connectNewUser(); - } else { - common_debug('Twitter bridge - ' . print_r($this->args, true)); - // TRANS: Form validation error displayed when an unhandled error occurs. - $this->showForm(_m('Something weird happened.'), - $this->trimmed('newname')); - } + // $this->oauth_token is only populated once Twitter authorizes our + // request token. If it's empty we're at the beginning of the auth + // process + if (empty($this->oauth_token)) { + // authorizeRequestToken either throws an exception or redirects + $this->authorizeRequestToken(); } else { - // $this->oauth_token is only populated once Twitter authorizes our - // request token. If it's empty we're at the beginning of the auth - // process - - if (empty($this->oauth_token)) { - $this->authorizeRequestToken(); - } else { - $this->saveAccessToken(); - } + $this->saveAccessToken(); } } + protected function doPost() + { + // User was not logged in to StatusNet before + + $this->twuid = $this->trimmed('twuid'); + + $this->tw_fields = array('screen_name' => $this->trimmed('tw_fields_screen_name'), + 'fullname' => $this->trimmed('tw_fields_fullname')); + + $this->access_token = new OAuthToken($this->trimmed('access_token_key'), $this->trimmed('access_token_secret')); + + if ($this->arg('create')) { + if (!$this->boolean('license')) { + // TRANS: Form validation error displayed when the checkbox to agree to the license has not been checked. + throw new ClientException(_m('You cannot register if you do not agree to the license.')); + } + return $this->createNewUser(); + } else if ($this->arg('connect')) { + return $this->connectNewUser(); + } + + common_debug('Twitter bridge - ' . print_r($this->args, true)); + // TRANS: Form validation error displayed when an unhandled error occurs. + throw new ClientException(_m('No known action for POST.')); + } + /** * Asks Twitter for a request token, and then redirects to Twitter * to authorize it. - * - * @return nothing */ - function authorizeRequestToken() + protected function authorizeRequestToken() { try { // Get a new request token and authorize it $client = new TwitterOAuthClient(); - $req_tok = $client->getRequestToken(); + $req_tok = $client->getTwitterRequestToken(); // Sock the request token away in the session temporarily @@ -176,10 +144,8 @@ class TwitterauthorizationAction extends Action $e->getMessage() ); common_log(LOG_INFO, 'Twitter bridge - ' . $msg); - $this->serverError( - // TRANS: Server error displayed when linking to a Twitter account fails. - _m('Could not link your Twitter account.') - ); + // TRANS: Server error displayed when linking to a Twitter account fails. + throw new ServerException(_m('Could not link your Twitter account.')); } common_redirect($auth_link); @@ -197,10 +163,8 @@ class TwitterauthorizationAction extends Action // token we sent them if ($_SESSION['twitter_request_token'] != $this->oauth_token) { - $this->serverError( - // TRANS: Server error displayed when linking to a Twitter account fails because of an incorrect oauth_token. - _m('Could not link your Twitter account: oauth_token mismatch.') - ); + // TRANS: Server error displayed when linking to a Twitter account fails because of an incorrect oauth_token. + throw new ServerException(_m('Could not link your Twitter account: oauth_token mismatch.')); } $twitter_user = null; @@ -212,7 +176,7 @@ class TwitterauthorizationAction extends Action // Exchange the request token for an access token - $atok = $client->getAccessToken($this->verifier); + $atok = $client->getTwitterAccessToken($this->verifier); // Test the access token and get the user's Twitter info @@ -235,8 +199,7 @@ class TwitterauthorizationAction extends Action if (common_logged_in()) { // Save the access token and Twitter user info - $user = common_current_user(); - $this->saveForeignLink($user->id, $twitter_user->id, $atok); + $this->saveForeignLink($this->scoped->getID(), $twitter_user->id, $atok); save_twitter_user($twitter_user->id, $twitter_user->screen_name); } else { @@ -297,24 +260,20 @@ class TwitterauthorizationAction extends Action $flink_id = $flink->insert(); + // We want to make sure we got a numerical >0 value, not just failed the insert (which would be === false) if (empty($flink_id)) { common_log_db_error($flink, 'INSERT', __FILE__); // TRANS: Server error displayed when linking to a Twitter account fails. - $this->serverError(_m('Could not link your Twitter account.')); + throw new ServerException(_m('Could not link your Twitter account.')); } return $flink_id; } - function showPageNotice() + function getInstructions() { - if ($this->error) { - $this->element('div', array('class' => 'error'), $this->error); - } else { - $this->element('div', 'instructions', - // TRANS: Page instruction. %s is the StatusNet sitename. - sprintf(_m('This is the first time you have logged into %s so we must connect your Twitter account to a local account. You can either create a new account, or connect with your existing account, if you have one.'), common_config('site', 'name'))); - } + // TRANS: Page instruction. %s is the StatusNet sitename. + return sprintf(_m('This is the first time you have logged into %s so we must connect your Twitter account to a local account. You can either create a new account, or connect with your existing account, if you have one.'), common_config('site', 'name')); } function title() @@ -323,19 +282,6 @@ class TwitterauthorizationAction extends Action return _m('Twitter Account Setup'); } - function showForm($error=null, $username=null) - { - $this->error = $error; - $this->username = $username; - - $this->showPage(); - } - - function showPage() - { - parent::showPage(); - } - /** * @fixme much of this duplicates core code, which is very fragile. * Should probably be replaced with an extensible mini version of @@ -380,7 +326,7 @@ class TwitterauthorizationAction extends Action $this->elementStart('li'); // TRANS: Field label. $this->input('newname', _m('New nickname'), - ($this->username) ? $this->username : '', + $this->username ?: '', // TRANS: Field title for nickname field. _m('1-64 lowercase letters or numbers, no punctuation or spaces.')); $this->elementEnd('li'); @@ -488,12 +434,13 @@ class TwitterauthorizationAction extends Action function createNewUser() { if (!Event::handle('StartRegistrationTry', array($this))) { - return; + // TRANS: Client error displayed when trying to create a new user but a plugin aborted the process. + throw new ClientException(_m('Registration of new user was aborted, maybe you failed a captcha?')); } if (common_config('site', 'closed')) { // TRANS: Client error displayed when trying to create a new user while creating new users is not allowed. - $this->clientError(_m('Registration not allowed.')); + throw new ClientException(_m('Registration not allowed.')); } $invite = null; @@ -502,23 +449,19 @@ class TwitterauthorizationAction extends Action $code = $_SESSION['invitecode']; if (empty($code)) { // TRANS: Client error displayed when trying to create a new user while creating new users is not allowed. - $this->clientError(_m('Registration not allowed.')); + throw new ClientException(_m('Registration not allowed.')); } - $invite = Invitation::getKV($code); + $invite = Invitation::getKV('code', $code); - if (empty($invite)) { + if (!$invite instanceof Invite) { // TRANS: Client error displayed when trying to create a new user with an invalid invitation code. - $this->clientError(_m('Not a valid invitation code.')); + throw new ClientException(_m('Not a valid invitation code.')); } } - try { - $nickname = Nickname::normalize($this->trimmed('newname'), true); - } catch (NicknameException $e) { - $this->showForm($e->getMessage()); - return; - } + // Nickname::normalize throws exception if the nickname is taken + $nickname = Nickname::normalize($this->trimmed('newname'), true); $fullname = trim($this->tw_fields['fullname']); @@ -533,23 +476,14 @@ class TwitterauthorizationAction extends Action $args['email'] = $email; } - try { - $user = User::register($args); - } catch (Exception $e) { - $this->serverError($e->getMessage()); - } + $user = User::register($args); - $result = $this->saveForeignLink($user->id, - $this->twuid, - $this->access_token); + $this->saveForeignLink($user->id, + $this->twuid, + $this->access_token); save_twitter_user($this->twuid, $this->tw_fields['screen_name']); - if (!$result) { - // TRANS: Server error displayed when connecting a user to a Twitter user has failed. - $this->serverError(_m('Error connecting user to Twitter.')); - } - common_set_user($user); common_real_login(true); @@ -569,28 +503,23 @@ class TwitterauthorizationAction extends Action if (!common_check_user($nickname, $password)) { // TRANS: Form validation error displayed when connecting an existing user to a Twitter user fails because // TRANS: the provided username and/or password are incorrect. - $this->showForm(_m('Invalid username or password.')); - return; + throw new ClientException(_m('Invalid username or password.')); } $user = User::getKV('nickname', $nickname); - if (!empty($user)) { + if ($user instanceof User) { common_debug('TwitterBridge Plugin - ' . "Legit user to connect to Twitter: $nickname"); } - $result = $this->saveForeignLink($user->id, - $this->twuid, - $this->access_token); + // throws exception on failure + $this->saveForeignLink($user->id, + $this->twuid, + $this->access_token); save_twitter_user($this->twuid, $this->tw_fields['screen_name']); - if (!$result) { - // TRANS: Server error displayed connecting a user to a Twitter user has failed. - $this->serverError(_m('Error connecting user to Twitter.')); - } - common_debug('TwitterBridge Plugin - ' . "Connected Twitter user $this->twuid to local user $user->id"); diff --git a/plugins/TwitterBridge/actions/twitterlogin.php b/plugins/TwitterBridge/actions/twitterlogin.php index ee00714c9f..f4af7d577d 100644 --- a/plugins/TwitterBridge/actions/twitterlogin.php +++ b/plugins/TwitterBridge/actions/twitterlogin.php @@ -28,9 +28,7 @@ * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } require_once dirname(__DIR__) . '/twitter.php'; @@ -46,20 +44,8 @@ require_once dirname(__DIR__) . '/twitter.php'; * * @see SettingsAction */ -class TwitterloginAction extends Action +class TwitterloginAction extends LoginAction { - function handle($args) - { - parent::handle($args); - - if (common_is_real_login()) { - // TRANS: Client error displayed when trying to log in using Twitter while already logged in to StatusNet. - $this->clientError(_m('Already logged in.')); - } - - $this->showPage(); - } - function title() { // TRANS: Title for login using Twitter page. @@ -72,15 +58,6 @@ class TwitterloginAction extends Action return _m('Login with your Twitter account'); } - function showPageNotice() - { - $instr = $this->getInstructions(); - $output = common_markup_to_html($instr); - $this->elementStart('div', 'instructions'); - $this->raw($output); - $this->elementEnd('div'); - } - function showContent() { $this->elementStart('a', array('href' => common_local_url('twitterauthorization', diff --git a/plugins/TwitterBridge/lib/twitteroauthclient.php b/plugins/TwitterBridge/lib/twitteroauthclient.php index f992a09079..9d3b244724 100644 --- a/plugins/TwitterBridge/lib/twitteroauthclient.php +++ b/plugins/TwitterBridge/lib/twitteroauthclient.php @@ -111,7 +111,7 @@ class TwitterOAuthClient extends OAuthClient * * @return OAuthToken $token the request token */ - function getRequestToken() + function getTwitterRequestToken() { return parent::getRequestToken( self::$requestTokenURL, @@ -142,7 +142,7 @@ class TwitterOAuthClient extends OAuthClient * * @return OAuthToken $token the access token */ - function getAccessToken($verifier = null) + function getTwitterAccessToken($verifier = null) { return parent::getAccessToken( self::$accessTokenURL, diff --git a/plugins/TwitterBridge/twitter.php b/plugins/TwitterBridge/twitter.php index b607fe6052..bdcd61db45 100644 --- a/plugins/TwitterBridge/twitter.php +++ b/plugins/TwitterBridge/twitter.php @@ -28,16 +28,17 @@ function add_twitter_user($twitter_id, $screen_name) // Clear out any bad old foreign_users with the new user's legit URL // This can happen when users move around or fakester accounts get // repoed, and things like that. - $luser = Foreign_user::getForeignUser($twitter_id, TWITTER_SERVICE); - - if (!empty($luser)) { - $result = $luser->delete(); + try { + $fuser = Foreign_user::getForeignUser($twitter_id, TWITTER_SERVICE); + $result = $fuser->delete(); if ($result != false) { common_log( LOG_INFO, "Twitter bridge - removed old Twitter user: $screen_name ($twitter_id)." ); } + } catch (NoResultException $e) { + // no old foreign users exist for this id } $fuser = new Foreign_user(); @@ -49,9 +50,8 @@ function add_twitter_user($twitter_id, $screen_name) $fuser->created = common_sql_now(); $result = $fuser->insert(); - if (empty($result)) { - common_log(LOG_WARNING, - "Twitter bridge - failed to add new Twitter user: $twitter_id - $screen_name."); + if ($result === false) { + common_log(LOG_WARNING, "Twitter bridge - failed to add new Twitter user: $twitter_id - $screen_name."); common_log_db_error($fuser, 'INSERT', __FILE__); } else { common_log(LOG_INFO, @@ -66,11 +66,10 @@ function save_twitter_user($twitter_id, $screen_name) { // Check to see whether the Twitter user is already in the system, // and update its screen name and uri if so. - $fuser = Foreign_user::getForeignUser($twitter_id, TWITTER_SERVICE); + try { + $fuser = Foreign_user::getForeignUser($twitter_id, TWITTER_SERVICE); - if (!empty($fuser)) { // Delete old record if Twitter user changed screen name - if ($fuser->nickname != $screen_name) { $oldname = $fuser->nickname; $fuser->delete(); @@ -80,11 +79,13 @@ function save_twitter_user($twitter_id, $screen_name) $screen_name, $oldname)); } - } else { + } catch (NoResultException $e) { + // No old users exist for this id + // Kill any old, invalid records for this screen name - $fuser = Foreign_user::getByNickname($screen_name, TWITTER_SERVICE); - - if (!empty($fuser)) { + // XXX: Is this really only supposed to be run if the above getForeignUser fails? + try { + $fuser = Foreign_user::getByNickname($screen_name, TWITTER_SERVICE); $fuser->delete(); common_log( LOG_INFO, @@ -95,6 +96,8 @@ function save_twitter_user($twitter_id, $screen_name) $fuser->id ) ); + } catch (NoResultException $e) { + // No old users exist for this screen_name } } From e10d081a56bf600b1babc1fa1c774dc8cbe874c1 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 17 Jul 2015 21:03:37 +0200 Subject: [PATCH 30/40] TwitterBridge is closer to working again --- classes/Foreign_link.php | 31 +++++---- classes/Foreign_user.php | 7 +- plugins/TwitterBridge/TwitterBridgePlugin.php | 15 ++-- .../actions/twitterauthorization.php | 69 ++++++++----------- .../TwitterBridge/actions/twittersettings.php | 6 +- 5 files changed, 61 insertions(+), 67 deletions(-) diff --git a/classes/Foreign_link.php b/classes/Foreign_link.php index 0d942443f3..6e050d0c94 100644 --- a/classes/Foreign_link.php +++ b/classes/Foreign_link.php @@ -56,34 +56,37 @@ class Foreign_link extends Managed_DataObject static function getByUserID($user_id, $service) { if (empty($user_id) || empty($service)) { - return null; + throw new ServerException('Empty user_id or service for Foreign_link::getByUserID'); } $flink = new Foreign_link(); - $flink->service = $service; $flink->user_id = $user_id; $flink->limit(1); - $result = $flink->find(true); + if (!$flink->find(true)) { + throw new NoResultException($flink); + } - return empty($result) ? null : $flink; + return $flink; } static function getByForeignID($foreign_id, $service) { if (empty($foreign_id) || empty($service)) { - return null; - } else { - $flink = new Foreign_link(); - $flink->service = $service; - $flink->foreign_id = $foreign_id; - $flink->limit(1); - - $result = $flink->find(true); - - return empty($result) ? null : $flink; + throw new ServerException('Empty foreign_id or service for Foreign_link::getByForeignID'); } + + $flink = new Foreign_link(); + $flink->service = $service; + $flink->foreign_id = $foreign_id; + $flink->limit(1); + + if (!$flink->find(true)) { + throw new NoResultException($flink); + } + + return $flink; } function set_flags($noticesend, $noticerecv, $replysync, $friendsync) diff --git a/classes/Foreign_user.php b/classes/Foreign_user.php index 3d23eabef9..1f6c77851d 100644 --- a/classes/Foreign_user.php +++ b/classes/Foreign_user.php @@ -41,7 +41,12 @@ class Foreign_user extends Managed_DataObject ); } - static function getForeignUser($id, $service) { + static function getForeignUser($id, $service) + { + if (empty($id) || empty($service)) { + throw new ServerException('Empty foreign user id or service for Foreign_user::getForeignUser'); + } + $fuser = new Foreign_user(); $fuser->id = $id; $fuser->service = $service; diff --git a/plugins/TwitterBridge/TwitterBridgePlugin.php b/plugins/TwitterBridge/TwitterBridgePlugin.php index 566038f2ec..84c6285029 100644 --- a/plugins/TwitterBridge/TwitterBridgePlugin.php +++ b/plugins/TwitterBridge/TwitterBridgePlugin.php @@ -512,16 +512,15 @@ class TwitterBridgePlugin extends Plugin { $fuser = null; - $flink = Foreign_link::getByUserID($profile->id, TWITTER_SERVICE); - - if (!empty($flink)) { + try { + $flink = Foreign_link::getByUserID($profile->id, TWITTER_SERVICE); $fuser = $flink->getForeignUser(); - if (!empty($fuser)) { - $links[] = array("href" => $fuser->uri, - "text" => sprintf(_("@%s on Twitter"), $fuser->nickname), - "image" => $this->path("icons/twitter-bird-white-on-blue.png")); - } + $links[] = array("href" => $fuser->uri, + "text" => sprintf(_("@%s on Twitter"), $fuser->nickname), + "image" => $this->path("icons/twitter-bird-white-on-blue.png")); + } catch (NoResultException $e) { + // no foreign link for Twitter on this profile ID } return true; diff --git a/plugins/TwitterBridge/actions/twitterauthorization.php b/plugins/TwitterBridge/actions/twitterauthorization.php index 90e4c35410..43a46835c3 100644 --- a/plugins/TwitterBridge/actions/twitterauthorization.php +++ b/plugins/TwitterBridge/actions/twitterauthorization.php @@ -31,6 +31,7 @@ if (!defined('GNUSOCIAL')) { exit(1); } require_once dirname(__DIR__) . '/twitter.php'; +require_once INSTALLDIR . '/lib/oauthclient.php'; /** * Class for doing OAuth authentication against Twitter @@ -81,16 +82,6 @@ class TwitterauthorizationAction extends FormAction } } } - - // $this->oauth_token is only populated once Twitter authorizes our - // request token. If it's empty we're at the beginning of the auth - // process - if (empty($this->oauth_token)) { - // authorizeRequestToken either throws an exception or redirects - $this->authorizeRequestToken(); - } else { - $this->saveAccessToken(); - } } protected function doPost() @@ -127,12 +118,10 @@ class TwitterauthorizationAction extends FormAction { try { // Get a new request token and authorize it - $client = new TwitterOAuthClient(); $req_tok = $client->getTwitterRequestToken(); // Sock the request token away in the session temporarily - $_SESSION['twitter_request_token'] = $req_tok->key; $_SESSION['twitter_request_token_secret'] = $req_tok->secret; @@ -170,16 +159,12 @@ class TwitterauthorizationAction extends FormAction $twitter_user = null; try { - - $client = new TwitterOAuthClient($_SESSION['twitter_request_token'], - $_SESSION['twitter_request_token_secret']); + $client = new TwitterOAuthClient($_SESSION['twitter_request_token'], $_SESSION['twitter_request_token_secret']); // Exchange the request token for an access token - $atok = $client->getTwitterAccessToken($this->verifier); // Test the access token and get the user's Twitter info - $client = new TwitterOAuthClient($atok->key, $atok->secret); $twitter_user = $client->verifyCredentials(); @@ -190,13 +175,11 @@ class TwitterauthorizationAction extends FormAction $e->getMessage() ); common_log(LOG_INFO, 'Twitter bridge - ' . $msg); - $this->serverError( - // TRANS: Server error displayed when linking to a Twitter account fails. - _m('Could not link your Twitter account.') - ); + // TRANS: Server error displayed when linking to a Twitter account fails. + throw new ServerException(_m('Could not link your Twitter account.')); } - if (common_logged_in()) { + if ($this->scoped instanceof Profile) { // Save the access token and Twitter user info $this->saveForeignLink($this->scoped->getID(), $twitter_user->id, $atok); @@ -208,7 +191,7 @@ class TwitterauthorizationAction extends FormAction $this->tw_fields = array("screen_name" => $twitter_user->screen_name, "fullname" => $twitter_user->name); $this->access_token = $atok; - $this->tryLogin(); + return $this->tryLogin(); } // Clean up the the mess we made in the session @@ -282,6 +265,21 @@ class TwitterauthorizationAction extends FormAction return _m('Twitter Account Setup'); } + public function showPage() + { + // $this->oauth_token is only populated once Twitter authorizes our + // request token. If it's empty we're at the beginning of the auth + // process + if (empty($this->oauth_token)) { + // authorizeRequestToken either throws an exception or redirects + $this->authorizeRequestToken(); + } else { + $this->saveAccessToken(); + } + + parent::showPage(); + } + /** * @fixme much of this duplicates core code, which is very fragile. * Should probably be replaced with an extensible mini version of @@ -289,11 +287,6 @@ class TwitterauthorizationAction extends FormAction */ function showContent() { - if (!empty($this->message_text)) { - $this->element('p', null, $this->message); - return; - } - $this->elementStart('form', array('method' => 'post', 'id' => 'form_settings_twitter_connect', 'class' => 'form_settings', @@ -310,7 +303,7 @@ class TwitterauthorizationAction extends FormAction $this->hidden('token', common_session_token()); // Don't allow new account creation if site is flagged as invite only - if (common_config('site', 'inviteonly') == false) { + if (common_config('site', 'inviteonly') == false) { $this->elementStart('fieldset'); $this->element('legend', null, // TRANS: Fieldset legend. @@ -425,12 +418,6 @@ class TwitterauthorizationAction extends FormAction return ''; } - function message($msg) - { - $this->message_text = $msg; - $this->showPage(); - } - function createNewUser() { if (!Event::handle('StartRegistrationTry', array($this))) { @@ -567,14 +554,12 @@ class TwitterauthorizationAction extends FormAction common_real_login(true); $this->goHome($user->nickname); } - - } else { - - common_debug('TwitterBridge Plugin - ' . - "No flink found for twuid: $this->twuid - new user"); - - $this->showForm(null, $this->bestNewNickname()); } + common_debug('TwitterBridge Plugin - ' . + "No flink found for twuid: $this->twuid - new user"); + + return; + throw new ServerException(_m('No foreign link found for Twitter user')); } function goHome($nickname) diff --git a/plugins/TwitterBridge/actions/twittersettings.php b/plugins/TwitterBridge/actions/twittersettings.php index 85d1a4bbe2..efde36797c 100644 --- a/plugins/TwitterBridge/actions/twittersettings.php +++ b/plugins/TwitterBridge/actions/twittersettings.php @@ -49,9 +49,11 @@ class TwittersettingsAction extends ProfileSettingsAction protected function doPreparation() { - $this->flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); - if ($this->flink instanceof Foreign_link) { + try { + $this->flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); $this->fuser = $this->flink->getForeignUser(); + } catch (NoResultException $e) { + // No foreign link found for this user! } } /** From beba2a25d0e1f89236702a9a2c739e4ca521e0de Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 01:09:50 +0200 Subject: [PATCH 31/40] Don't retry unhandled transports in OpportunisticQM It'd continue trying xmpp transports forever, for example... --- classes/Queue_item.php | 7 ++++++- lib/dbqueuemanager.php | 2 +- lib/queuemanager.php | 12 ++++++++++++ .../lib/opportunisticqueuemanager.php | 9 ++++++++- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/classes/Queue_item.php b/classes/Queue_item.php index 3a7d05adef..d41c53e0e0 100644 --- a/classes/Queue_item.php +++ b/classes/Queue_item.php @@ -40,7 +40,7 @@ class Queue_item extends Managed_DataObject * @param mixed $transports name of a single queue or array of queues to pull from * If not specified, checks all queues in the system. */ - static function top($transports=null) { + static function top($transports=null, array $ignored_transports=array()) { $qi = new Queue_item(); if ($transports) { @@ -52,6 +52,11 @@ class Queue_item extends Managed_DataObject $qi->transport = $transports; } } + if (!empty($ignored_transports)) { + // @fixme use safer escaping + $list = implode("','", array_map(array($qi, 'escape'), $ignored_transports)); + $qi->whereAdd("transport NOT IN ('$list')"); + } $qi->orderBy('created'); $qi->whereAdd('claimed is null'); diff --git a/lib/dbqueuemanager.php b/lib/dbqueuemanager.php index 45c4b694d2..9fb77eed96 100644 --- a/lib/dbqueuemanager.php +++ b/lib/dbqueuemanager.php @@ -72,7 +72,7 @@ class DBQueueManager extends QueueManager public function poll() { //$this->_log(LOG_DEBUG, 'Checking for notices...'); - $qi = Queue_item::top($this->activeQueues()); + $qi = Queue_item::top($this->activeQueues(), $this->getIgnoredTransports()); if (!$qi instanceof Queue_item) { //$this->_log(LOG_DEBUG, 'No notices waiting; idling.'); return false; diff --git a/lib/queuemanager.php b/lib/queuemanager.php index 45fe1e4ab4..487104099a 100644 --- a/lib/queuemanager.php +++ b/lib/queuemanager.php @@ -43,6 +43,7 @@ abstract class QueueManager extends IoManager protected $handlers = array(); protected $groups = array(); protected $activeGroups = array(); + protected $ignoredTransports = array(); /** * Factory function to pull the appropriate QueueManager object @@ -255,6 +256,17 @@ abstract class QueueManager extends IoManager return array_keys($queues); } + function getIgnoredTransports() + { + return array_keys($this->ignoredTransports); + } + + function ignoreTransport($transport) + { + // key is used for uniqueness, value doesn't mean anything + $this->ignoredTransports[$transport] = true; + } + /** * Initialize the list of queue handlers for the current site. * diff --git a/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php b/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php index 4b2b679b58..b2dc61e15f 100644 --- a/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php +++ b/plugins/OpportunisticQM/lib/opportunisticqueuemanager.php @@ -83,10 +83,17 @@ class OpportunisticQueueManager extends DBQueueManager // OpportunisticQM shouldn't discard items it can't handle, we're // only here to take care of what we _can_ handle! protected function noHandlerFound(Queue_item $qi, $rep=null) { - $this->_log(LOG_WARNING, "[{$qi->transport}:item {$qi->id}] Releasing claim for queue item without a handler"); + $this->_log(LOG_WARNING, "[{$qi->transport}:item {$qi->id}] Releasing claim for queue item without a handler"); $this->_fail($qi, true); // true here means "releaseOnly", so no error statistics since it's not an _error_ } + protected function _fail(Queue_item $qi, $releaseOnly=false) + { + parent::_fail($qi, $releaseOnly); + $this->_log(LOG_DEBUG, "[{$qi->transport}:item {$qi->id}] Ignoring this transport for the rest of this execution"); + $this->ignoreTransport($qi->transport); + } + /** * Takes care of running through the queue items, returning when * the limits setup in __construct are met. From 9fdf6474f8f466f8323bf8591b0d5037d926acc1 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 01:18:08 +0200 Subject: [PATCH 32/40] TwitterBridge working again (for signin and posting to Twitter at least) Now we just need to make it include the newname suggestion if the form fails (for example forgetting to check the license checkbox or so). --- .../actions/twitterauthorization.php | 77 +++++++++++-------- .../TwitterBridge/actions/twittersettings.php | 48 +++++------- .../TwitterBridge/lib/twitteroauthclient.php | 2 +- 3 files changed, 63 insertions(+), 64 deletions(-) diff --git a/plugins/TwitterBridge/actions/twitterauthorization.php b/plugins/TwitterBridge/actions/twitterauthorization.php index 43a46835c3..3eb302bda4 100644 --- a/plugins/TwitterBridge/actions/twitterauthorization.php +++ b/plugins/TwitterBridge/actions/twitterauthorization.php @@ -54,32 +54,28 @@ class TwitterauthorizationAction extends FormAction var $twuid = null; var $tw_fields = null; var $access_token = null; - var $signin = null; var $verifier = null; protected $needLogin = false; // authorization page can also be used to create a new user protected function doPreparation() { - $this->signin = $this->boolean('signin'); $this->oauth_token = $this->arg('oauth_token'); $this->verifier = $this->arg('oauth_verifier'); if ($this->scoped instanceof Profile) { - $flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); + try { + $flink = Foreign_link::getByUserID($this->scoped->getID(), TWITTER_SERVICE); + $fuser = $flink->getForeignUser(); - // If there's already a foreign link record and a foreign user - // it means the accounts are already linked, and this is unecessary. - // So go back. + // If there's already a foreign link record and a foreign user + // (no exceptions were thrown when fetching either of them...) + // it means the accounts are already linked, and this is unecessary. + // So go back. - if ($flink instanceof Foreign_link) { - try { - $fuser = $flink->getForeignUser(); - common_redirect(common_local_url('twittersettings')); - } catch (NoResultException $e) { - // ok, so no foreign user but there's a foreign link?? - // this logic is left since the StatusNet days - } + common_redirect(common_local_url('twittersettings')); + } catch (NoResultException $e) { + // but if we don't have a foreign user linked, let's continue authorization procedure. } } } @@ -96,16 +92,18 @@ class TwitterauthorizationAction extends FormAction $this->access_token = new OAuthToken($this->trimmed('access_token_key'), $this->trimmed('access_token_secret')); if ($this->arg('create')) { + common_debug('TwitterBridgeDebug - POST with create'); if (!$this->boolean('license')) { // TRANS: Form validation error displayed when the checkbox to agree to the license has not been checked. throw new ClientException(_m('You cannot register if you do not agree to the license.')); } return $this->createNewUser(); - } else if ($this->arg('connect')) { + } elseif ($this->arg('connect')) { + common_debug('TwitterBridgeDebug - POST with connect'); return $this->connectNewUser(); } - common_debug('Twitter bridge - ' . print_r($this->args, true)); + common_debug('TwitterBridgeDebug - ' . print_r($this->args, true)); // TRANS: Form validation error displayed when an unhandled error occurs. throw new ClientException(_m('No known action for POST.')); } @@ -125,7 +123,7 @@ class TwitterauthorizationAction extends FormAction $_SESSION['twitter_request_token'] = $req_tok->key; $_SESSION['twitter_request_token_secret'] = $req_tok->secret; - $auth_link = $client->getAuthorizeLink($req_tok, $this->signin); + $auth_link = $client->getTwitterAuthorizeLink($req_tok, $this->boolean('signin')); } catch (OAuthClientException $e) { $msg = sprintf( 'OAuth client error - code: %1s, msg: %2s', @@ -270,11 +268,13 @@ class TwitterauthorizationAction extends FormAction // $this->oauth_token is only populated once Twitter authorizes our // request token. If it's empty we're at the beginning of the auth // process - if (empty($this->oauth_token)) { - // authorizeRequestToken either throws an exception or redirects - $this->authorizeRequestToken(); - } else { - $this->saveAccessToken(); + if (empty($this->error)) { + if (empty($this->oauth_token)) { + // authorizeRequestToken either throws an exception or redirects + $this->authorizeRequestToken(); + } else { + $this->saveAccessToken(); + } } parent::showPage(); @@ -302,8 +302,8 @@ class TwitterauthorizationAction extends FormAction $this->hidden('tw_fields_name', $this->tw_fields['fullname']); $this->hidden('token', common_session_token()); - // Don't allow new account creation if site is flagged as invite only - if (common_config('site', 'inviteonly') == false) { + // Only allow new account creation if site is not flagged invite-only + if (!common_config('site', 'inviteonly')) { $this->elementStart('fieldset'); $this->element('legend', null, // TRANS: Fieldset legend. @@ -418,14 +418,17 @@ class TwitterauthorizationAction extends FormAction return ''; } - function createNewUser() + protected function createNewUser() { + common_debug('TwitterBridgeDebug - createNewUser'); if (!Event::handle('StartRegistrationTry', array($this))) { + common_debug('TwitterBridgeDebug - StartRegistrationTry failed'); // TRANS: Client error displayed when trying to create a new user but a plugin aborted the process. throw new ClientException(_m('Registration of new user was aborted, maybe you failed a captcha?')); } if (common_config('site', 'closed')) { + common_debug('TwitterBridgeDebug - site is closed for registrations'); // TRANS: Client error displayed when trying to create a new user while creating new users is not allowed. throw new ClientException(_m('Registration not allowed.')); } @@ -433,6 +436,7 @@ class TwitterauthorizationAction extends FormAction $invite = null; if (common_config('site', 'inviteonly')) { + common_debug('TwitterBridgeDebug - site is inviteonly'); $code = $_SESSION['invitecode']; if (empty($code)) { // TRANS: Client error displayed when trying to create a new user while creating new users is not allowed. @@ -442,11 +446,13 @@ class TwitterauthorizationAction extends FormAction $invite = Invitation::getKV('code', $code); if (!$invite instanceof Invite) { + common_debug('TwitterBridgeDebug - and we failed the invite code test'); // TRANS: Client error displayed when trying to create a new user with an invalid invitation code. throw new ClientException(_m('Not a valid invitation code.')); } } + common_debug('TwitterBridgeDebug - trying our nickname: '.$this->trimmed('newname')); // Nickname::normalize throws exception if the nickname is taken $nickname = Nickname::normalize($this->trimmed('newname'), true); @@ -463,12 +469,16 @@ class TwitterauthorizationAction extends FormAction $args['email'] = $email; } + common_debug('TwitterBridgeDebug - registering user with args:'.var_export($args,true)); $user = User::register($args); + common_debug('TwitterBridgeDebug - registered the user and saving foreign link for '.$user->id); + $this->saveForeignLink($user->id, $this->twuid, $this->access_token); + common_debug('TwitterBridgeDebug - saving twitter user after creating new local user '.$user->id); save_twitter_user($this->twuid, $this->tw_fields['screen_name']); common_set_user($user); @@ -534,19 +544,16 @@ class TwitterauthorizationAction extends FormAction common_redirect(common_local_url('twittersettings'), 303); } - function tryLogin() + protected function tryLogin() { common_debug('TwitterBridge Plugin - ' . "Trying login for Twitter user $this->twuid."); - $flink = Foreign_link::getByForeignID($this->twuid, - TWITTER_SERVICE); - - if (!empty($flink)) { + try { + $flink = Foreign_link::getByForeignID($this->twuid, TWITTER_SERVICE); $user = $flink->getUser(); - if (!empty($user)) { - + if ($user instanceof User) { common_debug('TwitterBridge Plugin - ' . "Logged in Twitter user $flink->foreign_id as user $user->id ($user->nickname)"); @@ -554,9 +561,11 @@ class TwitterauthorizationAction extends FormAction common_real_login(true); $this->goHome($user->nickname); } + } catch (NoResultException $e) { + // Either no Foreign_link was found or not the user connected to it. + // Let's just continue to allow creating or logging in as a new user. } - common_debug('TwitterBridge Plugin - ' . - "No flink found for twuid: $this->twuid - new user"); + common_debug("TwitterBridge Plugin - No flink found for twuid: {$this->twuid} - new user"); return; throw new ServerException(_m('No foreign link found for Twitter user')); diff --git a/plugins/TwitterBridge/actions/twittersettings.php b/plugins/TwitterBridge/actions/twittersettings.php index efde36797c..6665ae619e 100644 --- a/plugins/TwitterBridge/actions/twittersettings.php +++ b/plugins/TwitterBridge/actions/twittersettings.php @@ -216,25 +216,15 @@ class TwittersettingsAction extends ProfileSettingsAction * * @return void */ - function handlePost() + protected function doPost() { - // CSRF protection - $token = $this->trimmed('token'); - if (!$token || $token != common_session_token()) { - // TRANS: Client error displayed when the session token does not match or is not given. - $this->showForm(_m('There was a problem with your session token. '. - 'Try again, please.')); - return; - } - if ($this->arg('save')) { - $this->savePreferences(); + return $this->savePreferences(); } else if ($this->arg('disconnect')) { - $this->removeTwitterAccount(); - } else { - // TRANS: Client error displayed when the submitted form contains unexpected data. - $this->showForm(_m('Unexpected form submission.')); + return $this->removeTwitterAccount(); } + // TRANS: Client error displayed when the submitted form contains unexpected data. + throw new ClientException(_m('Unexpected form submission.')); } /** @@ -242,26 +232,26 @@ class TwittersettingsAction extends ProfileSettingsAction * * @return void */ - function removeTwitterAccount() + protected function removeTwitterAccount() { - $user = common_current_user(); - $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); - - if (empty($flink)) { - // TRANS: Client error displayed when trying to remove a connected Twitter account when there isn't one connected. - $this->clientError(_m('No Twitter connection to remove.')); + if (!$this->flink instanceof Foreign_link) { + // TRANS: Error message possibly displayed when trying to remove a connected Twitter account when there isn't one connected. + throw new AlreadyFulfilledException(_m('No Twitter connection to remove.')); } - $result = $flink->safeDelete(); + $result = $this->flink->safeDelete(); - if (empty($result)) { - common_log_db_error($flink, 'DELETE', __FILE__); + if ($result === false) { + common_log_db_error($this->flink, 'DELETE', __FILE__); // TRANS: Server error displayed when trying to remove a connected Twitter account fails. - $this->serverError(_m('Could not remove Twitter user.')); + throw new ServerException(_m('Could not remove Twitter user.')); } + $this->flink = null; + $this->fuser = null; + // TRANS: Success message displayed after disconnecting a Twitter account. - $this->showForm(_m('Twitter account disconnected.'), true); + return _m('Twitter account disconnected.'); } /** @@ -269,7 +259,7 @@ class TwittersettingsAction extends ProfileSettingsAction * * @return void */ - function savePreferences() + protected function savePreferences() { $noticesend = $this->boolean('noticesend'); $noticerecv = $this->boolean('noticerecv'); @@ -298,7 +288,7 @@ class TwittersettingsAction extends ProfileSettingsAction } // TRANS: Success message after saving Twitter integration preferences. - $this->showForm(_m('Twitter preferences saved.'), true); + return _m('Twitter preferences saved.'); } /** diff --git a/plugins/TwitterBridge/lib/twitteroauthclient.php b/plugins/TwitterBridge/lib/twitteroauthclient.php index 9d3b244724..93d10b8d48 100644 --- a/plugins/TwitterBridge/lib/twitteroauthclient.php +++ b/plugins/TwitterBridge/lib/twitteroauthclient.php @@ -126,7 +126,7 @@ class TwitterOAuthClient extends OAuthClient * * @return the link */ - function getAuthorizeLink($request_token, $signin = false) + function getTwitterAuthorizeLink($request_token, $signin = false) { $url = ($signin) ? self::$signinUrl : self::$authorizeURL; From 6f62adedfc44306e1df2a63e030f8b29e334f3a2 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 02:16:52 +0200 Subject: [PATCH 33/40] Infinite loop on CLI initiated profile deletion for local users profile deleting user deleting profile deleting user... --- classes/Profile.php | 5 +++++ classes/User.php | 6 ++++-- lib/deluserqueuehandler.php | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/classes/Profile.php b/classes/Profile.php index 5359ffb58b..09f9ca71d1 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -877,6 +877,11 @@ class Profile extends Managed_DataObject function delete($useWhere=false) { + // just in case it hadn't been done before... (usually set before adding deluser to queue handling!) + if (!$this->hasRole(Profile_role::DELETED)) { + $this->grantRole(Profile_role::DELETED); + } + $this->_deleteNotices(); $this->_deleteSubscriptions(); $this->_deleteTags(); diff --git a/classes/User.php b/classes/User.php index 5b9d7b51fe..e33c83e89c 100644 --- a/classes/User.php +++ b/classes/User.php @@ -598,8 +598,10 @@ class User extends Managed_DataObject } try { - $profile = $this->getProfile(); - $profile->delete(); + if (!$this->hasRole(Profile_role::DELETED)) { + $profile = $this->getProfile(); + $profile->delete(); + } } catch (UserNoProfileException $unp) { common_log(LOG_INFO, "User {$this->nickname} has no profile; continuing deletion."); } diff --git a/lib/deluserqueuehandler.php b/lib/deluserqueuehandler.php index 1baaf9331f..65866af418 100644 --- a/lib/deluserqueuehandler.php +++ b/lib/deluserqueuehandler.php @@ -74,8 +74,13 @@ class DelUserQueueHandler extends QueueHandler $qm = QueueManager::get(); $qm->enqueue($user, 'deluser'); } else { - // Out of notices? Let's finish deleting this guy! - $user->delete(); + // Out of notices? Let's finish deleting this profile! + try { + $user->getProfile()->delete(); + } catch (UserNoProfileException $e) { + // in case a profile didn't exist for some reason, just delete the User directly + $user->delete(); + } common_log(LOG_INFO, "User $user->id $user->nickname deleted."); return true; } From b609a3610fd3e61a11dc6b278bcc4eefee347897 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 11:04:35 +0200 Subject: [PATCH 34/40] Some missed exception throwing since fixing Foreign_link and Foreign_user --- plugins/TwitterBridge/TwitterBridgePlugin.php | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/plugins/TwitterBridge/TwitterBridgePlugin.php b/plugins/TwitterBridge/TwitterBridgePlugin.php index 84c6285029..dd3007e309 100644 --- a/plugins/TwitterBridge/TwitterBridgePlugin.php +++ b/plugins/TwitterBridge/TwitterBridgePlugin.php @@ -387,9 +387,9 @@ class TwitterBridgePlugin extends Plugin if ($n2s instanceof Notice_to_status) { - $flink = Foreign_link::getByUserID($notice->profile_id, TWITTER_SERVICE); // twitter service - - if (!$flink instanceof Foreign_link) { + try { + $flink = Foreign_link::getByUserID($notice->profile_id, TWITTER_SERVICE); // twitter service + } catch (NoResultException $e) { return true; } @@ -421,9 +421,9 @@ class TwitterBridgePlugin extends Plugin */ function onEndFavorNotice(Profile $profile, Notice $notice) { - $flink = Foreign_link::getByUserID($profile->getID(), TWITTER_SERVICE); // twitter service - - if (!$flink instanceof Foreign_link) { + try { + $flink = Foreign_link::getByUserID($profile->getID(), TWITTER_SERVICE); // twitter service + } catch (NoResultException $e) { return true; } @@ -460,10 +460,9 @@ class TwitterBridgePlugin extends Plugin */ function onEndDisfavorNotice(Profile $profile, Notice $notice) { - $flink = Foreign_link::getByUserID($profile->id, - TWITTER_SERVICE); // twitter service - - if (empty($flink)) { + try { + $flink = Foreign_link::getByUserID($profile->getID(), TWITTER_SERVICE); // twitter service + } catch (NoResultException $e) { return true; } @@ -520,7 +519,7 @@ class TwitterBridgePlugin extends Plugin "text" => sprintf(_("@%s on Twitter"), $fuser->nickname), "image" => $this->path("icons/twitter-bird-white-on-blue.png")); } catch (NoResultException $e) { - // no foreign link for Twitter on this profile ID + // no foreign link and/or user for Twitter on this profile ID } return true; @@ -564,16 +563,17 @@ class TwitterBridgePlugin extends Plugin if( count($noticeArray) != 1 ) { break; } $post = $noticeArray[0]; - $flink = Foreign_link::getByUserID($post->profile_id, TWITTER_SERVICE); - if( $flink ) { // Our local user has registered Twitter Gateway + try { + $flink = Foreign_link::getByUserID($post->profile_id, TWITTER_SERVICE); $fuser = Foreign_user::getForeignUser($flink->foreign_id, TWITTER_SERVICE); - if( $fuser ) { // Got nickname for local user's Twitter account - $action->element('meta', array('name' => 'twitter:creator', - 'content' => '@'.$fuser->nickname)); - } + $action->element('meta', array('name' => 'twitter:creator', + 'content' => '@'.$fuser->nickname)); + } catch (NoResultException $e) { + // no foreign link and/or user for Twitter on this profile ID } break; - default: break; + default: + break; } return true; From e0084a6fdf314365c6e58334bdd44a99f8e79bd1 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 11:39:34 +0200 Subject: [PATCH 35/40] Exception handling regarding Foreign_link --- classes/Foreign_link.php | 4 +- .../actions/facebookfinishlogin.php | 40 +++++++-------- plugins/FacebookBridge/lib/facebookclient.php | 11 ++-- .../actions/twitterauthorization.php | 13 +++-- .../TwitterBridge/actions/twittersettings.php | 2 +- .../daemons/synctwitterfriends.php | 50 +++++++++---------- .../daemons/twitterstatusfetcher.php | 9 +--- .../TwitterBridge/lib/tweetinqueuehandler.php | 6 +-- plugins/TwitterBridge/lib/twitterimport.php | 22 ++++---- plugins/TwitterBridge/scripts/fakestream.php | 7 +-- plugins/TwitterBridge/scripts/streamtest.php | 7 +-- plugins/TwitterBridge/twitter.php | 19 +++++-- 12 files changed, 88 insertions(+), 102 deletions(-) diff --git a/classes/Foreign_link.php b/classes/Foreign_link.php index 6e050d0c94..b3757448ad 100644 --- a/classes/Foreign_link.php +++ b/classes/Foreign_link.php @@ -136,12 +136,12 @@ class Foreign_link extends Managed_DataObject function getUser() { - return User::getKV($this->user_id); + return Profile::getByID($this->user_id)->getUser(); } function getProfile() { - return Profile::getKV('id', $this->user_id); + return Profile::getByID($this->user_id); } // Make sure we only ever delete one record at a time diff --git a/plugins/FacebookBridge/actions/facebookfinishlogin.php b/plugins/FacebookBridge/actions/facebookfinishlogin.php index 080c59612c..7cf493a994 100644 --- a/plugins/FacebookBridge/actions/facebookfinishlogin.php +++ b/plugins/FacebookBridge/actions/facebookfinishlogin.php @@ -519,34 +519,30 @@ class FacebookfinishloginAction extends Action function tryLogin() { - $flink = Foreign_link::getByForeignID($this->fbuid, FACEBOOK_SERVICE); - - if (!empty($flink)) { + try { + $flink = Foreign_link::getByForeignID($this->fbuid, FACEBOOK_SERVICE); $user = $flink->getUser(); - if (!empty($user)) { + common_log( + LOG_INFO, + sprintf( + 'Logged in Facebook user %s as user %d (%s)', + $this->fbuid, + $user->nickname, + $user->id + ), + __FILE__ + ); - common_log( - LOG_INFO, - sprintf( - 'Logged in Facebook user %s as user %d (%s)', - $this->fbuid, - $user->nickname, - $user->id - ), - __FILE__ - ); + common_set_user($user); + common_real_login(true); - common_set_user($user); - common_real_login(true); + // clear out the stupid cookie + setcookie('fb_access_token', '', time() - 3600); // one hour ago - // clear out the stupid cookie - setcookie('fb_access_token', '', time() - 3600); // one hour ago + $this->goHome($user->nickname); - $this->goHome($user->nickname); - } - - } else { + } catch (NoResultException $e) { $this->showForm(null, $this->bestNewNickname()); } } diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index e55e19d88b..5faad5dfb7 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -66,13 +66,12 @@ class Facebookclient $this->notice = $notice; $profile_id = $profile ? $profile->id : $notice->profile_id; - $this->flink = Foreign_link::getByUserID( - $profile_id, - FACEBOOK_SERVICE - ); - - if (!empty($this->flink)) { + try { + $this->flink = Foreign_link::getByUserID($profile_id, FACEBOOK_SERVICE); $this->user = $this->flink->getUser(); + } catch (NoResultException $e) { + // at least $this->flink could've gotten set to something, + // but the logic that was here before didn't care, so let's not care either } } diff --git a/plugins/TwitterBridge/actions/twitterauthorization.php b/plugins/TwitterBridge/actions/twitterauthorization.php index 3eb302bda4..c9b892b640 100644 --- a/plugins/TwitterBridge/actions/twitterauthorization.php +++ b/plugins/TwitterBridge/actions/twitterauthorization.php @@ -553,20 +553,19 @@ class TwitterauthorizationAction extends FormAction $flink = Foreign_link::getByForeignID($this->twuid, TWITTER_SERVICE); $user = $flink->getUser(); - if ($user instanceof User) { - common_debug('TwitterBridge Plugin - ' . - "Logged in Twitter user $flink->foreign_id as user $user->id ($user->nickname)"); + common_debug('TwitterBridge Plugin - ' . + "Logged in Twitter user $flink->foreign_id as user $user->id ($user->nickname)"); - common_set_user($user); - common_real_login(true); - $this->goHome($user->nickname); - } + common_set_user($user); + common_real_login(true); + $this->goHome($user->nickname); } catch (NoResultException $e) { // Either no Foreign_link was found or not the user connected to it. // Let's just continue to allow creating or logging in as a new user. } common_debug("TwitterBridge Plugin - No flink found for twuid: {$this->twuid} - new user"); + // FIXME: what do we want to do here? I forgot return; throw new ServerException(_m('No foreign link found for Twitter user')); } diff --git a/plugins/TwitterBridge/actions/twittersettings.php b/plugins/TwitterBridge/actions/twittersettings.php index 6665ae619e..ccdb44fcb9 100644 --- a/plugins/TwitterBridge/actions/twittersettings.php +++ b/plugins/TwitterBridge/actions/twittersettings.php @@ -184,7 +184,7 @@ class TwittersettingsAction extends ProfileSettingsAction $this->elementEnd('ul'); - if ($this->flink) { + if ($this->flink instanceof Foreign_link) { // TRANS: Button text for saving Twitter integration settings. $this->submit('save', _m('BUTTON','Save')); } else { diff --git a/plugins/TwitterBridge/daemons/synctwitterfriends.php b/plugins/TwitterBridge/daemons/synctwitterfriends.php index a3862eedfd..9fa3b282b4 100755 --- a/plugins/TwitterBridge/daemons/synctwitterfriends.php +++ b/plugins/TwitterBridge/daemons/synctwitterfriends.php @@ -104,6 +104,7 @@ class SyncTwitterFriendsDaemon extends ParallelizingDaemon return $flinks; } + // FIXME: make it so we can force a Foreign_link here without colliding with parent function childTask($flink) { // Each child ps needs its own DB connection @@ -124,7 +125,7 @@ class SyncTwitterFriendsDaemon extends ParallelizingDaemon unset($_DB_DATAOBJECT['CONNECTIONS']); } - function fetchTwitterFriends($flink) + function fetchTwitterFriends(Foreign_link $flink) { $friends = array(); @@ -192,8 +193,14 @@ class SyncTwitterFriendsDaemon extends ParallelizingDaemon return $friends; } - function subscribeTwitterFriends($flink) + function subscribeTwitterFriends(Foreign_link $flink) { + try { + $profile = $flink->getProfile(); + } catch (NoResultException $e) { + common_log(LOG_WARNING, 'Foreign_link has no matching local profile for local ID: '.$flink->user_id); + } + $friends = $this->fetchTwitterFriends($flink); if (empty($friends)) { @@ -203,8 +210,6 @@ class SyncTwitterFriendsDaemon extends ParallelizingDaemon return false; } - $profile = $flink->getProfile(); - foreach ($friends as $friend) { $friend_name = $friend->screen_name; @@ -219,31 +224,24 @@ class SyncTwitterFriendsDaemon extends ParallelizingDaemon continue; } - // Check to see if there's a related local user - - $friend_flink = Foreign_link::getByForeignID($friend_id, - TWITTER_SERVICE); - - if (!empty($friend_flink)) { + // Check to see if there's a related local user and try to subscribe + try { + $friend_flink = Foreign_link::getByForeignID($friend_id, TWITTER_SERVICE); // Get associated user and subscribe her + $friend_profile = $friend_flink->getProfile(); - $friend_profile = Profile::getKV('id', $friend_flink->user_id); - - if ($friend_profile instanceof Profile) { - try { - $other = Profile::getKV('id', $invites->user_id); - Subscription::start($profile, $friend_profile); - common_log(LOG_INFO, - $this->name() . ' - Subscribed ' . - "{$friend_profile->nickname} to {$profile->nickname}."); - } catch (Exception $e) { - common_debug($this->name() . - ' - Tried and failed subscribing ' . - "{$friend_profile->nickname} to {$profile->nickname} - " . - $e->getMessage()); - } - } + Subscription::start($profile, $friend_profile); + common_log(LOG_INFO, + $this->name() . ' - Subscribed ' . + "{$friend_profile->nickname} to {$profile->nickname}."); + } catch (NoResultException $e) { + // either no foreign link for this friend's foreign ID or no profile found on local ID. + } catch (Exception $e) { + common_debug($this->name() . + ' - Tried and failed subscribing ' . + "{$friend_profile->nickname} to {$profile->nickname} - " . + $e->getMessage()); } } diff --git a/plugins/TwitterBridge/daemons/twitterstatusfetcher.php b/plugins/TwitterBridge/daemons/twitterstatusfetcher.php index d444b8aa5d..83e8a0df5e 100755 --- a/plugins/TwitterBridge/daemons/twitterstatusfetcher.php +++ b/plugins/TwitterBridge/daemons/twitterstatusfetcher.php @@ -128,6 +128,7 @@ class TwitterStatusFetcher extends ParallelizingDaemon return $flinks; } + // FIXME: make it so we can force a Foreign_link here without colliding with parent function childTask($flink) { // Each child ps needs its own DB connection @@ -149,14 +150,8 @@ class TwitterStatusFetcher extends ParallelizingDaemon unset($_DB_DATAOBJECT['CONNECTIONS']); } - function getTimeline($flink, $timelineUri = 'home_timeline') + function getTimeline(Foreign_link $flink, $timelineUri = 'home_timeline') { - if (empty($flink)) { - common_log(LOG_ERR, $this->name() . - " - Can't retrieve Foreign_link for foreign ID $fid"); - return; - } - common_log(LOG_DEBUG, $this->name() . ' - Trying to get ' . $timelineUri . ' timeline for Twitter user ' . $flink->foreign_id); diff --git a/plugins/TwitterBridge/lib/tweetinqueuehandler.php b/plugins/TwitterBridge/lib/tweetinqueuehandler.php index cc0c05f9a6..69ce5a61e9 100644 --- a/plugins/TwitterBridge/lib/tweetinqueuehandler.php +++ b/plugins/TwitterBridge/lib/tweetinqueuehandler.php @@ -51,8 +51,8 @@ class TweetInQueueHandler extends QueueHandler $importer = new TwitterImport(); $notice = $importer->importStatus($status); if ($notice instanceof Notice) { - $flink = Foreign_link::getByForeignID($receiver, TWITTER_SERVICE); - if ($flink instanceof Foreign_link) { + try { + $flink = Foreign_link::getByForeignID($receiver, TWITTER_SERVICE); common_log(LOG_DEBUG, "TweetInQueueHandler - Got flink so add notice ". $notice->id." to attentions for user ".$flink->user_id); try { @@ -63,7 +63,7 @@ class TweetInQueueHandler extends QueueHandler common_log(LOG_ERR, "Failed adding notice {$notice->id} to attentions for user {$flink->user_id}: " . $e->getMessage()); } - } else { + } catch (NoResultException $e) { common_log(LOG_DEBUG, "TweetInQueueHandler - No flink found for foreign user ".$receiver); } } diff --git a/plugins/TwitterBridge/lib/twitterimport.php b/plugins/TwitterBridge/lib/twitterimport.php index 45b7547ce2..d929fecf83 100644 --- a/plugins/TwitterBridge/lib/twitterimport.php +++ b/plugins/TwitterBridge/lib/twitterimport.php @@ -542,17 +542,17 @@ class TwitterImport } foreach ($status->entities->user_mentions as $mention) { - $flink = Foreign_link::getByForeignID($mention->id, TWITTER_SERVICE); - if (!empty($flink)) { - $user = User::getKV('id', $flink->user_id); - if (!empty($user)) { - $reply = new Reply(); - $reply->notice_id = $notice->id; - $reply->profile_id = $user->id; - $reply->modified = $notice->created; - common_log(LOG_INFO, __METHOD__ . ": saving reply: notice {$notice->id} to profile {$user->id}"); - $id = $reply->insert(); - } + try { + $flink = Foreign_link::getByForeignID($mention->id, TWITTER_SERVICE); + $user = $flink->getUser(); + $reply = new Reply(); + $reply->notice_id = $notice->id; + $reply->profile_id = $user->id; + $reply->modified = $notice->created; + common_log(LOG_INFO, __METHOD__ . ": saving reply: notice {$notice->id} to profile {$user->id}"); + $id = $reply->insert(); + } catch (NoResultException $e) { + common_log(LOG_WARNING, 'No local user found for Foreign_link with local User id: '.$flink->user_id); } } } diff --git a/plugins/TwitterBridge/scripts/fakestream.php b/plugins/TwitterBridge/scripts/fakestream.php index e827a07117..5d965e7394 100644 --- a/plugins/TwitterBridge/scripts/fakestream.php +++ b/plugins/TwitterBridge/scripts/fakestream.php @@ -62,12 +62,7 @@ if (have_option('n')) { */ function twitterAuthForUser(User $user) { - $flink = Foreign_link::getByUserID($user->id, - TWITTER_SERVICE); - if (!$flink) { - throw new ServerException("No Twitter config for this user."); - } - + $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); $token = TwitterOAuthClient::unpackToken($flink->credentials); if (!$token) { throw new ServerException("No Twitter OAuth credentials for this user."); diff --git a/plugins/TwitterBridge/scripts/streamtest.php b/plugins/TwitterBridge/scripts/streamtest.php index 4e8340bb3f..a642920cee 100644 --- a/plugins/TwitterBridge/scripts/streamtest.php +++ b/plugins/TwitterBridge/scripts/streamtest.php @@ -63,12 +63,7 @@ if (have_option('n')) { */ function twitterAuthForUser(User $user) { - $flink = Foreign_link::getByUserID($user->id, - TWITTER_SERVICE); - if (!$flink) { - throw new ServerException("No Twitter config for this user."); - } - + $flink = Foreign_link::getByUserID($user->id, TWITTER_SERVICE); $token = TwitterOAuthClient::unpackToken($flink->credentials); if (!$token) { throw new ServerException("No Twitter OAuth credentials for this user."); diff --git a/plugins/TwitterBridge/twitter.php b/plugins/TwitterBridge/twitter.php index bdcd61db45..0f1e686ac8 100644 --- a/plugins/TwitterBridge/twitter.php +++ b/plugins/TwitterBridge/twitter.php @@ -181,11 +181,15 @@ function twitter_id($status, $field='id') */ function broadcast_twitter($notice) { - $flink = Foreign_link::getByUserID($notice->profile_id, - TWITTER_SERVICE); + try { + $flink = Foreign_link::getByUserID($notice->profile_id, TWITTER_SERVICE); + } catch (NoResultException $e) { + // Alright so don't broadcast it then! (since there's no foreign link) + return true; + } // Don't bother with basic auth, since it's no longer allowed - if (!empty($flink) && TwitterOAuthClient::isPackedToken($flink->credentials)) { + if (TwitterOAuthClient::isPackedToken($flink->credentials)) { if (is_twitter_bound($notice, $flink)) { if (!empty($notice->repeat_of) && is_twitter_notice($notice->repeat_of)) { $retweet = retweet_notice($flink, Notice::getKV('id', $notice->repeat_of)); @@ -273,8 +277,13 @@ function twitter_update_params($notice) return $params; } -function broadcast_oauth($notice, $flink) { - $user = $flink->getUser(); +function broadcast_oauth($notice, Foreign_link $flink) { + try { + $user = $flink->getUser(); + } catch (ServerException $e) { + common_log(LOG_WARNING, 'Discarding broadcast_oauth for notice '.$notice->id.' because of exception: '.$e->getMessage()); + return true; + } $statustxt = format_status($notice); $params = twitter_update_params($notice); From 5b09a150bc7face85a63e0deca019fb093ed4f48 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 18 Jul 2015 19:19:16 +0200 Subject: [PATCH 36/40] Increased debugging and fixing conversation stitching for saveActivity --- classes/Notice.php | 38 ++++++++++++++++++++++++++------------ lib/threadednoticelist.php | 2 +- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/classes/Notice.php b/classes/Notice.php index 6301f9ce62..050eeaf90d 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -523,18 +523,13 @@ class Notice extends Managed_DataObject // Handle repeat case - if (isset($repeat_of)) { + if (!empty($options['repeat_of'])) { // Check for a private one - $repeat = Notice::getKV('id', $repeat_of); + $repeat = Notice::getByID($options['repeat_of']); - if (!($repeat instanceof Notice)) { - // TRANS: Client exception thrown in notice when trying to repeat a missing or deleted notice. - throw new ClientException(_('Cannot repeat; original notice is missing or deleted.')); - } - - if ($profile->id == $repeat->profile_id) { + if ($profile->sameAs($repeat->getProfile())) { // TRANS: Client error displayed when trying to repeat an own notice. throw new ClientException(_('You cannot repeat your own notice.')); } @@ -610,12 +605,13 @@ class Notice extends Managed_DataObject if (empty($notice->conversation) and !empty($options['conversation'])) { $conv = Conversation::getKV('uri', $options['conversation']); if ($conv instanceof Conversation) { - common_debug('Conversation stitched together from (probably) reply to unknown remote user. Activity creation time ('.$notice->created.') should maybe be compared to conversation creation time ('.$conv->created.').'); + common_debug('Conversation stitched together from (probably) a reply to unknown remote user. Activity creation time ('.$notice->created.') should maybe be compared to conversation creation time ('.$conv->created.').'); $notice->conversation = $conv->id; } else { // Conversation URI was not found, so we must create it. But we can't create it // until we have a Notice ID because of the database layout... - $notice->tmp_conv_uri = $options['conversation']; + // $options['conversation'] will be used later after the $notice->insert(); + common_debug('Conversation URI not found, so we have to create it after inserting this Notice: '.$options['conversation']); } } else { // If we're not using the attached conversation URI let's remove it @@ -677,6 +673,7 @@ class Notice extends Managed_DataObject if (empty($notice->conversation)) { $orig = clone($notice); // $act->context->conversation will be null if it was not provided + $conv = Conversation::create($notice, $options['conversation']); $notice->conversation = $conv->id; $notice->update($orig); @@ -853,6 +850,22 @@ class Notice extends Managed_DataObject if (is_null($scope)) { $scope = $reply->scope; } + } else { + // If we don't know the reply, we might know the conversation! + // This will happen if a known remote user replies to an + // unknown remote user - within a known conversation. + if (empty($stored->conversation) and !empty($act->context->conversation)) { + $conv = Conversation::getKV('uri', $act->context->conversation); + if ($conv instanceof Conversation) { + common_debug('Conversation stitched together from (probably) a reply activity to unknown remote user. Activity creation time ('.$stored->created.') should maybe be compared to conversation creation time ('.$conv->created.').'); + $stored->conversation = $conv->id; + } else { + // Conversation URI was not found, so we must create it. But we can't create it + // until we have a Notice ID because of the database layout... + // $options['conversation'] will be used later after the $stored->insert(); + common_debug('Conversation URI from activity context not found, so we have to create it after inserting this Notice: '.$act->context->conversation); + } + } } if ($act->context instanceof ActivityContext) { @@ -898,10 +911,11 @@ class Notice extends Managed_DataObject throw new ServerException('Unsuccessful call to StoreActivityObject '.$stored->uri . ': '.$act->asString()); } - // If it's not part of a conversation, it's - // the beginning of a new conversation. + // If it's not part of a conversation, it's the beginning + // of a new one (or belongs to a previously unknown URI). if (empty($stored->conversation)) { // $act->context->conversation will be null if it was not provided + common_debug('Creating a new conversation for stored notice ID='.$stored->getID().' with URI: '.$act->context->conversation); $conv = Conversation::create($stored, $act->context->conversation); $stored->conversation = $conv->id; } diff --git a/lib/threadednoticelist.php b/lib/threadednoticelist.php index acb9efb972..ebf0a46089 100644 --- a/lib/threadednoticelist.php +++ b/lib/threadednoticelist.php @@ -195,7 +195,7 @@ class ThreadedNoticeListItem extends NoticeListItem function showEnd() { $max = $this->initialItems(); - if (!$this->repeat) { + if (!$this->repeat instanceof Notice) { $stream = new ConversationNoticeStream($this->notice->conversation, $this->userProfile); $notice = $stream->getNotices(0, $max + 2); $notices = array(); From 7ce32619cc793b5b18ec5814f5e0551dcb1b5490 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 21 Jul 2015 02:17:34 +0200 Subject: [PATCH 37/40] Missing getTarget function in targetedrss10action.php --- lib/targetedrss10action.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/targetedrss10action.php b/lib/targetedrss10action.php index c7615bd36a..ed7e262623 100644 --- a/lib/targetedrss10action.php +++ b/lib/targetedrss10action.php @@ -39,6 +39,11 @@ class TargetedRss10Action extends Rss10Action $this->target = User::getByNickname($this->trimmed('nickname'))->getProfile(); } + public function getTarget() + { + return $this->target; + } + function getImage() { return $this->target->avatarUrl(AVATAR_PROFILE_SIZE); From 266b032b170b0f4ad3d6d2f547a31e9e5ce33000 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 21 Jul 2015 02:32:17 +0200 Subject: [PATCH 38/40] UsergroupbyidAction now extends ManagedAction --- actions/groupbyid.php | 41 +++++------------------------------------ 1 file changed, 5 insertions(+), 36 deletions(-) diff --git a/actions/groupbyid.php b/actions/groupbyid.php index 8556675155..de87ec5c67 100644 --- a/actions/groupbyid.php +++ b/actions/groupbyid.php @@ -42,53 +42,22 @@ if (!defined('GNUSOCIAL')) { exit(1); } * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @link http://status.net/ */ -class GroupbyidAction extends Action +class GroupbyidAction extends ManagedAction { /** group we're viewing. */ - var $group = null; + protected $group = null; - /** - * Is this page read-only? - * - * @return boolean true - */ function isReadOnly($args) { return true; } - function prepare($args) + protected function doPreparation() { - parent::prepare($args); - - $id = $this->arg('id'); - - if (!$id) { - // TRANS: Client error displayed referring to a group's permalink without providing a group ID. - $this->clientError(_('No ID.')); - } - - common_debug("Got ID $id"); - - $this->group = User_group::getKV('id', $id); - - if (!$this->group) { - // TRANS: Client error displayed referring to a group's permalink for a non-existing group ID. - $this->clientError(_('No such group.'), 404); - } - - return true; + $this->group = User_group::getByID($this->arg('id')); } - /** - * Handle the request - * - * Shows a profile for the group, some controls, and a list of - * group notices. - * - * @return void - */ - function handle($args) + public function showPage() { common_redirect($this->group->homeUrl(), 303); } From b4342434167f0ce0785b2de0efb58b94cf7cf9d2 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sun, 2 Aug 2015 13:39:38 +0200 Subject: [PATCH 39/40] OpenID extlib updated: Fixes CVE-2014-8150 --- extlib/Auth/OpenID/URINorm.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/extlib/Auth/OpenID/URINorm.php b/extlib/Auth/OpenID/URINorm.php index c051b550aa..32e84588db 100644 --- a/extlib/Auth/OpenID/URINorm.php +++ b/extlib/Auth/OpenID/URINorm.php @@ -93,7 +93,17 @@ function Auth_OpenID_pct_encoded_replace_unreserved($mo) function Auth_OpenID_pct_encoded_replace($mo) { - return chr(intval($mo[1], 16)); + $code = intval($mo[1], 16); + + // Prevent request splitting by ignoring newline and space characters + if($code === 0xA || $code === 0xD || $code === ord(' ')) + { + return $mo[0]; + } + else + { + return chr($code); + } } function Auth_OpenID_remove_dot_segments($path) From c77bce12e53418c2457f17ce1e34238f7baa448d Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Sat, 15 Aug 2015 11:48:39 +0200 Subject: [PATCH 40/40] Mf2 extlib update from https://github.com/indieweb/php-mf2/ --- extlib/Mf2/Parser.php | 380 +++++++++++++++++++++++++----------------- 1 file changed, 227 insertions(+), 153 deletions(-) diff --git a/extlib/Mf2/Parser.php b/extlib/Mf2/Parser.php index 27805f2324..b8a954f2c8 100644 --- a/extlib/Mf2/Parser.php +++ b/extlib/Mf2/Parser.php @@ -13,17 +13,17 @@ use stdClass; /** * Parse Microformats2 - * + * * Functional shortcut for the commonest cases of parsing microformats2 from HTML. - * + * * Example usage: - * + * * use Mf2; * $output = Mf2\parse('Barnaby Walters'); * echo json_encode($output, JSON_PRETTY_PRINT); - * + * * Produces: - * + * * { * "items": [ * { @@ -35,7 +35,7 @@ use stdClass; * ], * "rels": {} * } - * + * * @param string|DOMDocument $input The HTML string or DOMDocument object to parse * @param string $url The URL the input document was found at, for relative URL resolution * @param bool $convertClassic whether or not to convert classic microformats @@ -84,7 +84,7 @@ function fetch($url, $convertClassic = true, &$curlInfo=null) { /** * Unicode to HTML Entities * @param string $input String containing characters to convert into HTML entities - * @return string + * @return string */ function unicodeToHtmlEntities($input) { return mb_convert_encoding($input, 'HTML-ENTITIES', mb_detect_encoding($input)); @@ -92,10 +92,10 @@ function unicodeToHtmlEntities($input) { /** * Collapse Whitespace - * + * * Collapses any sequences of whitespace within a string into a single space * character. - * + * * @deprecated since v0.2.3 * @param string $str * @return string @@ -113,10 +113,10 @@ function unicodeTrim($str) { /** * Microformat Name From Class string - * - * Given the value of @class, get the relevant mf classnames (e.g. h-card, + * + * Given the value of @class, get the relevant mf classnames (e.g. h-card, * p-name). - * + * * @param string $class A space delimited list of classnames * @param string $prefix The prefix to look for * @return string|array The prefixed name of the first microfomats class found or false @@ -127,9 +127,9 @@ function mfNamesFromClass($class, $prefix='h-') { $matches = array(); foreach ($classes as $classname) { - $compare_classname = strtolower(' ' . $classname); - $compare_prefix = strtolower(' ' . $prefix); - if (stristr($compare_classname, $compare_prefix) !== false && ($compare_classname != $compare_prefix)) { + $compare_classname = ' ' . $classname; + $compare_prefix = ' ' . $prefix; + if (strstr($compare_classname, $compare_prefix) !== false && ($compare_classname != $compare_prefix)) { $matches[] = ($prefix === 'h-') ? $classname : substr($classname, strlen($prefix)); } } @@ -139,10 +139,10 @@ function mfNamesFromClass($class, $prefix='h-') { /** * Get Nested µf Property Name From Class - * - * Returns all the p-, u-, dt- or e- prefixed classnames it finds in a + * + * Returns all the p-, u-, dt- or e- prefixed classnames it finds in a * space-separated string. - * + * * @param string $class * @return array */ @@ -153,19 +153,24 @@ function nestedMfPropertyNamesFromClass($class) { $class = str_replace(array(' ', ' ', "\n"), ' ', $class); foreach (explode(' ', $class) as $classname) { foreach ($prefixes as $prefix) { - $compare_classname = strtolower(' ' . $classname); - if (stristr($compare_classname, $prefix) && ($compare_classname != $prefix)) { - $propertyNames = array_merge($propertyNames, mfNamesFromClass($classname, ltrim($prefix))); + // Check if $classname is a valid property classname for $prefix. + if (mb_substr($classname, 0, mb_strlen($prefix)) == $prefix && $classname != $prefix) { + $propertyName = mb_substr($classname, mb_strlen($prefix)); + $propertyNames[$propertyName][] = $prefix; } } } + + foreach ($propertyNames as $property => $prefixes) { + $propertyNames[$property] = array_unique($prefixes); + } return $propertyNames; } /** * Wraps mfNamesFromClass to handle an element as input (common) - * + * * @param DOMElement $e The element to get the classname for * @param string $prefix The prefix to look for * @return mixed See return value of mf2\Parser::mfNameFromClass() @@ -192,28 +197,27 @@ function convertTimeFormat($time) { $hh = $mm = $ss = ''; preg_match('/(\d{1,2}):?(\d{2})?:?(\d{2})?(a\.?m\.?|p\.?m\.?)?/i', $time, $matches); - // if no am/pm specified + // If no am/pm is specified: if (empty($matches[4])) { return $time; - } - // else am/pm specified - else { + } else { + // Otherwise, am/pm is specified. $meridiem = strtolower(str_replace('.', '', $matches[4])); - // hours + // Hours. $hh = $matches[1]; - // add 12 to the pm hours + // Add 12 to hours if pm applies. if ($meridiem == 'pm' && ($hh < 12)) { $hh += 12; } $hh = str_pad($hh, 2, '0', STR_PAD_LEFT); - // minutes + // Minutes. $mm = (empty($matches[2]) ) ? '00' : $matches[2]; - // seconds, only if supplied + // Seconds, only if supplied. if (!empty($matches[3])) { $ss = $matches[3]; } @@ -229,11 +233,11 @@ function convertTimeFormat($time) { /** * Microformats2 Parser - * + * * A class which holds state for parsing microformats2 from HTML. - * + * * Example usage: - * + * * use Mf2; * $parser = new Mf2\Parser('

Barnaby Walters

'); * $output = $parser->parse(); @@ -244,18 +248,18 @@ class Parser { /** @var DOMXPath object which can be used to query over any fragment*/ public $xpath; - + /** @var DOMDocument */ public $doc; - + /** @var SplObjectStorage */ protected $parsed; - + public $jsonMode; /** * Constructor - * + * * @param DOMDocument|string $input The data to parse. A string of HTML or a DOMDocument * @param string $url The URL of the parsed document, for relative URL resolution * @param boolean $jsonMode Whether or not to use a stdClass instance for an empty `rels` dictionary. This breaks PHP looping over rels, but allows the output to be correctly serialized as JSON. @@ -271,20 +275,20 @@ class Parser { $doc = new DOMDocument(); @$doc->loadHTML(''); } - + $this->xpath = new DOMXPath($doc); - + $baseurl = $url; foreach ($this->xpath->query('//base[@href]') as $base) { $baseElementUrl = $base->getAttribute('href'); - + if (parse_url($baseElementUrl, PHP_URL_SCHEME) === null) { /* The base element URL is relative to the document URL. * * :/ * * Perhaps the author was high? */ - + $baseurl = resolveUrl($url, $baseElementUrl); } else { $baseurl = $baseElementUrl; @@ -296,31 +300,31 @@ class Parser { foreach ($this->xpath->query('//template') as $templateEl) { $templateEl->parentNode->removeChild($templateEl); } - + $this->baseurl = $baseurl; $this->doc = $doc; $this->parsed = new SplObjectStorage(); $this->jsonMode = $jsonMode; } - + private function elementPrefixParsed(\DOMElement $e, $prefix) { if (!$this->parsed->contains($e)) $this->parsed->attach($e, array()); - + $prefixes = $this->parsed[$e]; $prefixes[] = $prefix; $this->parsed[$e] = $prefixes; } - + private function isElementParsed(\DOMElement $e, $prefix) { if (!$this->parsed->contains($e)) return false; - + $prefixes = $this->parsed[$e]; - + if (!in_array($prefix, $prefixes)) return false; - + return true; } @@ -352,72 +356,72 @@ class Parser { // TODO: figure out if this has problems with sms: and geo: URLs public function resolveUrl($url) { - // If the URL is seriously malformed it’s probably beyond the scope of this + // If the URL is seriously malformed it’s probably beyond the scope of this // parser to try to do anything with it. if (parse_url($url) === false) return $url; - + $scheme = parse_url($url, PHP_URL_SCHEME); - + if (empty($scheme) and !empty($this->baseurl)) { return resolveUrl($this->baseurl, $url); } else { return $url; } } - + // Parsing Functions - + /** - * Parse value-class/value-title on an element, joining with $separator if + * Parse value-class/value-title on an element, joining with $separator if * there are multiple. - * + * * @param \DOMElement $e * @param string $separator = '' if multiple value-title elements, join with this string * @return string|null the parsed value or null if value-class or -title aren’t in use */ public function parseValueClassTitle(\DOMElement $e, $separator = '') { $valueClassElements = $this->xpath->query('./*[contains(concat(" ", @class, " "), " value ")]', $e); - + if ($valueClassElements->length !== 0) { // Process value-class stuff $val = ''; foreach ($valueClassElements as $el) { $val .= $this->textContent($el); } - + return unicodeTrim($val); } - + $valueTitleElements = $this->xpath->query('./*[contains(concat(" ", @class, " "), " value-title ")]', $e); - + if ($valueTitleElements->length !== 0) { // Process value-title stuff $val = ''; foreach ($valueTitleElements as $el) { $val .= $el->getAttribute('title'); } - + return unicodeTrim($val); } - + // No value-title or -class in this element return null; } - + /** * Given an element with class="p-*", get it’s value - * + * * @param DOMElement $p The element to parse * @return string The plaintext value of $p, dependant on type * @todo Make this adhere to value-class */ public function parseP(\DOMElement $p) { $classTitle = $this->parseValueClassTitle($p, ' '); - + if ($classTitle !== null) return $classTitle; - + if ($p->tagName == 'img' and $p->getAttribute('alt') !== '') { $pValue = $p->getAttribute('alt'); } elseif ($p->tagName == 'area' and $p->getAttribute('alt') !== '') { @@ -429,13 +433,13 @@ class Parser { } else { $pValue = unicodeTrim($this->textContent($p)); } - + return $pValue; } /** * Given an element with class="u-*", get the value of the URL - * + * * @param DOMElement $u The element to parse * @return string The plaintext value of $u, dependant on type * @todo make this adhere to value-class @@ -443,18 +447,18 @@ class Parser { public function parseU(\DOMElement $u) { if (($u->tagName == 'a' or $u->tagName == 'area') and $u->getAttribute('href') !== null) { $uValue = $u->getAttribute('href'); - } elseif ($u->tagName == 'img' and $u->getAttribute('src') !== null) { + } elseif (in_array($u->tagName, array('img', 'audio', 'video', 'source')) and $u->getAttribute('src') !== null) { $uValue = $u->getAttribute('src'); } elseif ($u->tagName == 'object' and $u->getAttribute('data') !== null) { $uValue = $u->getAttribute('data'); } - + if (isset($uValue)) { return $this->resolveUrl($uValue); } - + $classTitle = $this->parseValueClassTitle($u); - + if ($classTitle !== null) { return $classTitle; } elseif ($u->tagName == 'abbr' and $u->getAttribute('title') !== null) { @@ -468,7 +472,7 @@ class Parser { /** * Given an element with class="dt-*", get the value of the datetime as a php date object - * + * * @param DOMElement $dt The element to parse * @param array $dates Array of dates processed so far * @return string The datetime string found @@ -477,11 +481,11 @@ class Parser { // Check for value-class pattern $valueClassChildren = $this->xpath->query('./*[contains(concat(" ", @class, " "), " value ") or contains(concat(" ", @class, " "), " value-title ")]', $dt); $dtValue = false; - + if ($valueClassChildren->length > 0) { // They’re using value-class $dateParts = array(); - + foreach ($valueClassChildren as $e) { if (strstr(' ' . $e->getAttribute('class') . ' ', ' value-title ')) { $title = $e->getAttribute('title'); @@ -591,16 +595,16 @@ class Parser { $dtValue = $dt->nodeValue; } - if ( preg_match('/(\d{4}-\d{2}-\d{2})/', $dtValue, $matches) ) { + if (preg_match('/(\d{4}-\d{2}-\d{2})/', $dtValue, $matches)) { $dates[] = $matches[0]; } } /** - * if $dtValue is only a time and there are recently parsed dates, - * form the full date-time using the most recnetly parsed dt- value + * if $dtValue is only a time and there are recently parsed dates, + * form the full date-time using the most recently parsed dt- value */ - if ( (preg_match('/^\d{1,2}:\d{1,2}(Z?[+|-]\d{2}:?\d{2})?/', $dtValue) or preg_match('/^\d{1,2}[a|p]m/', $dtValue)) && !empty($dates) ) { + if ((preg_match('/^\d{1,2}:\d{1,2}(Z?[+|-]\d{2}:?\d{2})?/', $dtValue) or preg_match('/^\d{1,2}[a|p]m/', $dtValue)) && !empty($dates)) { $dtValue = convertTimeFormat($dtValue); $dtValue = end($dates) . 'T' . unicodeTrim($dtValue, 'T'); } @@ -613,15 +617,15 @@ class Parser { * * @param DOMElement $e The element to parse * @return string $e’s innerHTML - * + * * @todo need to mark this element as e- parsed so it doesn’t get parsed as it’s parent’s e-* too */ public function parseE(\DOMElement $e) { $classTitle = $this->parseValueClassTitle($e); - + if ($classTitle !== null) return $classTitle; - + // Expand relative URLs within children of this element // TODO: as it is this is not relative to only children, make this .// and rerun tests $this->resolveChildUrls($e); @@ -630,7 +634,7 @@ class Parser { foreach ($e->childNodes as $node) { $html .= $node->C14N(); } - + return array( 'html' => $html, 'value' => unicodeTrim($this->textContent($e)) @@ -639,7 +643,7 @@ class Parser { /** * Recursively parse microformats - * + * * @param DOMElement $e The element to parse * @return array A representation of the values contained within microformat $e */ @@ -660,26 +664,39 @@ class Parser { foreach ($this->xpath->query('.//*[contains(concat(" ", @class)," h-")]', $e) as $subMF) { // Parse $result = $this->parseH($subMF); - + // If result was already parsed, skip it if (null === $result) continue; + // In most cases, the value attribute of the nested microformat should be the p- parsed value of the elemnt. + // The only times this is different is when the microformat is nested under certain prefixes, which are handled below. $result['value'] = $this->parseP($subMF); // Does this µf have any property names other than h-*? $properties = nestedMfPropertyNamesFromElement($subMF); - + if (!empty($properties)) { // Yes! It’s a nested property µf - foreach ($properties as $property) { - $return[$property][] = $result; + foreach ($properties as $property => $prefixes) { + // Note: handling microformat nesting under multiple conflicting prefixes is not currently specified by the mf2 parsing spec. + $prefixSpecificResult = $result; + if (in_array('p-', $prefixes)) { + $prefixSpecificResult['value'] = $prefixSpecificResult['properties']['name'][0]; + } elseif (in_array('e-', $prefixes)) { + $eParsedResult = $this->parseE($subMF); + $prefixSpecificResult['html'] = $eParsedResult['html']; + $prefixSpecificResult['value'] = $eParsedResult['value']; + } elseif (in_array('u-', $prefixes)) { + $prefixSpecificResult['value'] = $this->parseU($subMF); + } + $return[$property][] = $prefixSpecificResult; } } else { // No, it’s a child µf $children[] = $result; } - + // Make sure this sub-mf won’t get parsed as a µf or property // TODO: Determine if clearing this is required? $this->elementPrefixParsed($subMF, 'h'); @@ -689,19 +706,24 @@ class Parser { $this->elementPrefixParsed($subMF, 'e'); } + if($e->tagName == 'area') { + $coords = $e->getAttribute('coords'); + $shape = $e->getAttribute('shape'); + } + // Handle p-* foreach ($this->xpath->query('.//*[contains(concat(" ", @class) ," p-")]', $e) as $p) { if ($this->isElementParsed($p, 'p')) continue; $pValue = $this->parseP($p); - + // Add the value to the array for it’s p- properties foreach (mfNamesFromElement($p, 'p-') as $propName) { if (!empty($propName)) $return[$propName][] = $pValue; } - + // Make sure this sub-mf won’t get parsed as a top level mf $this->elementPrefixParsed($p, 'p'); } @@ -710,32 +732,32 @@ class Parser { foreach ($this->xpath->query('.//*[contains(concat(" ", @class)," u-")]', $e) as $u) { if ($this->isElementParsed($u, 'u')) continue; - + $uValue = $this->parseU($u); - + // Add the value to the array for it’s property types foreach (mfNamesFromElement($u, 'u-') as $propName) { $return[$propName][] = $uValue; } - + // Make sure this sub-mf won’t get parsed as a top level mf $this->elementPrefixParsed($u, 'u'); } - + // Handle dt-* foreach ($this->xpath->query('.//*[contains(concat(" ", @class), " dt-")]', $e) as $dt) { if ($this->isElementParsed($dt, 'dt')) continue; - + $dtValue = $this->parseDT($dt, $dates); - + if ($dtValue) { // Add the value to the array for dt- properties foreach (mfNamesFromElement($dt, 'dt-') as $propName) { $return[$propName][] = $dtValue; } } - + // Make sure this sub-mf won’t get parsed as a top level mf $this->elementPrefixParsed($dt, 'dt'); } @@ -762,22 +784,43 @@ class Parser { if (!array_key_exists('name', $return)) { try { // Look for img @alt - if ($e->tagName == 'img' and $e->getAttribute('alt') != '') + if (($e->tagName == 'img' or $e->tagName == 'area') and $e->getAttribute('alt') != '') throw new Exception($e->getAttribute('alt')); - + if ($e->tagName == 'abbr' and $e->hasAttribute('title')) throw new Exception($e->getAttribute('title')); - + // Look for nested img @alt foreach ($this->xpath->query('./img[count(preceding-sibling::*)+count(following-sibling::*)=0]', $e) as $em) { - if ($em->getAttribute('alt') != '') + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames) && $em->getAttribute('alt') != '') { throw new Exception($em->getAttribute('alt')); + } } + // Look for nested area @alt + foreach ($this->xpath->query('./area[count(preceding-sibling::*)+count(following-sibling::*)=0]', $e) as $em) { + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames) && $em->getAttribute('alt') != '') { + throw new Exception($em->getAttribute('alt')); + } + } + + // Look for double nested img @alt foreach ($this->xpath->query('./*[count(preceding-sibling::*)+count(following-sibling::*)=0]/img[count(preceding-sibling::*)+count(following-sibling::*)=0]', $e) as $em) { - if ($em->getAttribute('alt') != '') + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames) && $em->getAttribute('alt') != '') { throw new Exception($em->getAttribute('alt')); + } + } + + // Look for double nested img @alt + foreach ($this->xpath->query('./*[count(preceding-sibling::*)+count(following-sibling::*)=0]/area[count(preceding-sibling::*)+count(following-sibling::*)=0]', $e) as $em) { + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames) && $em->getAttribute('alt') != '') { + throw new Exception($em->getAttribute('alt')); + } } throw new Exception($e->nodeValue); @@ -812,36 +855,58 @@ class Parser { // Check for u-url if (!array_key_exists('url', $return)) { // Look for img @src - if ($e->tagName == 'a') + if ($e->tagName == 'a' or $e->tagName == 'area') $url = $e->getAttribute('href'); - - // Look for nested img @src + + // Look for nested a @href foreach ($this->xpath->query('./a[count(preceding-sibling::a)+count(following-sibling::a)=0]', $e) as $em) { - $url = $em->getAttribute('href'); - break; + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames)) { + $url = $em->getAttribute('href'); + break; + } } - + + // Look for nested area @src + foreach ($this->xpath->query('./area[count(preceding-sibling::area)+count(following-sibling::area)=0]', $e) as $em) { + $emNames = mfNamesFromElement($em, 'h-'); + if (empty($emNames)) { + $url = $em->getAttribute('href'); + break; + } + } + if (!empty($url)) $return['url'][] = $this->resolveUrl($url); } // Make sure things are in alphabetical order sort($mfTypes); - + // Phew. Return the final result. $parsed = array( 'type' => $mfTypes, 'properties' => $return ); - if (!empty($children)) + + if (!empty($shape)) { + $parsed['shape'] = $shape; + } + + if (!empty($coords)) { + $parsed['coords'] = $coords; + } + + if (!empty($children)) { $parsed['children'] = array_values(array_filter($children)); + } return $parsed; } - + /** * Parse Rels and Alternatives - * - * Returns [$rels, $alternatives]. If the $rels value is to be empty, i.e. there are no links on the page + * + * Returns [$rels, $alternatives]. If the $rels value is to be empty, i.e. there are no links on the page * with a rel value *not* containing `alternate`, then the type of $rels depends on $this->jsonMode. If set * to true, it will be a stdClass instance, optimising for JSON serialisation. Otherwise (the default case), * it will be an empty array. @@ -849,18 +914,18 @@ class Parser { public function parseRelsAndAlternates() { $rels = array(); $alternates = array(); - + // Iterate through all a, area and link elements with rel attributes foreach ($this->xpath->query('//*[@rel and @href]') as $hyperlink) { if ($hyperlink->getAttribute('rel') == '') continue; - + // Resolve the href $href = $this->resolveUrl($hyperlink->getAttribute('href')); - + // Split up the rel into space-separated values $linkRels = array_filter(explode(' ', $hyperlink->getAttribute('rel'))); - + // If alternate in rels, create alternate structure, append if (in_array('alternate', $linkRels)) { $alt = array( @@ -869,10 +934,19 @@ class Parser { ); if ($hyperlink->hasAttribute('media')) $alt['media'] = $hyperlink->getAttribute('media'); - + if ($hyperlink->hasAttribute('hreflang')) $alt['hreflang'] = $hyperlink->getAttribute('hreflang'); - + + if ($hyperlink->hasAttribute('title')) + $alt['title'] = $hyperlink->getAttribute('title'); + + if ($hyperlink->hasAttribute('type')) + $alt['type'] = $hyperlink->getAttribute('type'); + + if ($hyperlink->nodeValue) + $alt['text'] = $hyperlink->nodeValue; + $alternates[] = $alt; } else { foreach ($linkRels as $rel) { @@ -880,38 +954,38 @@ class Parser { } } } - + if (empty($rels) and $this->jsonMode) { $rels = new stdClass(); } - + return array($rels, $alternates); } - + /** * Kicks off the parsing routine - * + * * If `$htmlSafe` is set, any angle brackets in the results from non e-* properties * will be HTML-encoded, bringing all output to the same level of encoding. - * + * * If a DOMElement is set as the $context, only descendants of that element will * be parsed for microformats. - * + * * @param bool $htmlSafe whether or not to html-encode non e-* properties. Defaults to false * @param DOMElement $context optionally an element from which to parse microformats * @return array An array containing all the µfs found in the current document */ public function parse($convertClassic = true, DOMElement $context = null) { $mfs = array(); - + if ($convertClassic) { $this->convertLegacy(); } - + $mfElements = null === $context ? $this->xpath->query('//*[contains(concat(" ", @class), " h-")]') : $this->xpath->query('.//*[contains(concat(" ", @class), " h-")]', $context); - + // Parser microformats foreach ($mfElements as $node) { // For each microformat @@ -920,64 +994,64 @@ class Parser { // Add the value to the array for this property type $mfs[] = $result; } - + // Parse rels list($rels, $alternates) = $this->parseRelsAndAlternates(); - + $top = array( 'items' => array_values(array_filter($mfs)), 'rels' => $rels ); - + if (count($alternates)) $top['alternates'] = $alternates; - + return $top; } - + /** * Parse From ID - * + * * Given an ID, parse all microformats which are children of the element with * that ID. - * + * * Note that rel values are still document-wide. - * - * If an element with the ID is not found, an empty skeleton mf2 array structure + * + * If an element with the ID is not found, an empty skeleton mf2 array structure * will be returned. - * + * * @param string $id * @param bool $htmlSafe = false whether or not to HTML-encode angle brackets in non e-* properties * @return array */ public function parseFromId($id, $convertClassic=true) { $matches = $this->xpath->query("//*[@id='{$id}']"); - + if (empty($matches)) return array('items' => array(), 'rels' => array(), 'alternates' => array()); - + return $this->parse($convertClassic, $matches->item(0)); } /** * Convert Legacy Classnames - * + * * Adds microformats2 classnames into a document containing only legacy * semantic classnames. - * + * * @return Parser $this */ public function convertLegacy() { $doc = $this->doc; $xp = new DOMXPath($doc); - + // replace all roots foreach ($this->classicRootMap as $old => $new) { foreach ($xp->query('//*[contains(concat(" ", @class, " "), " ' . $old . ' ") and not(contains(concat(" ", @class, " "), " ' . $new . ' "))]') as $el) { $el->setAttribute('class', $el->getAttribute('class') . ' ' . $new); } } - + foreach ($this->classicPropertyMap as $oldRoot => $properties) { $newRoot = $this->classicRootMap[$oldRoot]; foreach ($properties as $old => $new) { @@ -986,16 +1060,16 @@ class Parser { } } } - + return $this; } - + /** * XPath Query - * + * * Runs an XPath query over the current document. Works in exactly the same * way as DOMXPath::query. - * + * * @param string $expression * @param DOMNode $context * @return DOMNodeList @@ -1003,7 +1077,7 @@ class Parser { public function query($expression, $context = null) { return $this->xpath->query($expression, $context); } - + /** * Classic Root Classname map */ @@ -1013,11 +1087,11 @@ class Parser { 'hentry' => 'h-entry', 'hrecipe' => 'h-recipe', 'hresume' => 'h-resume', - 'hevent' => 'h-event', + 'vevent' => 'h-event', 'hreview' => 'h-review', 'hproduct' => 'h-product' ); - + public $classicPropertyMap = array( 'vcard' => array( 'fn' => 'p-name', @@ -1084,7 +1158,7 @@ class Parser { 'skill' => 'p-skill', 'affiliation' => 'p-affiliation h-card', ), - 'hevent' => array( + 'vevent' => array( 'dtstart' => 'dt-start', 'dtend' => 'dt-end', 'duration' => 'dt-duration', @@ -1246,7 +1320,7 @@ function resolveUrl($baseURI, $referenceURI) { # 5.2.3 Merge Paths function mergePaths($base, $reference) { # If the base URI has a defined authority component and an empty - # path, + # path, if($base['authority'] && $base['path'] == null) { # then return a string consisting of "/" concatenated with the # reference's path; otherwise,