Web url too may redirects

bluehash commented 2 years ago

Hello, Thank you for an amazing wish list manager.

I have mine on a docker container which works great when I access via local_ip:port number

In order to get this online, I registered with no-ip and have my router register via dynamic dnd (ddns) and forward the port.

So now I have a url like no-ip-url:port-no/wishlist. This works great once on the login page and then no longer works with "too many redirects". However the local ip method works.

Is this setup sound right or am I missing a step.

Thank you!

It

Wingysam commented 2 years ago

Could you check the network tab of the browser's developer tools please? What is it redirecting to?

bluehash commented 2 years ago

It magically works now. I'll reopen this if I find anything.

Wingysam commented 2 years ago

Great, probably a caching issue.

bluehash commented 2 years ago

Hello, I'm reopening this as it has come back. I may have a way of reproducing this. How do I send over a network log without sensitive data?

bluehash commented 2 years ago

I'm running the latest docker image.

This is how I can reproduce it:

Clear all cookies.
Pick a wishlist url: http://192.168.1.XXX:XXXX/wishlist/username
Go directly to the url
Redirected login
Login
Web url too many redirects issue.

Wingysam commented 2 years ago

Thank you, that should help with finding the issue.

Wingysam commented 1 year ago

Appears to be related to Passport. Will be fixed by #25.

TheTechmage commented 9 months ago

I did some investigation, @Wingysam. This does not appear to be related to Passport, but instead is related to express-session. If I create an anonymous function within app.use like the following code, as well as remove passport, I am still seeing the problem arise.

app.use((req, res, next) => {
  console.log(`Cookie Data: ${JSON.stringify(req.session.cookie.data)}`);
  req.session["testing"] = {};
  req.session["testing"].test = true;
  req.session.save(err => console.error(err));
  next()
});

When express-session is saving cookies, none of the options (such as path or max-age) are being set on the cookie headers themselves. What baffles me is that the log line there shows that the options are set in the right location and it's still not working.

Now, to describe the issue at hand and explain part of the why login errors are occurring:

A web request is made to /wishlist/user
The user is given a cookie and redirected to /login
- The path of the cookie is unset, so the browser sets the cookie to /wishlist since that's the "deepest folder"
The user lands on /login and is granted a cookie (since they "don't have one" already, path mismatch against /wishlist)
- The path of the cookie is unset, so the browser sets the cookie to / since that's the "deepest folder"
The user successfully logs in
- Login path is /login, so the new cookie that says we're logged in goes to /
- The user is redirected to /wishlist
- The unauthenticated cookie at /wishlist takes priority over the one set for /, so we use that one in req.session
- According to the cookie, the user is unauthenticated, redirect them to /login
- Upon redirection, we note that according to the most-relevant cookie at /, the user is logged in. Redirect them to /wishlist

And that's how we get the redirect loop. Even if you switched away from Passport, so long as express-session is being used, I believe that this would still be an issue and I don't think that #25 will fix it.

To work around the problem on my own setup, I have added the following middleware piece. This snippet tells the browser to delete the cookie that was set on the /wishlist path, if it is an unauthenticated cookie.

app.use((req, res, next) => {
  if(!req.session.passport || Object.keys(req.session.passport).length === 0)
    res.clearCookie('christmas_community.connect.sid', {path: '/wishlist'});
  next();
});

Hope this helps a bit!