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

Wrong (double language prefix in URL) redirection after success login - ends with 404 not found #3411

Closed
dexterxx-pl opened this issue Jul 21, 2021 · 5 comments
Assignees

Comments

@dexterxx-pl
Copy link

dexterxx-pl commented Jul 21, 2021

That issue is already well described here: https://discourse.getgrav.org/t/login-to-private-page-fail/12366

Config

Login plugin:

redirect_to_login: <whatever - true or false>
redirect_after_login: false

Site config

languages:
  supported:
    - pl               # whatever languages
    - en

Description

I have page with defined access rules on example url: www.mysite.com/pl/secret-page which requires user to authenticate.

---
title: 'lalala'
visible: false
access:
    private.my-whatever-grand: true

Unauthenticated user will see in first request login form.
After filling credentials and submitting form it's being wrongly redirected to page with double /lang/ URL prefix.

Form submit request

Request URL: http://somewhere/pl/secret-page
Request Method: POST
Status Code: 303 See Other

Response

Location: /pl/pl/secret-page

Some debugging

    public function redirectLangSafe($route, $code = null)
    {
        if (!$this['uri']->isExternal($route)) {
            $this->redirect($this['pages']->route($route), $code);       // here is happening additional, unwanted append
        } else {
            $this->redirect($route, $code);
        }
    }

What I've tried

1. Change login form action URL

I've overwritten login form template in my theme user/themes/.../templates/partials/login-form.html.twig and changed action URL in form from

<form method="post" action="{{ (base_url_relative ~ uri.path)|e }}" class="{{ form_outer_classes }} login-form">

to

<form method="post" action="{{ (base_url_simple ~ uri.path)|e }}" class="{{ form_outer_classes }} login-form">
                                          ^^^^--- here change

(base_url_simple doesn't append language prefix in url at the beginning)
but it didn't changed anything in result.

2. Found some pending PR changes

Looks related: #3004

I've applied left changes (PR base looks old / some changes looks already applied)

diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php
index 97f6e18..b0af5b6 100644
--- a/system/src/Grav/Common/Page/Pages.php
+++ b/system/src/Grav/Common/Page/Pages.php
@@ -1001,11 +1001,6 @@ class Pages
         /** @var Config $config */
         $config = $this->grav['config'];

-        /** @var Uri $uri */
-        $uri = $this->grav['uri'];
-        /** @var \Grav\Framework\Uri\Uri $source_url */
-        $source_url = $uri->uri(false);
-
         // Try Regex style redirects
         $site_redirects = $config->get('site.redirects');
         if (is_array($site_redirects)) {
@@ -1014,8 +1009,8 @@ class Pages
                 $pattern = '#^' . str_replace('/', '\/', $pattern) . '#';
                 try {
                     /** @var string $found */
-                    $found = preg_replace($pattern, $replace, $source_url);
-                    if ($found && $found !== $source_url) {
+                    $found = preg_replace($pattern, $replace, $route);
+                    if ($found && $found !== $route) {
                         $this->grav->redirectLangSafe($found);
                     }
                 } catch (ErrorException $e) {

Versions

grav: latest, 1.7.18
Login plugin: latest, v3.4.4

@dexterxx-pl
Copy link
Author

Looks like I've managed to fix it.

diff --git a/user/plugins/login/classes/Controller.php b/user/plugins/login/classes/Controller.php
index 54fe930..8c7e2e0 100644
--- a/user/plugins/login/classes/Controller.php
+++ b/user/plugins/login/classes/Controller.php
@@ -140,13 +140,13 @@ class Controller

                 $event->defRedirect(
                     $this->grav['session']->redirect_after_login ?:
-                        $login_redirect ?: $this->grav['uri']->referrer('/')
+                        $login_redirect ?: $this->grav['uri']->path()
                 );
             } else {
                 $redirect_to_login = $this->grav['config']->get('plugins.login.redirect_to_login');
                 $login_route = $this->grav['config']->get('plugins.login.route');
                 $redirect_route = $redirect_to_login && $login_route ? $login_route : false;
-                $event->defRedirect($redirect_route ?: $this->grav['uri']->referrer('/'));
+                $event->defRedirect($redirect_route ?: $this->grav['uri']->path());
             }
         } else {
             if ($user->authorized) {

Else path was not tested by me / it not fired on my second testuser without my custom grad - I've not tested and analyzed it more.

Please review, apply on your side and test it too :-). Also someone might PR it after sure that change is fine.

@mahagr
Copy link
Member

mahagr commented Sep 20, 2021

Yeah, I did run into this same issue, basically $uri->referrer() returns the whole path, not relative to the current Grav URL base.

PS. your code doesn't return to the previous page, but to the login page.

@mahagr
Copy link
Member

mahagr commented Sep 20, 2021

@dexterxx-pl Can you test the fix 5593327?

You will also need a commit in login: getgrav/grav-plugin-login@a171854

@mahagr mahagr self-assigned this Sep 20, 2021
mahagr added a commit that referenced this issue Sep 20, 2021
@dexterxx-pl
Copy link
Author

dexterxx-pl commented Sep 20, 2021

@mahagr Yup, tested at least with my case -- Works better ;-).

@mahagr
Copy link
Member

mahagr commented Sep 20, 2021

I have even an improved version, but it's not hooked up yet.

@mahagr mahagr closed this as completed Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants