Clear the OAuth token and warn when its refresh fails (#1184)

An expired global OAuth token whose refresh the server rejects was refreshed once per package during dependency resolution -- dozens of failed POSTs to /oauth/token, serialized by the token-refresh lock -- and the failure was swallowed, so resolution silently continued without credentials. This regressed the OnceCache memoization and warning dropped in cda8717.

Implement the new hex_cli_auth clear_oauth_tokens callback: drop the token from in-memory state (keeping the on-disk token so a transient failure can recover on the next run) and warn once that the session expired and to re-run `mix hex.user auth`. With the token gone, the rest of the run stops retrying the refresh.

Re-vendors hex_core.

Author: ericmj

lib/hex/auth.ex

@@ -40,6 +40,7 @@ defmodule Hex.Auth do
       get_auth_config: &get_auth_config/1,
       get_oauth_tokens: &get_oauth_tokens/0,
       persist_oauth_tokens: &persist_oauth_tokens/4,
+      clear_oauth_tokens: &clear_oauth_tokens/0,
       prompt_otp: &prompt_otp/1,
       get_client_id: &Hex.API.OAuth.client_id/0,
       should_authenticate: &should_authenticate/1
@@ -111,6 +112,22 @@ defmodule Hex.Auth do
     :ok
   end
 
+  # Invoked by hex_cli_auth when the stored global OAuth token is expired and
+  # could not be refreshed. Drop it from in-memory state only — keeping the
+  # on-disk token, since a refresh can fail transiently — so the rest of this
+  # run stops retrying the doomed refresh and proceeds unauthenticated.
+  defp clear_oauth_tokens do
+    Hex.State.put(:oauth_token, nil)
+
+    Hex.Shell.warn(
+      "Your authentication session has expired and could not be refreshed. " <>
+        "Continuing without credentials; requests for private resources will fail or " <>
+        "prompt for authentication. Run `mix hex.user auth` to re-authenticate"
+    )
+
+    :ok
+  end
+
   defp prompt_otp(message) do
     case Hex.Shell.prompt(message) do
       nil ->

src/mix_hex_cli_auth.erl

@@ -10,7 +10,7 @@
 %% == Callbacks ==
 %%
 %% Callbacks are provided via the `cli_auth_callbacks' key in the config map.
-%% All callbacks are required:
+%% All callbacks below are required unless marked optional:
 %%
 %% ```
 %% #{
@@ -30,6 +30,13 @@
 %%                                  RefreshToken :: binary() | undefined,
 %%                                  ExpiresAt :: integer()) -> ok),
 %%
+%%     %% Invalidate the stored global OAuth token after it expired and could
+%%     %% not be refreshed (optional). Lets the build tool drop the unusable
+%%     %% token so concurrent and subsequent callers stop retrying the doomed
+%%     %% refresh, and warn the user. Invoked at most once per resolution, while
+%%     %% holding the token-refresh lock.
+%%     clear_oauth_tokens => fun(() -> ok),
+%%
 %%     %% User interaction
 %%     prompt_otp => fun((Message :: binary()) -> {ok, OtpCode :: binary()} | cancelled),
 %%     should_authenticate => fun((Reason :: no_credentials | token_refresh_failed) -> boolean()),
@@ -114,6 +121,7 @@
             ExpiresAt :: integer()
         ) -> ok
     ),
+    clear_oauth_tokens => fun(() -> ok),
     prompt_otp := fun((Message :: binary()) -> {ok, OtpCode :: binary()} | cancelled),
     should_authenticate := fun((Reason :: auth_prompt_reason()) -> boolean()),
     get_client_id := fun(() -> binary())
@@ -602,7 +610,7 @@ do_resolve_oauth_token_with_context(Config) ->
                     is_binary(maps:get(refresh_token, Tokens)),
             case is_token_expired(ExpiresAt) of
                 true ->
-                    maybe_refresh_token_with_context(Config, Tokens);
+                    refresh_or_clear(Config, Tokens);
                 false ->
                     BearerToken = <<"Bearer ", AccessToken/binary>>,
                     {ok, BearerToken, #{source => oauth, has_refresh_token => HasRefreshToken}}
@@ -611,6 +619,21 @@ do_resolve_oauth_token_with_context(Config) ->
             {error, no_auth}
     end.
 
+%% @private
+%% Refresh an expired global token; if the refresh fails, invalidate the stored
+%% token via the optional clear_oauth_tokens callback. This runs inside the
+%% token_refresh lock, so the unusable token is dropped exactly once and the
+%% callers serialized behind the lock re-read it as absent instead of each
+%% retrying the doomed refresh against the server.
+refresh_or_clear(Config, Tokens) ->
+    case maybe_refresh_token_with_context(Config, Tokens) of
+        {ok, _Bearer, _Ctx} = Ok ->
+            Ok;
+        {error, _} = Error ->
+            maybe_call_callback(Config, clear_oauth_tokens, []),
+            Error
+    end.
+
 %% @private
 maybe_refresh_token_with_context(Config, #{refresh_token := RefreshToken}) when
     is_binary(RefreshToken)
@@ -758,3 +781,13 @@ call_callback(Config, Name, Args) ->
     #{cli_auth_callbacks := Callbacks} = Config,
     Fun = maps:get(Name, Callbacks),
     erlang:apply(Fun, Args).
+
+%% @private
+%% Like call_callback/3 but for optional callbacks: returns ok without doing
+%% anything when the callback is not provided.
+maybe_call_callback(Config, Name, Args) ->
+    #{cli_auth_callbacks := Callbacks} = Config,
+    case maps:find(Name, Callbacks) of
+        {ok, Fun} -> erlang:apply(Fun, Args);
+        error -> ok
+    end.

test/hex/auth_test.exs

@@ -51,4 +51,39 @@ defmodule Hex.AuthTest do
       assert get_auth_config("does-not-exist") == :undefined
     end
   end
+
+  describe "clear_oauth_tokens/0 callback" do
+    test "drops the in-memory token and warns to re-authenticate" do
+      Hex.State.put(:oauth_token, %{
+        access_token: "expired",
+        refresh_token: "refresh",
+        expires_at: System.system_time(:second) - 100
+      })
+
+      assert Hex.Auth.callbacks().clear_oauth_tokens.() == :ok
+
+      assert Hex.State.get(:oauth_token) == nil
+
+      output = Case.shell_output()
+      assert output =~ "could not be refreshed"
+      assert output =~ "mix hex.user auth"
+    end
+
+    test "keeps the on-disk token so a later run can retry the refresh" do
+      in_tmp("clear_oauth_tokens", fn ->
+        set_home_cwd()
+
+        Hex.OAuth.store_token(%{
+          access_token: "expired",
+          refresh_token: "refresh",
+          expires_at: System.system_time(:second) - 100
+        })
+
+        assert Hex.Auth.callbacks().clear_oauth_tokens.() == :ok
+
+        assert Hex.State.get(:oauth_token) == nil
+        assert Hex.Config.read()[:"$oauth_token"] != nil
+      end)
+    end
+  end
 end

test/hex/remote_converger_test.exs

@@ -133,6 +133,11 @@ defmodule Hex.RemoteConvergerTest do
       end)
 
       refute_received {:mix_shell, :yes?, _}
+
+      # The unusable session is dropped after the first failed refresh, so the
+      # rest of the resolution falls back to unauthenticated fetches instead of
+      # retrying the doomed refresh for every package.
+      assert Hex.State.get(:oauth_token) == nil
     end)
   end