Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Redirect to last page after frontend login bug? #8632

Closed
rorych opened this issue Jan 25, 2017 · 14 comments
Closed

Redirect to last page after frontend login bug? #8632

rorych opened this issue Jan 25, 2017 · 14 comments
Assignees
Labels
Milestone

Comments

@rorych
Copy link

rorych commented Jan 25, 2017

I'm not shure if this is the correct behavior or not, so i will just explain myself.

Reproduction:
Create a login page and set the redirect option to: last page
Create a new page and create a link to the Login-Page.
Now visit the new page and click on the login-link.
Enter wrong login data and then correct login data (what definitely could happen in real life).
Now you are not correctly redirected, in my case, the login page stays put as it now is the last page, but the login form disappears, which resolves in a empty page.

I will try and build a workaroun for my case.
Do you also see this as a bug or at least not optimal?
I haven't looked at the redirect creation process yet, but i would suggest to check, if the newly to be set last page is the same as the current page, if so, don't overwrite the existing entry.
Normally people want to go to the last page anyway and are annoyed if they have to click "back" twice not?

Not shure if this would affect to many other situations.

Thanks for your feedback in advance

@fritzmg
Copy link
Contributor

fritzmg commented Jan 25, 2017

Sounds like a bug to me. Is it also reproducible in the current Contao demo?

@rorych
Copy link
Author

rorych commented Jan 25, 2017

I just checked the ModuleLogin.php to check what happens.
Seems like the empty page only appears, if you "protect" articles inside a page instead of the page.
in case of the page being protected, you get redirected to the start-page, so that is maybe my mistake.

But in the:
core/system/modules/core/modules/ModuleLogin.php
You set the last visited page like this:

// Set the last page visited
if (!$_POST && $this->redirectBack)
{
	$_SESSION['LAST_PAGE_VISITED'] = $this->getReferer();
} 

I used this now:

// Set the last page visited
if (!$_POST && $this->redirectBack)
{
	if (strpos($this->getReferer(), $this->replaceInsertTags('{{page::alias}}')) === false) {
		$_SESSION['LAST_PAGE_VISITED'] = $this->getReferer();
	}
}

I presume, there is a "cleaner" way but this would be my idea.

What do you think?

@rorych
Copy link
Author

rorych commented Jan 25, 2017

And to your question yes it is, in Contao 3.5.24 and the version 4 demo.
If you are on the "about" page and click on login in the header-nav, then enter correct login data, you are redirected to about.
If you enter wrong data first, and then the correct data, you are not redirected.

@fritzmg
Copy link
Contributor

fritzmg commented Jan 25, 2017

You set the last visited page like this:

This should actually suffice (in theory). It checks if there is a POST request and if there is, it does not set the LAST_PAGE_VISITED. But should be easy to debug what's going on.

@rorych
Copy link
Author

rorych commented Jan 25, 2017

Entering wrong login data can only be checked by the backend so it must be passed back to php via the POST Request, so i think that check doesn't work for this case.

@fritzmg
Copy link
Contributor

fritzmg commented Jan 25, 2017

Well yes of course. When you send the login data, you send a POST request. In that case the LAST_PAGE_VISITED variable is not updated with the current page (which would be the login page in this case).

@rorych
Copy link
Author

rorych commented Jan 25, 2017

Maybe empty($_POST) would be better? It should recognize the empty array() that is being returned more safely.

Although that would solve the posting the wrong data problem, still if you reload the page or timeout and your browser does it for you, no POST ist sent and the login-page is set as last_page_visited...
That's why i would still try and check if the current page is being saved as last_visited_page (which never would make sense...).

@leofeyer
Copy link
Member

leofeyer commented Feb 1, 2017

Is this issue related to #8324?

@rorych
Copy link
Author

rorych commented Feb 7, 2017

No, it is not.
The Problem described in this Issue is based on the "Zurück zur letzten Seite"/"Back to last page" Option being set, the issue #8324 doesn't.

To sum it up:
in the File -> core/system/modules/core/modules/ModuleLogin.php
the Check for a form post is not working correctly (maybe based on PHP Version?)
I propose to change
if (!$_POST &&
to
if(empty($_POST) &&

@leofeyer
Copy link
Member

if (!$_POST) and if (empty($_POST)) is exactly the same, isn't it?

@leofeyer
Copy link
Member

@contao/developers /cc

@rorych
Copy link
Author

rorych commented Feb 17, 2017

I'm sorry you are right, of course it is.

The problem still exists after adding empty();

What i had to do ist add Code inside the if statement because it seems the POST Value Check is not working. You can test it:

  1. Set Login Module to Redirect to last page.
  2. Go to Start-Page
  3. Go to Login-Page
  4. Enter wrong Login Data
  5. Enter real Login Date
  6. You will not be redirected to the Start-Page because the if statement still triggers and adds the login page to the "LAST_PAGE_VISITED" in step 4

I solved it by adding a second check inside your statement, like this:
if (strpos($this->getReferer(), $this->replaceInsertTags('{{page::alias}}')) === false) { $_SESSION['LAST_PAGE_VISITED'] = $this->getReferer(); }

@leofeyer
Copy link
Member

As discussed in Mumble on March 23rd, we must not set the last page visited if the referrer is the same as the current URL here. (Mind the ampersand() when comparing URLs.)

@leofeyer
Copy link
Member

Fixed in 8c92d98.

jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Apr 25, 2017
# Contao calendar bundle change log

### 4.3.8 (2017-04-24)

 * Correctly use the en dash in the calendar modules (see contao/core#8690).
 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao comments bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao core bundle change log

### 4.3.9 (2017-04-25)

 * Revert the Punycode library changes (see contao/core#8693).

### 4.3.8 (2017-04-24)

 * Inline small images in protected folders in the file manager (see #636).
 * Correctly encode the URL in the DataContainer::switchToEdit() method (see #762).
 * Fix the parent view drag and drop in Firefox (see #666).
 * Correctly display the search results in the extended tree view (see #739).
 * Update the Punycode library to version 2 (see #748).
 * Fix the "delete file" button for non-admin users (see #764).
 * Prevent endless loops in the book navigation module (see contao/core#8665).
 * Limit the maximum size of dimensionless SVGs in the back end (see contao/core#8684).
 * Correctly support 64 character template names everywhere (see contao/core#6819).
 * Remove the UTF-8 BOM when combining files (see contao/core#8689).
 * Correctly move folders with an "@" in their name (see contao/core#8674).
 * Correctly redirect to the last page visited upon login (see contao/core#8632).

### 4.3.7 (2017-03-23)

 * Check the database connection in the WebsiteRootsConfigProvider class.
 * Fix the %2B conversion in the Controller::addToUrl() method.

# Contao listing bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao news bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).

# Contao newsletter bundle change log

### 4.3.8 (2017-04-24)

 * Correctly support 64 character template names everywhere (see contao/core#6819).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants