Improving invalid token error

[drjjw]

Invalid token errors in Joomla are handled sub-optimally. A white screen appears showing the error:

'Invalid Token'

This does not mean anything to the casual web user; it certainly does not tell anyone how to recover form the error.

Expected result

Instead of an invalid token error, the Joomla/template error page should be loaded with instructions on how to recover

Actual result

White screen: Invalid token

System information (as much as possible)

Jooomla 3.4.5

Additional comments

In order to simulate an invalid token, head to http://yoursjoomla.com/index.php?option=com_users&view=login

Using browser developer tools, explore the login form and at this line:

<input type="hidden" name="21c90b1ad7d44fcdad27ae14eb9d3461" value="1">

delete the contents of 'name', leaving it as "". Without reloading the page enter your username and password. Result: Invalid Token.

My proposal:

In:

/components/com_users/controllers/user.php

Replace line 30:

JSession::checkToken('post') or jexit(JText::_('JINVALID_TOKEN'));

with:

$currenturl = JURI::current();
        JSession::checkToken('post') or jexit(JError::raiseError( 'Woops', 'Something went wrong.<br><br><a href= ' .  $currenturl . '   >Please <span style="text-decoration:underline">click here</span></a> to reload the page you were trying to access and try logging in again' ));

This code will load the error page on invalid token at login events and provide a link to navigate back to the page you were on in order to try again.

[Bakual]

Copied the title and comment from the original issue so we have all the details here.

[Bakual]

Instead of using a hardcoded error message, you should instead use a language string so it can be translated. Maybe the existing JINVALID_TOKEN can be adjusted to achieve that.

[wilsonge]

Also whilst this works as a proof of concept for people to test/approve obviously before we merge this we would need to implement it across all the uses of this check token in the CMS :)

[dgrammatiko]

I think that JINVALID_TOKEN is also used in json responses (eg media/controllers/file.json.php), so it might be a good idea to leave that as is and introduce another string JINVALID_TOKEN_HTML or something similar.

[peterpeter]

I have tested this item 🔴 unsuccessfully on da57531

Redirect to the template's error page works (with a deprecated JError::raiseError call!), but the HTML of the link is not interpreted by the browser and thus not clickable. And the URL is not correct, JURI::current returns the URL without the query.

---

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

[peterpeter]

A solution for this white 'invalid token' screen is needed, no doubt about. But why not with a simple redirect back to the form and a simple 'Invalid Token, please try again' system message? Without force a user to read (and discover the relevant message at the bottom of that error page, see screenshot). Something like

if(!JSession::checkToken('post')) {
    $app = JFactory::getApplication();
    $app->enqueueMessage(JText::_('IVALID_TOKEN_HTML'));
    $app->redirect(JURI::current());
}

(just with the right URL)
Would be much more UX/User friendly, no deprecated JError, no reading/thinking/clicking for users....

And if you add a return $this in JApplication::enqueueMessage this could be a one-liner too :)

---

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

[peterpeter]

With a redirect back to the form the page would look

like this, wich is much more UX friendly

[drjjw]

Hi Peter,

I submitted this originally. I am an advanced Joomla user but not PHP programmer. I started this as a proof of concept, for the most part. Your idea is obviously excellent. And the UX is great. I would think it's important to redirect to the original page from which the error was generated. If your suggestion includes that (as it seems to) then I think this is great.

Jordan

[peterpeter]

Great, thanks. It's all about the code, nothing personally :) So you are able to do this patch/PR, as you created a branch for that in your Joomla-Repo fork?

[joomla-cms-bot]

This PR has received new commits.

CC: @peterpeter

---

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

[joomla-cms-bot]

This PR has received new commits.

CC: @peterpeter

---

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

[joomla-cms-bot]

This PR has received new commits.

CC: @peterpeter

---

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

[drjjw]

OK that's done and language file updated.

[infograf768]

Please correct code style ( https://travis-ci.org/joomla/joomla-cms/builds/91165438 )

FILE: ...ravis/build/joomla/joomla-cms/components/com_users/controllers/user.php

--------------------------------------------------------------------------------

FOUND 2 ERROR(S) AFFECTING 2 LINE(S)

--------------------------------------------------------------------------------

 30 | ERROR | Expected "if (...)\n"; found "if(...) "

 35 | ERROR | Whitespace found at end of line

[peterpeter]

JURI::current() is still not providing the complete URL :) The function that does that job is $url = JURI::getInstance()->toString();
But looked into JUri: Problem one is, in JURI is all about the requested URL, not where you came from. Problem two is that there is a redirect from a controller in between. But as we know the URL to the login form, and to make things not too complicated I propose to doing a simple '->redirect(JRoute::_('index.php?option=com_users&view=login'))`

I found 128(!) use cases of checkToken, but the two that are imho most important for better UX are the login on frontend and backend.

---

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

[drjjw]

Do you think we could append the return url to index.php?option=com_users&view=login so that from UX perspective, users are at least redirected on login to wherever they might have been if the invalid token hadn’t occurred?

Jordan

[peterpeter]

yepp....there are two cases: user comes with invalid login token from the login site (component) itself, or from antother site with the login-module. In both cases a redirect to the login (component) site seems to me not too bad, better than this white 'invalid-token-page'. And if the login site is a menu-item, JRoute() should be able to find and address them. That's what I would do. But It's your patch ;-)
The main thing is to get rid of this white, unfrendly, confusing error page somehow :)

[drjjw]

That’s great. I’m afraid while it’s my patch, I have reached my own PHP limits here :)

At least in its current form it’s far superior to the white screen of death. But it would be good get the return URL baked in to this eventually.

Dr. Jordan Weinstein, MD
Division of Nephrology, St. Michael's Hospital
Assistant Professor of Medicine
University of Toronto
Director, UKidney.com
61 Queen St East, 9th Floor
Toronto, Ontario M5C 2T2
PH (416) 867-3703
FX (416) 867-3709

On November 15, 2015 at 9:50:23 AM, peterpeter (notifications@github.com) wrote:

yepp....there are two cases: user comes with invalid login token from the login site (component) itself, or from antother site with the login-module. In both cases a redirect to the login (component) site seems to me not too bad, better than this white 'invalid-token-page'. And if the login site is a menu-item, JRoute() should be able to find and address them. That's what I would do. But It's your patch ;-)
The main thing is to get rid of this white, unfrendly, confusing error page somehow :)

—
Reply to this email directly or view it on GitHub.

Untracked with Trackbuster

[izharaazmi]

Wow! This weekend I've been working on this too. Let me commit and PR my changes so that we can further continue this discussion.

[peterpeter]

Thats an idea. But it is the wrong way and against design principles: JSession is a specialised class for sessionhandling and not about redirection handling.

I got a possible solution but here at work I had to do 'work-stuff' ;-) I am in a few hours able to write and test it.

[izharaazmi]

@peterpeter Thanks for pointing this out. I agree.

This is about #8445.

The JSession::checkToken() is already being used at several places in the entire cms.
I was trying to minimize the changes in these places by modifying the method itself. So this thing slipped off my mind.

However, I can see the checkToken is already handling redirection in case of a new session.

Anyway we still need to make this work across the CMS. As also pointed out in #8416 (comment) by @wilsonge.

What we can do is to move this validateToken method somewhere else that fits better and call it instead.

Any other suggestions?

[Devportobello]

This is controller responsability to handle this "error" in first attempt.

JSession::checkToken('post') or jexit(JText::_('JINVALID_TOKEN'));

to

JSession::checkToken('post') or $this->handleErrorToken();

/* Docblock */
public function handleErrorToken()
{
    /* @todo: logic here */
    jexit(JText::_('JINVALID_TOKEN'));
}

Then maybe we could move to Exception to let application handle kind of error (eg: html, json, ...)

throw new RuntimeException(JText::_('JINVALID_TOKEN'),'403');

[conconnl]

Can you fix the Travis errors and tell me if i can start testing this PR?

---

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

[Devportobello]

cross with: #8445

[ghost]

@Bakual should this PR be tested, cause #8445 has 2 successfully Tests?

[Bakual]

The other PR is already merged and seems to achieve the same goal. I'm closing this one.

If anyone feels that the issue isn't fully solved, feel free to reopen this one or create a new issue for it.