HawkForge opened a new pull request, #14428: URL: https://github.com/apache/cloudstack/pull/14428
### Description This PR fixes project admins being unable to change `ispublic`, `isextractable` or `isfeatured` on a template or ISO owned by their project through `updateTemplatePermissions` / `updateIsoPermissions`. Thanks to @prashanthr2 for the report and analysis. Root cause: `TemplateManagerImpl.updateTemplateOrIsoPermissions()` only allows the change if `caller.getId() == ownerId` or the caller is an admin. For a project owned template the owner is the project account, so no user account ever matches and the call fails with "Unable to grant permission to Account X as it is neither admin nor owner or the Template", while the same flags can be set when registering the template into the project. This is an authorization change, limited to members with the project `Admin` role. The caller is treated as the owner only when the template is owned by a project account and `ProjectManager.canModifyProjectAccount(caller, ownerId)` returns true. That is the helper `DomainChecker` already uses for `ModifyProject` access; for regular users it only returns true for accounts/users with the `Admin` role in the project. Plain project members are still refused. The same owner flag is used for the existing `isextractable` owner check. Behaviour for template owners, root/domain admins and non-project templates is unchanged; the project check is only evaluated for non-admin, non-owner callers of project owned templates. Sent against 4.20 so it can be merged forward; the same code is present on 4.22 and main. Fixes: #14174 ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] build/CI - [ ] test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### How Has This Been Tested? Added unit tests to `TemplateManagerImplTest` for `updateTemplateOrIsoPermissions()`: - project admin on a project owned template succeeds (fails without the fix with the error from the issue) - project member who cannot modify the project account is still rejected - owner of a normal template succeeds, project check not consulted - admin on a project owned template succeeds, project check not consulted - non-owner, non-admin on a normal template is still rejected Ran: - `mvn -pl server test -Dtest=TemplateManagerImplTest`: 42 tests (37 existing + 5 new), 0 failures. With the fix reverted, `updateTemplateOrIsoPermissionsTestProjectAdminCanUpdateProjectTemplate` fails with the `InvalidParameterValueException` from the issue. - `mvn -pl server test -Dtest='com.cloud.template.*Test,com.cloud.projects.*Test'` with checkstyle enabled: `TemplateManagerImplTest` 42, `HypervisorTemplateAdapterTest` 17, `ProjectManagerImplTest` 6; 65 tests in total, 0 failures, checkstyle passed. The full server test suite and integration/Marvin tests were not run. #### How did you try to break this feature and the system with this change? Checked that project members without the Admin role and unrelated users are still rejected, and that the existing owner and admin paths do not call the project check. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
