Skip to content

If website is offline, then after logging in, return to the previous page#22480

Closed
csthomas wants to merge 2 commits intojoomla:stagingfrom
csthomas:fix_offline_redirect
Closed

If website is offline, then after logging in, return to the previous page#22480
csthomas wants to merge 2 commits intojoomla:stagingfrom
csthomas:fix_offline_redirect

Conversation

@csthomas
Copy link
Contributor

@csthomas csthomas commented Oct 3, 2018

Summary of Changes

If website is offline, then after logging in, return to the previous page

Testing Instructions

As a guest go to any subpage (article) of your website.
On backend in another tab set website offline.
Refresh page on front end and log in as Super User.

Expected result

After log in you stay on the same page.

Actual result

After login you are redirected to the home page.

Documentation Changes Required

No

@Quy
Copy link
Contributor

Quy commented Oct 6, 2018

I have tested this item ✅ successfully on 7a7fa48


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22480.

@infograf768
Copy link
Member

Although this works, I see no real use for it. The case is so much to the edge that one needs binoculars to even consider it. (That was a joke: 😺 )

@brianteeman
Copy link
Contributor

@csthomas thanks for this. It will save me a lot of time.

@csthomas
Copy link
Contributor Author

csthomas commented Oct 8, 2018

This PR does not do any important thing but correctly describe (in code) how to back to previous page after logged in.
If nobody tests it, I will close it after a few weeks and I will not bother any more.

This PR is a part of #22229, which I wanted to do separately.

I would like to standardize the way of generating the action link of the form:

  • parameters, such as option and view (and others used to generate valid canonical SEF links), should be placed in the form's action link.

@csthomas
Copy link
Contributor Author

@vc-development @viocassel
I know it's trivial improvement but we have to start with something easy to test.
Can I ask for one more test?

Test patch, go to https://issues.joomla.org/tracker/joomla-cms/22480 and click "Test this" and fill in a form.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 7a7fa48


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22480.

@viocassel
Copy link
Contributor

@csthomas done 👍

@vc-development
Copy link

I have tested this item ✅ successfully on 7a7fa48


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22480.

@brianteeman
Copy link
Contributor

Please do not use multiple github accounts to report that you have tested something more than once

@viocassel
Copy link
Contributor

@brianteeman the second account is not mine, but this is my friend's account 😉

<input type="hidden" name="option" value="com_users" />
<input type="hidden" name="task" value="user.login" />
<input type="hidden" name="return" value="<?php echo base64_encode(JUri::base()); ?>" />
<?php $uri = new Uri('index.php'); ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do this in a single line with:

<input type="hidden" name="return" value="<?php echo base64_encode(Uri::getInstance()->tostring()); ?>" />

<input type="hidden" name="option" value="com_users" />
<input type="hidden" name="task" value="user.login" />
<input type="hidden" name="return" value="<?php echo base64_encode(JUri::base()); ?>" />
<?php $uri = new Uri('index.php'); ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@csthomas
Copy link
Contributor Author

@phproberto
There is a difference between

echo Uri::getInstance()->tostring(); // this is get from $_SERVER

and

$uri = new Uri('index.php');
$uri->setQuery($app->getRouter()->getVars());
echo $uri->tostring();

When SEF is ON the first one can display e.g.
sef-url/to-my/1-article
but the second one show you
index.php?option=com_content&view=article&id=1-article&cat=2&Itemid=102.

Following https://docs.joomla.org/How_do_you_redirect_users_after_a_successful_login%3F#Overriding the return field should contains non SEF URL.

@phproberto
Copy link
Contributor

I've just tested it with SEF ON with an URL like http://localhost/joomla-cms/article-category-list?start=10:

// Prints http://localhost/joomla-cms/article-category-list?start=10
var_dump(Uri::getInstance()->toString());

// True
var_dump(Uri::isInternal(Uri::getInstance()->toString()));

$uri = new Uri('index.php');
$uri->setQuery($app->getRouter()->getVars());

// Prints index.php?Itemid=260&option=com_content
var_dump($uri->toString());

Note that start=10 is missing in your URL.

This code is already used and tested in in mod_login:

https://github.com/joomla/joomla-cms/blob/staging/modules/mod_login/helper.php#L33

So I'd definitely use Uri::getInstance()->toString().

I promise to test your PR if you change it 😄

@csthomas
Copy link
Contributor Author

I have fixed issue with start=10 in your URL.
The start/limitstart parameters should work in the same way on parsing and building.

Why I force it?

$uri = new Uri('index.php');
$uri->setQuery($app->getRouter()->getVars());

For offline page Uri::getInstance()->toString() will work for a lot of cases but I want to encourage to use the same technique on different places.

If you start using association on multilingual website then you will see the difference for offline page.

The form is generated as <form action="<?php echo JRoute::_('index.php?option=com_users&view=login'); ?>" method="post" id="form-login">

For e.g. create 2 pages for English and German and associate them (/en/english-blog, /de/german-blog).

As user frontend language is German and you go to /en/english-blog (still offline) then after login you will go to ...
a) your solution - english blog
b) my solution - german blog - association work correctly

I know that for offline pages this is not important but I we will use this at

$this->setUserState('users.login.form.data', array('return' => \JUri::getInstance()->toString()));

diff --git a/libraries/src/Application/SiteApplication.php b/libraries/src/Application/SiteApplication.php
index f6dbe5408a..39c25c8acf 100644
--- a/libraries/src/Application/SiteApplication.php
+++ b/libraries/src/Application/SiteApplication.php
@@ -86,8 +86,11 @@ final class SiteApplication extends CMSApplication
                {
                        if ($user->get('id') == 0)
                        {
+                               $uri = new \JUri('index.php');
+                               $uri->setQuery($this->getRouter()->getVars());
+
                                // Set the data
-                               $this->setUserState('users.login.form.data', array('return' => \JUri::getInstance()->toString()));
+                               $this->setUserState('users.login.form.data', array('return' => $uri->toString()));
 
                                $url = \JRoute::_('index.php?option=com_users&view=login', false);
 

then association start working when you go to restricted area (registered only) e.g. /en/restricted-blog as a guest. Then after log in:
a) current joomla - you back to /en/restricted-blog-en - association does not work
b) my solution - you back to /de/restricted-blog-de - association work, ping @infograf768

@phproberto
Copy link
Contributor

phproberto commented Oct 16, 2018

As user frontend language is German and you go to /en/english-blog (still offline) then after login you will go to ...
a) your solution - english blog
b) my solution - german blog - association work correctly

False. My solution works as expected with mod_login so it's not about using Uri::getInstance() or not.

My setup:

  1. Install Spanish language
  2. Enable language filter plugin
  3. Create menu for spanish language
  4. Create menu for english language
  5. Create an article an assign language to Spanish
  6. Create an article an assign language to English
  7. In spanish menu create a link of type Single Article and assign the spanish article created previously.
  8. Set previous menu item home for spanish language
  9. In english menu create a link of type Single Article and assign the english article created previously.
  10. Set previous menu item home for english language and in Associations tab assign the spanish menu item.
  11. Create an user and assign spanish as frontend language.
  12. Publish a language switcher module and navigate to the english article. In my example: http://localhost/joomla-cms/index.php/en/an-article
  13. In the mod_login box introduce the login+pass of the user created previously with spanish frontend language
  14. Note that after login you are redirected to the spanish language. In my example: http://localhost/joomla-cms/index.php/es/un-articulo

I also tested the previous setup with links that are not the home page. It also works redirecting.

So then if my solution works with mod_login (and it has been used & tested for years) why does it fail in your offline? Because the changes you did. Use previous form action:

<form action="<?php echo JRoute::_('index.php', true); ?>" method="post" id="form-login">

And add back the $_POST data that you removed:

  <input type="hidden" name="option" value="com_users" />
  <input type="hidden" name="task" value="user.login" />

You will notice that it works.

So changing one line of code everything would work as you wanted. From:

<input type="hidden" name="return" value="<?php echo base64_encode(JUri::base()); ?>" />

to:

<input type="hidden" name="return" value="<?php echo base64_encode(JUri::getInstance()->toString()); ?>" />

That's all!

Summary:

  • mod_login is doing what you try to do. For years. With issues reported and fixed.
  • Changing 1 line of code does it.
  • Devs already know and use Uri::getInstance().
  • I guess you will want to change the behaviour of mod_login. And then fix issues caused by switching to the new method (as it happened with the start fix).

About the changes you propose for:

/libraries/src/Application/SiteApplication.php

I cannot make it working with your code because return is hardcoded in offline template and not reading from the login data in session. It requires a more complicated solution.

Anyway I'm abandoning here. I've invested enough time testing things. You already decided to follow your way.

Good luck!

@csthomas
Copy link
Contributor Author

So then if my solution works with mod_login (and it has been used & tested for years) why does it fail in your offline? Because the changes you did. Use previous form action:

I knew from the beginning that it works if you put it back

<form action="<?php echo JRoute::_('index.php', true); ?>" method="post" id="form-login">

but it looks ugly, the action URL is an URL of the current page and you have to override option parameter to com_users in the POST request. For me it is ugly.

The form action URL should point to the login form (JRoute::_('index.php?option=com_users&view=login')) and the return value should contain full information where you want to back. When you use association (your way, mod_login) joomla get information where to back from the action URL. This is ugly and create mess in the language filter plugin, but I have to admin that it works in most cases.

I will prepare a separate PR about the changes I proposed in /libraries/src/Application/SiteApplication.php.

@csthomas
Copy link
Contributor Author

I created a new PR to fix issue for restricted menu item with association enabled on multilingual website. #22677

@csthomas
Copy link
Contributor Author

The PR died because a problem with B/C. See #22677.

@csthomas csthomas closed this Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Comments