File download does not return an error when user is missing the download right #121603

[a-schild]

When addind attachments to events, we can get hold of the file entries via the FileRequest::forEvent($eventId)->get() method.

$eventFiles= FileRequest::forEvent($eventId)->get();

This returns the array with all files, but when i later on try to download the file via $ctFile->downloadToPath('mytempfile.txt) and the user has no rights to download file:
a) A file with that name is still created
b) Content is wrong (Access error message with HTML)
c) Api still returns OK for the download

Message in downloaded file:
Keine ausreichende Berechtigung. Das Recht 'churchservice:view' ist notwendig.

Attached is such an example of a download without enough rights
Flyer.pdf

[DumbergerL]

The issue is probably caused by this request: https://github.com/5pm-HDH/churchtools-api/blob/master/src/Models/Common/File/File.php#L62-L70

I'll take care of it.

[a-schild]

At the moment I look into the file content this way, of course has potential to break if the text changes/translated etc.
Perhaps comparing file size from metadata + dowloaded file could help

    $invalidRightsHeader= "Keine ausreichende Berechtigung"; // File starts with this content when rights are missing
    $fileContent = file_get_contents($tmpFile, false, null, 0, 64);
    if (str_starts_with($fileContent, $invalidRightsHeader)) {
        logError("Not enough rights to access file ".$tmpFile);
        // wp_delete_file($tmpFile);
        return false;
    }

[DumbergerL]

Nice. Thanks for providing a workaround for this bug.

I have reproduced the problem and in my opinion the main issue is that a user who does not have permissions for this particular file should not be able to retrieve information about the file from the api. If this were the case, the user would not be able to retrieve the file anyway.

Nevertheless, it is bad practice to return an http status 200 with this simple "Keine ausreichende Berechtigung ..." string. I have opened an issue for both on the churchtools bugtracker (incident id #121603) and will keep you updated on their response. If they do not fix it, I will consider implementing your workaround into the api wrapper code.

[a-schild]

Indeed, it looks strange that the events/{id} returns any data at all about this file