Remove OAuth token exchange (#1089)

We don't need a short lived write refresh token since the API server now enforces 2FA for all write actions.

Author: ericmj

lib/hex/api/oauth.ex

@@ -47,24 +47,6 @@ defmodule Hex.API.OAuth do
     :mix_hex_api_oauth.poll_device_token(config, @client_id, device_code)
   end
 
-  @doc """
-  Exchanges a token for a new token with different scopes using RFC 8693 token exchange.
-
-  ## Examples
-
-      iex> Hex.API.OAuth.exchange_token(subject_token, "api:write")
-      {:ok, {200, _headers, %{
-        "access_token" => "...",
-        "refresh_token" => "...",
-        "token_type" => "Bearer",
-        "expires_in" => 3600
-      }}}
-  """
-  def exchange_token(subject_token, scope) do
-    config = Client.config()
-    :mix_hex_api_oauth.exchange_token(config, @client_id, subject_token, scope)
-  end
-
   @doc """
   Refreshes an access token using a refresh token.
 

lib/hex/oauth.ex

@@ -4,78 +4,73 @@ defmodule Hex.OAuth do
   alias Hex.API.OAuth
 
   @doc """
-  Retrieves a valid access token for the given permission type.
+  Retrieves a valid access token.
 
   Automatically refreshes the token if it's expired.
   Returns {:error, :no_auth} if no tokens are available.
+
+  Since we now use 2FA for write operations, we use a single token for both read and write.
+  The permission parameter is kept for backward compatibility but is no longer used.
   """
   def get_token(permission) when permission in [:read, :write] do
-    case get_stored_tokens() do
+    case get_stored_token() do
       nil ->
         {:error, :no_auth}
 
-      tokens ->
-        token_data = Map.get(tokens, to_string(permission))
-
-        if token_data && valid_token?(token_data) do
+      token_data ->
+        if valid_token?(token_data) do
           {:ok, token_data["access_token"]}
         else
           # Try to refresh the token
-          refresh_token_if_possible(permission, token_data)
+          refresh_token_if_possible(token_data)
         end
     end
   end
 
   @doc """
-  Stores OAuth tokens for both read and write permissions.
+  Stores OAuth token data.
+
+  Since we now use 2FA for write operations, we only store a single token.
 
   Expected format:
   %{
-    "write" => %{
-      "access_token" => "...",
-      "refresh_token" => "...",
-      "expires_at" => unix_timestamp
-    },
-    "read" => %{
-      "access_token" => "...",
-      "refresh_token" => "...",
-      "expires_at" => unix_timestamp
-    }
+    "access_token" => "...",
+    "refresh_token" => "...",
+    "expires_at" => unix_timestamp
   }
   """
-  def store_tokens(tokens) do
-    Hex.Config.update([{:"$oauth_tokens", tokens}])
-    Hex.State.put(:oauth_tokens, tokens)
+  def store_token(token_data) do
+    Hex.Config.update([{:"$oauth_token", token_data}])
+    Hex.State.put(:oauth_token, token_data)
   end
 
   @doc """
   Clears all stored OAuth tokens.
   """
   def clear_tokens do
-    Hex.Config.remove([:"$oauth_tokens"])
-    Hex.State.put(:oauth_tokens, nil)
+    Hex.Config.remove([:"$oauth_token"])
+    Hex.State.put(:oauth_token, nil)
   end
 
   @doc """
   Checks if we have any OAuth tokens stored.
   """
   def has_tokens? do
-    get_stored_tokens() != nil
+    get_stored_token() != nil
   end
 
   @doc """
-  Refreshes a token for the given permission type.
+  Refreshes the stored OAuth token.
+
+  The permission parameter is kept for backward compatibility but is no longer used.
   """
   def refresh_token(permission) when permission in [:read, :write] do
-    case get_stored_tokens() do
+    case get_stored_token() do
       nil ->
         {:error, :no_auth}
 
-      tokens ->
-        permission_str = to_string(permission)
-        token_data = Map.get(tokens, permission_str)
-
-        if token_data && token_data["refresh_token"] do
+      token_data ->
+        if token_data["refresh_token"] do
           case OAuth.refresh_token(token_data["refresh_token"]) do
             {:ok, {200, _, new_token_data}} ->
               # Update the token data with new values
@@ -86,9 +81,8 @@ defmodule Hex.OAuth do
                 |> Map.put("expires_at", expires_at)
                 |> Map.take(["access_token", "refresh_token", "expires_at"])
 
-              # Update stored tokens
-              updated_tokens = Map.put(tokens, permission_str, new_token_data)
-              store_tokens(updated_tokens)
+              # Update stored token
+              store_token(new_token_data)
 
               {:ok, new_token_data["access_token"]}
 
@@ -117,8 +111,8 @@ defmodule Hex.OAuth do
     |> Map.take(["access_token", "refresh_token", "expires_at"])
   end
 
-  defp get_stored_tokens do
-    Hex.State.get(:oauth_tokens)
+  defp get_stored_token do
+    Hex.State.get(:oauth_token)
   end
 
   defp valid_token?(token_data) do
@@ -133,8 +127,8 @@ defmodule Hex.OAuth do
     end
   end
 
-  defp refresh_token_if_possible(permission, token_data) do
-    case refresh_token(permission) do
+  defp refresh_token_if_possible(token_data) do
+    case refresh_token(:write) do
       {:ok, access_token} ->
         {:ok, access_token}
 

lib/mix/tasks/hex.ex

@@ -179,8 +179,8 @@ defmodule Mix.Tasks.Hex do
     Hex.Shell.info("Waiting for authentication...")
 
     case poll_for_token(device_code, interval) do
-      {:ok, initial_token} ->
-        exchange_and_store_tokens(initial_token)
+      {:ok, token} ->
+        store_token(token)
 
       :error ->
         :error
@@ -232,108 +232,15 @@ defmodule Mix.Tasks.Hex do
       "-" <> String.slice(user_code, mid, String.length(user_code))
   end
 
-  defp exchange_and_store_tokens(initial_token) do
-    # Parse the granted scopes from the initial token
-    granted_scopes = parse_granted_scopes(initial_token["scope"] || "")
+  defp store_token(token) do
+    # Create token data with expiration time
+    token_data = Hex.OAuth.create_token_data(token)
 
-    # Determine what tokens we can create based on granted scopes
-    write_token = create_write_token(initial_token, granted_scopes)
-    read_token = create_read_token(initial_token, granted_scopes)
-
-    # Store the tokens based on what we obtained
-    case {write_token, read_token} do
-      {nil, nil} ->
-        # Couldn't get proper tokens - this is an error condition
-        Hex.Shell.error("Failed to obtain proper access tokens")
-        Hex.Shell.error("Please try authenticating again or check your permissions")
-        :error
-
-      {write, read} ->
-        tokens = %{
-          "write" => if(write, do: Hex.OAuth.create_token_data(write)),
-          "read" => if(read, do: Hex.OAuth.create_token_data(read))
-        }
-
-        Hex.OAuth.store_tokens(tokens)
-        Hex.Shell.info("Authentication completed successfully!")
-        {:ok, tokens}
-    end
-  end
-
-  defp parse_granted_scopes(scope_string) when is_binary(scope_string) do
-    String.split(scope_string, " ", trim: true)
-  end
-
-  defp parse_granted_scopes(_), do: []
-
-  defp create_write_token(initial_token, granted_scopes) do
-    # Check if we have write permission
-    cond do
-      "api" in granted_scopes || "api:write" in granted_scopes ->
-        # We have write permission, exchange for api:write token
-        case Hex.API.OAuth.exchange_token(initial_token["access_token"], "api:write") do
-          {:ok, {200, _, write_token_response}} ->
-            write_token_response
-
-          {:ok, {status, _, error}} ->
-            Hex.Shell.warn("Could not exchange for write token (#{status}): #{inspect(error)}")
-            nil
-
-          {:error, reason} ->
-            Hex.Shell.warn("Write token exchange error: #{inspect(reason)}")
-            nil
-        end
-
-      true ->
-        # No write permission granted
-        nil
-    end
-  end
-
-  defp create_read_token(initial_token, granted_scopes) do
-    # Always create a separate read token - they have different refresh rates and conditions
-    # Determine what read scopes we can request
-    read_scopes = build_read_scopes(granted_scopes)
-
-    if read_scopes != "" do
-      case Hex.API.OAuth.exchange_token(initial_token["access_token"], read_scopes) do
-        {:ok, {200, _, read_token_response}} ->
-          read_token_response
-
-        {:ok, {status, _, error}} ->
-          Hex.Shell.warn("Could not exchange for read token (#{status}): #{inspect(error)}")
-          nil
-
-        {:error, reason} ->
-          Hex.Shell.warn("Read token exchange error: #{inspect(reason)}")
-          nil
-      end
-    else
-      # No read scopes available
-      nil
-    end
-  end
-
-  defp build_read_scopes(granted_scopes) do
-    read_scopes = []
-
-    # Add repositories if granted
-    read_scopes =
-      if "repositories" in granted_scopes do
-        ["repositories" | read_scopes]
-      else
-        read_scopes
-      end
-
-    # Add api:read if we have any API read permission
-    read_scopes =
-      if "api" in granted_scopes || "api:read" in granted_scopes || "api:write" in granted_scopes do
-        ["api:read" | read_scopes]
-      else
-        read_scopes
-      end
-
-    Enum.join(read_scopes, " ")
+    # Store a single token for both read and write operations
+    # With 2FA now required for write operations, we don't need separate tokens
+    Hex.OAuth.store_token(token_data)
+    Hex.Shell.info("You are authenticated!")
+    {:ok, token_data}
   end
 
   @doc false
@@ -382,25 +289,22 @@ defmodule Mix.Tasks.Hex do
 
   @doc false
   def revoke_existing_oauth_tokens do
-    case Hex.Config.read()[:"$oauth_tokens"] do
+    case Hex.Config.read()[:"$oauth_token"] do
       nil ->
         :ok
 
-      tokens when is_map(tokens) ->
-        Enum.each(tokens, fn {_type, token_data} ->
-          if access_token = token_data["access_token"] do
-            case Hex.API.OAuth.revoke_token(access_token) do
-              {:ok, {code, _, _}} when code in 200..299 ->
-                :ok
+      token_data when is_map(token_data) ->
+        if access_token = token_data["access_token"] do
+          case Hex.API.OAuth.revoke_token(access_token) do
+            {:ok, {code, _, _}} when code in 200..299 ->
+              :ok
 
-              _ ->
-                :ok
-            end
+            _ ->
+              :ok
           end
-        end)
+        end
 
-        Hex.Config.remove([:"$oauth_tokens"])
-        Hex.Shell.info("Revoked existing OAuth tokens.")
+        Hex.Config.remove([:"$oauth_token"])
 
       _ ->
         :ok
@@ -414,33 +318,19 @@ defmodule Mix.Tasks.Hex do
     # Check for old write key
     if write_key = config[:"$write_key"] do
       # Try to revoke on server (might fail if already revoked or invalid)
-      case Hex.API.Key.delete(write_key, key: write_key) do
-        {:ok, {code, _, _}} when code in 200..299 ->
-          Hex.Shell.info("Revoked old write API key.")
-
-        _ ->
-          # Key might already be invalid, continue anyway
-          :ok
-      end
+      _ = Hex.API.Key.delete(write_key, key: write_key)
     end
 
     # Check for old read key (only if different from write key)
     if read_key = config[:"$read_key"] do
       if read_key != config[:"$write_key"] do
-        case Hex.API.Key.delete(read_key, key: read_key) do
-          {:ok, {code, _, _}} when code in 200..299 ->
-            Hex.Shell.info("Revoked old read API key.")
-
-          _ ->
-            :ok
-        end
+        _ = Hex.API.Key.delete(read_key, key: read_key)
       end
     end
 
     # Remove from config if they existed
     if config[:"$write_key"] || config[:"$read_key"] do
       Hex.Config.remove([:"$write_key", :"$read_key"])
-      Hex.Shell.info("Removed deprecated API keys from config.")
     end
   end
 

mix.exs

@@ -17,7 +17,7 @@ defmodule Hex.MixProject do
       elixirc_options: elixirc_options(Mix.env()),
       elixirc_paths: elixirc_paths(Mix.env()),
       test_ignore_filters: [
-        "test/fixtures/**/*.exs",
+        ~r"^test/fixtures/",
         "test/setup_hexpm.exs"
       ]
     ]