diff --git a/lib/hex/remote_converger.ex b/lib/hex/remote_converger.ex index 76314fab..c66e54b1 100644 --- a/lib/hex/remote_converger.ex +++ b/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 diff --git a/lib/hex/repo.ex b/lib/hex/repo.ex index 6de27afa..8ce400f3 100644 --- a/lib/hex/repo.ex +++ b/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 diff --git a/test/hex/remote_converger_test.exs b/test/hex/remote_converger_test.exs index a2669649..12328a5b 100644 --- a/test/hex/remote_converger_test.exs +++ b/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 diff --git a/test/hex/repo_test.exs b/test/hex/repo_test.exs index 076ae91f..925d89a1 100644 --- a/test/hex/repo_test.exs +++ b/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!()) diff --git a/test/setup_hexpm.exs b/test/setup_hexpm.exs index 3677d27a..43067665 100644 --- a/test/setup_hexpm.exs +++ b/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"))} ])