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

[BUG] "External Path" incorrectly cached #5807

Closed
1 of 3 tasks
SlavikCA opened this issue Dec 17, 2023 · 4 comments
Closed
1 of 3 tasks

[BUG] "External Path" incorrectly cached #5807

SlavikCA opened this issue Dec 17, 2023 · 4 comments

Comments

@SlavikCA
Copy link
Contributor

SlavikCA commented Dec 17, 2023

The bug

When user updates the "External Path", it shows incorrect (old) value in the ACCOUNT SETTINGS

The OS that Immich Server is running on

Docker on Synology

Version of Immich Server

1.91.3

Version of Immich Mobile App

n/a

Platform with the issue

  • Server
  • Web
  • Mobile

Your docker-compose.yml content

n/a

Your .env content

n/a

Reproduction steps

In the web app:
1. Click ADMINISTRATION -> User -> Edit button (the pen icon)
2. Update the value in the EXTERNAL PATH field
3. Click CONFIRM
4. Click Circle Icon in the top-right, -> ACCOUNT SETTINGS
5. Expand Account

Error:
* The value EXTERNAL PATH is incorrect. It shows the old value.

If user click the browser button REFRESH (F5 or Ctrl-R), then the webpage will show correct value for the EXTERNAL PATH
@SlavikCA
Copy link
Contributor Author

Also, it's confusing that EXTERNAL PATH entry present in both screens:

  1. ADMINISTRATION -> Users -> Action
  2. USER -> Account setting -> Account (read only here)

When I wanted to change EXTERNAL PATH, I found it in the USER -> ACCOUNT SETTING and was really struggling to figure out why I can't edit it? Why it's read only?

It took me half an hour to figure out, that it's also available at the other location: ADMINISTRATION -> USERS.

Can it somehow be consolidated, so it's only present in one place?

@jrasm91
Copy link
Contributor

jrasm91 commented Dec 18, 2023

I actually don't know if we even need to show it on any screen except the admin one tbh.

@bhugh
Copy link

bhugh commented Jan 11, 2024

I came in to say something similar, so I'll just add it here:

  • It is confusing that we need to set the external library path (or portions of it, anyway) in BOTH the User/Settings/Libraries and Administration/Edit User/External Path

  • I understand the logic of it: The admin sets a LIMIT on where the path can be (ie, a parent directory which the user must remain inside), whereas the individual user then selects a directory (or directories, or perhaps several sets of directories for different libraries) from within the parent directory that the admin has set.

  • As explained in the documentation, this is to head off "path traversal attacks" whereby users can gain access to directories they shouldn't have access to, by using paths like ../../../../var/passwords

So there is a reason for it, but it is just plain confusing. I spent 45 minutes trying to figure out my external libraries TWICE now. Because it just doesn't make sense to set it up in two disparate places.

Suggestions:

    1. (easiest) definitely update the documentation to clearly lay out, step by step, that you must 1. Have an ADMIN go to Administration/User Settings (for this user) and set up the allowed External Path folder. 2. The USER then goes to Settings/directories and sets up an external directory which must be WITHIN the folder the ADMIN has set. 3. Save thew new External Library folder. 4. Hit "Scan All Libraries". 5. Check Admin/Jobs/Library and you should see at least one job running under "Library". If you don't see that, something is wrong and your library is not working. 6. You can also watch the logs of microservices, which will give an error when you hit "Scan All Libraries" if you don't have a working library path.
    1. (2nd easiest) Give a message to the user when the external library path is entered, telling them clearly whether or not it succeeded. If it didn't succeed, nudge them what to do to solve it: 1. Make sure your path is a real/correct path that exists within the container, 2. Make sure an Admin has set up your External Path folder correctly (ie, not blank), 3. Make sure your library folder is within that path." Perhaps give a specific error message depending on what exactly went wrong?
    1. Perhaps set the default Administration/User Settings/External Path for each user to be /usr/src/app/external - the Admin can go and make it even more restrictive if desired. But from the git-go at least we have a simple, working, and (reasonably) secure setup that will work for most situations.
    1. Why does the User have to enter the entire path /usr/src/app/external/Parent_Directory_Set_By_Admin/My_Desired_Library_Directory? Their path "jail" is set by the Admin, so surely they can just enter a path starting from what the Admin has entered. In this case, it can be simply "My_Desired_Library_Directory".
    1. It is confusing to call both the path that the Admin sets and that one the User sets very similar things. They are both, basically, "External Path". (The user's is "External Library Path" I guess. Whatever - it's confusingly similar. So they need better names - and names that are clearly distinct. I don't know what, though! Maybe the one the admin sets is "Home Library Folder for the User's External Libraries" and the User's is "External Library Folder" (and including a note or something making it clear that the path they give is relative to the User's Home Library Folder set by the Admin).
    1. When the Admin sets the User's Home Library Folder AND when the User chooses their specific External Library Path within that folder, both need to receive immediate feedback as to whether what they set is correct and works. So errors like "That directory does not exist," "You do not have permission to access that directory," "That directory contains no image files," "Success! You have selected a directory that contains image files" etc. Maybe even give a quick count of images found - even if it is just "this Library Path contains NO images," "over 100 images", "over 500 images" or similar.

Of course it would be nice to pop up with a select box of available directories so that the user can select the directory without the possibility of mis-typing. Or perhaps just give a list of available directories within the User's Home Folder so that they at least have SOME clue about what would be a correct thing to type. But there might be security ramifications to these things, and the whole concept of an External Library is a more or less technical thing that users/admins must set up on the back end. So I think it is OK that it is not 100% "user friendly for complete dummies". The whole point of the External Library is that you must have access to the machine, and directories within it, to put the library files there. So it's always going to take some admin type setup, and it's OK that it is not totally idiot proof, so to speak. But I think users do need feedback that at least they have set up something that is going to work.

    1. Thinking it through, here is what I think the real solution to this is - and one that is only a little more complicated that what already exists, but will make more sense to both users & admins.

First, we realize that giving users access to a directory within the system - in the way we must do to make the External Library concept work - is DEFINITELY a security risk. So . . . this rather powerful permission should not be granted to every user by default.

Rather, it should be turned OFF for all users by default. And when it is turned off in the admin panel (where there is an on/off toggle), the External Library option does not even appear in the User's settings. Maybe there is, instead, a note that if they would like to use an External Library they should contact the Admin, who can grant them the required permission under Admin/User Setting/Allow External Libraries.

This completely avoids the confusing situation we have now, where the user goes in & sets up a perfectly valid External Library Folder but it doesn't work because another obscure admin setting somewhere is not properly set up.

Then when the Admin goes into Admin/User Settings to click the toggle to Allow External Libraries for that user, that is when the Home Directory for that user pre-populates with "/usr/src/app/external" and prompts the admin to further restrict the home directory of the user, from that starting point, if so desired.

(Note that "/usr/src/app/external" is a directory that is set up by the admin when the docker volume is set up. So it can be changed and vary in each install. So the default entry here should not be "/usr/src/app/external" but rather, whatever directory is set up as the root of the external directories within this particular installation. Hopefully that won't be too difficult to customize?)

And once that is done, the option for External Libraries appears on the User's settings page, allowing the user to set the External Library up. The prompt for them to do so includes the path the Admin has set up with the place for subdirectories they can add to that base clearly indicated - so something like Choose External Library folder: /usr/src/app/external/Jones_Family/Fred/[ . . . . . . . . ] The text entry field, which the user fills in, is only the part within the brackets.

That way the user can see the Base Directory the Admin has set up for them, and they can also see exactly which part of the directory path they need to fill in to get the directory they wish.

My $.02! Thanks for a great piece of software!

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 22, 2024

External path has been removed in favor of admin management

@jrasm91 jrasm91 closed this as completed Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants