Close what the second CLI review found

A repository request that triggered inline device auth retried with the
API key set, and repository requests read repo_key, so the retry went
out unauthenticated and 401ed again. It re-resolves repository auth
after authenticating. with_repo also executed once and never acted on a
401 from a token it had already resolved, so a token the server rejects
as expired was never refreshed or re-exchanged; it retries once with
renewed credentials.

A transport failure during a refresh was reported as an expired session
and dropped the stored token, so a DNS blip printed that the session had
expired and downgraded the rest of the run to anonymous. Only a refusal
from the server clears the token now.

with_repo documented auth_inline as defaulting to false and then took
true on the 401 path, so a private package could open an interactive
device flow against the caller's stated default.

One transport error while polling ended a device authorization the user
was minutes into. Polling continues until the device code expires.

A malformed verification URI crashed the CLI, because the URL check
threw where its caller expected a return value.

The refresh token sentinel had two owners: an absent one was undefined
in one place and absent in another, and undefined reached persistence,
where Elixir stored it as a refresh token. The grant is normalized once.

has_refresh_token was derived three ways in one module, and the source
member of the auth context was written on eight paths and read nowhere.

Author: ericmj

src/hex_api_oauth.erl

@@ -19,7 +19,7 @@
 
 -type oauth_tokens() :: #{
     access_token := binary(),
-    refresh_token => binary() | undefined,
+    refresh_token => binary(),
     expires_at := integer(),
     %% Organizations the session must authenticate against their identity
     %% provider for. Their scopes are not in this token and re-requesting them
@@ -182,14 +182,12 @@ poll_for_token_loop(Config, ClientId, DeviceCode, IntervalSeconds, ExpiresAt) ->
                         <<"access_token">> := AccessToken,
                         <<"expires_in">> := ExpiresIn
                     } = TokenResponse,
-                    RefreshToken = maps:get(<<"refresh_token">>, TokenResponse, undefined),
-                    TokenExpiresAt = erlang:system_time(second) + ExpiresIn,
-                    {ok, #{
+                    Tokens = #{
                         access_token => AccessToken,
-                        refresh_token => RefreshToken,
-                        expires_at => TokenExpiresAt,
+                        expires_at => erlang:system_time(second) + ExpiresIn,
                         sso_reauth_required => sso_reauth_required(TokenResponse)
-                    }};
+                    },
+                    {ok, put_refresh_token(Tokens, TokenResponse)};
                 {ok, {400, _, #{<<"error">> := <<"authorization_pending">>}}} ->
                     poll_for_token_loop(Config, ClientId, DeviceCode, IntervalSeconds, ExpiresAt);
                 {ok, {400, _, #{<<"error">> := <<"slow_down">>}}} ->
@@ -203,8 +201,12 @@ poll_for_token_loop(Config, ClientId, DeviceCode, IntervalSeconds, ExpiresAt) ->
                     {error, {access_denied, Status, Body}};
                 {ok, {Status, _, Body}} ->
                     {error, {poll_failed, Status, Body}};
-                {error, Reason} ->
-                    {error, Reason}
+                {error, _Reason} ->
+                    %% A request that did not get through says nothing about the
+                    %% authorization, which the user may be minutes into. The
+                    %% device code lives on the server until it expires, so keep
+                    %% polling until then.
+                    poll_for_token_loop(Config, ClientId, DeviceCode, IntervalSeconds, ExpiresAt)
             end
     end.
 
@@ -391,19 +393,27 @@ sso_reauth_required(TokenResponse) ->
 %% Opens a URL in the default browser using the platform's opener: `open' on
 %% macOS, `xdg-open' on Linux, `start' on Windows. Returns
 %% `{error, browser_not_found}' when none of them exists, which is the ordinary
-%% case on a headless machine.
--spec open_browser(binary()) -> ok | {error, browser_not_found}.
+%% case on a headless machine, and `{error, invalid_url}' for anything that is
+%% not an http(s) URL.
+-spec open_browser(binary()) -> ok | {error, browser_not_found | invalid_url}.
 open_browser(Url) when is_binary(Url) ->
-    ok = ensure_valid_http_url(Url),
-    UrlStr = binary_to_list(Url),
+    case valid_http_url(Url) of
+        true ->
+            spawn_browser(binary_to_list(Url));
+        false ->
+            {error, invalid_url}
+    end.
+
+%% @private
+spawn_browser(Url) ->
     {Cmd, Args} =
         case os:type() of
             {unix, darwin} ->
-                {"open", [UrlStr]};
+                {"open", [Url]};
             {unix, _} ->
-                {"xdg-open", [UrlStr]};
+                {"xdg-open", [Url]};
             {win32, _} ->
-                {"cmd", ["/c", "start", "", UrlStr]}
+                {"cmd", ["/c", "start", "", Url]}
         end,
     case os:find_executable(Cmd) of
         false ->
@@ -414,15 +424,23 @@ open_browser(Url) when is_binary(Url) ->
     end.
 
 %% @private
-%% Validates that a URL uses http:// or https:// scheme.
--spec ensure_valid_http_url(binary()) -> ok.
-ensure_valid_http_url(Url) when is_binary(Url) ->
+%% Whether a URL uses the http:// or https:// scheme.
+-spec valid_http_url(binary()) -> boolean().
+valid_http_url(Url) when is_binary(Url) ->
     case uri_string:parse(Url) of
-        #{scheme := <<"https">>} -> ok;
-        #{scheme := <<"http">>} -> ok;
-        _ -> throw({invalid_url, Url})
+        #{scheme := <<"https">>} -> true;
+        #{scheme := <<"http">>} -> true;
+        _ -> false
     end.
 
+%% @private
+%% A response without a refresh token carries no key at all rather than a
+%% placeholder, so what a build tool stores is only ever a real token.
+put_refresh_token(Tokens, #{<<"refresh_token">> := RefreshToken}) when is_binary(RefreshToken) ->
+    Tokens#{refresh_token => RefreshToken};
+put_refresh_token(Tokens, _TokenResponse) ->
+    Tokens.
+
 %% @private
 %% Get the hostname of the current machine.
 -spec get_hostname() -> binary().