Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redundant jcr:read permissions on /conf #3376

Open
kwin opened this issue Jul 4, 2024 · 7 comments
Open

Redundant jcr:read permissions on /conf #3376

kwin opened this issue Jul 4, 2024 · 7 comments

Comments

@kwin
Copy link
Contributor

kwin commented Jul 4, 2024

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
Copy link
Contributor Author

kwin commented Jul 4, 2024

@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
Copy link
Contributor

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

@kwin
Copy link
Contributor Author

kwin commented Jul 8, 2024

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

@davidjgonzalez
Copy link
Contributor

@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
Copy link
Contributor Author

kwin commented Jul 8, 2024

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

@YegorKozlov
Copy link
Contributor

YegorKozlov commented Jul 8, 2024

@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
Copy link
Contributor

@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.

YegorKozlov added a commit to YegorKozlov/acs-aem-commons that referenced this issue Jul 15, 2024
YegorKozlov added a commit to YegorKozlov/acs-aem-commons that referenced this issue Jul 15, 2024
YegorKozlov added a commit to YegorKozlov/acs-aem-commons that referenced this issue Jul 16, 2024
davidjgonzalez pushed a commit that referenced this issue Aug 14, 2024
* #3376 Redirect Manager: refactor code to not require service user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants