Refactor service users and service user mappings to use principal name and principal-based authorization

anchela commented 3 years ago

Required Information

[ ] AEM Version, including Service Packs, Cumulative Fix Packs, etc:
[ ] ACS AEM Commons Version: master branch
[ ] Reproducible on Latest? yes

Expected Behavior

service-user-mappings in acs-aem-commons as defined in ServiceUserMapperImpl.amended-acs-commons.xml still use the old format with user-id instead of one or multiple principal names (feature introduced in 2017). see

this is the source where i found the outdated mappings: https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/master/ui.apps/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl.amended-acs-commons.xml

that's how the new mapping format would look like in the feature-model (might require escaping the [] in xml) assuming that none of them is defined to be member of a group:

"org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl.amended~acs-commons":{
  "user.mapping":[
    "com.adobe.acs.acs-aem-commons-bundle:ensure-oak-index=[acs-commons-ensure-oak-index-service]",
    "com.adobe.acs.acs-aem-commons-bundle:email-service=[acs-commons-email-service]v,
    "com.adobe.acs.acs-aem-commons-bundle:httpcache-jcr-storage-service=[acs-commons-httpcache-jcr-storage-service]",
    "com.adobe.acs.acs-aem-commons-bundle:review-task-asset-mover=[acs-commons-review-task-asset-mover-service]",
    "com.adobe.acs.acs-aem-commons-bundle:error-page-handler=[acs-commons-error-page-handler-service]",
    "com.adobe.acs.acs-aem-commons-bundle:form-helper=[acs-commons-form-helper-service]",
    "com.adobe.acs.acs-aem-commons-bundle:dispatcher-flush=[acs-commons-dispatcher-flush-service]",
    "com.adobe.acs.acs-aem-commons-bundle:package-replication-status-event-listener=[acs-commons-package-replication-status-event-service]v,
    "com.adobe.acs.acs-aem-commons-bundle:component-error-handler=[acs-commons-component-error-handler-service]",
    "com.adobe.acs.acs-aem-commons-bundle:system-notifications=[acs-commons-system-notifications-service]",
    "com.adobe.acs.acs-aem-commons-bundle-twitter:twitter-updater=[acs-commons-twitter-updater-service]",
    "com.adobe.acs.acs-aem-commons-bundle:workflow-remover=[acs-commons-workflow-remover-service]",
    "com.adobe.acs.acs-aem-commons-bundle:bulk-workflow=[acs-commons-bulk-workflow-service]",
    "com.adobe.acs.acs-aem-commons-bundle:bulk-workflow-runner=[workflow-process-service]",
    "com.adobe.acs.acs-aem-commons-bundle:ensure-service-user=[acs-commons-ensure-service-user-service]",
    "com.adobe.acs.acs-aem-commons-bundle:shared-component-props=[acs-commons-shared-component-props-service]",
    "com.adobe.acs.acs-aem-commons-bundle:manage-controlled-processes=[acs-commons-manage-controlled-processes-service]",
    "com.adobe.acs.acs-aem-commons-bundle:automatic-package-replicator=[acs-commons-automatic-package-replicator-service]",
    "com.adobe.acs.acs-aem-commons-bundle:on-deploy-scripts=[acs-commons-on-deploy-scripts-service]",
    "com.adobe.acs.acs-aem-commons-bundle:remote-assets=[acs-commons-remote-assets-service]",
    "com.adobe.acs.acs-aem-commons-bundle:workflowpackagemanager-service=[acs-commons-workflowpackagemanager-service]",
    "com.adobe.acs.acs-aem-commons-bundle:file-fetch=[acs-commons-file-fetch-service]" 
  ]
}

once this is complated all service users should come with principal-based access control setup. in fact principal-based ac setup is easier to define in repo-init, because it doesn't require the corresponding path to exist (among other benefits that relate to the ability to treat service permissions as application content).

to pick just one example 'acs-commons-manage-controlled-processes-service' it would look as follows in 'repo-init' statements:

"create service user acs-commons-manage-controlled-processes-service with forced path system/cq:services/acs-commons",
"set principal ACL for acs-commons-manage-controlled-processes-service",
"allow jcr:read on /jcr_root/var/acs-commons/mcp"
"end",

Actual Behavior

see above.

Steps to Reproduce

install in AEM and check

service-user-mapping report in system-console => mappings are not listed as mapping-by-principal
verify access control setup => not principal-based ac setup defined with service users

kwin commented 3 years ago

Principal based authorization requires AEMaaCS and ACS-AEM-Commons is currently still supporting AEM 6.3 (https://adobe-consulting-services.github.io/acs-aem-commons/pages/compatibility.html). With 5.x there is the plan to support AEM 6.4 (shipping with Jackrabbit 2.16). This features requires Jackrabbit 2.18 and the bundle oak-authorization-principalbased (https://jackrabbit.apache.org/oak/docs/security/authorization/principalbased.html#general) which doesn't ship with any AEM on prem versions (including the most recent 6.5.15).

@anchela Is this just an optimization or is it prerequisite for supporting AEMaaCS?

anchela commented 3 years ago

@kwin is a prerequisite for AEMaaCS. today the mappings defined in acs-aem-commons log warnings but in the future mappings with the old format (i.e. 'service:subservice=userid' instead of 'service:subservice=[one,or,many,principalnames]') and service users that don't come with principal-based ac setup will no longer be valid in AEMaaCS. so, this is was intended to serve as an early heads up. hope it helps.

anchela commented 3 years ago

@kwin , btw.... since you link it to the neetcentric/accsscontroltool: that one also defines service user mappings with the old format. i will create a ticket there as well.

davidjgonzalez commented 3 years ago

This is partially (the ServiceUserMapperImpl) resolved in the 5.0.0 release.

The principal-based ACLs will be rolled into the next breaking change release 6.0.0.

Thanks for the ticket!

kwin commented 3 years ago

AEMaaCS supports principal based authorization by default only for all users below /home/users/system/cq:services so we would need to move the service users for this ticket.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

anchela commented 2 years ago

@kwin , @davidjgonzalez , IMHO this issue should be fixed. i am not sure though i have enough karma to remove the 'wontfix' label. would you be able to remove it?

anchela commented 2 years ago

oh.... magically removed from the bot..... that was easy. so, @kwin , @davidjgonzalez no action required from your side :)

kwin commented 2 years ago

@anchela Your comment removed it automatically. However, I added also the "pinned" label which makes the issue persistent.

kwin commented 2 years ago

@anchela I don't see the bundle oak-authorization-principalbased shipped with AEM 6.5.13. Is there no plan to support this ever in AEM 6.5.x? In this case libraries like ACS AEM Commons will not be able to switch anytime soon.

anchela commented 2 years ago

hi @kwin , you would need to talk to adobe product management. from my point of view there is no reason not to support it

kwin commented 1 year ago

If we want to support this in ACS AEM Commons this requires a dedicated package for cloud only (similar to the approach in https://github.com/Netcentric/accesscontroltool/blob/f8f0c792711ec07b53aaf6a0c787f59fcb18d8ec/accesscontroltool-package/pom.xml#L80-L102)

anchela commented 1 year ago

@kwin thanks for the update. if having a dedicated package is feasible, it would IMHO make sense. also, because acs aem commons is widely used and i believe it should reflect best practices as used in AEMaaCS out of the box.

kwin commented 1 year ago

@anchela Maybe you can internally reach out to product management to get this also supported in AEM 6.5.x.

anchela commented 1 year ago

@kwin , will do.

royteeuwen commented 1 year ago

+1 on the principal based support on AEM on-prem, it complicates the things a lot for frameworks wanting to support both

davidjgonzalez commented 1 year ago

Did we ever figure out what version of AEM 6.5 support this?

ACS Commons 6.0.0 is getting close - would be a great time to make the jump. (ex. if 6.5.10+ supports it, then we can update ACS Commons 6.0.0 to require 6.5.10+)

/cc @anchela

kwin commented 1 year ago

The necessary bundle is not even part of most recent 6.5.16.

davidjgonzalez commented 1 year ago

FWIW users/perms are defined in the 6.0.0 release branch are now all repo init, but they arent principal based.

https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/3054

anchela commented 1 year ago

hi @davidjgonzalez which is the very core of this ticket, no? it would be really good if acs services would reflect best practices i.e. use principal-based authorization for service users.

kwin commented 1 year ago

@anchela Any update on principal support for AEM 6.5.x? Maintaining differently for Cloud and On-Prem is not really manageable…

anchela commented 1 year ago

@kwin , nope.... but i mentioned it in the general discussion about whether to update AEM 6.5.x to the latest oak.

davidjgonzalez commented 1 year ago

@anchela - as @kwin notes - this project supports 6.5.x and AEM CS - we dont have the capacity to support both (nor i'd argue is it a good to try to manage both; its a recipe for confusion).

Can you provide us counsel on the best way to have principal-based authorization added to AEM 6.5? Is this something you can take a lead on? Im not sure what you mean by "general discussion" - or what weight that carries. Thanks!

anchela commented 1 year ago

@davidjgonzalez , the general discussion covered

supporting java 17 (and the fact that java.security.acl.Group has been deprecated in java 9 and was removed in in java 14)
updating guava to a new version that has vulnerabilities fixed
and this topic here which i referenced in the internal jira and sent a heads up to prod mgt

as much as i wanted i can't take the lead to driving this because i am not in the position of making the decisions needed. i am developer not prod mgt. all i can do is trying to explain the cost of not supporting (and enforcing) principal-based authorization for service users.

as you and @kwin may be aware of i have been working on the sling cp-converter to make sure we can automatically convert access control setup for service users in AEMaaCS (with 2 different configuration options) as well as the ability to roll back the conversion.

since you an Adobe employee as well please share your point with prod mgt as well.

kwin commented 2 weeks ago

@anchela / @davidjgonzalez Any update on principal authentication support in AEM 6.5?

Adobe-Consulting-Services / acs-aem-commons