-
Notifications
You must be signed in to change notification settings - Fork 601
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
REDIRECT MANAGER - None of the OPTIONS Context Aware Configs are working (ISSUE ROOT CAUSED IN AEM CLOUD) #3284
Comments
@thcharan I can't reproduce it. ACS Commons grants read access to redirects in the repo-init script: Line 46 in 3d72570
I tried with aem-sdk-2023.12.14697.20231215T125030Z-231200 and 6.5.20 and redirects worked fine to me. |
Ok -- we did further test on a vanilla localhost publish instance (aem-sdk-quickstart-2023.12.14697.20231215T125030Z-231200) with ACS 6.4.0 pkg deployed and (no other custom code on it) --- The issue is still reproducible. Also - i checked if the ACL's of "anonymous" user acc in the above vanilla localhost with acs 6.4.0 -> They look exact same as 'what's in your screenshot'. Can you double check ?
http://localhost:4503/conf/global/settings/redirects.json I can definitely say "anonymous" being part of "everyone" group... the out of the box acl restrictions defined at the "everyone" group is conflicting with the ACL defined at the 'anonymous' user acc which is being modified by acs-repoinit --- that is completely possible. I'm very certain this is a issue. |
I'm able to reproduce the issue with vanilla aem sdk local setup(aem-sdk-quickstart-2023.12.14697.20231215T125030Z-231200) and acs 6.4.0. Also checked conf/global/settings/redirects.json value as an anonymous user and an admin. Attaching the screenshot of the same |
@thcharan @vc-architha I was able to reproduce it on my local. Thanks for spotting it out. @thcharan the statement that none of the context-aware configs are working is not quite correct . Regular redirects are working fine. The issue only impacts the functionality that reads optional configuration parameters from the context, i.e. from this tab: that's why I could not reproduce it first. The fix seems to be easy, we need to set another ACL for /conf :
I tried it on my local and with these two rules it works properly. The PR with the fix is coming. |
@YegorKozlov / @vc-architha - Thank you for checking on it. Glad to know that it's reproducible on your end. Yes Apologies - I will update the title of this "Issue" to make it clear.. i probably did not phrase that statement correctly - Yes. Any of the CA Option params directly stored on that specific /redirects node being the only problem was my intended statement. Out of curiosity -- I have this Question in my mind .... why not completely rely on user service account ? "acs-commons-manage-redirects-service" which is already being used in RedirectFilter.java .. This already gives you read permission on complete /conf node? any specific reasons not to ? was wondering -- why take the painful approach of modifying the 'anonymous' user ACL on publish..as i know it is a very sensitive user acc. and potentially can cause some conflicts with a custom code base of a specific client... IF we rely on our own ACS user service acc "acs-commons-manage-redirects-service" ... may be it provides better insulation in a customer envt where custom code won't interfere with acs-commons-manage-redirects-service. Another reason to consider the user service acc usage is that... IF we modify the "Configuration Name" and "Configuration Bucket Name"... your ACL's which assumes /settings/redirects node being the bucket won't work...that would in-turn require customer implementing their own ACL and troubleshooting cycles that it would cause and all that down the road. Any thoughts ? I am sure you may have some valid reasons not to rely user service account..would like to know what could be that.. ? |
Performance is one of the reasons. Redirect Manager was designed to work in high-load environments. Opening a service resolver on every request would add a delta , so why not to use the resolver from the request? |
@YegorKozlov 👍 Ok. Thank you for clarifying the context there. |
…ig options + readme
Fixed in 6.5.0 |
Redirect manager works well for anonymous users, but the same issue occurs and the config is unable to be accessed for logged in user. I am working on a site where a portion is behind login and am exploring using redirect manager. Once a user logs in none, of the redirects are processed. As a short term workaround, I am looking to give everyone jcr:read for redirect folder that is applied to anonymous per this fix. A better fix would seem to be to use a service user as mentioned by @YegorKozlov |
@josh-ellingsworth I was able to reproduce it in WKND. It's a different issue that the one reported in this ticket and we've always had it. We allow anonymous to read redirect rules, and my assumption was if anonymous could read redirects, then any other user would be able too. It's not correct and like you said, we need to grant read access to anonymous and any other users/groups that access the site. It's a good argument for using a service user. The fix might take some time though. |
Please don't use service users but just grant the necessary permissions to the |
@kwin thanks for the tip. we are never late to learn:) I see what's wrong with the current version of repo-init:
it should grant it to |
… everyone instead of anonymous
@YegorKozlov, @kwin - Yes, granting everyone access also fixes the issue. This is the temporary workaround I am implementing on our onPrem installation. I think this makes sense as an exception, but Adobe clearly documents that it is not recommended to change ACLs on everyone. |
This is a major issue with the current version of ACS v6.4.0.,.. but possibly many older versions as well.
Issue is reproducible on localhost publish on the latest AEM Cloud SDK.
Issue is reproducible on all AEMasCloud Service environments.
Note - It is possible that - Same issue may exist in AMS OR ON PREM provided if the Everyone group permissions is same as in the screenshot provided below from aem-cloud. ( I did not validate the permissions setup on AMS or on-prem-aem65 environments )
@YegorKozlov - So, My Team investigated this issue by debugging runtime on a AEM Cloud SDK on localhost. Below is our findings. It's a major issue.
In a nutshell, None of the "Context Aware Cfg Options" is working in AEM-Cloud. So technically this is not just impacting "Ignore Selectors" option, but impacts all the CA options. Findings below
com.adobe.acs.commons.redirects.filter.RedirectFilter.java is having this below line of code,
@reference
ConfigurationResourceResolver configResolver;
The configResolver is injected with a valid configResolver object --- no issues with that. However important thing to note here -- this configResolver is still operating with a anonymous user context ( NOT AS A SERVICE USER CONTEXT WITH ELEVATED PERMISSIONS).
when code flow control reaches the match() method,
we have this line, which works fine, returning the path of an valid context aware path applicable to that site.
String configPath = configResource.getPath();
The code fails here -- returns an empty valuemap (size=0) since the anonymous user context didn't have permissions on that node. it was not able to read any context aware properties stored on that node. "redirects".
ValueMap properties = configResource.getValueMap();
On local, when we are logged in as admin on that browser session -- the same code worked,...that's self explanatory.
Solution -
Any Read operations on the Redirect configs under /conf -- always rely on resource resolver of the SERVICE USER ACCOUNT.
Note - Anonymous is part of everyone group... the Out of the box permissions in the below screenshot should make it clear of the issue described above. We should not be modifying the ACL's of the everyone group as short fix.
The text was updated successfully, but these errors were encountered: