fix: restore SCRAM as the default auth method over TLS (#1249)

config/runtime.exs

@@ -271,7 +271,8 @@ if config_env() != :test do
     "app_name_metric" => Supavisor.Helpers.get_env_bool("APP_NAME_METRIC_ENABLED", false),
     "check_simple_query_prepare" =>
       Supavisor.Helpers.get_env_bool("CHECK_SIMPLE_QUERY_PREPARE_ENABLED", false),
-    "scram_auth_over_tls" => Supavisor.Helpers.get_env_bool("SCRAM_AUTH_OVER_TLS_ENABLED", false)
+    "cleartext_auth_over_tls" =>
+      Supavisor.Helpers.get_env_bool("CLEARTEXT_AUTH_OVER_TLS_ENABLED", false)
   }
 
   config :supavisor, Supavisor.Repo,

docs/configuration/env.md

@@ -46,7 +46,7 @@ This document provides a categorized list of all environment variables used in S
 | `NAMED_PREPARED_STATEMENTS_ENABLED` | Enable named prepared statements feature (true, false, 1, 0) | `false`       | No       |
 | `APP_NAME_METRIC_ENABLED`           | Add client `app_name` label to connection metrics (true, false, 1, 0) | `false` | No       |
 | `CHECK_SIMPLE_QUERY_PREPARE_ENABLED` | Reject `PREPARE`/`EXECUTE`/`DEALLOCATE` sent over the simple query protocol in transaction mode. Parses every simple query, so it adds latency proportional to query size (true, false, 1, 0) | `false` | No       |
-| `SCRAM_AUTH_OVER_TLS_ENABLED`       | Use SCRAM instead of cleartext password authentication for TLS clients (true, false, 1, 0) | `false` | No       |
+| `CLEARTEXT_AUTH_OVER_TLS_ENABLED`   | Use cleartext password instead of SCRAM authentication for TLS clients (true, false, 1, 0) | `false` | No       |
 
 ---
 

lib/supavisor/client_handler/auth_methods.ex

@@ -13,8 +13,8 @@ defmodule Supavisor.ClientHandler.AuthMethods do
   When `client_jit` is true (client passed `--jit=true` in options) and the tenant
   has `use_jit` enabled, returns `:jit` to route to the dedicated JIT auth module.
 
-  Over TLS, cleartext password authentication is used unless the
-  `scram_auth_over_tls` feature flag is enabled, in which case SCRAM is used.
+  Over TLS, SCRAM is used unless the `cleartext_auth_over_tls` feature flag
+  is enabled, in which case cleartext password authentication is used.
   """
   @spec fetch_authentication_method(
           Supavisor.Tenants.Tenant.t(),
@@ -33,8 +33,8 @@ defmodule Supavisor.ClientHandler.AuthMethods do
   end
 
   defp tls_authentication_method(tenant) do
-    if FeatureFlag.enabled?(tenant, "scram_auth_over_tls"),
-      do: :scram_sha_256,
-      else: :password
+    if FeatureFlag.enabled?(tenant, "cleartext_auth_over_tls"),
+      do: :password,
+      else: :scram_sha_256
   end
 end

priv/repo/seeds_after_migration.exs

@@ -10,29 +10,36 @@ if Tenants.get_tenant_by_external_id(tenant_name) do
   Tenants.delete_tenant_by_external_id(tenant_name)
 end
 
-if !Tenants.get_tenant_by_external_id("is_manager") do
-  {:ok, _} =
-    %{
-      db_host: db_conf[:hostname],
-      db_port: db_conf[:port],
-      db_database: db_conf[:database],
-      default_parameter_status: %{},
-      external_id: "is_manager",
-      require_user: false,
-      auth_query: "SELECT rolname, rolpassword FROM pg_authid WHERE rolname=$1;",
-      users: [
-        %{
-          "db_user" => db_conf[:username],
-          "db_password" => db_conf[:password],
-          "pool_size" => 2,
-          "mode_type" => "transaction",
-          "is_manager" => true,
-          "pool_checkout_timeout" => 1000
-        }
-      ]
-    }
-    |> Tenants.create_tenant()
-end
+[
+  {"is_manager", %{}},
+  {"is_manager_cleartext", %{"cleartext_auth_over_tls" => true}}
+]
+|> Enum.each(fn {tenant, feature_flags} ->
+  if !Tenants.get_tenant_by_external_id(tenant) do
+    {:ok, _} =
+      %{
+        db_host: db_conf[:hostname],
+        db_port: db_conf[:port],
+        db_database: db_conf[:database],
+        default_parameter_status: %{},
+        external_id: tenant,
+        require_user: false,
+        auth_query: "SELECT rolname, rolpassword FROM pg_authid WHERE rolname=$1;",
+        feature_flags: feature_flags,
+        users: [
+          %{
+            "db_user" => db_conf[:username],
+            "db_password" => db_conf[:password],
+            "pool_size" => 2,
+            "mode_type" => "transaction",
+            "is_manager" => true,
+            "pool_checkout_timeout" => 1000
+          }
+        ]
+      }
+      |> Tenants.create_tenant()
+  end
+end)
 
 [
   "proxy_tenant1",

test/integration/protocol_integration_test.exs

@@ -234,6 +234,7 @@ defmodule Supavisor.Integration.ProtocolIntegrationTest do
     @password String.duplicate("！", 8)
     @role "fullwidth_pw_user"
     @tenant "is_manager"
+    @cleartext_tenant "is_manager_cleartext"
 
     setup do
       Supavisor.Support.SSLHelper.setup_downstream_certs()
@@ -280,7 +281,7 @@ defmodule Supavisor.Integration.ProtocolIntegrationTest do
       {:ok, "S"} = :gen_tcp.recv(tcp, 1, 5000)
       {:ok, ssl} = :ssl.connect(tcp, [verify: :verify_none, active: false], 5000)
 
-      send_startup(:ssl, ssl, ctx)
+      send_startup(:ssl, ssl, %{ctx | username: "#{@role}.#{@cleartext_tenant}"})
 
       {:ok, <<?R, _::32, 3::32>>} = :ssl.recv(ssl, 0, 5000)
 

test/integration/proxy_test.exs

@@ -726,7 +726,7 @@ defmodule Supavisor.Integration.ProxyTest do
 
   test "change role password with cleartext auth retries against new secrets" do
     SSLHelper.setup_downstream_certs()
-    %{origin: origin, db_conf: db_conf} = setup_tenant_connections("is_manager")
+    %{origin: origin, db_conf: db_conf} = setup_tenant_connections("is_manager_cleartext")
 
     on_exit(fn ->
       {:ok, origin} =
@@ -746,7 +746,7 @@ defmodule Supavisor.Integration.ProxyTest do
         hostname: db_conf[:hostname],
         port: Application.get_env(:supavisor, :proxy_port_transaction),
         database: db_conf[:database],
-        username: "dev_postgres_password_test.is_manager",
+        username: "dev_postgres_password_test.is_manager_cleartext",
         password: password,
         ssl: [verify: :verify_none]
       ]

test/supavisor/client_handler/auth_methods_test.exs

@@ -13,35 +13,35 @@ defmodule Supavisor.ClientHandler.AuthMethodsTest do
                {:ok, :scram_sha_256}
     end
 
-    test "uses cleartext password over TLS by default" do
+    test "uses SCRAM over TLS by default" do
       tenant = %Tenant{use_jit: false, feature_flags: %{}}
 
       assert AuthMethods.fetch_authentication_method(tenant, false, true, "user") ==
-               {:ok, :password}
+               {:ok, :scram_sha_256}
     end
 
-    test "uses SCRAM over TLS when scram_auth_over_tls is enabled for the tenant" do
-      tenant = %Tenant{use_jit: false, feature_flags: %{"scram_auth_over_tls" => true}}
+    test "uses cleartext password over TLS when cleartext_auth_over_tls is enabled for the tenant" do
+      tenant = %Tenant{use_jit: false, feature_flags: %{"cleartext_auth_over_tls" => true}}
 
       assert AuthMethods.fetch_authentication_method(tenant, false, true, "user") ==
-               {:ok, :scram_sha_256}
+               {:ok, :password}
     end
 
-    test "uses SCRAM over TLS when scram_auth_over_tls is enabled globally" do
+    test "uses cleartext password over TLS when cleartext_auth_over_tls is enabled globally" do
       original = Application.get_env(:supavisor, Supavisor.FeatureFlag, %{})
 
       Application.put_env(
         :supavisor,
         Supavisor.FeatureFlag,
-        Map.put(original, "scram_auth_over_tls", true)
+        Map.put(original, "cleartext_auth_over_tls", true)
       )
 
       on_exit(fn -> Application.put_env(:supavisor, Supavisor.FeatureFlag, original) end)
 
       tenant = %Tenant{use_jit: false, feature_flags: %{}}
 
       assert AuthMethods.fetch_authentication_method(tenant, false, true, "user") ==
-               {:ok, :scram_sha_256}
+               {:ok, :password}
     end
 
     test "tenant flag overrides the global setting" do
@@ -50,19 +50,19 @@ defmodule Supavisor.ClientHandler.AuthMethodsTest do
       Application.put_env(
         :supavisor,
         Supavisor.FeatureFlag,
-        Map.put(original, "scram_auth_over_tls", true)
+        Map.put(original, "cleartext_auth_over_tls", true)
       )
 
       on_exit(fn -> Application.put_env(:supavisor, Supavisor.FeatureFlag, original) end)
 
-      tenant = %Tenant{use_jit: false, feature_flags: %{"scram_auth_over_tls" => false}}
+      tenant = %Tenant{use_jit: false, feature_flags: %{"cleartext_auth_over_tls" => false}}
 
       assert AuthMethods.fetch_authentication_method(tenant, false, true, "user") ==
-               {:ok, :password}
+               {:ok, :scram_sha_256}
     end
 
-    test "uses JIT over TLS regardless of scram_auth_over_tls" do
-      tenant = %Tenant{use_jit: true, feature_flags: %{"scram_auth_over_tls" => true}}
+    test "uses JIT over TLS regardless of cleartext_auth_over_tls" do
+      tenant = %Tenant{use_jit: true, feature_flags: %{"cleartext_auth_over_tls" => true}}
 
       assert AuthMethods.fetch_authentication_method(tenant, true, true, "user") ==
                {:ok, :jit}