Logout: fix redirect #3304
Logout: fix redirect #3304
Conversation
|
I don't see a loop but I do see a long delay until the logout finally succeeds. This seems to be a (Firefox?) browser issue as FTL sees the logout request immediately, terminates the session accordingly and replies with 204. This can be seen in the Network Overview (Dev Tools) tab, what we can also see is that the browser then hangs trying to (re)load the page to get to the login page. However, this request never arrives at the web server (confirmed by an independent However, I don't think the first approach in this PR (static redirect) is the right solution. Mostly because of #3269 where we are currently trying to get rid of any hard-coded paths very like the one you are adding here. But if this is really the only option here, I will find a solution for #3269 and we don't have to worry about it here. |
Agree. No issues to logout using Chrome, this is instant. But Firefox loops |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
i did some testing with the development branch in this regard (logout loop with firefox) If the tab with Pi-hole is the focus, the page loops forever. If the tab with Pi-hole is not the focus, the logout succeeds quicker (not as fast as chrome). (click logout then switch to a different tab) also worth noting is the logout happens without a loop, still not as fast chrome, when deleting the SID from this page https://pi.hole/admin/settings/api for the current session (highlighted) |
|
Conflicts have been resolved. |
|
Yeah that's exactly what I suggested to Dominik on matter most.
…On Sun, Mar 9, 2025, 20:25 RD WebDesign ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/js/utils.js
<#3304 (comment)>:
> @@ -468,7 +468,7 @@ function doLogout() {
url: "/api/auth",
method: "DELETE",
}).always(function () {
- location.reload();
+ globalThis.location = "/admin/login";
We could probably hack something around and pass the URL
If this is really needed, we can use pihole.webhome() in Lua code to set
an attribute in any HTML tag.
Maybe set data-webhome="<?= pihole.webhome() ?>" in <body> tag (or any
other attribute name).
—
Reply to this email directly, view it on GitHub
<#3304 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACVLNIQMJOIURWV4TW7ZWL2TSBQHAVCNFSM6AAAAABYT5ISQWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNRZGQ4TCMJVGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
XhmikosR
force-pushed
the
patch-3
branch
3 times, most recently
from
ca1f186 to
345d271
Compare
|
Login might take a long time on Firefox too. There might be more places that we need to be explicit with the For example, logout and then manually change the URL in the browser to something that requires authentication, like |
|
I agree with what this PR is doing and it tests okay. However, I wonder if @rdwebdesign is right and we should instead add this here: web/scripts/lua/header_authenticated.lp Line 25 in e25554d Just in case we want/need to reuse it in the future. |
|
I don't mind moving it to |
|
My main concern is that there might be more places this might be needed, see my previous comment. |
Without this, we were getting into a loop with keep alive enabled when using Firefox. Signed-off-by: XhmikosR <[email protected]>
Without this, we were getting into a loop with keep alive enabled
/CC @DL6ER to test it out and squash when merging if it LGTY.
Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
What does this PR aim to accomplish?:
How does this PR accomplish the above?:
Link documentation PRs if any are needed to support this PR:
By submitting this pull request, I confirm the following:
git rebase)