Server: Some of your link shares have been removed

[sciurius]

Issue #15318 addresses this but was quickly closed without a satisfactory solution.

I, and several other users, frequently get messages "Some of your link shares have been removed" without an indication which shares. Personally I have a single share and it is still there so I think this message is erroneous and irritating.

[tessus]

/ref https://help.nextcloud.com/t/some-of-your-link-shares-have-been-removed-15-0-8/53522

Is there a log file that shows which links were removed? Nothing was in the nextcloud.log, but I don’t know, if the process just doesn’t log anything or if nothing has been removed (and the message was thus emitted in error).

In any case, you can’t just notify a user that something has been removed without telling anyone what it was.

[tessus]

To put it in a nutshell: Which links were removed?

[rockmn]

I have this issue on 16.0.1. We have link that we have put into manuals so it would be nice to know what the problems is. I just a upgrade from owncloud9>ownclous10>nextcloud>12.nextcloud13>nextcloud>14> nextcloud 15>nextcloud 16.

[stratege1401]

yes, put in an other prospective, 1789548254 files for 11.7 To and 85 users !!!

I am thinking taking a vacation to find out witch files, but my boss said no to my request ! :)

[garyhowell]

Hello. I was also concerned about this issue so I did some digging in the codebase.
In short, the notification is send to admins even if no shares were deleted.

To confirm, look at https://YOUR_SITE/index.php/settings/admin/logging with an admin account (see screen shot attached)

When the process starts, we get "\OC\Repair::startProgress: Starting ... Remove potentially over exposing share links (0)". Note the 0, which seems to indicate there are no dodgy shares.

then, we get
\OC\Repair::info: Repair info: Sending notifications to admins and affected users
\OC\Repair::info: Repair info: Removed potentially over exposing link shares

Looking at the code:

    private function repair(IOutput $output): void {
        $total = $this->getTotal();
        $output->startProgress($total);

        $shareCursor = $this->getShares();
        while($data = $shareCursor->fetch()) {
            $this->processShare($data);
            $output->advance();
        }
        $output->finishProgress();
        $shareCursor->closeCursor();

        // Notifiy all admins
        $adminGroup = $this->groupManager->get('admin');
        $adminUsers = $adminGroup->getUsers();
        foreach ($adminUsers as $user) {
            $this->addToNotify($user->getUID());
        }

        $output->info('Sending notifications to admins and affected users');
        $this->sendNotification();
    }

The getTotal() function seems to use the same SQL query to get the total number of dodgy shares but it is only used for reporting purposes. The repair function then continues to delete 'zero' shares and reports that it has deleted any 'potential' dodgy ones.

The following could probably resolve this poor reporting by only deleting/reporting dodgy shares if the total was more than 0, so

    private function repair(IOutput $output): void {
        $total = $this->getTotal();

        if ($total > 0) {   // WRAP THE DELETION/REPORTING PROCESS
            $output->startProgress($total);

            $shareCursor = $this->getShares();
            while($data = $shareCursor->fetch()) {
                $this->processShare($data);
                $output->advance();
            }
            $output->finishProgress();
            $shareCursor->closeCursor();

            // Notifiy all admins
            $adminGroup = $this->groupManager->get('admin');
            $adminUsers = $adminGroup->getUsers();
            foreach ($adminUsers as $user) {
                $this->addToNotify($user->getUID());
            }

            $output->info('Sending notifications to admins and affected users');
            $this->sendNotification();
        }
    }

I don't have the infrastructure to test this and make a pull request, but perhaps the original developer could do so. It might then prevent this false reporting, albeit not adding the details of the shares that are actually deleted.

[kesselb]

Thank you @garyhowell for digging :+1:

[tessus]

@garyhowell

Note the 0, which seems to indicate there are no dodgy shares.

It could also mean that the return code was 0.
It also does not tell us what the names of the removed links were. Without a dev explaining this, nobody will ever know which links were removed, but we are all still waiting to get any info from any dev.

[kesselb]

It could also mean that the return code was 0.

No ;) 0 is the number of removed shares.

[tessus]

@kesselb the number of removed shares is known when the process starts? Usually you know that info only _after_ the process had been run.
anyway, so now to the most important information of this entire issue:

Which links were removed?

[kesselb]

Which links were removed?

@rullzer :ping_pong:

the number of removed shares is known when the process starts? Usually you know that info only after the process had been run.

https://github.com/nextcloud/server/blob/7f689af6f4c8465f7525c17f5ca9d836ae6a53ae/lib/private/Repair/RemoveLinkShares.php#L101-L131

[garyhowell]

@tessus Looking at the SQL the links appear to be external 'shares' that have been shared by someone else on the 'master' share list (i.e. by someone the admin has given permission to allow a share to be shared).

On the face of it looks like the shares being deleted are essentially shares used by third parties and not the 'master' share holder. Operationally this seems reasonable to prevent third parties accessing stuff they should not.

My concern (as yours I'm sure) was that files or file links (as in symbolic links) were being deleted and this does not appear to be the case.

If someone can point me to the database documentation for the 'shares' table I might be able to work through it to determine if it's possible to come up with a simply 'list' of shares affected. However, even it is possible to do this, the performance overhead of doing so might be restrictive on a large NC installation causing the maintenance jobs to be running for a longer period.

[tessus]

@garyhowell

My concern (as yours I'm sure) was that files or file links (as in symbolic links) were being deleted and this does not appear to be the case.

Right, in either case removed links should be logged and presented to the end user. The fact that the message was emitted in error is an entirely different issue.

However, even it is possible to do this, the performance overhead of doing so might be restrictive on a large NC installation causing the maintenance jobs to be running for a longer period.

No need to create an additional statement. The statements run by this script already give you all the info you need. The info is just not logged.

Btw, the performance is already at risk by running this statement in the first place. Actually this repair step runs the statement twice. Once for counting and a second time to process the list.
Something is a bit wonky, because if 0 is returned the output should have been No need to remove link shares..
Also, I didn't see any logging in case a share is removed. I guess there's room for improvement. ;-)

[r2evans]

Nextcloud is supposed to be about personal control of information, so I'm surprised when the server "does things" that affect (destroy or make unavailable) data in a completely opaque manner.

Requests:

• The changelog for 15.0.8 does not explicitly list anything about changing links in this way. I know keeping an accurate and useful changelog needs to be balanced with readable and not-gargantuan, but this is a big missing-link.
• The advisory says to upgrade to 15.0.0. Since I only received this notification after upgrading from 15.0.7 to 15.0.8, this looks wrong. Either (1) the advisory is wrong, please correct it; (2) the notification sending me to that advisory page sent me to the wrong advisory; or (3) something completely different happened, in which case some code is really confused and/or (unintentionally) deceiving us.
• Suggest some way to determine what links were closed. I do understand that security holes should not be kept open, but I cannot start on a workaround or alternative method without knowing which of my links I need to repair.
• Is there any suggestion somewhere on what was specifically "bad" about a particular link? The advisory might be a good start, but I believe most admins are not going to be reading the geek-speak.

In the interim, I now need to do the dreaded: wait until one of my users complains. For internal links this is annoying. For customer-facing links, this looks bad.

Thanks.

[r2evans]

On more digging, the upgrade log suggests that nothing needed to be removed:

{"log":"Remove potentially over exposing share links\n","stream":"stdout","time":"2019-06-03T16:26:17.119641983Z"}
{"log":"\n","stream":"stdout","time":"2019-06-03T16:26:17.119674407Z"}
{"log":"    0/0 [\u003e---------------------------]   0% Starting ...\n","stream":"stderr","time":"2019-06-03T16:26:17.119729063Z"}

This adds one bullet to my requests/suggestions above:

• Don't prematurely alarm us (causing unnecessary hate/discontent) if nothing was done.

I still think the previous-comment bullets are valid, but at least now my dread (waiting until users complain) is reduced.

[rullzer]

The extra notifications was a bug that crept in. A fix is already in master https://github.com/nextcloud/server/pull/15738 and the backports are pending. So in the next maintenance releases this is fixed.

Further allow me to clear up a few things. The issue is fixed in 15.0.0 indeed. But we noticed that old shares could still be lying around. That is why they were removed. We could indeed have done a better job at collecting the links. But it was deemed more important to fix the integrity of the system. If a bug like this ever happens again we will take better care of that next time.

[djermann]

I updated my Nextcloud instance to 16.0.2 yesterday and still receive this notification. I did not save the upgrade log so I cannot tell if there were actually links removed but as an administrator i definitely need to know if and which links were removed, so I could go on with informing the affected users.

[kesselb]

Link shares are gone. It's not possible to restore them hence figure out which links afterwards. Okay with you to close this one?

[garyhowell]

@kesselb - it's been a while since I looked at this, but my understanding is the current release only emits a warning message if 1 or more links were deleted. In other words, nothing is emitted if no links were found to be removed. If that is the case, I have not objection to this ticket being closed.

[rockmn]

I did not get any feedback from users on a link that no longer
works. If we do get on we will make a new link.
You can close it.
On Thu, 2020-01-30 at 09:21 -0800, garyhowell wrote:

@kesselb - it's been a while since I looked at this, but my
understanding is the current release only emits a warning message if
1 or more links were deleted. In other words, nothing is emitted if
no links were found to be removed. If that is the case, I have not
objection to this ticket being closed.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "
https://github.com/nextcloud/server/issues/15643?email_source=notifications\u0026email_token=AEUCJCQTQBMLEBPFLMQIMI3RAMEBBA5CNFSM4HOHXMQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLZ4SQ#issuecomment-580361802
",
"url": "
https://github.com/nextcloud/server/issues/15643?email_source=notifications\u0026email_token=AEUCJCQTQBMLEBPFLMQIMI3RAMEBBA5CNFSM4HOHXMQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLZ4SQ#issuecomment-580361802
",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]