Repository navigation
Conversation
…es (apache#14174) updateTemplateOrIsoPermissions() only treated the caller as owner when the caller's account id matched the template owner. For project owned templates/ISOs the owner is the project account, so project admins were rejected even though they can set the same flags at registration. Treat callers that can modify the project account (project admins) as owners. Fixes: apache#14174
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes project admins being unable to change
ispublic,isextractableorisfeaturedon a template or ISO owned by their project throughupdateTemplatePermissions/updateIsoPermissions. Thanks to @prashanthr2 for the report and analysis.Root cause:
TemplateManagerImpl.updateTemplateOrIsoPermissions()only allows the change ifcaller.getId() == ownerIdor 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
Adminrole. The caller is treated as the owner only when the template is owned by a project account andProjectManager.canModifyProjectAccount(caller, ownerId)returns true. That is the helperDomainCheckeralready uses forModifyProjectaccess; for regular users it only returns true for accounts/users with theAdminrole in the project. Plain project members are still refused. The same owner flag is used for the existingisextractableowner 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
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
Added unit tests to
TemplateManagerImplTestforupdateTemplateOrIsoPermissions():Ran:
mvn -pl server test -Dtest=TemplateManagerImplTest: 42 tests (37 existing + 5 new), 0 failures. With the fix reverted,updateTemplateOrIsoPermissionsTestProjectAdminCanUpdateProjectTemplatefails with theInvalidParameterValueExceptionfrom the issue.mvn -pl server test -Dtest='com.cloud.template.*Test,com.cloud.projects.*Test'with checkstyle enabled:TemplateManagerImplTest42,HypervisorTemplateAdapterTest17,ProjectManagerImplTest6; 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.
Replaces #14428, which got closed when my fork was accidentally detached.