[OP#51321]fix: Virus infected File is not removed by background job when integration app is enabled

[SwikritiT]

Description

We have a hook to protect the deletion of the folders inside the OpenProject groupfolder and to do so we intercept each deletion request to check if the node is inside of OpenProject groupfolder. We expect the user to have a user session but in case of background jobs doing some cleanup jobs there's no user session. In the case where the files antivirus is trying to scan the files and remove any virus infected file our app didn't allow the deletion of such files.

In this PR we check if the user is null and the user is returned null we assume that it's a background job and return without performing further operation

Related Issues:

• https://community.openproject.org/work_packages/51321
• File is not removed by background job (no user backend) files_antivirus#297

[SwikritiT]

I've checked if the user is null and if the user is null then it's a "background job " . I couldn't find a way to determine if we can know an event is triggered by a background job. @julien-nc would you have any suggestion on if I can determine that a task is done by background job?

[julien-nc]

You can't really know if you're in a background job or in an occ command. I think your check makes sense, it covers both bg job and occ command cases.

[individual-it]

That looks good to me.
@wielinde can you ask the people who reported the issue in the first place to test this fix?

[wielinde]

Hey @Rello, could you please verify that this PR solves the problem? Thank you!

[SagarGi]

LGTM 👍

[SwikritiT]

Hi, @wielinde @Rello any updates on if we can move ahead with the changes of this PR?

[Rello]

will test today...

[Rello]

@SwikritiT just tested and confirmed working 👍🏻

[wielinde]

Thank you @Rello! Much apreciated.

Now, that this is fixed, please allow me this little comment: Nextcloud could really benefit from a concept that some objects, such as files, folders, groups, users, group folders are owned by an application and can get protected from getting deleted or modified by somebody else. Then, we would not need to hook into every deletion every file, folder, group and user in order to prevent them from being deleted by some uninformed admin. And still a app could use those objects for creating cool work flow optimization such as our project folder concept.

[github-actions]

JS Code Coverage

Coverage after merging bug/51321-file-is-not-removed-by-background-job-when-integration-app-is-enabled into release/2.4 will be

87.93%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src
adminSettings.js	0%	0%	0%	0%	1, 1, 10–19, 2, 20–25, 3–9
bootstrap.js	0%	0%	0%	0%	1, 1–7
dashboard.js	0%	0%	0%	0%	1, 1, 10–19, 2, 20–25, 3–9
fileActions.js	0%	0%	0%	0%	1, 1, 10–17, 2–9
personalSettings.js	0%	0%	0%	0%	1, 1, 10–19, 2, 20–25, 3–9
projectTab.js	0%	0%	0%	0%	1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–66, 7–9
reference.js	0%	0%	0%	0%	1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60, 7–9
utils.js	63.64%	33.33%	50%	66%	10–14, 17–26, 6–9
src/components
AdminSettings.vue	99.29%	95.60%	85.19%	99.91%	1, 1, 1, 1, 1
OAuthConnectButton.vue	99.17%	87.50%	100%	100%	1
PersonalSettings.vue	98.87%	93.33%	85.71%	100%	1
src/components/admin
FieldValue.vue	97.12%	83.33%	100%	98.89%	1, 23, 23
FormHeading.vue	97.66%	75%	100%	99.36%	1, 1, 34, 34
ProjectFolderError.vue	96.83%	80%	100%	98.21%	1, 1
TextInput.vue	99.25%	95%	88.89%	100%	1
src/components/icons
ClippyIcon.vue	93.18%	50%	50%	97.50%	1, 1
OpenProjectIcon.vue	93.75%	100%	0%	96.77%	1
src/components/settings
CheckBox.vue	92.45%	80%	66.67%	97.62%	1, 1
SettingsTitle.vue	94.74%	50%	100%	97.14%	1, 1
src/components/tab
EmptyContent.vue	97.04%	78.95%	100%	99.32%	1, 1, 1, 33, 33
SearchInput.vue	99.19%	88.89%	80%	100%	1
WorkPackage.vue	98.42%	40%	100%	99.35%	1, 1, 1, 43, 43
src/utils
workpackageHelper.js	94.21%	88%	100%	95.65%	17–19, 48, 48–50
src/views
Dashboard.vue	98.97%	71.43%	100%	99.65%	1, 1, 1
ProjectsTab.vue	99.74%	93.75%	100%	100%	23
WorkPackagePickerElement.vue	0%	0%	0%	0%	1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–96
WorkPackageReferenceWidget.vue	0%	0%	0%	0%	1, 1, 10, 100–109, 11, 110–119, 12, 120–129, 13, 130–134, 14–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–99

[github-actions]

PHP Code Coverage

Coverage after merging bug/51321-file-is-not-removed-by-background-job-when-integration-app-is-enabled into release/2.4 will be

56.82%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
server/apps/integration_openproject/lib
Capabilities.php	0%	100%	0%	0%	19, 26–29
server/apps/integration_openproject/lib/AppInfo
Application.php	11.11%	100%	25%	9.38%	100, 102–104, 108–111, 113, 117–122, 133, 137, 69–71, 74, 78, 81, 85, 87, 91, 96, 98–99
server/apps/integration_openproject/lib/BackgroundJob
RemoveExpiredDirectUploadTokens.php	0%	100%	0%	0%	42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
ConfigController.php	66.40%	100%	50%	67.36%	134, 151–152, 154, 156–158, 160–161, 166–167, 169, 219, 280, 369–370, 373–374, 504–507, 509–510, 513, 521, 532, 546–548, 563–567, 569–570, 572–575, 577–579, 597–602, 604–605, 607–609, 612, 614–617, 619–621, 635, 644–647, 649–652, 662–667
DirectDownloadController.php	0%	100%	0%	0%	36–38, 53–55, 57, 60–61
DirectUploadController.php	71.43%	100%	100%	70.43%	131–132, 177, 190, 194–197, 199, 209, 216, 232–234, 236–237, 240–242, 248, 250, 255–256, 263–264, 267–268, 271–272, 288–289, 309, 314, 320
FilesController.php	77%	100%	85.71%	76.34%	209–210, 263, 269, 271–273, 275, 277, 288–290, 293–294, 296–297, 301–304, 307, 313
OpenProjectAPIController.php	82.27%	100%	73.33%	82.93%	138, 175, 192–195, 199, 203, 205–210, 212, 221–224, 227, 229, 231–234, 236–237, 242, 254, 263, 281, 290, 555, 557, 95
server/apps/integration_openproject/lib/Dashboard
OpenProjectWidget.php	0%	100%	0%	0%	101–102, 104–108, 116, 123–124, 126, 128–129, 131–132, 134, 137–138, 140–141, 143, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
OpenprojectErrorException.php	100%	100%	100%	100%
OpenprojectFileNotUploadedException.php	100%	100%	100%	100%
OpenprojectGroupfolderSetupConflictException.php	100%	100%	100%	100%
OpenprojectResponseException.php	100%	100%	100%	100%
OpenprojectUnauthorizedUserException.php	0%	100%	0%	0%	16
server/apps/integration_openproject/lib/Listener
BeforeGroupDeletedListener.php	0%	100%	0%	0%	48, 56–57, 60–63
BeforeNodeInsideOpenProjectGroupfilderChangedListener.php	0%	100%	0%	0%	41–43, 47–52, 54, 57–58, 60–65, 67, 69, 73
BeforeUserDeletedListener.php	0%	100%	0%	0%	48, 55–56, 58–61
LoadSidebarScript.php	0%	100%	0%	0%	100, 102, 104, 106–108, 110, 112, 114–115, 117–118, 120, 122, 75–81, 83–84, 86–87, 89–90, 96–97, 99
OpenProjectReferenceListener.php	0%	100%	0%	0%	53–54, 58–59, 62–63, 65, 67
UserChangedListener.php	0%	100%	0%	0%	52, 59–60, 63–68
server/apps/integration_openproject/lib/Migration
Version2001Date20221213083550.php	0%	100%	0%	0%	47, 57, 60, 63, 67, 70, 73, 77–79, 81
Version2310Date20230116153411.php	0%	100%	0%	0%	46, 49–52, 54–56, 60, 64, 68, 72, 76, 81–82, 84
Version2400Date20230504144300.php	0%	100%	0%	0%	47, 57, 60
server/apps/integration_openproject/lib/Reference
WorkPackageReferenceProvider.php	17.02%	100%	10%	18.92%	102, 109–112, 115–117, 120, 127–130, 132, 134–136, 138, 140, 142, 146, 155, 163–164, 172, 52, 59, 66, 73–74
server/apps/integration_openproject/lib/Search
OpenProjectSearchProvider.php	0%	100%	0%	0%	103–104, 107–114, 116–118, 121–122, 124–125, 129–134, 140–141, 143, 66–69, 76, 83, 91, 93, 96
OpenProjectSearchResultEntry.php	100%	100%	100%	100%
server/apps/integration_openproject/lib/Service
DatabaseService.php	43.90%	100%	60%	41.67%	125–128, 131, 80–87, 89–93, 95–97
DirectDownloadService.php	88%	100%	100%	86.96%	65–66, 68
DirectUploadService.php	54.55%	100%	66.67%	52.63%	112, 118, 79–82, 84, 89, 91
OauthService.php	0%	100%	0%	0%	108–115, 45–47, 56–66, 68, 70–72, 75–78, 89, 91–94, 96–97
OpenProjectAPIService.php	70.61%	100%	70.73%	70.60%	1000, 1006, 1011, 1015, 1025, 1027, 1030, 1032, 1163–1170, 1172, 1180–1184, 1192–1199, 1216–1223, 1232–1237, 180–184, 255, 364–365, 367, 381–382, 391, 395, 416, 504–505, 512, 515–518,

[Rello]

Thank you @Rello! Much apreciated.

Now, that this is fixed, please allow me this little comment: Nextcloud could really benefit from a concept that some objects, such as files, folders, groups, users, group folders are owned by an application and can get protected from getting deleted or modified by somebody else. Then, we would not need to hook into every deletion every file, folder, group and user in order to prevent them from being deleted by some uninformed admin. And still a app could use those objects for creating cool work flow optimization such as our project folder concept.

Hallo @wielinde
thank you for your suggestion. Problem is here, that this would result in objects that are not owned by a real user and this might create auditing challenges. With everything tied to a user, its clear who maintained or changed what content of the system. When switching to alternatives with "app user" or "service user", the whole setup will get very complex.