Fix authenticated downloading (#3868)

.github/workflows/unix_impl.yml

@@ -171,8 +171,8 @@ jobs:
               ${{ runner.debug == 'true' && '-v --capture=tee-sys' || '--exitfirst' }} \
               ${{ github.ref == 'refs/heads/main' && '--reruns 3' || '' }}
 
-  verify_pkg_tests:
-    name: mamba-content-trust tests
+  verify_pkg_and_auth_tests:
+    name: mamba-content-trust and auth tests
     needs: ["build_shared_unix"]
     runs-on: ${{ inputs.os }}
     if: startsWith(inputs.os, 'ubuntu')
@@ -198,4 +198,4 @@ jobs:
           unset CONDARC  # Interferes with tests
           cd micromamba/test-server
           ./generate_gpg_keys.sh
-          ./testserver_pkg_signing.sh
+          ./testserver_auth_pkg_signing.sh

libmamba/include/mamba/core/package_fetcher.hpp

@@ -102,6 +102,7 @@ namespace mamba
         struct CheckSumParams;
 
         bool use_oci() const;
+        bool use_auth() const;
         const std::string& filename() const;
         std::string channel() const;
         std::string url_path() const;

libmamba/include/mamba/core/util.hpp

@@ -11,6 +11,7 @@
 #include <fstream>
 #include <map>
 #include <optional>
+#include <regex>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -26,6 +27,9 @@ namespace mamba
 {
     class Context;
 
+    const std::regex& token_regex();
+    const std::regex& http_basicauth_regex();
+
     // Used when we want a callback which does nothing.
     struct no_op
     {

libmamba/src/core/package_fetcher.cpp

@@ -333,8 +333,14 @@ namespace mamba
         return util::starts_with(m_package_info.package_url, oci_scheme);
     }
 
+    bool PackageFetcher::use_auth() const
+    {
+        return std::regex_search(m_package_info.package_url, http_basicauth_regex())
+               || std::regex_search(m_package_info.package_url, token_regex());
+    }
+
     // NOTE
-    // In the general case (not fetching from an oci registry),
+    // - In the general case (not fetching from an oci registry),
     // `channel()` and `url_path()` are concatenated when passed to `HTTPMirror`
     // and the channel is resolved if needed (using the channel alias).
     // Therefore, `util::url_concat("", m_package_info.package_url)`
@@ -343,12 +349,16 @@ namespace mamba
     // with `--override-channels` option.
     // Hence, we are favoring the first option (returning "" and `m_package_info.package_url`
     // to be concatenated), valid for all the mentioned cases used with `HTTPMirror`.
-    // In the case of fetching from oci registries (using `OCIMirror`),the actual url
+    // - In the case of fetching from oci registries (using `OCIMirror`),the actual url
     // used is built differently, and returning `m_package_info.package_url` is not relevant
     // (i.e oci://ghcr.io/<mirror>/<channel>/<platform>/<filename>).
+    // - With authenticated downloading (private packages for e.g), we should use
+    // `util::url_concat(m_package_info.channel, m_package_info.platform,
+    // m_package_info.filename)` as `m_package_info.package_url` would contain the
+    // authentication info which shouldn't be passed in the url but set using libcurl's CURLUPart.
     std::string PackageFetcher::channel() const
     {
-        if (use_oci())
+        if (use_oci() || use_auth())
         {
             return m_package_info.channel;
         }
@@ -357,7 +367,7 @@ namespace mamba
 
     std::string PackageFetcher::url_path() const
     {
-        if (use_oci())
+        if (use_oci() || use_auth())
         {
             return util::concat(m_package_info.platform, '/', m_package_info.filename);
         }

libmamba/src/core/util.cpp

@@ -14,7 +14,6 @@
 #include <memory>
 #include <mutex>
 #include <optional>
-#include <regex>
 #include <sstream>
 #include <string>
 #include <string_view>
@@ -76,6 +75,20 @@ namespace mamba
         std::atomic<bool> persist_temporary_directories{ false };
     }
 
+    const std::regex& token_regex()
+    {
+        // usernames on anaconda.org can have a underscore, which influences the
+        // first two characters
+        static const std::regex token_regex{ "/t/([a-zA-Z0-9-_]{0,2}[a-zA-Z0-9-]*)" };
+        return token_regex;
+    }
+
+    const std::regex& http_basicauth_regex()
+    {
+        static const std::regex http_basicauth_regex{ "(://|^)([^\\s]+):([^\\s]+)@" };
+        return http_basicauth_regex;
+    }
+
     bool must_persist_temporary_files()
     {
         return persist_temporary_files;
@@ -1647,24 +1660,16 @@ namespace mamba
         return std::nullopt;
     }
 
-    namespace
-    {
-        // usernames on anaconda.org can have a underscore, which influences the
-        // first two characters
-        inline const std::regex token_regex{ "/t/([a-zA-Z0-9-_]{0,2}[a-zA-Z0-9-]*)" };
-        inline const std::regex http_basicauth_regex{ "(://|^)([^\\s]+):([^\\s]+)@" };
-    }
-
     std::string hide_secrets(std::string_view str)
     {
         std::string copy(str);
 
         if (util::contains(str, "/t/"))
         {
-            copy = std::regex_replace(copy, token_regex, "/t/*****");
+            copy = std::regex_replace(copy, token_regex(), "/t/*****");
         }
 
-        copy = std::regex_replace(copy, http_basicauth_regex, "$1$2:*****@");
+        copy = std::regex_replace(copy, http_basicauth_regex(), "$1$2:*****@");
 
         return copy;
     }

micromamba/test-server/testserver_auth_pkg_signing.sh

@@ -27,27 +27,47 @@ start_server() {
 	exec python "${reposerver}" -n mychannel -d "${repo}" "$@"
 }
 
-test_install() {
-	local tmp=$(mktemp -d)
-	local condarc="${tmp}/condarc"
-	"${TEST_MAMBA_EXE}" create -y -p "${tmp}/env1" --override-channels -c $1/mychannel test-package -vvv --repodata-ttl=0 --trusted-channels "http://localhost:8000/mychannel" --verify-artifacts
-}
-
 check_dwd_pkg() {
     if [ -e "${test_dir}/pkgs/test-package-0.1-0.tar.bz2" ]; then
-        echo -e "\e[32mtest-package-0.1-0.tar.bz2 successfully verified and downloaded!\e[0m"
+        echo -e "\e[32mtest-package-0.1-0.tar.bz2 successfully (verified) and downloaded!\e[0m"
     else
         echo -e "\e[31mtest-package-0.1-0.tar.bz2 does not exist!\e[0m"
         exit 1
     fi
 }
 
+test_install() {
+	local tmp=$(mktemp -d)
+	local condarc="${tmp}/condarc"
+	"${TEST_MAMBA_EXE}" create -y -p "${tmp}/env1" --override-channels -c $1/mychannel test-package -vvv --repodata-ttl=0 "${@:2}"
+	check_dwd_pkg
+	"${TEST_MAMBA_EXE}" clean --all -f -y
+}
+
+# Test without authentication
+start_server & PID=$!
+test_install http://localhost:8000
+kill -TERM $PID
+
+# Tests with authentication
+start_server --auth basic --user user --password test & PID=$!
+test_install http://user:test@localhost:8000
+kill -TERM $PID
+
+start_server --auth basic --user user@email.com --password test & PID=$!
+test_install http://user%40email.com:test@localhost:8000
+kill -TERM $PID
+
+start_server --token xy-12345678-1234-1234-1234-123456789012 & PID=$!
+test_install http://localhost:8000/t/xy-12345678-1234-1234-1234-123456789012
+kill -TERM $PID
+
+# Verify signed packages
 if [[ "$(uname -s)" == "Linux" ]]; then
 	export KEY1=$(gpg --fingerprint "MAMBA1")
 	export KEY2=$(gpg --fingerprint "MAMBA2")
 
 	start_server --auth none --sign & PID=$!
-	test_install http://localhost:8000
-	check_dwd_pkg
+	test_install http://localhost:8000 --trusted-channels "http://localhost:8000/mychannel" --verify-artifacts
 	kill -TERM $PID
 fi