diff --git a/.credo.exs b/.credo.exs index 17e9431a7..7f6eff047 100644 --- a/.credo.exs +++ b/.credo.exs @@ -118,7 +118,7 @@ # {Credo.Check.Refactor.Apply, []}, {Credo.Check.Refactor.CondStatements, []}, - {Credo.Check.Refactor.CyclomaticComplexity, []}, + {Credo.Check.Refactor.CyclomaticComplexity, [max_complexity: 12]}, {Credo.Check.Refactor.FunctionArity, []}, {Credo.Check.Refactor.LongQuoteBlocks, []}, {Credo.Check.Refactor.MatchInCondition, []}, diff --git a/apps/fz_http/lib/fz_http/auth.ex b/apps/fz_http/lib/fz_http/auth.ex index b04eb7629..a14566eb6 100644 --- a/apps/fz_http/lib/fz_http/auth.ex +++ b/apps/fz_http/lib/fz_http/auth.ex @@ -3,8 +3,13 @@ defmodule FzHttp.Auth do def fetch_oidc_provider_config(provider_id) do with {:ok, provider} <- fetch_provider(:openid_connect_providers, provider_id) do - external_url = FzHttp.Config.fetch_env!(:fz_http, :external_url) - redirect_uri = provider.redirect_uri || "#{external_url}/auth/oidc/#{provider.id}/callback/" + redirect_uri = + if provider.redirect_uri do + provider.redirect_uri + else + external_url = FzHttp.Config.fetch_env!(:fz_http, :external_url) + "#{external_url}auth/oidc/#{provider.id}/callback/" + end {:ok, %{ diff --git a/apps/fz_http/lib/fz_http/config/definitions.ex b/apps/fz_http/lib/fz_http/config/definitions.ex index 766c4f9f2..0e6522597 100644 --- a/apps/fz_http/lib/fz_http/config/definitions.ex +++ b/apps/fz_http/lib/fz_http/config/definitions.ex @@ -156,7 +156,7 @@ defmodule FzHttp.Config.Definitions do defconfig(:external_url, :string, changeset: fn changeset, key -> changeset - |> FzHttp.Validator.validate_uri(key) + |> FzHttp.Validator.validate_uri(key, require_trailing_slash: true) |> FzHttp.Validator.normalize_url(key) end ) diff --git a/apps/fz_http/lib/fz_http/validator.ex b/apps/fz_http/lib/fz_http/validator.ex index 48475b474..871cc7750 100644 --- a/apps/fz_http/lib/fz_http/validator.ex +++ b/apps/fz_http/lib/fz_http/validator.ex @@ -18,6 +18,7 @@ defmodule FzHttp.Validator do def validate_uri(changeset, field, opts \\ []) when is_atom(field) do valid_schemes = Keyword.get(opts, :schemes, ~w[http https]) + require_trailing_slash? = Keyword.get(opts, :require_trailing_slash, false) validate_change(changeset, field, fn _current_field, value -> case URI.new(value) do @@ -32,6 +33,10 @@ defmodule FzHttp.Validator do uri.scheme not in valid_schemes -> [{field, "only #{Enum.join(valid_schemes, ", ")} schemes are supported"}] + require_trailing_slash? and not is_nil(uri.path) and + not String.ends_with?(uri.path, "/") -> + [{field, "does not end with a trailing slash"}] + true -> [] end @@ -48,7 +53,7 @@ defmodule FzHttp.Validator do uri = URI.parse(value) scheme = uri.scheme || "https" port = URI.default_port(scheme) - path = uri.path || "/" + path = maybe_add_trailing_slash(uri.path || "/") uri = %{uri | scheme: scheme, port: port, path: path} uri_string = URI.to_string(uri) put_change(changeset, field, uri_string) @@ -57,6 +62,14 @@ defmodule FzHttp.Validator do end end + defp maybe_add_trailing_slash(value) do + if String.ends_with?(value, "/") do + value + else + value <> "/" + end + end + def validate_no_duplicates(changeset, field) when is_atom(field) do validate_change(changeset, field, fn _current_field, list when is_list(list) -> list diff --git a/apps/fz_http/lib/fz_http_web/live/setting_live/oidc_form_component.ex b/apps/fz_http/lib/fz_http_web/live/setting_live/oidc_form_component.ex index 9a9a62200..e3d0b305c 100644 --- a/apps/fz_http/lib/fz_http_web/live/setting_live/oidc_form_component.ex +++ b/apps/fz_http/lib/fz_http_web/live/setting_live/oidc_form_component.ex @@ -133,10 +133,7 @@ defmodule FzHttpWeb.SettingLive.OIDCFormComponent do
- <%= Path.join(
- @external_url,
- "auth/oidc/#{input_value(f, :id) || "{CONFIG_ID}"}/callback/"
- ) %>
+ <%= "#{@external_url}auth/oidc/#{input_value(f, :id) || "{CONFIG_ID}"}/callback/" %>
is used.
diff --git a/apps/fz_http/test/fz_http/auth_test.exs b/apps/fz_http/test/fz_http/auth_test.exs
index 86a8f2a5f..b362ebc46 100644
--- a/apps/fz_http/test/fz_http/auth_test.exs
+++ b/apps/fz_http/test/fz_http/auth_test.exs
@@ -36,7 +36,7 @@ defmodule FzHttp.AuthTest do
client_id: attrs["client_id"],
client_secret: attrs["client_secret"],
discovery_document_uri: attrs["discovery_document_uri"],
- redirect_uri: "http://foo.bar.com//auth/oidc/google/callback/",
+ redirect_uri: "http://foo.bar.com/auth/oidc/google/callback/",
response_type: attrs["response_type"],
scope: attrs["scope"]
}}
diff --git a/apps/fz_http/test/fz_http/config_test.exs b/apps/fz_http/test/fz_http/config_test.exs
index e0afbf1cd..33be698e9 100644
--- a/apps/fz_http/test/fz_http/config_test.exs
+++ b/apps/fz_http/test/fz_http/config_test.exs
@@ -155,6 +155,34 @@ defmodule FzHttp.ConfigTest do
eg: `https://firezone.mycorp.com/vpn`.
+ You can find more information on configuration here: https://www.firezone.dev/docs/reference/env-vars/#environment-variable-listing
+ """
+
+ assert_raise RuntimeError, message, fn ->
+ fetch_source_and_configs!([:external_url])
+ end
+ end
+
+ test "raises an error when value is invalid" do
+ put_system_env_override(:external_url, "https://example.com/vpn")
+
+ message = """
+ Invalid configuration for 'external_url' retrieved from environment variable EXTERNAL_URL.
+
+ Errors:
+
+ - `"https://example.com/vpn"`: does not end with a trailing slash
+
+ ## Documentation
+
+ The external URL the web UI will be accessible at.
+
+ Must be a valid and public FQDN for ACME SSL issuance to function.
+
+ You can add a path suffix if you want to serve firezone from a non-root path,
+ eg: `https://firezone.mycorp.com/vpn`.
+
+
You can find more information on configuration here: https://www.firezone.dev/docs/reference/env-vars/#environment-variable-listing
"""