Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 64 additions & 37 deletions lib/hex/remote_converger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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
9 changes: 7 additions & 2 deletions lib/hex/repo.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
129 changes: 129 additions & 0 deletions test/hex/remote_converger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
24 changes: 24 additions & 0 deletions test/hex/repo_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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!())
Expand Down
3 changes: 3 additions & 0 deletions test/setup_hexpm.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))}
])
Expand Down
Loading