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

Login does not work for certain paths #887

Open
golinski opened this issue Apr 25, 2021 · 8 comments
Open

Login does not work for certain paths #887

golinski opened this issue Apr 25, 2021 · 8 comments
Assignees

Comments

@golinski
Copy link
Collaborator

golinski commented Apr 25, 2021

When in private mode one goes to etc. http://localhost:8080/system/console/bundles, we are presented with the normal login form at http://localhost:8080/system/sling/form/login?resource=%2Fsystem%2Fconsole%2Fbundles, but after entering admin/admin we are redirected to http://localhost:8080/system/sling/form/login?resource=%2Fsystem%2Fconsole%2Fbundles%2Fj_security_check. Each subsequent login attempt just adds %2Fj_security_check.

@reggie7
Copy link
Contributor

reggie7 commented Apr 26, 2021

@golinski please check if this covers #33 too. If so - let's close that one too. #721 is the original for it.

@golinski
Copy link
Collaborator Author

golinski commented Apr 26, 2021

No, peregrine-cms/enhancements#33 works this way because /content is anonymously readable, therefore no authentication is performed (and the form is not displayed) and markup is simply returned. Afterwards the /perapi endpoints are not visible to the felibs (because we are an anonymous user), and the page stays blank.

@reggie7
Copy link
Contributor

reggie7 commented Apr 27, 2021

Great analysis @golinski, thank you for that 👍🏼

@cmrockwell
Copy link
Collaborator

cmrockwell commented Apr 27, 2021

Indeed. Thanks both to Reggie and Michał for investigating the issues related to expired login. To summarize the discussion from the water-cooler meeting, there were a few points...

  1. Whether anonymous users should have access to /content for default localFS replication use-case. Instances with author runmode for the remote use-case do not grant 'everyone' access.
  2. Sling distinguishes between anonymous access and granting access to the everyone group. I think that Peregrine CMS does not give read access on /content to anonymous users, rather it grants read access to the everyone group. That is to say that org.apache.sling.engine.impl.auth.SlingAuthenticator is not configured to grant access to anonymous unauthenticated users. But any authenticated user (members of everyone) has read access.
    image
  3. Another facet is that "content" is optional as is apparent upon login the path is /admin/pages/index.html which is enabled by the Resource Resolver Factor configuration URL Mappings. I don't this makes ant difference to the issue per se, and rather I want to make sure the team understands

image
4. The question about what to do about this came up in the meeting. We brainstormed a few possible solutions to consider:

  • Maybe remove the ACL granting read for everyone group on /content would be a good idea. The remote use case and this condition, and the login experience seems a but little nicer. It could be a good idea for the security of the system as well if tenant users are not totally friendly.
  • The option of a frontend approach was discussed whereby the servlet /perapi/admin/access.json is already polled at regular intervals. If user should lose their sessions, then the response should not be 404 as is the current state. Rather the response could be userID: "anonymous" and the JS could redirect the page to /system/sling/form/login?resource=
  • The option of a backend approach, where the user may be expected to refresh the screen upon observing the whitescreen. The redirect would be initiated form the backend.

@reusr1
Copy link
Contributor

reusr1 commented Apr 29, 2021

@golinski @cmrockwell @reggie7 great discussion here - one comment on the access servlet: that is probably a permission issue in /perapi/admin/access.json that one should allow read for everyone - since the original issue was fixed and this relates more to peregrine-cms/enhancements#33 - should we take the discussion there?

also, I think we should turn off the default /content/:/ mapping in our launcher - it is the default in sling but it seems confusing?

@cmrockwell
Copy link
Collaborator

cmrockwell commented Apr 30, 2021

Hi @reusr1
Peregrine CMS would greatly benefit from friendlier URL's generally. The default content mapping offers a simple way to have that not just for the CMS pages, but also the tenant pages. No site owner wants to look at /content/ at root constantly. As CMS developers, we should not settle on that either. I mentioned the default content mapping in the context of this issue only to ensure the team was aware about it. But it is a separate topic, not related inherently related to these login issues. If you want to propose changes to the content mapping, could we discuss in a separate ticket? For what it's worth, I do not feel turning off the default /content/:/ mapping is for the better.

@reusr1
Copy link
Contributor

reusr1 commented Apr 30, 2021

@cmrockwell I think that is what I was trying to say - we should split the discussion into a different ticket - this issue is about login does not work for certain paths and from what I see that was fixed.

@cmrockwell
Copy link
Collaborator

Ah I see. My comments on this issue 887 were a bit off topic. They were more relevant to issue 33 which I had read before the water-cooler meeting. It was on my mind, so maybe I delete the comments move them. Because I was mistaken about what issue 887 was really about. My apologies!

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

4 participants