Skip to content

Commit 3d642f7

Browse files
rubenvdlinderubenvdlinde
authored
Merge pull request #146 from ConductionNL/hotfix/aanbodendpoint
Dont set parent organisation
2 parents 0aabda4 + a0318b9 commit 3d642f7

File tree

6 files changed

+955
-12
lines changed

6 files changed

+955
-12
lines changed

‎.phpqa.yml‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# PHPQA Configuration
2+
# This file configures the PHP Quality Analyzer tool
3+
# Run with: composer phpqa or composer qa:check
4+
5+
# Directories to analyze
6+
analyzedDirs: lib
7+
8+
# Build directory for reports
9+
buildDir: phpqa
10+
11+
# Ignore patterns
12+
ignoredDirs:
13+
- vendor
14+
- node_modules
15+
- tests
16+
- build
17+
- coverage
18+
- phpmetrics
19+
20+
ignoredFiles: []
21+
22+
# Tools configuration
23+
tools:
24+
# PHP CodeSniffer - Coding standards
25+
phpcs:
26+
standard: phpcs.xml
27+
reports:
28+
- full
29+
- summary
30+
ignoreWarnings: false
31+
32+
# PHP Mess Detector - Code quality
33+
phpmd:
34+
ruleset: phpmd.xml
35+
36+
# PHP Lines of Code - Code metrics
37+
phploc:
38+
enabled: true
39+
40+
# PHP Metrics - Complexity and maintainability
41+
phpmetrics:
42+
enabled: true
43+
config:
44+
- '--report-html=phpqa/phpmetrics'
45+
- '--report-json=phpqa/phpmetrics/metrics.json'
46+
47+
# PHP Copy/Paste Detector - Duplicate code
48+
phpcpd:
49+
enabled: true
50+
minLines: 5
51+
minTokens: 70
52+
53+
# Parallel Lint - Syntax checking
54+
parallel-lint:
55+
enabled: true
56+
exclude:
57+
- vendor
58+
- node_modules
59+
60+
# Security Checker - Check for known vulnerabilities
61+
security-checker:
62+
enabled: false # Requires composer.lock
63+
64+
# Report configuration
65+
report:
66+
# Create HTML report
67+
html: true
68+
69+
# Create CLI output
70+
cli: true
71+
72+
# File formats
73+
file:
74+
# JSON report with all data
75+
json: phpqa/phpqa.json
76+
77+
# Offline HTML report
78+
offline: phpqa/phpqa-offline.html
79+
80+
# Execution configuration
81+
execution:
82+
# Number of parallel processes
83+
parallel: 4
84+
85+
# Timeout per tool in seconds
86+
timeout: 300
87+
88+
# Thresholds - Set to 0 to report but not fail
89+
allowedErrorsCount: 0
90+
91+
# Verbose output
92+
verbose: false
93+

‎composer.json‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,28 @@
2222
"@composer bin all update --ansi"
2323
],
2424
"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './vendor-bin/*' -not -path './build/*' -print0 | xargs -0 -n1 php -l",
25-
"cs:check": "php-cs-fixer fix --dry-run --diff",
26-
"cs:fix": "php-cs-fixer fix",
25+
"cs:check": "./vendor/bin/phpcs --standard=phpcs.xml",
26+
"cs:fix": "./vendor/bin/phpcbf --standard=phpcs.xml",
27+
"phpcs": "./vendor/bin/phpcs --standard=phpcs.xml",
28+
"phpcs:fix": "./vendor/bin/phpcbf --standard=phpcs.xml",
29+
"phpmd": "phpmd lib text phpmd.xml || echo 'PHPMD not installed, skipping...'",
30+
"phpmetrics": "./vendor/bin/phpmetrics --report-html=phpmetrics lib/",
2731
"psalm": "psalm --threads=1 --no-cache",
2832
"test:unit": "phpunit tests -c tests/phpunit.xml --colors=always --fail-on-warning --fail-on-risky",
29-
"openapi": "generate-spec"
33+
"test:all": "./vendor/bin/phpunit --colors=always || echo 'Tests require Nextcloud environment, skipping...'",
34+
"openapi": "generate-spec",
35+
"grumphp": "./vendor/bin/grumphp run",
36+
"grumphp:init": "./vendor/bin/grumphp git:init",
37+
"grumphp:deinit": "./vendor/bin/grumphp git:deinit",
38+
"phpqa": "./vendor/bin/phpqa --report --analyzedDirs lib --buildDir phpqa",
39+
"phpqa:full": "./vendor/bin/phpqa --report --analyzedDirs lib --buildDir phpqa --tools phpcs:0,phpmd:0,phploc:0,phpmetrics,phpcpd:0,parallel-lint:0",
40+
"phpqa:ci": "./vendor/bin/phpqa --report --analyzedDirs lib --buildDir phpqa --tools phpcs,phpmd,phploc,phpmetrics,phpcpd,parallel-lint",
41+
"qa:check": [
42+
"@phpqa"
43+
],
44+
"qa:full": [
45+
"@phpqa:full"
46+
]
3047
},
3148
"require": {
3249
"php": "^8.1",
@@ -40,14 +57,21 @@
4057
"twig/twig": "^3.8"
4158
},
4259
"require-dev": {
60+
"edgedesign/phpqa": "^1.30",
61+
"guzzlehttp/guzzle": "^7.8",
62+
"phpmd/phpmd": "^2.15",
63+
"phpmetrics/phpmetrics": "^2.8",
64+
"phpro/grumphp": "^2.9",
4365
"phpunit/phpunit": "^10.5",
4466
"roave/security-advisories": "dev-latest",
45-
"guzzlehttp/guzzle": "^7.8"
67+
"squizlabs/php_codesniffer": "^3.9"
4668
},
4769
"config": {
4870
"allow-plugins": {
4971
"bamarni/composer-bin-plugin": true,
50-
"php-http/discovery": true
72+
"php-http/discovery": true,
73+
"phpro/grumphp": true,
74+
"dealerdirect/phpcodesniffer-composer-installer": true
5175
},
5276
"optimize-autoloader": true,
5377
"sort-packages": true,

‎grumphp.yml‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
grumphp:
2+
# Process runs mode (linux/windows)
3+
process_timeout: 300
4+
5+
# Stop on first failure
6+
stop_on_failure: true
7+
8+
# Ignore unstaged changes
9+
ignore_unstaged_changes: false
10+
11+
# Hide circumvention tip
12+
hide_circumvention_tip: false
13+
14+
# Git hooks configuration
15+
git_hook_variables:
16+
EXEC_GRUMPHP_COMMAND: 'docker exec -u 33 master-nextcloud-1 bash -c "cd /var/www/html/apps-extra/openregister && php vendor/bin/grumphp"'
17+
18+
# Environment configuration
19+
environment:
20+
files: []
21+
variables: {}
22+
paths: []
23+
24+
# Fixer configuration - automatically fix issues when possible
25+
fixer:
26+
enabled: true
27+
fix_by_default: false
28+
29+
# Tasks to run
30+
tasks:
31+
# PHP Lint - Check for syntax errors
32+
phplint:
33+
exclude: ['vendor', 'node_modules']
34+
jobs: ~
35+
short_open_tag: false
36+
ignore_patterns: []
37+
triggered_by: ['php']
38+
39+
# PHP CodeSniffer - Check coding standards
40+
phpcs:
41+
standard: phpcs.xml
42+
triggered_by: [php]
43+
whitelist_patterns: []
44+
encoding: UTF-8
45+
ignore_patterns:
46+
- vendor/
47+
- node_modules/
48+
sniffs: []
49+
severity: ~
50+
error_severity: ~
51+
warning_severity: ~
52+
tab_width: ~
53+
report: full
54+
report_width: ~
55+
56+
# PHP Mess Detector - Check for code smells
57+
phpmd:
58+
ruleset: ['phpmd.xml']
59+
triggered_by: ['php']
60+
exclude:
61+
- vendor
62+
- node_modules
63+
- tests
64+
65+
# PHPUnit - Run unit tests
66+
phpunit:
67+
config_file: phpunit.xml
68+
testsuite: ~
69+
group: []
70+
always_execute: false
71+
order: ~
72+
73+
# Composer validation
74+
composer:
75+
file: composer.json
76+
no_check_all: false
77+
no_check_lock: false
78+
no_check_publish: false
79+
with_dependencies: false
80+
strict: false
81+
82+
# YAML Lint - Check YAML files
83+
yamllint:
84+
whitelist_patterns: []
85+
ignore_patterns:
86+
- vendor/
87+
- node_modules/
88+
object_support: false
89+
exception_on_invalid_type: false
90+
parse_constant: false
91+
parse_custom_tags: false
92+
93+
# JSON Lint - Check JSON files
94+
jsonlint:
95+
detect_key_conflicts: true
96+
ignore_patterns:
97+
- vendor/
98+
- node_modules/
99+
100+
# Test suites - Group tasks together
101+
testsuites:
102+
# Quick checks for pre-commit
103+
git_commit_msg:
104+
tasks: []
105+
106+
git_pre_commit:
107+
tasks:
108+
- phplint
109+
- phpcs
110+
- jsonlint
111+
- yamllint
112+
- composer
113+
114+
# Full checks for pre-push
115+
git_pre_push:
116+
tasks:
117+
- phplint
118+
- phpcs
119+
- phpmd
120+
- phpunit
121+
- composer
122+
123+
# Extensions configuration
124+
extensions: []
125+

‎lib/BackgroundJob/CronjobContextTrait.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,3 +232,5 @@ protected function getCronjobOrganisationUuid(): ?string
232232
abstract protected function getLogger(): LoggerInterface;
233233
}
234234

235+
236+

‎lib/Service/OrganisatieService.php‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@ private function mapStatus(string $status): bool
215215
/**
216216
* Internal method to create organization entity
217217
*
218+
* HOTFIX: Parent organisation setting has been disabled due to RBAC issues.
219+
* Previously, new organisations were automatically set as children of the active organisation,
220+
* but this caused permission problems where users could not access newly created organisations.
221+
* TODO: Re-enable parent organisation setting after fixing RBAC logic.
222+
*
218223
* @param \OCA\OpenRegister\Service\OrganisationService $organisationService The organisation service
219224
* @param array $mappedData The mapped data
220225
* @param string $organizationUuid The organization UUID
@@ -227,14 +232,17 @@ private function createOrganisationEntityInternal(
227232
string $organizationUuid
228233
): \OCA\OpenRegister\Db\Organisation {
229234

230-
// Get the currently active organisation UUID to set as parent.
231-
$parentOrganisationUuid = $this->getActiveOrganisationUuid($organisationService);
235+
// HOTFIX: Commented out automatic parent organisation setting due to RBAC issues.
236+
// When child organisations are created, the parent relationship causes permission problems
237+
// where users cannot access the newly created organisations due to hierarchical RBAC filtering.
238+
// TODO: Investigate and fix RBAC logic to properly handle parent-child organisation relationships.
239+
// $parentOrganisationUuid = $this->getActiveOrganisationUuid($organisationService);
232240

233241
$this->logger->info('OrganisatieService: Creating organisation entity', [
234242
'uuid' => $organizationUuid,
235243
'name' => $mappedData['naam'],
236244
'active' => $mappedData['active'],
237-
'parentOrganisation' => $parentOrganisationUuid
245+
// 'parentOrganisation' => $parentOrganisationUuid // HOTFIX: Commented out
238246
]);
239247

240248
// Use OrganisationService to create the entity with correct parameters.
@@ -252,10 +260,14 @@ private function createOrganisationEntityInternal(
252260
$organisationEntity->setActive($mappedData['active']);
253261
$organisationEntity->setUsers([]); // Will be populated by contact person processing.
254262

255-
// Set the parent organisation to the currently active organisation.
256-
if ($parentOrganisationUuid !== null) {
257-
$organisationEntity->setParent($parentOrganisationUuid);
258-
}
263+
// HOTFIX: Commented out automatic parent organisation setting due to RBAC issues.
264+
// Setting the parent organisation causes users to lose access to newly created organisations
265+
// because the RBAC filtering expects users to belong to the parent organisation chain.
266+
// This needs to be properly resolved with RBAC logic updates.
267+
// TODO: Re-enable this once RBAC properly handles parent-child organisation relationships.
268+
// if ($parentOrganisationUuid !== null) {
269+
// $organisationEntity->setParent($parentOrganisationUuid);
270+
// }
259271

260272
// Save the updated entity.
261273
$organisationMapper = $this->container->get('OCA\OpenRegister\Db\OrganisationMapper');

0 commit comments

Comments
 (0)