(ofbiz-framework) branch release24.09 updated: Improved: Anchor content role-creation permission checks to the target content (#1734) (#1737)

[as...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ashishvijaywargiya pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release24.09 by this push:
     new 9bdcd614ea Improved: Anchor content role-creation permission checks to the target content (#1734) (#1737)
9bdcd614ea is described below

commit 9bdcd614ea463f9a71d85480bf40567f5c3393c4
Author: Ashish Vijaywargiya <as...@apache.org>
AuthorDate: Wed Aug 26 16:52:01 2026 +0530

    Improved: Anchor content role-creation permission checks to the target content (#1734) (#1737)
    
    - genericContentPermission now falls back to the request's own contentId
    as the ownership anchor when a role assignment is being established with
    no owner/parent context supplied, instead of silently dropping
    roleTypeId (it was never declared in the service definition).
    
    - checkContentOperationSecurity no longer defaults to granting a
    role-assignment action when no ContentPurposeOperation rule matches,
    deferring instead to the ownership check above.
    
    - Corrected findAllAssociatedPartyIds's declared return type (List, not
    Map) to match what it actually returns, which the ownership check above
    now depends on.
    
    Thank you Krishna Uprit for the contribution
    
    (cherry picked from commit c157da7eba43441cf0b5b5f88c658128d0b7c1a9)
    
    Co-authored-by: Krishna Uprit <kr...@gmail.com>
    Co-authored-by: Krishnauprit18 <kr...@users.noreply.github.com>
---
 applications/content/servicedef/services.xml          |  1 +
 .../permission/ContentPermissionServices.groovy       | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/applications/content/servicedef/services.xml b/applications/content/servicedef/services.xml
index 4330262773..ecaa587dca 100644
--- a/applications/content/servicedef/services.xml
+++ b/applications/content/servicedef/services.xml
@@ -883,6 +883,7 @@
         <attribute name="statusId" type="String" mode="IN" optional="true"/>
         <attribute name="contentPurposeTypeId" type="String" mode="IN" optional="true"/>
         <attribute name="contentOperationId" type="String" mode="IN" optional="true"/>
+        <attribute name="roleTypeId" type="String" mode="IN" optional="true"/>
     </service>
     <service name="checkOwnership" engine="groovy" auth="true"
             location="component://content/src/main/groovy/org/apache/ofbiz/content/permission/ContentPermissionServices.groovy" invoke="checkOwnership">
diff --git a/applications/content/src/main/groovy/org/apache/ofbiz/content/permission/ContentPermissionServices.groovy b/applications/content/src/main/groovy/org/apache/ofbiz/content/permission/ContentPermissionServices.groovy
index a196c1adda..d0d710ab80 100644
--- a/applications/content/src/main/groovy/org/apache/ofbiz/content/permission/ContentPermissionServices.groovy
+++ b/applications/content/src/main/groovy/org/apache/ofbiz/content/permission/ContentPermissionServices.groovy
@@ -64,6 +64,13 @@ Map genericContentPermission() {
     // here we can use contentIdTo to check parent(s) ownership
     if (!parameters.ownerContentId && parameters.contentIdFrom) {
         ownerContentId = parameters.contentIdFrom
+    } else if (!parameters.ownerContentId && !parameters.contentIdFrom && parameters.roleTypeId && parameters.contentId) {
+        // establishing a role assignment (roleTypeId present, e.g. creating a ContentRole) with no
+        // owner/parent context supplied: anchor the ownership-based compensating check in
+        // createContentPermission/updateContentPermission against the actual target content instead
+        // of leaving it unbound, so only someone who already has standing on that content can grant
+        // a role on it.
+        ownerContentId = parameters.contentId
     }
 
     //  mainAction based call outs
@@ -412,6 +419,16 @@ Map checkContentOperationSecurity(String contentOperationId, String contentPurpo
                 }
             }
         }
+    } else if (parameters.roleTypeId) {
+        // there are no ContentPurposeOperation entries for this operation/purpose, and this call is
+        // establishing a role assignment (roleTypeId present, e.g. creating/updating a ContentRole).
+        // Granting by default here would let any caller assign themselves an arbitrary role - including
+        // OWNER or CONTENT_ADMIN - on any content whenever no explicit rule happens to be configured.
+        // Fail closed for role-assignment actions; the caller must instead satisfy the ownership-based
+        // compensating check in createContentPermission/updateContentPermission (see the ownerContentId
+        // fallback in genericContentPermission above).
+        logVerbose('No operations found for a role-assignment action; permission denied by default')
+        hasPermission = false
     } else {
         // there are no ContentPurposeOperation entries for this operation/purpose; default is approve permission
         logVerbose('No operations found; permission granted!')
@@ -527,7 +544,7 @@ Map findAllContentPurposes(String checkId) {
 /**
  * Finds all associated party Ids for a use
  */
-Map findAllAssociatedPartyIds () {
+List findAllAssociatedPartyIds () {
     Map serviceResult = run service: 'getRelatedParties', with: [partyIdFrom: userLogin.partyId,
                                                                  partyRelationshipTypeId: 'GROUP_ROLLUP',
                                                                  includeFromToSwitched: 'Y']