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

Module/options request: filebrowser #236165

Open
nielsegberts opened this issue Jun 5, 2023 · 4 comments
Open

Module/options request: filebrowser #236165

nielsegberts opened this issue Jun 5, 2023 · 4 comments
Assignees
Labels
8.has: module (new) This PR adds a module in `nixos/`

Comments

@nielsegberts
Copy link
Contributor

Project description

This is a continuation of #122339 to add options to easily set up File Browser now that it's packaged.

I've started development with this commit:

nielsegberts@6d39a0b

@nielsegberts nielsegberts added the 0.kind: packaging request Request for a new package to be added label Jun 5, 2023
@nielsegberts nielsegberts self-assigned this Jun 5, 2023
@nielsegberts nielsegberts added 8.has: module (new) This PR adds a module in `nixos/` and removed 0.kind: packaging request Request for a new package to be added labels Jun 5, 2023
@kraftnix
Copy link

I've tested your module and it appears to be running correctly. I would support you submitting a PR to upstream this.

The only comment I have on the implementation is that the services.filebrowser.baseUrl option example is misleading/incorrect, since baseurl upstream seems to actually be a base url path like /filebrowser rather than a FQDN like the example suggests (there doesn't appear to be documentation for what baseurl actually does in the upstream docs other than "base url").

When I set baseUrl to a FQDN like the example suggests, the filebrowser UI was attempting to make requests to https://filebrowser.mydomain.com/filebrowser.mydomain.com, setting this to "" fixed the issue.

@cimm
Copy link
Contributor

cimm commented Apr 21, 2024

I was annoyed by the lack of options as well and created my own File Browser module over the weekend: cimm@92285eb

Only then did I notice this GitHub issue. I should have searched here before spending the time redoing the same work. 🤦‍♂️ Ah, well, I learned something in the process and took a slightly different approach. I noticed this issue is almost a year old, so have there been any attempts to get it merged?

  • I went for the DynamicUser = "yes"; approach in the systemd unit. It doesn't need a user, but more importantly, it sandboxes the service. This should improve security but also means the dataDir and database locations are limited to the /var/lib/filebrowser path. I'm not claiming this is better; I never wrote a Nix module before, but it might be interesting to discuss?
  • I added all the command line options, maybe this is overkill?
  • I escapeShellArg all string options to make sure special characters don't break the command.
  • I also added a simple test to make sure the service is up.

@pinpox
Copy link
Member

pinpox commented Apr 22, 2024

Only then did I notice this GitHub issue. I should have searched here before spending the time redoing the same work.

I haven't looked much into this lately, but if you have improvements to the module I would be awesome if you can make a PR. Feel free to ping me for review and testing.

@cimm
Copy link
Contributor

cimm commented Apr 22, 2024

When creating the pull request, I noticed there was already a pending pull request: #289750. I won't open another one, better focus our efforts on the pending one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: module (new) This PR adds a module in `nixos/`
Projects
None yet
Development

No branches or pull requests

4 participants