Make oauth exchange opt-in for now

Author: ericmj

lib/hex/repo.ex

@@ -64,6 +64,7 @@ defmodule Hex.Repo do
       url: trusted_mirror_url || mirror_url,
       public_key: @hexpm_public_key,
       auth_key: auth_key,
+      oauth_exchange: true,
       trusted: trusted_mirror_url != nil or mirror_url == nil
     }
   end
@@ -73,6 +74,7 @@ defmodule Hex.Repo do
       url: @hexpm_url,
       public_key: @hexpm_public_key,
       auth_key: nil,
+      oauth_exchange: true,
       trusted: true
     }
   end

lib/mix/tasks/hex.repo.ex

@@ -41,7 +41,12 @@ defmodule Mix.Tasks.Hex.Repo do
 
     * `--fetch-public-key FINGERPRINT` - Download public key from the repository and verify against the fingerprint (optional).
 
-    * `--no-oauth-exchange` - Disable OAuth token exchange for API keys. Use the API key directly instead of exchanging it for a short-lived OAuth token (optional).
+    * `--oauth-exchange` - Enable OAuth token exchange for API keys. Exchange the API key for a short-lived OAuth token
+      instead of using the API key directly. Defaults to enabled for hexpm, disabled for other repositories.
+      In the future, this will default to enabled for all repositories (optional).
+
+    * `--no-oauth-exchange` - Disable OAuth token exchange for API keys. Use the API key directly instead of exchanging
+      it for a short-lived OAuth token. Currently a no-op, but will disable OAuth token exchange  in the future (optional).
 
     * `--oauth-exchange-url URL` - Custom URL for OAuth token exchange. By default, the API URL is used (optional).
 
@@ -50,6 +55,7 @@ defmodule Mix.Tasks.Hex.Repo do
       $ mix hex.repo set NAME --url URL
       $ mix hex.repo set NAME --public-key PATH
       $ mix hex.repo set NAME --auth-key KEY
+      $ mix hex.repo set NAME --oauth-exchange
       $ mix hex.repo set NAME --no-oauth-exchange
       $ mix hex.repo set NAME --oauth-exchange-url URL
 
@@ -72,14 +78,14 @@ defmodule Mix.Tasks.Hex.Repo do
     public_key: :string,
     auth_key: :string,
     fetch_public_key: :string,
-    no_oauth_exchange: :boolean,
+    oauth_exchange: :boolean,
     oauth_exchange_url: :string
   ]
   @set_switches [
     url: :string,
     public_key: :string,
     auth_key: :string,
-    no_oauth_exchange: :boolean,
+    oauth_exchange: :boolean,
     oauth_exchange_url: :string
   ]
   @show_switches [
@@ -143,15 +149,17 @@ defmodule Mix.Tasks.Hex.Repo do
   end
 
   defp add(name, url, opts) do
-    opts_with_exchange = normalize_oauth_exchange_opt(opts)
+    # Default oauth_exchange to true for hexpm/repo.hex.pm, false for others
+    default_oauth_exchange = name == "hexpm"
+    oauth_exchange = Keyword.get(opts, :oauth_exchange, default_oauth_exchange)
 
     public_key =
-      read_public_key(opts_with_exchange[:public_key]) ||
+      read_public_key(opts[:public_key]) ||
         fetch_public_key(
-          opts_with_exchange[:fetch_public_key],
+          opts[:fetch_public_key],
           url,
-          opts_with_exchange[:auth_key],
-          opts_with_exchange[:oauth_exchange]
+          opts[:auth_key],
+          oauth_exchange
         )
 
     repo =
@@ -160,11 +168,11 @@ defmodule Mix.Tasks.Hex.Repo do
         public_key: nil,
         fetch_public_key: nil,
         auth_key: nil,
-        oauth_exchange: true,
+        oauth_exchange: oauth_exchange,
         oauth_exchange_url: nil,
         trusted: true
       }
-      |> Map.merge(Map.new(opts_with_exchange))
+      |> Map.merge(Map.new(opts))
       |> Map.put(:public_key, public_key)
 
     Hex.State.fetch!(:repos)
@@ -180,8 +188,6 @@ defmodule Mix.Tasks.Hex.Repo do
         opts
       end
 
-    opts = normalize_oauth_exchange_opt(opts)
-
     Hex.State.fetch!(:repos)
     |> Map.update!(name, &Map.merge(&1, Map.new(opts)))
     |> Hex.Config.update_repos()
@@ -299,16 +305,4 @@ defmodule Mix.Tasks.Hex.Repo do
         Mix.raise("Config does not contain repo #{name}")
     end
   end
-
-  defp normalize_oauth_exchange_opt(opts) do
-    if Keyword.has_key?(opts, :no_oauth_exchange) do
-      oauth_exchange = !opts[:no_oauth_exchange]
-
-      opts
-      |> Keyword.delete(:no_oauth_exchange)
-      |> Keyword.put(:oauth_exchange, oauth_exchange)
-    else
-      opts
-    end
-  end
 end

test/mix/tasks/hex.repo_test.exs

@@ -71,8 +71,7 @@ defmodule Mix.Tasks.Hex.RepoTest do
         "--fetch-public-key",
         @public_key_fingerprint,
         "--auth-key",
-        "AAAA",
-        "--no-oauth-exchange"
+        "AAAA"
       ])
 
       assert [
@@ -180,24 +179,24 @@ defmodule Mix.Tasks.Hex.RepoTest do
   end
 
   describe "OAuth exchange configuration" do
-    test "add with --no-oauth-exchange disables OAuth token exchange" do
+    test "add with --oauth-exchange enables OAuth token exchange" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
         Mix.Tasks.Hex.Repo.run([
           "add",
           "reponame",
           "http://example.com",
-          "--no-oauth-exchange"
+          "--oauth-exchange"
         ])
 
         config = Hex.Config.read()
         repo = config[:"$repos"]["reponame"]
-        assert repo.oauth_exchange == false
+        assert repo.oauth_exchange == true
       end)
     end
 
-    test "add without --no-oauth-exchange enables OAuth token exchange by default" do
+    test "add without --oauth-exchange disables OAuth token exchange by default for non-hexpm repos" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
@@ -209,7 +208,7 @@ defmodule Mix.Tasks.Hex.RepoTest do
 
         config = Hex.Config.read()
         repo = config[:"$repos"]["reponame"]
-        assert repo.oauth_exchange == true
+        assert repo.oauth_exchange == false
       end)
     end
 
@@ -231,36 +230,36 @@ defmodule Mix.Tasks.Hex.RepoTest do
       end)
     end
 
-    test "add with both --no-oauth-exchange and --oauth-exchange-url" do
+    test "add with both --oauth-exchange and --oauth-exchange-url" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
         Mix.Tasks.Hex.Repo.run([
           "add",
           "reponame",
           "http://example.com",
-          "--no-oauth-exchange",
+          "--oauth-exchange",
           "--oauth-exchange-url",
           "http://custom-oauth.com"
         ])
 
         config = Hex.Config.read()
         repo = config[:"$repos"]["reponame"]
-        assert repo.oauth_exchange == false
+        assert repo.oauth_exchange == true
         assert repo.oauth_exchange_url == "http://custom-oauth.com"
       end)
     end
 
-    test "set with --no-oauth-exchange disables OAuth token exchange" do
+    test "set with --oauth-exchange enables OAuth token exchange" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
         Mix.Tasks.Hex.Repo.run(["add", "reponame", "http://example.com"])
-        Mix.Tasks.Hex.Repo.run(["set", "reponame", "--no-oauth-exchange"])
+        Mix.Tasks.Hex.Repo.run(["set", "reponame", "--oauth-exchange"])
 
         config = Hex.Config.read()
         repo = config[:"$repos"]["reponame"]
-        assert repo.oauth_exchange == false
+        assert repo.oauth_exchange == true
       end)
     end
 
@@ -290,8 +289,7 @@ defmodule Mix.Tasks.Hex.RepoTest do
         Mix.Tasks.Hex.Repo.run([
           "add",
           "reponame",
-          "http://example.com",
-          "--no-oauth-exchange"
+          "http://example.com"
         ])
 
         Mix.Tasks.Hex.Repo.run(["show", "reponame", "--oauth-exchange"])
@@ -318,7 +316,7 @@ defmodule Mix.Tasks.Hex.RepoTest do
       end)
     end
 
-    test "add with auth key and --no-oauth-exchange" do
+    test "add with auth key defaults to no oauth exchange for non-hexpm repos" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
@@ -327,8 +325,7 @@ defmodule Mix.Tasks.Hex.RepoTest do
           "reponame",
           "http://example.com",
           "--auth-key",
-          "my-api-key",
-          "--no-oauth-exchange"
+          "my-api-key"
         ])
 
         config = Hex.Config.read()
@@ -338,15 +335,15 @@ defmodule Mix.Tasks.Hex.RepoTest do
       end)
     end
 
-    test "list displays repos with OAuth exchange disabled" do
+    test "list displays repos with different OAuth exchange settings" do
       in_tmp(fn ->
         Hex.State.put(:config_home, File.cwd!())
 
         Mix.Tasks.Hex.Repo.run([
           "add",
           "repo1",
           "http://example1.com",
-          "--no-oauth-exchange"
+          "--oauth-exchange"
         ])
 
         Mix.Tasks.Hex.Repo.run(["add", "repo2", "http://example2.com"])
@@ -371,5 +368,22 @@ defmodule Mix.Tasks.Hex.RepoTest do
         assert all_output =~ "repo2"
       end)
     end
+
+    test "add repo named 'hexpm' defaults to oauth_exchange enabled" do
+      in_tmp(fn ->
+        Hex.State.put(:config_home, File.cwd!())
+
+        Mix.Tasks.Hex.Repo.run([
+          "add",
+          "hexpm",
+          "http://example.com"
+        ])
+
+        # Check the actual repo state (after merging with defaults)
+        # not the raw config (which may have oauth_exchange removed if it matches default)
+        repo = Hex.State.fetch!(:repos)["hexpm"]
+        assert repo.oauth_exchange == true
+      end)
+    end
   end
 end

test/support/case.ex

@@ -258,6 +258,7 @@ defmodule HexTest.Case do
     Hex.State.update!(:repos, &put_in(&1["hexpm"].url, "http://localhost:4043/repo"))
     Hex.State.update!(:repos, &put_in(&1["hexpm"].public_key, public_key))
     Hex.State.update!(:repos, &put_in(&1["hexpm"].auth_key, nil))
+    Hex.State.update!(:repos, &put_in(&1["hexpm"].oauth_exchange, true))
     Hex.State.put(:repos_key, nil)
     Hex.State.put(:pbkdf2_iters, 10)
     Hex.State.put(:clean_pass, false)