From a303c0b6dc71b1e0d6a57986c3f7a9b61361cd92 Mon Sep 17 00:00:00 2001 From: Greg Hale Date: Wed, 17 Jun 2020 15:08:59 -0400 Subject: [PATCH] Fetch commits from github/gitlab using Auth header `nix flake info` calls the github 'commits' API, which requires authorization when the repository is private. Currently this request fails with a 404. This commit adds an authorization header when calling the 'commits' API. It also changes the way that the 'tarball' API authenticates, moving the user's token from a query parameter into the Authorization header. The query parameter method is recently deprecated and will be disallowed in November 2020. Using them today triggers a warning email. --- src/libexpr/common-eval-args.cc | 2 +- src/libexpr/parser.y | 2 +- src/libexpr/primops/fetchTree.cc | 4 +- src/libfetchers/fetchers.hh | 2 + src/libfetchers/github.cc | 76 +++++++++++++++++++++++--------- src/libfetchers/registry.cc | 2 +- src/libfetchers/tarball.cc | 9 ++-- src/libstore/filetransfer.cc | 3 ++ src/libstore/filetransfer.hh | 4 ++ src/libstore/globals.hh | 3 ++ src/libutil/types.hh | 2 + src/nix-channel/nix-channel.cc | 6 +-- 12 files changed, 84 insertions(+), 31 deletions(-) diff --git a/src/libexpr/common-eval-args.cc b/src/libexpr/common-eval-args.cc index 10c1a6975..d71aa22f1 100644 --- a/src/libexpr/common-eval-args.cc +++ b/src/libexpr/common-eval-args.cc @@ -76,7 +76,7 @@ Path lookupFileArg(EvalState & state, string s) if (isUri(s)) { return state.store->toRealPath( fetchers::downloadTarball( - state.store, resolveUri(s), "source", false).first.storePath); + state.store, resolveUri(s), Headers {}, "source", false).first.storePath); } else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') { Path p = s.substr(1, s.size() - 2); return state.findFile(p); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 24b21f7da..28e31f46b 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -719,7 +719,7 @@ std::pair EvalState::resolveSearchPathElem(const SearchPathEl if (isUri(elem.second)) { try { res = { true, store->toRealPath(fetchers::downloadTarball( - store, resolveUri(elem.second), "source", false).first.storePath) }; + store, resolveUri(elem.second), Headers {}, "source", false).first.storePath) }; } catch (FileTransferError & e) { logWarning({ .name = "Entry download", diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 06e8304b8..3001957b4 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -201,8 +201,8 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, auto storePath = unpack - ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).first.storePath - : fetchers::downloadFile(state.store, *url, name, (bool) expectedHash).storePath; + ? fetchers::downloadTarball(state.store, *url, Headers {}, name, (bool) expectedHash).first.storePath + : fetchers::downloadFile(state.store, *url, Headers{}, name, (bool) expectedHash).storePath; auto path = state.store->toRealPath(storePath); diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 89b1e6e7d..62807e53b 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -118,12 +118,14 @@ struct DownloadFileResult DownloadFileResult downloadFile( ref store, const std::string & url, + const Headers & headers, const std::string & name, bool immutable); std::pair downloadTarball( ref store, const std::string & url, + const Headers & headers, const std::string & name, bool immutable); diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 1cc0c5e2e..d8d0351b9 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -3,11 +3,24 @@ #include "fetchers.hh" #include "globals.hh" #include "store-api.hh" +#include "types.hh" #include namespace nix::fetchers { +struct DownloadUrl +{ + std::string url; + std::optional> access_token_header; + + DownloadUrl(const std::string & url) + : url(url) { } + + DownloadUrl(const std::string & url, const std::pair & access_token_header) + : url(url), access_token_header(access_token_header) { } +}; + // A github or gitlab url const static std::string urlRegexS = "[a-zA-Z0-9.]*"; // FIXME: check std::regex urlRegex(urlRegexS, std::regex::ECMAScript); @@ -16,6 +29,8 @@ struct GitArchiveInputScheme : InputScheme { virtual std::string type() = 0; + virtual std::pair accessHeaderFromToken(const std::string & token) const = 0; + std::optional inputFromURL(const ParsedURL & url) override { if (url.scheme != type()) return {}; @@ -131,7 +146,7 @@ struct GitArchiveInputScheme : InputScheme virtual Hash getRevFromRef(nix::ref store, const Input & input) const = 0; - virtual std::string getDownloadUrl(const Input & input) const = 0; + virtual DownloadUrl getDownloadUrl(const Input & input) const = 0; std::pair fetch(ref store, const Input & _input) override { @@ -160,7 +175,12 @@ struct GitArchiveInputScheme : InputScheme auto url = getDownloadUrl(input); - auto [tree, lastModified] = downloadTarball(store, url, "source", true); + Headers headers; + if (url.access_token_header) { + headers.push_back(*url.access_token_header); + } + + auto [tree, lastModified] = downloadTarball(store, url.url, headers, "source", true); input.attrs.insert_or_assign("lastModified", lastModified); @@ -182,11 +202,8 @@ struct GitHubInputScheme : GitArchiveInputScheme { std::string type() override { return "github"; } - void addAccessToken(std::string & url) const - { - std::string accessToken = settings.githubAccessToken.get(); - if (accessToken != "") - url += "?access_token=" + accessToken; + std::pair accessHeaderFromToken(const std::string & token) const { + return std::pair("Authorization", fmt("token %s", token)); } Hash getRevFromRef(nix::ref store, const Input & input) const override @@ -195,18 +212,21 @@ struct GitHubInputScheme : GitArchiveInputScheme auto url = fmt("https://api.%s/repos/%s/%s/commits/%s", // FIXME: check host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), *input.getRef()); - addAccessToken(url); + Headers headers; + std::string accessToken = settings.githubAccessToken.get(); + if (accessToken != "") + headers.push_back(accessHeaderFromToken(accessToken)); auto json = nlohmann::json::parse( readFile( store->toRealPath( - downloadFile(store, url, "source", false).storePath))); + downloadFile(store, url, headers, "source", false).storePath))); auto rev = Hash::parseAny(std::string { json["sha"] }, htSHA1); debug("HEAD revision for '%s' is %s", url, rev.gitRev()); return rev; } - std::string getDownloadUrl(const Input & input) const override + DownloadUrl getDownloadUrl(const Input & input) const override { // FIXME: use regular /archive URLs instead? api.github.com // might have stricter rate limits. @@ -215,9 +235,13 @@ struct GitHubInputScheme : GitArchiveInputScheme host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), input.getRev()->to_string(Base16, false)); - addAccessToken(url); - - return url; + std::string accessToken = settings.githubAccessToken.get(); + if (accessToken != "") { + auto auth_header = accessHeaderFromToken(accessToken); + return DownloadUrl(url, auth_header); + } else { + return DownloadUrl(url); + } } void clone(const Input & input, const Path & destDir) override @@ -234,21 +258,31 @@ struct GitLabInputScheme : GitArchiveInputScheme { std::string type() override { return "gitlab"; } + std::pair accessHeaderFromToken(const std::string & token) const { + return std::pair("Authorization", fmt("Bearer %s", token)); + } + Hash getRevFromRef(nix::ref store, const Input & input) const override { auto host_url = maybeGetStrAttr(input.attrs, "url").value_or("gitlab.com"); auto url = fmt("https://%s/api/v4/projects/%s%%2F%s/repository/commits?ref_name=%s", host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), *input.getRef()); + + Headers headers; + std::string accessToken = settings.gitlabAccessToken.get(); + if (accessToken != "") + headers.push_back(accessHeaderFromToken(accessToken)); + auto json = nlohmann::json::parse( readFile( store->toRealPath( - downloadFile(store, url, "source", false).storePath))); + downloadFile(store, url, headers, "source", false).storePath))); auto rev = Hash::parseAny(std::string(json[0]["id"]), htSHA1); debug("HEAD revision for '%s' is %s", url, rev.gitRev()); return rev; } - std::string getDownloadUrl(const Input & input) const override + DownloadUrl getDownloadUrl(const Input & input) const override { // FIXME: This endpoint has a rate limit threshold of 5 requests per minute auto host_url = maybeGetStrAttr(input.attrs, "url").value_or("gitlab.com"); @@ -256,12 +290,14 @@ struct GitLabInputScheme : GitArchiveInputScheme host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), input.getRev()->to_string(Base16, false)); - /* # FIXME: add privat token auth (`curl --header "PRIVATE-TOKEN: "`) - std::string accessToken = settings.githubAccessToken.get(); - if (accessToken != "") - url += "?access_token=" + accessToken;*/ + std::string accessToken = settings.gitlabAccessToken.get(); + if (accessToken != "") { + auto auth_header = accessHeaderFromToken(accessToken); + return DownloadUrl(url, auth_header); + } else { + return DownloadUrl(url); + } - return url; } void clone(const Input & input, const Path & destDir) override diff --git a/src/libfetchers/registry.cc b/src/libfetchers/registry.cc index 4367ee810..551e7684a 100644 --- a/src/libfetchers/registry.cc +++ b/src/libfetchers/registry.cc @@ -145,7 +145,7 @@ static std::shared_ptr getGlobalRegistry(ref store) auto path = settings.flakeRegistry.get(); if (!hasPrefix(path, "/")) { - auto storePath = downloadFile(store, path, "flake-registry.json", false).storePath; + auto storePath = downloadFile(store, path, Headers {}, "flake-registry.json", false).storePath; if (auto store2 = store.dynamic_pointer_cast()) store2->addPermRoot(storePath, getCacheDir() + "/nix/flake-registry.json"); path = store->toRealPath(storePath); diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index a2d16365e..cf6d6e3d2 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -5,12 +5,14 @@ #include "store-api.hh" #include "archive.hh" #include "tarfile.hh" +#include "types.hh" namespace nix::fetchers { DownloadFileResult downloadFile( ref store, const std::string & url, + const Headers & headers, const std::string & name, bool immutable) { @@ -36,7 +38,7 @@ DownloadFileResult downloadFile( if (cached && !cached->expired) return useCached(); - FileTransferRequest request(url); + FileTransferRequest request(url, headers); if (cached) request.expectedETag = getStrAttr(cached->infoAttrs, "etag"); FileTransferResult res; @@ -110,6 +112,7 @@ DownloadFileResult downloadFile( std::pair downloadTarball( ref store, const std::string & url, + const Headers & headers, const std::string & name, bool immutable) { @@ -127,7 +130,7 @@ std::pair downloadTarball( getIntAttr(cached->infoAttrs, "lastModified") }; - auto res = downloadFile(store, url, name, immutable); + auto res = downloadFile(store, url, headers, name, immutable); std::optional unpackedStorePath; time_t lastModified; @@ -222,7 +225,7 @@ struct TarballInputScheme : InputScheme std::pair fetch(ref store, const Input & input) override { - auto tree = downloadTarball(store, getStrAttr(input.attrs, "url"), "source", false).first; + auto tree = downloadTarball(store, getStrAttr(input.attrs, "url"), Headers {}, "source", false).first; return {std::move(tree), input}; } }; diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 4149f8155..13ed429fa 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -112,6 +112,9 @@ struct curlFileTransfer : public FileTransfer requestHeaders = curl_slist_append(requestHeaders, ("If-None-Match: " + request.expectedETag).c_str()); if (!request.mimeType.empty()) requestHeaders = curl_slist_append(requestHeaders, ("Content-Type: " + request.mimeType).c_str()); + for (auto it = request.headers.begin(); it != request.headers.end(); ++it){ + requestHeaders = curl_slist_append(requestHeaders, fmt("%s: %s", it->first, it->second).c_str()); + } } ~TransferItem() diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index 0d608c8d8..7e302ff39 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -51,6 +51,7 @@ extern FileTransferSettings fileTransferSettings; struct FileTransferRequest { std::string uri; + Headers headers; std::string expectedETag; bool verifyTLS = true; bool head = false; @@ -65,6 +66,9 @@ struct FileTransferRequest FileTransferRequest(const std::string & uri) : uri(uri), parentAct(getCurActivity()) { } + FileTransferRequest(const std::string & uri, Headers headers) + : uri(uri), headers(headers) { } + std::string verb() { return data ? "upload" : "download"; diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 02721285a..b2e7610ee 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -863,6 +863,9 @@ public: Setting githubAccessToken{this, "", "github-access-token", "GitHub access token to get access to GitHub data through the GitHub API for `github:<..>` flakes."}; + Setting gitlabAccessToken{this, "", "gitlab-access-token", + "GitLab access token to get access to GitLab data through the GitLab API for gitlab:<..> flakes."}; + Setting experimentalFeatures{this, {}, "experimental-features", "Experimental Nix features to enable."}; diff --git a/src/libutil/types.hh b/src/libutil/types.hh index 3af485fa0..55d02bcf9 100644 --- a/src/libutil/types.hh +++ b/src/libutil/types.hh @@ -24,6 +24,8 @@ typedef string Path; typedef list Paths; typedef set PathSet; +typedef vector> Headers; + /* Helper class to run code at startup. */ template struct OnStartup diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index 3ccf620c9..94d33a75c 100755 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -87,7 +87,7 @@ static void update(const StringSet & channelNames) // We want to download the url to a file to see if it's a tarball while also checking if we // got redirected in the process, so that we can grab the various parts of a nix channel // definition from a consistent location if the redirect changes mid-download. - auto result = fetchers::downloadFile(store, url, std::string(baseNameOf(url)), false); + auto result = fetchers::downloadFile(store, url, Headers {}, std::string(baseNameOf(url)), false); auto filename = store->toRealPath(result.storePath); url = result.effectiveUrl; @@ -112,9 +112,9 @@ static void update(const StringSet & channelNames) if (!unpacked) { // Download the channel tarball. try { - filename = store->toRealPath(fetchers::downloadFile(store, url + "/nixexprs.tar.xz", "nixexprs.tar.xz", false).storePath); + filename = store->toRealPath(fetchers::downloadFile(store, url + "/nixexprs.tar.xz", Headers {}, "nixexprs.tar.xz", false).storePath); } catch (FileTransferError & e) { - filename = store->toRealPath(fetchers::downloadFile(store, url + "/nixexprs.tar.bz2", "nixexprs.tar.bz2", false).storePath); + filename = store->toRealPath(fetchers::downloadFile(store, url + "/nixexprs.tar.bz2", Headers {}, "nixexprs.tar.bz2", false).storePath); } }