Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
454 stars 601 forks source link

Redundant jcr:read permissions on /conf #3376

Open kwin opened 4 months ago

kwin commented 4 months ago

The repoinit script from https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/master/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config grants jcr:read in /conf to several system users. That is redundant as AEM 6.5 and AEMaaCS ship with the following default permissions for everyone:

allow jcr:read on /conf with restrictions: [rep:subtrees: '/global/site-templates/,/settings/wcm/,/sling:configs/,/settings/dam/cfm/models/,/settings/graphql/persistentQueries' ]

kwin commented 4 months ago

@YegorKozlov Any idea why we still ran into #3284? Was the aforementioned access control entry not enough (even the service user should inherit from everyone)...

Update: Nevermind, found that redirects are stored below settings/redirects which is not covered by any of the rep:subtrees from above!.

davidjgonzalez commented 4 months ago

@kwin I assume this is the same for marketo? Can we close this?

kwin commented 4 months ago

@davidjgonzalez Sorry, I am not following. Which marketo path/config are you referring to? Why should this justify closing this ticket?

davidjgonzalez commented 4 months ago

@kwin Wrt to the redirects (#3284) based on this thread, it sounds like the current ACLs are necessary due to the OOTB with restrictions (ie we cant remove them), or am i missing something?

kwin commented 4 months ago

I am not familiar with the service users, but as long as they only use content below the allowed subtrees there is no need for it. Potential candidates are

  1. https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/70679422ea50ba281a62abccdf77c1b0690f7b5c/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config#L34
  2. https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/70679422ea50ba281a62abccdf77c1b0690f7b5c/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config#L40
YegorKozlov commented 4 months ago

@kwin

the only ACL block need is

# web requests need read access to redirect configurations, e.g. /conf/global/settings/redirects
set ACL for everyone
    allow jcr:read on /conf  restriction(rep:glob,/*/settings/redirects)
    allow jcr:read on /conf  restriction(rep:glob,/*/settings/redirects/*)
end

this one is obsolete, but let me confirm first.


create service user acs-commons-manage-redirects-service with path system/acs-commons
set ACL for acs-commons-manage-redirects-service
    allow jcr:read on /
    allow jcr:read on /conf
end```
YegorKozlov commented 4 months ago

@kwin I'm going to remove this one

create service user acs-commons-manage-redirects-service with path system/acs-commons
set ACL for acs-commons-manage-redirects-service
    allow jcr:read on /
    allow jcr:read on /conf
end

and refactor Redirect Manager to not require service users at all. With redirects readable to everyone we can access them using request's resolver. The service user and the reference to @ResourceResolverFactory will go away.