Unauthenticated users are not redirected to /login about grocy-docker

[berrnd]

So this seems to be a specific problem of the Docker container (grocy.env is nothing grocy uses itself).

I can't reproduce this (for MODE = production - anything else should not be used for this setting, if you want to disable authentication, use DISABLE_AUTH).

Just as a reference, the redirect is done here - so always, when the session could not be validated, regardless which page was requested.

Feel free to use my private grocy instance to test this, I've found no path which doesn't redirect to /login so far...
https://grocy.berrnd.org/choresoverview
https://grocy.berrnd.org/products
...
(Note that there are some feature flags disabled on this instance, therefore e. g. https://grocy.berrnd.org/tasks results in an 404 error instead of redirecting).

(I'm moving this issue to grocy/grocy-docker.)

[jayaddison]

@tet3 Do you mind updating this issue description to link to grocy.env as-it-was when you opened the issue, just for anyone gathering context in future?

That should be https://github.com/grocy/grocy-docker/blob/d5163459e9211010167834254847eb62a8241fe2/grocy.env as best I can tell

(NB: it looks like I could technically edit your description myself but I figure it's probably best to edit our own comments & descriptions unless really necessary)

[jayaddison]

Currently investigating the root cause here.

[tet3]

Edited. While not understanding the root cause is always frustrating, since there seem to be other advantages to the work done on #67 and that fixes it, I'd just go with that, myself. :-)
I got that branch up and running on my VPS last night, though I have to sort out why the SSL isn't working.

[jayaddison]

@berrnd I've nearly tracked down the cause here - certainly there's enough information that I can pinpoint a potential security issue in, and have a proposed fix.

The issue comes down to what content should be considered permissible for non-authenticated users to view. In general for a production system it's likely sensible to deny read access (i.e. redirect to login before rendering any content) for non-authenticated users, since that seems to be the intent of the middleware code.

Although I've pinpointed a fix in the application code, I'm still digging further to determine why this particular pull request alters the behaviour.

[berrnd]

Thanks, I see - the rendered originally requested page is rendered in addition to the 302 redirect (browsers, including the developer tools, seem to not show the response body of a 302 response).

Should be easy to fix, I think we should do this immediately (today).

[berrnd]

We do this also already for the API, here:

https://github.com/grocy/grocy/blob/11bf89e13a7c0b356910a9a39514b80c0a6f2eb2/middleware/ApiKeyAuthMiddleware.php#L82

(Sorry for the off-topic comments here...)

[berrnd]

I've created grocy/grocy#696 to track and fix the security issue - thanks a lot for finding that!