Fix “You are not authorised to view this” when mod_expires enabled by PhilETaylor · Pull Request #13516 · joomla/joomla-cms

PhilETaylor · 2017-01-08T16:01:30Z

THE ULTIMATE FIX !!!!!

Closes #8731 Dec 18, 2015
Closes #8757 Dec 21, 2015
Closes #9013 Jan 28, 2016
Closes #9145 Feb 17, 2016
Closes #9615 Mar 26, 2016
Closes #10753 Jun 7, 2016
and many many more...

To replicate the root issue with minimal configuration

To replicate the issue - with 100% reliability:

Install LAMP & Joomla
Enable expires module in apache
Add to .htaccess

ExpiresActive On
ExpiresDefault "access plus 30 days"

Attempt to edit a content item BY CLICKING ON THE TITLE of the content item in admin (and NOT using the checkboxes!)
Do this twice, note on the second attempt you get

"You are not permitted to use that link to directly access that page (#n)."

Note that on subsequent clicks the error goes away but it is still impossible to get to the edit page by clicking on the title of the content
Note that if you open Google Dev Tools Panel, Network Tab, and enable Disable Cache that the problem "disappears" and while disable cache is enabled, you can edit the item by clicking its article title.

The root cause of the problem

The root cause of the problem is that with the Expires Module enabled in apache, responses from Joomla, which are redirects, are not outputting the correct headers, which allows browsers to cache the redirects. On subsequent visits the browser "skips" evn asking Joomla for a response, and simply "skips" over the request and opens the redirect destination url. Unfortunately the "skipped" url checks out the item for the user and is a prerequisite to the edit form being able to load - as this is "skipped" there is no check out record in the session and so a not authorised message is displayed instead.

The solution this PR makes

The final solution to resolve this issue is two fold:

Ensure that any "edit" controller action is not cached by the use of JFactory::getApplication()->allowCache(FALSE);
Instead of closing the connection at the edit fo the redirect method, correctly run the ->respond() method which correctly checks if the response is cacheable and if not, sets appropriate headers

NOTE TO TESTERS

IF YOU ALREADY HAVE CACHED URLS IN YOUR BROWSER, (or hitting a known Google Chrome bug https://bugs.chromium.org/p/chromium/issues/detail?id=91740) THEN THIS NEW CODE IS NOT EVEN RUN - DO NOT REPORT THIS PR IS NOT WORKING FOR YOU, BECAUSE IT DOES WORK IF THE CODE IS "RUN". Simply clearing your browser cache IS NOT ENOUGH sometimes, REDIRECTS ARE STILL CACHED!

YOU CANNOT test this on a server you dont have full access to - like Site Ground demo, people (like @durian808) will not be entertained in this PR comments should they attempt to rail road the comments again. Personal attacks will not be tolerated either.

It you do not have FULL CONTROL over your apache server, .htaccess, and a FULL UNDERSTANDING of the above issues, please DO NOT ATTEMPT to test this PR.

Hattips, Thanks, And Patience and pinging others ...

@ggppdk @mbabker @lyquix-owner @gwsdesk @brianteeman @durian808 @karendunne @OzzieBob @MATsxm @andrepereiradasilva @lukasz-pawlowski @revers28 @level420 and many many others.

Closes #8731 Dec 18, 2015 Closes #8757 Dec 21, 2015 Closes #9013 Jan 28, 2016 Closes #9145 Feb 17, 2016 Closes #9615 Mar 26, 2016 Closes #10753 Jun 7, 2016

…ates in headers)

…r/joomla-cms into FixNotAuthRedirectWithExpires

dgrammatiko · 2017-01-08T17:12:14Z

I have tested this item ✅ successfully on 1d28e54

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

mbabker · 2017-01-08T18:01:46Z

libraries/joomla/application/web.php


-		// Close the application after the redirect.
-		$this->close();
+		$this->respond();


You still need to call $this->close(); otherwise the application code will continue to process when it is expected that calling redirect results in PHP's exit() being triggered.

mbabker · 2017-01-08T18:02:16Z

libraries/legacy/controller/form.php

 	public function edit($key = null, $urlVar = null)
 	{
+		// Do not cache the response to this, its a redirect, and mod_expires and google chrome browser bugs cache it forever!
+		JFactory::getApplication()->allowCache(false);


Is it just the edit task needing this directive?

Are there any components in core extending this class and overriding this method?

ggppdk · 2017-01-08T21:00:39Z

@durian808

As most of you know, I was able to precipitate this problem easily on the SiteGround server, without any special setup

The "without any special setup" case,
is not fixed in this PR (rare / random case occurence of the error),
identified as "RACE conditions" during updating user session

so you do not need to post here , that it does not work in some rare / random cases,

The cases that should be fixed by this PR, are due to browser caching the redirect to the edit form

either due server configuration sending default HTTP headers instructing browser to cache
or due to missing headers regarding cache behaviour, some browsers , sometimes deciding to cache the redirect

ggppdk · 2017-01-08T21:18:05Z

have been unable to replicate a TRUE race condition relating to this ever. Therefore I believe it can be ruled out.

hhmm, i am just saying that this PR is good in fixing the caching case, the RACE conditions issue is irrelevant to fixing caching, it is simply a ... different case

to explain more about race conditions, i mean this:

In Joomla articles manager

during 6-8 seconds, quickly middle click (to open in new browser TAB / Window)
on 20 article editing links

Some of them will fail, e.g. 3-6 of them should fail (yes not 1 failure but several)
and message will appear propably only 1 or 2 times

Please note that on failure the Joomla articles manager will appear sometimes WITHOUT the error message (because multiple "same text" enqueued messages are displayed once and then of course cleaned by the first page view to be able to read them)

It is happening like this:
thread 1 e.g. gets as (existing) editable ids array (31,35)
thread 2 e.g. gets as (existing) editable ids array (31,35)

thread 2 adds 47 and saves array (31,35, 47)
thread 1 adds 58 and saves array (31,35, 58)

Now the editable ids are (31,35, 58) you see that id 47 is missing

lyquix-owner · 2017-01-08T21:24:42Z

I confirm that this PR fixes the issues I had observed when opening/editing Users, Articles, Categories, Modules, Plugins, and Templates. 👍

lyquix-owner · 2017-01-08T21:26:46Z

From my perspective the issues solved by the PR are significantly more prevalent and disruptive than the race conditions issue.

dgrammatiko · 2017-01-08T21:27:53Z

I have tested this item ✅ successfully on c766975

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

dgrammatiko · 2017-01-08T21:29:01Z

I'll set this to RTC, but more tests are welcome!!!
Thanks @PhilETaylor great stuff

lyquix-owner · 2017-01-08T21:37:40Z

Thanks very much for looking into this issue!

ggppdk · 2017-01-08T21:42:59Z

I have tested this item ✅ successfully on c766975

My initial comment was to prevent irrelevant and partly frustrating, useless comments, of someone thinking that it fixes a different issue / case too,
despite the fact that this has been discussed extensively

i never implied something negative of this change, (as long as we test it of course like any PR)
in fact i had asked for these changes in the other PR, i am happy that someone took time to do it

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

ot2sen · 2017-01-08T22:06:48Z

I have tested this item ✅ successfully on c766975

As described in testing instructions this PR solves the 'You are not permitted to use that link' that do appear on second try. After patch you can edit again with no issues seen. Thanks @PhilETaylor nice solution.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

gwsdesk · 2017-01-09T05:30:17Z

I have tested this item ✅ successfully on c766975

I have tested this item ✅ successfully

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

laoneo · 2017-01-09T05:45:44Z

I'm getting this error also when the cache plugin is on and a I try to directly open a link on the front end from a menu item which has access to registered. Is this related to this one?

Bakual · 2017-01-09T07:25:26Z

@durian808 Please open a new issue for the race condition if you feel it is important. Don't derail this PR which explicitely is not about that.

Also an obvious one: please all stop attacking eachother. It's bad enough this has to be said.

laoneo · 2017-01-09T07:39:41Z

Yes the menu item. I'm logged in with remind me enabled. After a few hours of not browsing the site, when I directly want to open that registered link, it gives me the error.

dgrammatiko · 2017-01-09T07:48:43Z

I'm logged in with remind me enabled. After a few hours of not browsing the site, when I directly want to open that registered link, it gives me the error.

@laoneo after all this time do you have any errors logged in the browser's console before hitting the link?

laoneo · 2017-01-09T07:54:42Z

@DGT41 no, it redirects me to the start page.

@PhilETaylor ok, just tough because it is the same error message, probably the caching needs to be disabled on another place as well.

laoneo · 2017-01-09T09:15:47Z

@PhilETaylor that's what I thought at beginning as well, but my session hasn't expired. I'm still logged in after the error. But I don't want to pollute this PR anymore. Will set up another issue.

rdeutz · 2017-01-10T13:56:49Z

I merge a lot commits, most of the time these are tested and have a RTC state :-)

GASOLINE · 2017-01-11T11:25:49Z

Thanks Phil,
I assume Joomla will implement this in the next update? I'v got this on all my and clients sites. Annoying.

Bakual · 2017-01-11T11:30:41Z

It's merged, so it will be in Joomla 3.7.0

paulcu · 2017-02-02T21:27:40Z

Apologies if this is a rehash but in case this provides a little more information - - -

We have a clean install of v3.6.5 which we've been working with for a couple of weeks without this issue, and today it appeared. What we are seeing is:

As described above this is occurring when clicking on admin links for all Articles but also for the links in Modules and Categories. We haven't seen this happen with menu items.
Selecting the checkbox to the left of an Article, Module, or Category and then using the EDIT button doesn't replicate the problem and we can repeatedly access.
When we clear ONLY 'cached images and files' in Chrome, it temporarily fixes issue but as described above the second attempt gives us the error. Same with Safari and we haven't tested other browsers.
PHP v 5.6.29
_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13516.}

Lumiga · 2017-02-21T14:39:56Z

Exactly the same issue over here.

For me the best solution was to moving away from the hoster mijndomein.nl to byte.nl which is one of the best Joomla hosting company in the Netherlands.

So in my opinion it has something to do with the hosting company.

gwsdesk · 2017-02-21T14:43:32Z

Stop promoting your hosting company. This has nothing to do with hosting

demis-palma · 2017-03-07T12:27:29Z

This same patch exists here: #12504 and can be closed now.

Fix “You are not authorised to view this” when mod_expires enabled

8d8be19

Closes #8731 Dec 18, 2015 Closes #8757 Dec 21, 2015 Closes #9013 Jan 28, 2016 Closes #9145 Feb 17, 2016 Closes #9615 Mar 26, 2016 Closes #10753 Jun 7, 2016

joomla-cms-bot added the PR-staging label Jan 8, 2017

Phil Taylor added 3 commits January 8, 2017 16:06

code style

c6b1d73

Unit test refactoring (cannot do it the old way as relative dynamic d…

adaceac

…ates in headers)

Merge branch 'FixNotAuthRedirectWithExpires' of github.com:PhilETaylo…

7f81ee7

…r/joomla-cms into FixNotAuthRedirectWithExpires

joomla-cms-bot added the Unit/System Tests label Jan 8, 2017

Phil E. Taylor added 3 commits January 8, 2017 16:26

Remove duplicate header output for charset

a5efab7

Better Unit Tests

a637965

More unit tests :)

1d28e54

mbabker reviewed Jan 8, 2017

View reviewed changes

Phil E. Taylor added 2 commits January 8, 2017 18:19

Close after setting headers

68c4b76

Do not cache the response to this, its a redirect

c766975

This comment was marked as abuse.

Sign in to view

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 8, 2017

This comment was marked as abuse.

Sign in to view

rdeutz merged commit a6316f0 into joomla:staging Jan 10, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 10, 2017

This comment was marked as abuse.

Sign in to view

zero-24 added this to the Joomla 3.7.0 milestone Jan 10, 2017

This comment was marked as abuse.

Sign in to view

demis-palma mentioned this pull request Mar 7, 2017

Can't logout with the menu item Users > Logout #12504

Closed

brianteeman mentioned this pull request Mar 10, 2017

Not permitted error when Expires Headers enabled #12522

Closed

Fedik mentioned this pull request May 10, 2017

[Associations] You are not permitted to use that link... #15945

Closed

wilsonge mentioned this pull request May 21, 2017

Port fixes to the redirect method from the CMS joomla-framework/application#72

Merged

Uh oh!

Conversation

PhilETaylor commented Jan 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

THE ULTIMATE FIX !!!!!

To replicate the root issue with minimal configuration

The root cause of the problem

The solution this PR makes

NOTE TO TESTERS

Hattips, Thanks, And Patience and pinging others ...

Uh oh!

dgrammatiko commented Jan 8, 2017

Uh oh!

mbabker Jan 8, 2017

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

mbabker Jan 8, 2017

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

This comment was marked as abuse.

ggppdk commented Jan 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as abuse.

ggppdk commented Jan 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as abuse.

lyquix-owner commented Jan 8, 2017

Uh oh!

lyquix-owner commented Jan 8, 2017

Uh oh!

This comment was marked as abuse.

dgrammatiko commented Jan 8, 2017

Uh oh!

This comment was marked as abuse.

dgrammatiko commented Jan 8, 2017

Uh oh!

lyquix-owner commented Jan 8, 2017

Uh oh!

ggppdk commented Jan 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ot2sen commented Jan 8, 2017

Uh oh!

This comment was marked as abuse.

gwsdesk commented Jan 9, 2017

Uh oh!

laoneo commented Jan 9, 2017

Uh oh!

This comment was marked as abuse.

Bakual commented Jan 9, 2017

Uh oh!

laoneo commented Jan 9, 2017

Uh oh!

This comment was marked as abuse.

dgrammatiko commented Jan 9, 2017

Uh oh!

laoneo commented Jan 9, 2017

Uh oh!

This comment was marked as abuse.

This comment was marked as abuse.

laoneo commented Jan 9, 2017

Uh oh!

This comment was marked as abuse.

rdeutz commented Jan 10, 2017

Uh oh!

GASOLINE commented Jan 11, 2017

Uh oh!

PhilETaylor commented Jan 8, 2017 •

edited

Loading

ggppdk commented Jan 8, 2017 •

edited

Loading

ggppdk commented Jan 8, 2017 •

edited

Loading

ggppdk commented Jan 8, 2017 •

edited

Loading

demis-palma commented Mar 7, 2017 •

edited

Loading