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.

Sign up for GitHub

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

Jump to bottom

Nextcloud: add openFirewall setting #50256

Closed
wants to merge 2 commits into from
Closed

Nextcloud: add openFirewall setting #50256

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ba26d2
nextcloud: add an openFirewall boolean
teto Nov 12, 2018
bc11a5d
nextcloud: improve doc around hostName
teto Nov 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions nixos/modules/services/web-apps/nextcloud.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ in {
enable = mkEnableOption "nextcloud";
hostName = mkOption {
type = types.str;
description = "FQDN for the nextcloud instance.";
description = ''
FQDN for the nextcloud instance. Automatically added to
<literal>extraTrustedDomains</literal> are accepted.
'';
};
home = mkOption {
type = types.str;
Expand All @@ -50,6 +53,13 @@ in {
default = false;
description = "Enable if there is a TLS terminating proxy in front of nextcloud.";
};
openFirewall = mkOption {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may be better placed in services.nginx, as it would open the firewall for all of nginx, and not only for opensmtpd.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would make sense th have the setting for both modules (with nextcloud passing it to nginx if nginx is enabled). Now if that slows evaluation, that looks bad xD

type = types.bool;
default = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we don't want openFirewall to default to true: sshd is the exception to the rule, and while it could make sense to provide the openFirewall option, having it defaulting to opening the firewall would be unexpected and potentially a source of security issues (because people wouldn't expect their nextcloud to be opened to the whole internet).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad I definitely wanted to set it to false but forgot.

description = ''
Whether to automatically open the specified ports in the firewall.
'';
};

maxUploadSize = mkOption {
default = "512M";
Expand Down Expand Up @@ -209,8 +219,8 @@ in {
default = [];
description = ''
Trusted domains, from which the nextcloud installation will be
acessible. You don't need to add
<literal>services.nextcloud.hostname</literal> here.
acessible. You don't need to add either localhost or
<literal>services.nextcloud.hostName</literal> here.
'';
};
};
Expand Down Expand Up @@ -256,6 +266,10 @@ in {
];
}

{
networking.firewall.allowedTCPPorts = if cfg.openFirewall then [ 80 443 ] else [];
}

{ systemd.timers."nextcloud-cron" = {
wantedBy = [ "timers.target" ];
timerConfig.OnBootSec = "5m";
Expand Down