Limit deps.get auth preflight and trust-gate user OAuth fallback (#1145)

Author: ericmj

lib/hex/remote_converger.ex

@@ -31,11 +31,6 @@ defmodule Hex.RemoteConverger do
 
     Registry.open()
 
-    # Check and refresh OAuth token before fetching packages
-    # This ensures we only prompt once for authentication instead of
-    # getting multiple prompts during concurrent package fetches
-    check_and_refresh_auth()
-
     # We cannot use given lock here, because all deps that are being
     # converged have been removed from the lock by Mix
     # We need the old lock to get the children of Hex packages
@@ -44,14 +39,20 @@ defmodule Hex.RemoteConverger do
     overridden = Hex.Mix.overridden_deps(deps)
     requests = Hex.Mix.deps_to_requests(deps)
 
-    [
-      Hex.Mix.packages_from_lock(lock),
-      Hex.Mix.packages_from_lock(old_lock),
-      packages_from_requests(requests)
-    ]
-    |> Enum.concat()
-    |> verify_prefetches()
-    |> Registry.prefetch()
+    prefetches =
+      [
+        Hex.Mix.packages_from_lock(lock),
+        Hex.Mix.packages_from_lock(old_lock),
+        packages_from_requests(requests)
+      ]
+      |> Enum.concat()
+      |> verify_prefetches()
+
+    # Only preflight user OAuth when one of the relevant repos would
+    # actually rely on the stored OAuth token. Public-only deps.get
+    # should not prompt just because an unrelated token expired.
+    check_and_refresh_auth(prefetches)
+    Registry.prefetch(prefetches)
 
     locked = prepare_locked(lock, old_lock, deps)
 
@@ -734,34 +735,60 @@ defmodule Hex.RemoteConverger do
     version1.major == 0 and version2.major == 0 and version1.minor != version2.minor
   end
 
-  defp check_and_refresh_auth do
-    # Try to get token with authentication prompting enabled
-    # The OnceCache ensures only one process prompts even if multiple processes
-    # detect the expired token concurrently
-    case Hex.OAuth.get_token(prompt_auth: true) do
-      {:ok, _access_token} ->
-        # Token is valid, was successfully refreshed, or user authenticated
-        :ok
+  defp check_and_refresh_auth(prefetches) do
+    if auth_preflight_required?(prefetches) do
+      # Try to get token with authentication prompting enabled
+      # The OnceCache ensures only one process prompts even if multiple processes
+      # detect the expired token concurrently
+      case Hex.OAuth.get_token(prompt_auth: true) do
+        {:ok, _access_token} ->
+          # Token is valid, was successfully refreshed, or user authenticated
+          :ok
 
-      {:error, :auth_failed} ->
-        Hex.Shell.warn(
-          "Authentication failed. Private packages will not be available. " <>
-            "Run `mix hex.user auth` to authenticate."
-        )
+        {:error, :auth_failed} ->
+          Hex.Shell.warn(
+            "Authentication failed. Private packages will not be available. " <>
+              "Run `mix hex.user auth` to authenticate."
+          )
 
-      {:error, :auth_declined} ->
-        Hex.Shell.warn(
-          "Private packages will not be available. " <>
-            "Run `mix hex.user auth` to authenticate."
-        )
+        {:error, :auth_declined} ->
+          Hex.Shell.warn(
+            "Private packages will not be available. " <>
+              "Run `mix hex.user auth` to authenticate."
+          )
 
-      {:error, :no_auth} ->
-        # No OAuth token - this is OK, user might only be fetching public packages
-        :ok
+        {:error, :no_auth} ->
+          # No OAuth token - this is OK, user might only be fetching public packages
+          :ok
 
-      {:error, _other} ->
-        # Other errors (shouldn't happen with prompt_auth: true, but handle gracefully)
-        :ok
+        {:error, _other} ->
+          # Other errors (shouldn't happen with prompt_auth: true, but handle gracefully)
+          :ok
+      end
+    else
+      :ok
     end
   end
+
+  @doc false
+  def auth_preflight_required?(prefetches) do
+    prefetches
+    |> Enum.map(fn {repo, _package} -> repo end)
+    |> Enum.uniq()
+    |> Enum.any?(&repo_requires_user_oauth?/1)
+  end
+
+  defp repo_requires_user_oauth?("hexpm:" <> _ = repo) do
+    repo
+    |> Hex.Repo.get_repo()
+    |> repo_uses_user_oauth?()
+  end
+
+  defp repo_requires_user_oauth?(_repo) do
+    false
+  end
+
+  defp repo_uses_user_oauth?(repo_config) do
+    Map.get(repo_config, :trusted, true) && !repo_config.auth_key
+  end
 end

lib/hex/repo.ex

@@ -331,8 +331,10 @@ defmodule Hex.Repo do
               raise "Failed to exchange API key for OAuth token: #{inspect(reason)}"
           end
 
-        # Third priority: fallback to OAuth token if available (from device flow or other sources)
-        true ->
+        # Third priority: fallback to OAuth token if available, but only for
+        # trusted repos. Untrusted mirrors/custom repos must not receive the
+        # user bearer token.
+        Map.get(repo_config, :trusted, true) ->
           case Hex.OAuth.get_token() do
             {:ok, access_token} ->
               # Format as Bearer token for OAuth authentication
@@ -343,6 +345,9 @@ defmodule Hex.Repo do
               # Server will return 401/403 if authentication is required
               config
           end
+
+        true ->
+          config
       end
 
     if etag do

test/hex/remote_converger_test.exs

@@ -61,6 +61,61 @@ defmodule Hex.RemoteConvergerTest do
     end
   end
 
+  defmodule PublicDepsWithExpiredOAuth.MixProject do
+    def project do
+      [
+        app: :public_deps_with_expired_oauth,
+        version: "0.1.0",
+        deps: [
+          {:postgrex, "0.2.1"}
+        ]
+      ]
+    end
+  end
+
+  defmodule OrganizationDepsWithExpiredOAuth.MixProject do
+    def project do
+      [
+        app: :organization_deps_with_expired_oauth,
+        version: "0.1.0",
+        deps: [
+          {:private_prompt_pkg, "0.1.0", repo: "hexpm:remote_converger_org"}
+        ]
+      ]
+    end
+  end
+
+  defp with_project(project, fun) do
+    Mix.Project.push(project)
+
+    try do
+      fun.()
+    after
+      Mix.Project.pop()
+    end
+  end
+
+  defp store_expired_oauth_token do
+    Hex.OnceCache.clear(Hex.OAuth.RefreshCache)
+
+    Hex.OAuth.store_token(%{
+      "access_token" => "expired_access_token",
+      "refresh_token" => "invalid_refresh_token",
+      "expires_at" => System.system_time(:second) - 3600
+    })
+  end
+
+  defp new_repo_auth_user(prefix) do
+    suffix = System.unique_integer([:positive])
+
+    Hexpm.new_user(
+      "#{prefix}_#{suffix}",
+      "#{prefix}_#{suffix}@mail.com",
+      "password",
+      "#{prefix}_#{suffix}_key"
+    )
+  end
+
   test "deps with warn_if_outdated: true and hex: option" do
     in_tmp(fn ->
       Mix.Project.push(WarnOutdatedWithHexOption.MixProject)
@@ -69,4 +124,78 @@ defmodule Hex.RemoteConvergerTest do
       assert :ok = Mix.Tasks.Deps.Get.run([])
     end)
   end
+
+  test "deps.get does not prompt for auth when only public deps are requested" do
+    in_tmp(fn ->
+      set_home_cwd()
+      store_expired_oauth_token()
+
+      with_project(PublicDepsWithExpiredOAuth.MixProject, fn ->
+        assert :ok = Mix.Tasks.Deps.Get.run([])
+      end)
+
+      refute_received {:mix_shell, :yes?, _}
+    end)
+  end
+
+  test "auth preflight is skipped for public repos" do
+    assert Hex.RemoteConverger.auth_preflight_required?([{"hexpm", "postgrex"}]) == false
+  end
+
+  test "auth preflight is required for organization repos without repo auth" do
+    in_tmp(fn ->
+      set_home_cwd()
+
+      assert Hex.RemoteConverger.auth_preflight_required?([
+               {"hexpm:remote_converger_org", "private_prompt_pkg"}
+             ])
+    end)
+  end
+
+  test "auth preflight is skipped when repo auth is already available" do
+    in_tmp(fn ->
+      set_home_cwd()
+
+      auth = new_repo_auth_user("remote_converger_repo_auth_preflight")
+
+      repos = Hex.State.fetch!(:repos)
+      repos = put_in(repos["hexpm"].auth_key, auth[:key])
+      Hex.State.put(:repos, repos)
+
+      assert Hex.RemoteConverger.auth_preflight_required?([
+               {"hexpm:remote_converger_org", "private_prompt_pkg"}
+             ]) == false
+    end)
+  end
+
+  test "auth preflight is skipped for untrusted org repos" do
+    in_tmp(fn ->
+      set_home_cwd()
+      Hex.State.put(:mirror_url, "http://example.com")
+
+      assert Hex.RemoteConverger.auth_preflight_required?([
+               {"hexpm:remote_converger_org", "private_prompt_pkg"}
+             ]) == false
+    end)
+  end
+
+  test "deps.get does not prompt when repo auth is already available" do
+    in_tmp(fn ->
+      set_home_cwd()
+
+      auth = new_repo_auth_user("remote_converger_repo_auth_deps_get")
+
+      repos = Hex.State.fetch!(:repos)
+      repos = put_in(repos["hexpm"].auth_key, auth[:key])
+      Hex.State.put(:repos, repos)
+
+      store_expired_oauth_token()
+
+      with_project(OrganizationDepsWithExpiredOAuth.MixProject, fn ->
+        assert :ok = Mix.Tasks.Deps.Get.run([])
+      end)
+
+      refute_received {:mix_shell, :yes?, _}
+    end)
+  end
 end

test/hex/repo_test.exs

@@ -56,6 +56,30 @@ defmodule Hex.RepoTest do
     assert {:ok, {404, _, "not found"}} = Hex.Repo.get_public_key(config)
   end
 
+  test "does not send OAuth token fallback to untrusted hexpm mirror" do
+    bypass = Bypass.open()
+    hexpm = Hex.Repo.default_hexpm_repo()
+
+    Hex.OAuth.store_token(%{
+      "access_token" => "device_flow_token",
+      "refresh_token" => "device_refresh",
+      "expires_at" => System.system_time(:second) + 3600
+    })
+
+    Hex.State.put(:mirror_url, "http://localhost:#{bypass.port}")
+
+    Bypass.expect(bypass, fn %Plug.Conn{request_path: "/public_key"} = conn ->
+      assert Plug.Conn.get_req_header(conn, "authorization") == []
+      Plug.Conn.resp(conn, 200, hexpm.public_key)
+    end)
+
+    repo = Hex.Repo.get_repo("hexpm")
+    assert repo.trusted == false
+
+    assert {:ok, {200, _, public_key}} = Hex.Repo.get_public_key(repo)
+    assert public_key == hexpm.public_key
+  end
+
   test "add repo persists oauth_exchange through config round-trip" do
     in_tmp(fn ->
       Hex.State.put(:config_home, File.cwd!())

test/setup_hexpm.exs

@@ -89,6 +89,9 @@ Hexpm.new_repo("testorg", auth)
 Hexpm.new_package("testorg", "foo", "0.1.0", [], pkg_meta, auth)
 Hexpm.new_package("testorg", "bar", "0.1.0", [foo: "~> 0.1.0"], pkg_meta, auth)
 
+Hexpm.new_repo("remote_converger_org", auth)
+Hexpm.new_package("remote_converger_org", "private_prompt_pkg", "0.1.0", [], pkg_meta, auth)
+
 Hexpm.new_package("hexpm", "ecto_sql", "3.3.2", [ecto: "~> 3.3.1"], pkg_meta, auth, [
   {"mix.exs", File.read!(Case.fixture_path("ecto_sql_3_3_2/mix.exs"))}
 ])