Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More rework to make private communities working #11253

Merged
merged 11 commits into from
Feb 18, 2022

Conversation

annando
Copy link
Collaborator

@annando annando commented Feb 16, 2022

This is another follow up to the previous PR.

It contains several different things to make private forums work. We hadn't stored the forum members when publishing a post. And also we are now working with "followers" in the group permissions and in the AP payload.

We also had a problem detecting the best user when no receiver was tagged. Also when fetching content locally we have to add ourselves to the permissions as well.

And finally some settings are now hardcoded for communities.

foreach (Tag::getByURIId($target_item['uri-id'], [Tag::EXCLUSIVE_MENTION]) as $tag) {
$target_contact = Contact::getByURL(Strings::normaliseLink($tag['url']), null, [], $uid);
if (($target_contact['contact-type'] == Contact::TYPE_COMMUNITY) && $target_contact['manually-approve']) {
Group::getMembersForForum($target_contact['id']);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this inaccurately named method? It should be named updateMembersForforum but more to the point, why do we need a made-up group of followers now that Group::FOLLOWERS exists?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I send a message to a private forum then this message will be announced to the members. The members then receive the announce and then will request the message. Since the message is not public, I need to know who is allowed to fetch the message. So I request a current list of forum subscribers before performing the transmission.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But can't this just be done with the rel field in the contact table, surely the forum node records which remote contacts are following the forum?

Copy link
Collaborator Author

@annando annando Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the forum that needs to know the forum users. It's the forum users who need to grant access to the other forum users.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's a hidden user group per user per forum?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't sound very scalable, is there no way we can go through the forum account to forward the posts submitted by members using the forum node local list of followers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all about the trust. The message flow is like this:

You have got Anna and Bob. Both are following the private community Alpha. Both aren't following each other. Now Anna creates a post. This post is transmitted to Alpha. Alpha then announces the post URL to their followers. Bob receives the announce, so he now fetches the content from Anna. At this point Anna has got to know whom she have to grant access to the post.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I get it. Can't we do this with the contact-relation table instead? It would have everything in one place, including showing the group member list to its members on their respective node.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. The contact-relation contains only public data. So when a group disallow fetching the followers then it won't return anything. And in fact this PR introduces an automatic so that group members can never be fetched by non members.

The solution in this PR has the advantage that we need it any way for the post permissions.

src/Model/Group.php Outdated Show resolved Hide resolved
src/Worker/Notifier.php Outdated Show resolved Hide resolved
@MrPetovan MrPetovan merged commit c03ff78 into friendica:develop Feb 18, 2022
@MrPetovan MrPetovan added this to the 2022.05 milestone Feb 18, 2022
@annando annando deleted the forum3 branch February 18, 2022 17:30
@MrPetovan
Copy link
Collaborator

Causing #11218 (comment)

PHP Notice: Trying to access array offset on value of type bool in /friendica/src/Model/Post/UserNotification.php on line 240

At this point all we have is a list of contact ids, I don't know how to easily get the contact.contact-type value without changing too much, this one is for you @annando

@tobiasd
Copy link
Collaborator

tobiasd commented Feb 19, 2022

That PHP notice alternates with a PHP warning of the same wording on my system.

PHP Warning: Trying to access array offset on value of type bool in /src/Model/Post/UserNotification.php on line 240

@MrPetovan
Copy link
Collaborator

Yep, same issue.

@annando
Copy link
Collaborator Author

annando commented Feb 19, 2022

Yeah, I have a fix ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants