From 254054c0c1e107f85ef75c61973bcedb789108ec Mon Sep 17 00:00:00 2001
From: Kieran <kieran.eglin@gmail.com>
Date: Mon, 4 Mar 2024 13:29:26 -0800
Subject: [PATCH] [Bugfix] Fix basic auth in deployed apps (#52)

* Renamed auth env vars and updated them to work in prod env

* Updated basic auth parsing to ignore empty strings
---
 README.md                           | 2 +-
 config/config.exs                   | 6 +++---
 config/runtime.exs                  | 4 ++++
 lib/pinchflat_web/router.ex         | 6 +++++-
 test/pinchflat_web/routing_test.exs | 9 +++++++++
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index be7a82c..c40fdf4 100644
--- a/README.md
+++ b/README.md
@@ -28,7 +28,7 @@ docker run \
 
 ## Authentication
 
-Currently HTTP basic auth is optionally supported. To use it, set the `AUTH_USERNAME` and `AUTH_PASSWORD` environment variables when starting the container. If you don't set both of these, no authentication will be required.
+Currently HTTP basic auth is optionally supported. To use it, set the `BASIC_AUTH_USERNAME` and `BASIC_AUTH_PASSWORD` environment variables when starting the container. If you don't set both of these, no authentication will be required.
 
 ## License
 
diff --git a/config/config.exs b/config/config.exs
index 617613b..4718265 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -17,10 +17,10 @@ config :pinchflat,
   # The user may or may not store metadata for their needs, but the app will always store its copy
   metadata_directory: "/config/metadata",
   tmpfile_directory: Path.join([System.tmp_dir!(), "pinchflat", "data"]),
-  # Setting AUTH_USERNAME and AUTH_PASSWORD implies you want to use basic auth.
+  # Setting BASIC_AUTH_USERNAME and BASIC_AUTH_PASSWORD implies you want to use basic auth.
   # If either is unset, basic auth will not be used.
-  basic_auth_username: System.get_env("AUTH_USERNAME"),
-  basic_auth_password: System.get_env("AUTH_PASSWORD"),
+  basic_auth_username: System.get_env("BASIC_AUTH_USERNAME"),
+  basic_auth_password: System.get_env("BASIC_AUTH_PASSWORD"),
   file_watcher_poll_interval: 1000
 
 # Configures the endpoint
diff --git a/config/runtime.exs b/config/runtime.exs
index 8fb0033..e151336 100644
--- a/config/runtime.exs
+++ b/config/runtime.exs
@@ -21,6 +21,10 @@ if System.get_env("PHX_SERVER") do
   config :pinchflat, PinchflatWeb.Endpoint, server: true
 end
 
+config :pinchflat,
+  basic_auth_username: System.get_env("BASIC_AUTH_USERNAME"),
+  basic_auth_password: System.get_env("BASIC_AUTH_PASSWORD")
+
 if config_env() == :prod do
   config_path =
     System.get_env("CONFIG_PATH") ||
diff --git a/lib/pinchflat_web/router.ex b/lib/pinchflat_web/router.ex
index 3326764..e68e799 100644
--- a/lib/pinchflat_web/router.ex
+++ b/lib/pinchflat_web/router.ex
@@ -54,10 +54,14 @@ defmodule PinchflatWeb.Router do
     username = Application.get_env(:pinchflat, :basic_auth_username)
     password = Application.get_env(:pinchflat, :basic_auth_password)
 
-    if username && password do
+    if credential_set?(username) && credential_set?(password) do
       Plug.BasicAuth.basic_auth(conn, username: username, password: password, realm: "Pinchflat")
     else
       conn
     end
   end
+
+  defp credential_set?(credential) do
+    credential && credential != ""
+  end
 end
diff --git a/test/pinchflat_web/routing_test.exs b/test/pinchflat_web/routing_test.exs
index a7ee8a8..37c047d 100644
--- a/test/pinchflat_web/routing_test.exs
+++ b/test/pinchflat_web/routing_test.exs
@@ -44,5 +44,14 @@ defmodule PinchflatWeb.RoutingTest do
 
       assert conn.status == 200
     end
+
+    test "it treats empty strings as not being set when using basic auth", %{conn: conn} do
+      Application.put_env(:pinchflat, :basic_auth_username, "")
+      Application.put_env(:pinchflat, :basic_auth_password, "pass")
+
+      conn = get(conn, "/")
+
+      assert conn.status == 200
+    end
   end
 end