From b5e195b41526b12b292e0814a955ee5d0a8708ed Mon Sep 17 00:00:00 2001 From: Dean Townsley Date: Sat, 22 Jun 2019 12:24:30 -0500 Subject: [PATCH 1/4] Add auth to load sequence for photos This allows private photos to load on any page. Previously auth depended on some other thing like the enclosing page triggering the authentication of the specific contact for the photo owner. --- src/Model/Photo.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 68665126fb..7df96fccdb 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -16,6 +16,7 @@ use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\Model\Storage\IStorage; use Friendica\Object\Image; +use Friendica\Protocol\DFRN; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; use Friendica\Util\Security; @@ -133,8 +134,16 @@ class Photo extends BaseObject if ($r === false) { return false; } + $uid = $r["uid"]; - $sql_acl = Security::getPermissionsSQLByUserId($r["uid"]); + // This is the first place, when retrieving just a photo, that we know who owns the photo. + // Make sure that the requester's session is appropriately authenticated to that user + // otherwise permissions checks done by getPermissionsSQLByUserId() won't work correctly + $r = DBA::selectFirst("user", ["nickname"], ["uid" => $uid], []); + // this will either just return (if auth all ok) or will redirect and exit (starting over) + DFRN::autoRedir(self::getApp(), $r["nickname"]); + + $sql_acl = Security::getPermissionsSQLByUserId($uid); $conditions = [ "`resource-id` = ? AND `scale` <= ? " . $sql_acl, From 042fcfeb508cde2647fedee53c28f4037255c5ed Mon Sep 17 00:00:00 2001 From: Dean Townsley Date: Sat, 22 Jun 2019 12:34:54 -0500 Subject: [PATCH 2/4] Enable multi-auth in dfrn autoRedir Update checks to account for a user being authenticated to multiple contacts on the local server at the same time. It was also necessary to remove a looping procection to make this work correcly with browsers that open multiple connections because the information about what contacts are authenticated is stored in the PHP session. --- src/Protocol/DFRN.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index ec4557e822..0e2bfc579b 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2899,7 +2899,12 @@ class DFRN { // prevent looping if (!empty($_REQUEST['redir'])) { - return; + Logger::log('autoRedir might be looping because is redir', Logger::DEBUG); + // looping prevention also appears to sometimes prevent authentication for images + // because browser may have multiple connections open and load an image on a connection + // whose session wasn't updated when a previous redirect authenticated + // Leaving commented in case looping reappears + //return; } if ((! $contact_nick) || ($contact_nick === $a->user['nickname'])) { @@ -2923,6 +2928,9 @@ class DFRN $baseurl = substr($baseurl, $domain_st + 3); $nurl = Strings::normaliseLink($baseurl); + $r = DBA::selectFirst("user", ["uid"], ["nickname" => DBA::escape($contact_nick)], []); + $contact_uid = $r["uid"]; + /// @todo Why is there a query for "url" *and* "nurl"? Especially this normalising is strange. $r = q("SELECT `id` FROM `contact` WHERE `uid` = (SELECT `uid` FROM `user` WHERE `nickname` = '%s' LIMIT 1) AND `nick` = '%s' AND NOT `self` AND (`url` LIKE '%%%s%%' OR `nurl` LIKE '%%%s%%') AND NOT `blocked` AND NOT `pending` LIMIT 1", @@ -2931,9 +2939,18 @@ class DFRN DBA::escape($baseurl), DBA::escape($nurl) ); - if ((! DBA::isResult($r)) || $r[0]['id'] == remote_user()) { + if ((! DBA::isResult($r))) { return; } + // test if redirect authentication already succeeded + // Note that "contact" in the sense used in $contact_nick and the sense in the $remote[]["cid"] + // in the session are opposite. In the session variable the user currently fetching is the contact + // while $contact_nick is the nick of tho user who owns the stuff being fetched. + foreach (\Friendica\Core\Session::get('remote', []) as $visitor) { + if ($visitor['uid'] == $contact_uid && $visitor['cid'] == $r[0]['id']) { + return; + } + } $r = q("SELECT * FROM contact WHERE nick = '%s' AND network = '%s' AND uid = %d AND url LIKE '%%%s%%' LIMIT 1", From 9dff3d2b6d27a3917b7ca998c5d33c7b7cbaa84a Mon Sep 17 00:00:00 2001 From: Dean Townsley Date: Sat, 22 Jun 2019 18:56:33 -0500 Subject: [PATCH 3/4] Use User:: API insteadd of direct database read --- src/Protocol/DFRN.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index 0e2bfc579b..e47cfc72c0 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2928,7 +2928,7 @@ class DFRN $baseurl = substr($baseurl, $domain_st + 3); $nurl = Strings::normaliseLink($baseurl); - $r = DBA::selectFirst("user", ["uid"], ["nickname" => DBA::escape($contact_nick)], []); + $r = User::getByNickname($contact_nick, ["uid"]); $contact_uid = $r["uid"]; /// @todo Why is there a query for "url" *and* "nurl"? Especially this normalising is strange. From 0c6a0942cc220128b448b922aba2a5777b47a360 Mon Sep 17 00:00:00 2001 From: Dean Townsley Date: Sat, 22 Jun 2019 19:08:34 -0500 Subject: [PATCH 4/4] Clarify comment and log message --- src/Protocol/DFRN.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index e47cfc72c0..91ca2545db 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2899,7 +2899,7 @@ class DFRN { // prevent looping if (!empty($_REQUEST['redir'])) { - Logger::log('autoRedir might be looping because is redir', Logger::DEBUG); + Logger::log('autoRedir might be looping because redirect has been redirected', Logger::DEBUG); // looping prevention also appears to sometimes prevent authentication for images // because browser may have multiple connections open and load an image on a connection // whose session wasn't updated when a previous redirect authenticated @@ -2943,8 +2943,9 @@ class DFRN return; } // test if redirect authentication already succeeded - // Note that "contact" in the sense used in $contact_nick and the sense in the $remote[]["cid"] - // in the session are opposite. In the session variable the user currently fetching is the contact + // Note that "contact" in the sense used in the $contact_nick argument to this function + // and the sense in the $remote[]["cid"] in the session are opposite. + // In the session variable the user currently fetching is the contact // while $contact_nick is the nick of tho user who owns the stuff being fetched. foreach (\Friendica\Core\Session::get('remote', []) as $visitor) { if ($visitor['uid'] == $contact_uid && $visitor['cid'] == $r[0]['id']) {