Skip to content

Commit 573f566

Browse files
authored
Gate API key to OAuth token exchange on oauth_exchange flag (#1148)
The second-priority branch in build_hex_core_config attempted to exchange the API key for an OAuth token whenever auth_key and trusted were set, relying on the first branch to short-circuit for the disabled case. That left the branch's condition incomplete: any state not caught by branch 1 would fire exchange, even states that should not (e.g. organization repos propagated from a parent without oauth_exchange set, which resolve to an explicit nil). Make both branches self-gated on the oauth_exchange flag so the intent is local to each clause. Also clarify preflight comments in remote_converger.
1 parent 0d5d10d commit 573f566

3 files changed

Lines changed: 62 additions & 4 deletions

File tree

lib/hex/remote_converger.ex

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,11 +758,13 @@ defmodule Hex.RemoteConverger do
758758
)
759759

760760
{:error, :no_auth} ->
761-
# No OAuth token - this is OK, user might only be fetching public packages
761+
# No stored OAuth token to preflight; continue unauthenticated
762+
# and let the repo fetch fail normally if authentication is required.
762763
:ok
763764

764765
{:error, _other} ->
765-
# Other errors (shouldn't happen with prompt_auth: true, but handle gracefully)
766+
# Do not fail dependency resolution during OAuth preflight; continue
767+
# unauthenticated and let the repo fetch surface any auth error.
766768
:ok
767769
end
768770
else

lib/hex/repo.ex

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,12 @@ defmodule Hex.Repo do
318318
cond do
319319
# First priority: explicit repo auth key with OAuth exchange disabled - use API key directly
320320
repo_config.auth_key && Map.get(repo_config, :trusted, true) &&
321-
Map.get(repo_config, :oauth_exchange, false) == false ->
321+
!Map.get(repo_config, :oauth_exchange, false) ->
322322
%{config | repo_key: repo_config.auth_key}
323323

324324
# Second priority: Exchange API key for OAuth token if enabled
325-
repo_config.auth_key && Map.get(repo_config, :trusted, true) ->
325+
repo_config.auth_key && Map.get(repo_config, :trusted, true) &&
326+
Map.get(repo_config, :oauth_exchange, false) ->
326327
case exchange_api_key_for_token(repo_config, repo_name) do
327328
{:ok, access_token} ->
328329
%{config | repo_key: "Bearer #{access_token}"}

test/hex/repo_test.exs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,61 @@ defmodule Hex.RepoTest do
117117
assert Map.get(repos_after["hexpm"], :oauth_token) == nil
118118
end
119119

120+
test "non-hexpm repo with oauth_exchange: false uses API key directly and does not exchange" do
121+
bypass = Bypass.open()
122+
123+
Bypass.expect(bypass, fn %Plug.Conn{request_path: path} = conn ->
124+
case path do
125+
"/public_key" ->
126+
assert Plug.Conn.get_req_header(conn, "authorization") == ["rawkey"]
127+
Plug.Conn.resp(conn, 200, "fake_public_key_body")
128+
129+
"/oauth/token" ->
130+
flunk("OAuth exchange must not be attempted when oauth_exchange is false")
131+
end
132+
end)
133+
134+
myrepo_config = %{
135+
url: "http://localhost:#{bypass.port}",
136+
public_key: nil,
137+
auth_key: "rawkey",
138+
trusted: true,
139+
oauth_exchange: false,
140+
oauth_exchange_url: "http://localhost:#{bypass.port}"
141+
}
142+
143+
assert {:ok, {200, _, "fake_public_key_body"}} = Hex.Repo.get_public_key(myrepo_config)
144+
end
145+
146+
test "non-hexpm repo with oauth_exchange not true does not attempt API key exchange" do
147+
bypass = Bypass.open()
148+
test_pid = self()
149+
150+
Bypass.expect(bypass, fn %Plug.Conn{request_path: path} = conn ->
151+
case path do
152+
"/public_key" ->
153+
Plug.Conn.resp(conn, 200, "fake_public_key_body")
154+
155+
"/oauth/token" ->
156+
send(test_pid, :exchange_attempted)
157+
Plug.Conn.resp(conn, 500, "")
158+
end
159+
end)
160+
161+
myrepo_config = %{
162+
url: "http://localhost:#{bypass.port}",
163+
public_key: nil,
164+
auth_key: "rawkey",
165+
trusted: true,
166+
oauth_exchange: nil,
167+
oauth_exchange_url: "http://localhost:#{bypass.port}"
168+
}
169+
170+
assert {:ok, {200, _, _}} = Hex.Repo.get_public_key(myrepo_config)
171+
172+
refute_received :exchange_attempted
173+
end
174+
120175
test "fetch_repo/1" do
121176
assert Hex.Repo.fetch_repo("foo") == :error
122177

0 commit comments

Comments
 (0)