Repository: huggingface/huggingface_hub
Pull request #277 (2021-08-13 10:21:23)

Git credentials
*Thoroughly tested in practice, will implement unit tests in a bit but the API will remain as it is so pinging you for review.*

Fixes https://github.com/huggingface/huggingface_hub/issues/211 and https://github.com/huggingface/moon-landing/issues/150

#### Problem

This PR offers a long-awaited overhaul of the login system through the `huggingface_hub` library. The way the login system is currently designed is complex and led to a lot of errors mid-training during the Jax/Flax community week - as well as a general misunderstanding of what was happening behind the scenes.

The core issue is that logging in through `huggingface_cli login` does *not* log the user through git. The user will have to reauthenticate when pushing. This is particularly an issue on Google Colab, where a simple `git push` is met by a fatal error:
```
fatal: could not read Username for 'https://huggingface.co': No such device or address
```
The workarounds are cumbersome (see [#12572](https://github.com/huggingface/transformers/issues/12572)).

#### Solving the issue

This PR offers to use the `git-credential` API to store the username and password in a way that git can leverage when authenticating the user on `huggingface.co`. It leverages the `git credential.helper store` helper, which stores the username/password under plain text in a file with the user's permission.

It does not set the global credential helper to `store` as this could disrupt the user's git configuration - it instead detects what is the current helper, and if it isn't `store`, mentions that users will need to reauthenticate. If users do not wish to reauthenticate, a command to be run (which will set the helper to `store`) is printed out.

Additionally, in order to improve the experience across platforms, this PR adjusts two configuration attributes which are linked to the above:
- Cloning a repository through `Repository(xxx, clone_from=yyy)` will set the helper of that repository to `store`. 
- Using the `Repository` without explicitly passing a `git_email` or a `git_user`, instead of leveraging the system's default, will leverage the token to be used by that `Repository`.



Patch:
diff --git a/src/huggingface_hub/commands/user.py b/src/huggingface_hub/commands/user.py
index 1da7debe..1755033a 100644
--- a/src/huggingface_hub/commands/user.py
+++ b/src/huggingface_hub/commands/user.py
@@ -117,6 +117,27 @@ def tabulate(rows: List[List[Union[str, int]]], headers: List[str]) -> str:
     return "\n".join(lines)
 
 
+def currently_setup_credential_helpers(directory=None) -> List[str]:
+    try:
+        output = subprocess.run(
+            "git config --list".split(),
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            encoding="utf-8",
+            check=True,
+            cwd=directory,
+        ).stdout.split("\n")
+
+        current_credential_helpers = []
+        for line in output:
+            if "credential.helper" in line:
+                current_credential_helpers.append(line.split("=")[-1])
+    except subprocess.CalledProcessError as exc:
+        raise EnvironmentError(exc.stderr)
+
+    return current_credential_helpers
+
+
 class BaseUserCommand:
     def __init__(self, args):
         self.args = args
@@ -147,6 +168,16 @@ class LoginCommand(BaseUserCommand):
         HfFolder.save_token(token)
         print("Login successful")
         print("Your token has been saved to", HfFolder.path_token)
+        helpers = currently_setup_credential_helpers()
+
+        if "store" not in helpers:
+            print(
+                ANSI.red(
+                    "Authenticated through git-crendential store but this isn't the helper defined on your machine.\nYou "
+                    "will have to re-authenticate when pushing to the Hugging Face Hub. Run the following command in your "
+                    "terminal to set it as the default\n\ngit config --global credential.helper store"
+                )
+            )
 
 
 class WhoamiCommand(BaseUserCommand):
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
index b730bfbc..b1a32e19 100644
--- a/src/huggingface_hub/hf_api.py
+++ b/src/huggingface_hub/hf_api.py
@@ -16,11 +16,12 @@
 
 import os
 import re
+import subprocess
 import sys
 import warnings
 from io import BufferedIOBase, RawIOBase
 from os.path import expanduser
-from typing import BinaryIO, Dict, Iterable, List, Optional, Union
+from typing import BinaryIO, Dict, Iterable, List, Optional, Tuple, Union
 
 import requests
 from requests.exceptions import HTTPError
@@ -224,6 +225,79 @@ class DatasetInfo:
         return r
 
 
+def write_to_credential_store(username: str, password: str):
+    with subprocess.Popen(
+        "git credential-store store".split(),
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+    ) as process:
+        input_username = f"username={username.lower()}"
+        input_password = f"password={password}"
+
+        process.stdin.write(
+            f"url={ENDPOINT}\n{input_username}\n{input_password}\n\n".encode("utf-8")
+        )
+        process.stdin.flush()
+
+
+def read_from_credential_store(
+    username=None,
+) -> Tuple[Union[str, None], Union[str, None]]:
+    """
+    Reads the credential store relative to huggingface.co. If no `username` is specified, will read the first
+    entry for huggingface.co, otherwise will read the entry corresponding to the username specified.
+
+    The username returned will be all lowercase.
+    """
+    with subprocess.Popen(
+        "git credential-store get".split(),
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+    ) as process:
+        standard_input = f"url={ENDPOINT}\n"
+
+        if username is not None:
+            standard_input += f"username={username.lower()}\n"
+
+        standard_input += "\n"
+
+        process.stdin.write(standard_input.encode("utf-8"))
+        process.stdin.flush()
+        output = process.stdout.read()
+        output = output.decode("utf-8")
+
+    if len(output) == 0:
+        return None, None
+
+    username, password = [line for line in output.split("\n") if len(line) != 0]
+    return username.split("=")[1], password.split("=")[1]
+
+
+def erase_from_credential_store(username=None):
+    """
+    Erases the credential store relative to huggingface.co. If no `username` is specified, will erase the first
+    entry for huggingface.co, otherwise will erase the entry corresponding to the username specified.
+    """
+    with subprocess.Popen(
+        "git credential-store erase".split(),
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+    ) as process:
+        standard_input = f"url={ENDPOINT}\n"
+
+        if username is not None:
+            standard_input += f"username={username.lower()}\n"
+
+        standard_input += "\n"
+
+        process.stdin.write(standard_input.encode("utf-8"))
+        print(standard_input)
+        process.stdin.flush()
+
+
 class HfApi:
     def __init__(self, endpoint=None):
         self.endpoint = endpoint if endpoint is not None else ENDPOINT
@@ -240,6 +314,8 @@ class HfApi:
         r = requests.post(path, json={"username": username, "password": password})
         r.raise_for_status()
         d = r.json()
+
+        write_to_credential_store(username, password)
         return d["token"]
 
     def whoami(self, token: str) -> Dict:
@@ -258,6 +334,9 @@ class HfApi:
         """
         Call HF API to log out.
         """
+        username = self.whoami(token)["name"]
+        erase_from_credential_store(username)
+
         path = "{}/api/logout".format(self.endpoint)
         r = requests.post(path, headers={"authorization": "Bearer {}".format(token)})
         r.raise_for_status()
diff --git a/src/huggingface_hub/repository.py b/src/huggingface_hub/repository.py
index cda08e79..2d7b2769 100644
--- a/src/huggingface_hub/repository.py
+++ b/src/huggingface_hub/repository.py
@@ -300,11 +300,22 @@ class Repository:
                     "If not specifying `clone_from`, you need to pass Repository a valid git clone."
                 )
 
-        # overrides .git config if user and email is provided.
+        if self.huggingface_token is not None and (
+            git_email is None or git_user is None
+        ):
+            user = HfApi().whoami(self.huggingface_token)
+
+            if git_email is None:
+                git_email = user["email"]
+
+            if git_user is None:
+                git_user = user["fullname"]
+
         if git_user is not None or git_email is not None:
             self.git_config_username_and_email(git_user, git_email)
 
         self.lfs_enable_largefiles()
+        self.git_credential_helper_store()
 
         if revision is not None:
             self.git_checkout(revision, create_branch_ok=True)
@@ -501,6 +512,22 @@ class Repository:
         except subprocess.CalledProcessError as exc:
             raise EnvironmentError(exc.stderr)
 
+    def git_credential_helper_store(self):
+        """
+        sets the git credential helper to `store`
+        """
+        try:
+            subprocess.run(
+                ["git", "config", "credential.helper", "store"],
+                stderr=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+                check=True,
+                encoding="utf-8",
+                cwd=self.local_dir,
+            )
+        except subprocess.CalledProcessError as exc:
+            raise EnvironmentError(exc.stderr)
+
     def git_head_hash(self) -> str:
         """
         Get commit sha on top of HEAD.


Test patch:
diff --git a/tests/test_hf_api.py b/tests/test_hf_api.py
index ffb941c6..a66ac12b 100644
--- a/tests/test_hf_api.py
+++ b/tests/test_hf_api.py
@@ -29,6 +29,8 @@ from huggingface_hub.hf_api import (
     HfFolder,
     ModelInfo,
     RepoObj,
+    erase_from_credential_store,
+    read_from_credential_store,
     repo_type_and_id_from_hf_id,
 )
 from requests.exceptions import HTTPError
@@ -67,6 +69,13 @@ class HfApiCommonTest(unittest.TestCase):
 
 
 class HfApiLoginTest(HfApiCommonTest):
+    def setUp(self) -> None:
+        erase_from_credential_store(USER)
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        cls._api.login(username=USER, password=PASS)
+
     def test_login_invalid(self):
         with self.assertRaises(HTTPError):
             self._api.login(username=USER, password="fake")
@@ -75,6 +84,13 @@ class HfApiLoginTest(HfApiCommonTest):
         token = self._api.login(username=USER, password=PASS)
         self.assertIsInstance(token, str)
 
+    def test_login_git_credentials(self):
+        self.assertTupleEqual(read_from_credential_store(USER), (None, None))
+        self._api.login(username=USER, password=PASS)
+        self.assertTupleEqual(read_from_credential_store(USER), (USER.lower(), PASS))
+        erase_from_credential_store(username=USER)
+        self.assertTupleEqual(read_from_credential_store(USER), (None, None))
+
 
 class HfApiCommonTestWithLogin(HfApiCommonTest):
     @classmethod
diff --git a/tests/test_repository.py b/tests/test_repository.py
index 0b78b091..2208334b 100644
--- a/tests/test_repository.py
+++ b/tests/test_repository.py
@@ -22,6 +22,7 @@ import unittest
 from io import BytesIO
 
 import requests
+from huggingface_hub.commands.user import currently_setup_credential_helpers
 from huggingface_hub.hf_api import HfApi
 from huggingface_hub.repository import (
     Repository,
@@ -815,6 +816,76 @@ class RepositoryOfflineTest(RepositoryCommonTest):
         files = os.listdir(current_head_repo.local_dir)
         self.assertIn("file.txt", files)
 
+    def test_repo_user(self):
+        api = HfApi(endpoint=ENDPOINT_STAGING)
+        token = api.login(USER, PASS)
+
+        repo = Repository(WORKING_REPO_DIR, use_auth_token=token)
+        user = api.whoami(token)
+
+        username = subprocess.run(
+            ["git", "config", "user.name"],
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            check=True,
+            encoding="utf-8",
+            cwd=repo.local_dir,
+        ).stdout.strip()
+        email = subprocess.run(
+            ["git", "config", "user.email"],
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            check=True,
+            encoding="utf-8",
+            cwd=repo.local_dir,
+        ).stdout.strip()
+
+        self.assertEqual(username, user["fullname"])
+        self.assertEqual(email, user["email"])
+
+    def test_repo_passed_user(self):
+        api = HfApi(endpoint=ENDPOINT_STAGING)
+        token = api.login(USER, PASS)
+        repo = Repository(
+            WORKING_REPO_DIR,
+            git_user="RANDOM_USER",
+            git_email="EMAIL@EMAIL.EMAIL",
+            use_auth_token=token,
+        )
+        username = subprocess.run(
+            ["git", "config", "user.name"],
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            check=True,
+            encoding="utf-8",
+            cwd=repo.local_dir,
+        ).stdout.strip()
+        email = subprocess.run(
+            ["git", "config", "user.email"],
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            check=True,
+            encoding="utf-8",
+            cwd=repo.local_dir,
+        ).stdout.strip()
+
+        self.assertEqual(username, "RANDOM_USER")
+        self.assertEqual(email, "EMAIL@EMAIL.EMAIL")
+
+    def test_correct_helper(self):
+        subprocess.run(
+            ["git", "config", "--global", "credential.helper", "get"],
+            stderr=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            check=True,
+            encoding="utf-8",
+        )
+        repo = Repository(WORKING_REPO_DIR)
+        self.assertListEqual(
+            currently_setup_credential_helpers(repo.local_dir), ["get", "store"]
+        )
+        self.assertEqual(currently_setup_credential_helpers(), ["get"])
+
 
 class RepositoryDatasetTest(RepositoryCommonTest):
     @classmethod