Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

fix: M1-M7 and L1-L4 medium/low quality issues #357

Merged
rubenvdlinde merged 1 commit into from
May 27, 2026
Merged

fix: M1-M7 and L1-L4 medium/low quality issues #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .distignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Development and debug scripts — excluded from release packages
check_*.php
debug_*.php
cleanup_*.php
enhance_*.php
find_*.php
compare_*.php
test-*.php
test-*.sh
phpstan-bootstrap.php

# Test infrastructure
tests/
postman/
reacties/
issues/
issues.md
*.md
!README.md
!CHANGELOG.md
!LICENSE

# Build/dev artifacts
node_modules/
src/
webpack.config.js
package.json
package-lock.json
tsconfig.json
jest.config.js
eslint.config.js
stylelint.config.js
grumphp.yml
phpcs.xml
phpmd.xml
phpstan.neon
psalm.xml
phpunit.xml
phpcs-custom-sniffs/
coverage/
coverage-frontend/
.last-update
76 changes: 51 additions & 25 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,38 @@ public function getConfigurationService(): ?ConfigurationService

}//end getConfigurationService()

/**
* Return request params with known credential fields redacted.
*
* Prevents SMTP passwords, API keys and similar secrets from appearing
* in application logs when an error occurs during a settings-update request.
*
* @return array<string,mixed> Sanitised copy of the request parameters.
*/
private function getRedactedParams(): array
{
$sensitiveKeys = [
'smtpPassword',
'sendgridApiKey',
'mailgunApiKey',
'postmarkApiKey',
'sesSecretKey',
'mailjetSecretKey',
'apiKey',
'password',
'secret',
];

$params = $this->request->getParams();
foreach ($sensitiveKeys as $key) {
if (isset($params[$key]) === true) {
$params[$key] = '***';
}
}

return $params;
}//end getRedactedParams()

/**
* Retrieve the current settings.
*
Expand Down Expand Up @@ -282,7 +314,7 @@ function ($key) {
'Failed to update settings',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(['error' => $e->getMessage()], 500);
Expand Down Expand Up @@ -361,7 +393,7 @@ public function updateGeneralConfig(): JSONResponse
'Failed to update general config',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -445,7 +477,7 @@ public function updateSyncConfig(): JSONResponse
'Failed to update sync config',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -647,11 +679,10 @@ public function stats(): JSONResponse
}//end stats()

/**
* Get debug information for settings
* Get debug information for settings (admin-only)
*
* @return JSONResponse JSON response containing debug information
*
* @NoAdminRequired
* @NoCSRFRequired
*
* @spec openspec/changes/retrofit-2026-05-24-method-decomposition/tasks.md#task-4
Expand All @@ -662,6 +693,10 @@ public function debug(): JSONResponse
return new JSONResponse(['message' => 'Not authenticated'], Http::STATUS_UNAUTHORIZED);
}

if ($this->groupManager->isAdmin($this->userSession->getUser()->getUID()) === false) {
return new JSONResponse(['message' => 'Admin privileges required'], Http::STATUS_FORBIDDEN);
}

try {
$debugInfo = $this->settingsService->getDebugInfo();
return new JSONResponse($debugInfo);
Expand Down Expand Up @@ -709,7 +744,7 @@ public function sendTestEmail(): JSONResponse
[
'exception_class' => get_class($e),
'exception_message' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -780,10 +815,7 @@ public function performSync(int $minutesBack=0): JSONResponse
// For incremental sync, use the original method.
$result = $this->orgSyncSvc->performManualSync($minutesBack);

if ($result['success'] === true) {
}

return new JSONResponse($result, 500);
return new JSONResponse($result, $result['success'] === true ? 200 : 500);
} catch (\Exception $e) {
$this->logger->error(
'Manual sync failed',
Expand Down Expand Up @@ -926,10 +958,7 @@ public function resetAutoConfig(): JSONResponse

$result = $this->settingsService->resetAutoConfiguration($resetConfiguration);

if ($result['success'] === true) {
}

return new JSONResponse($result, 400);
return new JSONResponse($result, $result['success'] === true ? 200 : 400);
} catch (\Exception $e) {
return new JSONResponse(
[
Expand Down Expand Up @@ -1017,10 +1046,7 @@ public function manualImport(): JSONResponse
// Add timestamp for cache busting.
$result['timestamp'] = time();

if ($result['success'] === true) {
}

return new JSONResponse($result, 400);
return new JSONResponse($result, $result['success'] === true ? 200 : 400);
} catch (\Exception $e) {
$this->logger->error(
'SettingsController: Manual import failed',
Expand Down Expand Up @@ -1894,7 +1920,7 @@ public function testEmailConnection(): JSONResponse
[
'exception_class' => get_class($e),
'exception_message' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -2002,7 +2028,7 @@ public function updateEmailSettings(): JSONResponse
'Failed to update email settings',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -2156,7 +2182,7 @@ public function updateEmailTemplate(string $templateName): JSONResponse
"Failed to update email template {$templateName}",
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -2340,7 +2366,7 @@ public function setGenericUserGroups(): JSONResponse
'Failed to set generic user groups',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -2432,7 +2458,7 @@ public function setOrganizationAdminGroups(): JSONResponse
'Failed to set organization admin groups',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -2524,7 +2550,7 @@ public function setSuperUserGroups(): JSONResponse
'Failed to set super user groups',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down Expand Up @@ -3565,7 +3591,7 @@ public function updateCronjobConfig(): JSONResponse
'Failed to update cronjob config',
[
'exception' => $e->getMessage(),
'requestData' => $this->request->getParams(),
'requestData' => $this->getRedactedParams(),
]
);
return new JSONResponse(
Expand Down
45 changes: 38 additions & 7 deletions lib/Service/OrganizationSyncService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,20 @@ public function performOrganizationsSync(int $batchSize=50, int $maxExecutionSec
return $stats;
}

// Cast config values to integers before using in a table name to prevent injection.
$registerIdOrg = (int) $register;
$schemaIdOrg = (int) $organizationSchema;
if ($registerIdOrg <= 0 || $schemaIdOrg <= 0) {
$this->logger->warning('OrganizationSync: register or organisatie_schema is not a valid positive integer', [
'register' => $register,
'organizationSchema' => $organizationSchema,
]);
return $stats;
}

// Build table name dynamically from config (environment-agnostic).
// Objects live in per-schema MagicMapper tables, NOT in openregister_objects.
$magicTableName = 'openregister_table_'.$register.'_'.$organizationSchema;
$magicTableName = 'openregister_table_'.$registerIdOrg.'_'.$schemaIdOrg;

// Count total remaining orgs without entities for progress logging.
$countQb = $this->db->getQueryBuilder();
Expand Down Expand Up @@ -382,8 +393,19 @@ public function performContactSync(int $batchSize=100, int $maxExecutionSeconds=
return $stats;
}

// Cast config values to integers before using in a table name to prevent injection.
$registerIdContact = (int) $register;
$schemaIdContact = (int) $contactSchema;
if ($registerIdContact <= 0 || $schemaIdContact <= 0) {
$this->logger->warning('ContactSync: register or contactpersoon_schema is not a valid positive integer', [
'register' => $register,
'contactSchema' => $contactSchema,
]);
return $stats;
}

// Query per-schema magic table directly (NOT the empty openregister_objects blob table).
$contactTableName = 'openregister_table_'.$register.'_'.$contactSchema;
$contactTableName = 'openregister_table_'.$registerIdContact.'_'.$schemaIdContact;

// Find contacts without a username that DO have a matching Nextcloud account (by email).
$qb = $this->db->getQueryBuilder();
Expand Down Expand Up @@ -486,8 +508,19 @@ public function performUserSync(): array
return [];
}

// Cast to int and validate before using in a table name to prevent injection.
$registerId = (int) $register;
$schemaId = (int) $contactSchema;
if ($registerId <= 0 || $schemaId <= 0) {
$this->logger->warning('UserSync: register or contactpersoon_schema is not a valid positive integer', [
'register' => $register,
'contactSchema' => $contactSchema,
]);
return [];
}

// Query per-schema magic table directly (NOT the empty openregister_objects blob table).
$contactTableName = 'openregister_table_'.$register.'_'.$contactSchema;
$contactTableName = 'openregister_table_'.$registerId.'_'.$schemaId;

// Build JSON contains check - platform-specific.
$platform = $this->db->getDatabasePlatform();
Expand Down Expand Up @@ -2784,10 +2817,8 @@ public function performOptimizedManualSync(int $maxRounds=10, int $batchSize=100

$allResults['totalRounds'] = $round;

// Small pause between rounds to prevent resource exhaustion.
if ($round < $maxRounds) {
sleep(1);
}
// No sleep between rounds — this runs in an HTTP worker;
// blocking the worker for up to 10 s degrades concurrency.
}//end for

// Final user sync.
Expand Down
47 changes: 10 additions & 37 deletions lib/Service/SoftwareCatalogue/ContactPersonHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,11 @@ public function createUserAccount(object $contactpersoonObject, bool $isFirstCon
]
);

$randomPw = $this->_secureRandom->generate(length: 12);
// Build a password that satisfies NC default policy (≥10 chars, upper+lower+digit+special).
$randomPw = $this->_secureRandom->generate(length: 4, characters: ISecureRandom::CHAR_UPPER)
. $this->_secureRandom->generate(length: 4, characters: ISecureRandom::CHAR_LOWER)
. $this->_secureRandom->generate(length: 2, characters: ISecureRandom::CHAR_DIGITS)
. $this->_secureRandom->generate(length: 2, characters: '!@#$%^&*()-_=+[]');
$user = $this->_userManager->createUser(uid: $username, password: $randomPw);

if (empty($user) === false) {
Expand All @@ -478,42 +482,11 @@ public function createUserAccount(object $contactpersoonObject, bool $isFirstCon
]
);

// Fire-and-forget filesystem pre-warm in a background process.
// This replicates Session::prepareUserLogin() (setupFS, copySkeleton,.
// updateLastLoginTimestamp) so the user's first login is instant.
// Uses exec('... &') for true async — returns immediately, the forked.
// process does the ~5s work without blocking the admin's request.
try {
$phpBin = PHP_BINARY;
if (empty($phpBin) === true) {
$phpBin = 'php';
}

$serverRoot = \OC::$SERVERROOT;
$safeUser = escapeshellarg($username);
$exportedUser = var_export($username, true);
$requirePart = 'require "'.$serverRoot.'/lib/base.php";';
$setupPart = ' \OC_Util::setupFS('.$exportedUser.');';
$folderPart = ' $f = \OC::$server->getUserFolder('.$exportedUser.');';
$skelPart = ' \OC_Util::copySkeleton('.$exportedUser.', $f);';
// phpcs:ignore Generic.Files.LineLength.TooLong
$loginPart = ' \OC::$server->get(\OCP\IUserManager::class)->get('.$exportedUser.')->updateLastLoginTimestamp();';
$script = $requirePart.$setupPart.$folderPart.$skelPart.$loginPart;
$cmd = sprintf(
'%s -r %s > /dev/null 2>&1 &',
escapeshellarg($phpBin),
escapeshellarg($script)
);
exec($cmd);
} catch (\Exception $e) {
$this->_logger->warning(
'Filesystem pre-warm exec failed for '.$username,
[
'app' => 'softwarecatalog',
'error' => $e->getMessage(),
]
);
}//end try
// Note: filesystem pre-warming via exec() has been removed.
// The exec() spawned a raw PHP process that used \OC::$server (fatal on NC 34)
// and created a fork-bomb risk when user creation is triggered from an
// unauthenticated path. NC performs setupFS/copySkeleton automatically on
// first login without any pre-warming.

// Set user details.
$this->_logger->info(
Expand Down
Loading