You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by as...@apache.org on 2026/08/26 10:40:33 UTC

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

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

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


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

commit c157da7eba43441cf0b5b5f88c658128d0b7c1a9
Author: Krishna Uprit <kr...@gmail.com>
AuthorDate: Wed Aug 26 16:10:27 2026 +0530

    Improved: Anchor content role-creation permission checks to the target content (#1734)
    
    - 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
    
    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']