From 5cac43fe55465dd72dee5cd6577a1f9c620bc3dc Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 23 Jan 2021 15:49:40 -0500 Subject: [PATCH] refactor: fix computation test and remove the app-specific hsts value The default for an app is empty, meaning it will fallback to the global value (which defaults to true). This will allow users to properly override the global value, regardless of what it is. --- plugins/nginx-vhosts/command-functions | 4 ++-- plugins/nginx-vhosts/internal-functions | 17 ++++++++--------- plugins/nginx-vhosts/subcommands/set | 9 ++++++++- tests/unit/nginx-vhosts_6.bats | 20 ++++++++++++++++---- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/plugins/nginx-vhosts/command-functions b/plugins/nginx-vhosts/command-functions index 4a78838a2..d410de3e3 100755 --- a/plugins/nginx-vhosts/command-functions +++ b/plugins/nginx-vhosts/command-functions @@ -46,8 +46,8 @@ cmd-nginx-report-single() { "--nginx-disable-custom-config: $(fn-plugin-property-get-default "nginx" "$APP" "disable-custom-config" "false")" "--nginx-error-log-path: $(fn-nginx-error-log-path "$APP")" "--nginx-global-hsts: $(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" - "--nginx-applied-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")")" - "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true")" + "--nginx-computed-hsts: $(fn-nginx-hsts-is-enabled "$APP")" + "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "")" "--nginx-hsts-include-subdomains: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" "--nginx-hsts-max-age: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" "--nginx-hsts-preload: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-preload" "false")" diff --git a/plugins/nginx-vhosts/internal-functions b/plugins/nginx-vhosts/internal-functions index f969a00de..91e7ea528 100755 --- a/plugins/nginx-vhosts/internal-functions +++ b/plugins/nginx-vhosts/internal-functions @@ -13,21 +13,20 @@ fn-nginx-vhosts-last-visited-at() { fi } -fn-nginx-hsts-is-applied() { +fn-nginx-hsts-is-enabled() { declare APP="$1" - # if hsts is disabled globally then only apply it if an app has it set explicitly - if [[ "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" == "false" ]] && \ - [[ "$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "false")" == "false" ]]; then - echo "false"; - return; + local hsts_is_enabled="$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "")" + + if [[ "$hsts_is_enabled" == "" ]]; then + hsts_is_enabled="$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" fi - # if app has explicitly turned it off else it is default on - echo $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true"); + + echo "$hsts_is_enabled" } fn-nginx-vhosts-manage-hsts() { declare APP="$1" SSL_ENABLED="$2" - local HSTS="$(fn-nginx-hsts-is-applied "$APP")" + local HSTS="$(fn-nginx-hsts-is-enabled "$APP")" local HSTS_INCLUDE_SUBDOMAINS="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" local HSTS_MAX_AGE="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" local HSTS_PRELOAD="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-preload" "false")" diff --git a/plugins/nginx-vhosts/subcommands/set b/plugins/nginx-vhosts/subcommands/set index 34925ed99..c65fbe78f 100755 --- a/plugins/nginx-vhosts/subcommands/set +++ b/plugins/nginx-vhosts/subcommands/set @@ -10,7 +10,7 @@ cmd-nginx-set() { [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" local VALID_KEYS=("access-log-format" "access-log-path" "bind-address-ipv4" "bind-address-ipv6" "client-max-body-size" "disable-custom-config" "error-log-path" "hsts" "hsts-include-subdomains" "hsts-preload" "hsts-max-age" "proxy-read-timeout" "proxy-buffer-size" "proxy-buffering" "proxy-buffers" "proxy-busy-buffers-size" "x-forwarded-for-value" "x-forwarded-port-value" "x-forwarded-proto-value") - verify_app_name "$APP" + local GLOBAL_KEYS=("hsts") [[ -z "$KEY" ]] && dokku_log_fail "No key specified" @@ -18,6 +18,13 @@ cmd-nginx-set() { dokku_log_fail "Invalid key specified, valid keys include: access-log-format, access-log-path, bind-address-ipv4, bind-address-ipv6, client-max-body-size, disable-custom-config, error-log-path, hsts, hsts-include-subdomains, hsts-preload, hsts-max-age, proxy-read-timeout, proxy-buffer-size, proxy-buffering, proxy-buffers, proxy-busy-buffers-size, x-forwarded-for-value, x-forwarded-port-value, x-forwarded-proto-value" fi + if ! fn-in-array "$KEY" "${GLOBAL_KEYS[@]}"; then + if [[ "$APP" == "--global" ]]; then + dokku_log_fail "The key '$KEY' cannot be set globally" + fi + verify_app_name "$APP" + fi + if [[ -n "$VALUE" ]]; then dokku_log_info2_quiet "Setting ${KEY} to ${VALUE}" fn-plugin-property-write "nginx" "$APP" "$KEY" "$VALUE" diff --git a/tests/unit/nginx-vhosts_6.bats b/tests/unit/nginx-vhosts_6.bats index 824d142ea..1ce7b384f 100644 --- a/tests/unit/nginx-vhosts_6.bats +++ b/tests/unit/nginx-vhosts_6.bats @@ -119,8 +119,12 @@ teardown() { # disable hsts globally run /bin/bash -c "dokku nginx:set --global hsts false" + echo "output: $output" + echo "status: $status" + assert_success + # check it is now not applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success @@ -128,17 +132,25 @@ teardown() { # apply on app run /bin/bash -c "dokku nginx:set $TEST_APP hsts true" + echo "output: $output" + echo "status: $status" + assert_success + # check it is now applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success assert_output_contains "Enabling HSTS" 1 - # disable globally + # set global value to default run /bin/bash -c "dokku nginx:set --global hsts" + echo "output: $output" + echo "status: $status" + assert_success + # check it is still applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success