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

403 error on cookie login when visiting a protected page after session expires #15824

Open
sakicnet opened this issue May 5, 2017 · 16 comments

Comments

@sakicnet
Copy link
Contributor

sakicnet commented May 5, 2017

Steps to reproduce the issue

  • Install Joomla 3.7 (or 3.7.1-dev)
  • Create a menu link with Access set to Registered (e.g. /test)
  • Enable System - Language Filter plugin (required for multi-lingual sites)
  • Login on frontend with "Remember me" checkbox ticked
  • Wait 15 minutes for session to expire ;) (not really, just remove your session cookie, or delete the session record from #__session table)
  • Visit the protected page (e.g. /en/test)

Expected result

You should be automatically logged in and land on open protected page.

Actual result

You are redirected to frontpage with message:
Error: You are not authorised to view this resource.
If your homepage requires access then you end up on 403 page.

System information (as much as possible)

What happens is that the language filter plugin builds the menu with access levels before the remember plugin had a chance to login the user. The menu is built with access levels of the guest user and updates only on refresh. So, on first visit, after the session expires, the menu thinks it's a guest and denies the access.

The quick fix is to re-build the menu after the remember plugin has logged in the user.
Here is the Gist, line 66: https://gist.github.com/sakicnet/f2b8e2486011093d08e544423d8e5124

Additional comments

@brianteeman
Copy link
Contributor

@sakicnet can you submit this as a PR please Emir

@alikon
Copy link
Contributor

alikon commented May 5, 2017

i wonder, can we do in an alternative way just loading first plg_system_remember plugin and after plg_system_languagefilter changing the system plugin order?

@sakicnet
Copy link
Contributor Author

sakicnet commented May 5, 2017

@brianteeman Hi Brian, is this enough? sakicnet@d6fa7f9

@alikon: no, not really. Remember plugin does the login, which triggers onUserLogin events, which builds the menu before user is logged in. So it would involve bigger structural changes to fix it.

@mbabker
Copy link
Contributor

mbabker commented May 5, 2017

We shouldn't be direct calling __construct() again after an object is already constructed. Especially because it could result in class properties being changed. What you really need to do is call the load() method.

@sakicnet
Copy link
Contributor Author

sakicnet commented May 5, 2017

@mbabker that would probably require extending the JMenu class because its $user property is protected?

@mbabker
Copy link
Contributor

mbabker commented May 5, 2017

JMenu::load() is public. There's no need to make any other changes than to (re-)call the load() method.

@mbabker
Copy link
Contributor

mbabker commented May 5, 2017

Wait, nevermind. Realizing you need an updated JUser object too.

@mbabker
Copy link
Contributor

mbabker commented May 5, 2017

Either way, you shouldn't reconstruct an object. A new one should be loaded in replacing the existing one.

@sakicnet
Copy link
Contributor Author

sakicnet commented May 5, 2017

JMenu::load() doesn't fix the issue. What we need is $menu->user->load($id)
OK, will try that.

@mbabker
Copy link
Contributor

mbabker commented May 5, 2017

Putting public setters for the properties (at least a setUser() method) would help. Then you'd just need to do $this->app->getMenu()->setUser($user)->load();.

sakicnet added a commit to sakicnet/joomla-cms that referenced this issue May 5, 2017
@sakicnet
Copy link
Contributor Author

sakicnet commented May 5, 2017

If we can change JMenu then great. New PR: #15839
Feel free to modify / improve.
Thanks.

@zero-24 zero-24 closed this as completed May 5, 2017
@HLeithner
Copy link
Member

Reopen because both PR that should fix them are closed #21230 and #15839

@HLeithner HLeithner reopened this Jun 21, 2020
@AndySDH
Copy link
Contributor

AndySDH commented Oct 7, 2020

Same issue as #11541

@AndySDH
Copy link
Contributor

AndySDH commented Oct 7, 2020

Please close this and re-open #21230

@brianteeman
Copy link
Contributor

Maintainers please action the request above

@zero-24 zero-24 closed this as completed Aug 23, 2022
@zero-24
Copy link
Member

zero-24 commented Aug 23, 2022

That PR is against staging which does not exists anymore and the PR can not be reopend but need to be created against 4.x-dev

@zero-24 zero-24 reopened this Aug 23, 2022
@jwaisner jwaisner added the JBS label Oct 10, 2022
@Hackwar Hackwar added bug and removed JBS labels Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Still Relevant, Needs Attention
Development

No branches or pull requests

10 participants