bolt / users

Bolt users extension.
MIT License
8 stars 7 forks source link

No exception handling for routes when sessions are nullified or inactive #11

Open crim3hound opened 3 years ago

crim3hound commented 3 years ago

Trying to retrieve the ContentType slug using $contentTypeSlug = $this->getExtension()->getExtConfig('contenttype', $user->getRoles()[0]); in FrontendUsersProfileController.php throws the exception Call to a member function getRoles() on null.

This happens when a user tries to access profile routes without an active session, either because it expired or was invalidated.

session_issue

I have found a solution for this issue and will share for review.

crim3hound commented 3 years ago

Similar issue when accessing /profile/edit route. 403: Access Denied/Forbidden exception thrown as shown below.

session_issue_2

Exploring a way to handle the exception a little more 'gracefully'. Any help would be appreciated.

I-Valchev commented 3 years ago

Hi @crim3hound , that happens because there is no authenticated user. In the first screenshot, you'll see a user icon next to which it says "anon" -> that means, in Symfony terms, an anonymously authenticated user, i.e. no user.

You can handle 403 exceptions within Bolt itself, by configuring a 403 page. You can check the config.yaml file, there will be a forbidden key which you can use for that. There is documentation in the file itself :-)

I-Valchev commented 3 years ago

There may well be a better ways to handle this for people who were already logged in -> maybe by setting some session that we can check in this repo somewhere? I'm open to any ideas!

crim3hound commented 3 years ago

Thanks, @I-Valchev. I'll check it out in more detail and get back to you! At the moment I have opted for redirection as opposed to using the 403 handler for my use case. Trying to keep front-end users as far as possible from anything that may point them to the backend. Just customising the extension from a security by obscurity perspective. What do you think? Same thing applies to the logout route, where I'm looking to not let users see the /bolt/login screen.

I-Valchev commented 3 years ago

I think that makes sense too :-)

Perhaps allow people to configure where to go to on logout, with default to / (homepage)?

crim3hound commented 3 years ago

Exactly @I-Valchev! That would probably be better and would allow a more granular config without touching core files. For now in my case, I have set my logout listener in security.yaml as follows:

logout:
          handler: Bolt\Security\LogoutListener
          path: bolt_logout
          target: extension_frontend_user_profile # redirection to frontend login will be handled by extension
          # target: bolt_login (redirects to backend login - we want to avoid this for frontend users)

This way, and together with the exception handling for users accessing profiles, I am able to render to front-end login page. While I wouldn't prefer this method, it's of little consequence at this time if admin users are also redirected to the same screen on logout.

Would there be a quick way to maybe add a redirect_on_logout option to the extension config as well? And redirect to the homepage by default like you suggested? So far thank you for the great work! This extension has worked pretty well for my case.

I-Valchev commented 3 years ago

hi @crim3hound, sorry I thought I had replied to this! Glad that it works for you thus far, thanks a lot for the nice work and PRs!

I think it does make sense to add a redirect_on_logout, yes :-)

Do you think you can do that in a PR?

crim3hound commented 3 years ago

You're welcome, @I-Valchev! Let me look into the logout bit.