Repository: repobee/repobee

Issue:
Move `_insert_token` and `_insert_user_and_token` from the `git` module
The `_insert_token` and `_insert_user_and_token` functions in the `git` module should be extracted somewhere else, because the use differs between GitHub and GitLab. For example, GitLab requires the token inserted on the form `oauth2:<TOKEN>@url` for _all_ token-related commands, while GitHub is satisfied with `<TOKEN>@url` for all but pushing, which requires `<USERNAME>:<TOKEN>@url`.

It may make sense to make this particular functionality part of the plugin system, or possibly just put it in the API class as this is platform-dependent authentication. Could have e.g. push auth and pull auth functions that do something to the URL.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=h1) Report
> Merging [#185](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=desc) into [master](https://codecov.io/gh/repobee/repobee/commit/a89a3aaf62a8ff8b0029013f64bac94d7a7ce3c9?src=pr&el=desc) will **decrease** coverage by `0.22%`.
> The diff coverage is `95.34%`.

[![Impacted file tree graph](https://codecov.io/gh/repobee/repobee/pull/185/graphs/tree.svg?width=650&token=5vOKgkphZe&height=150&src=pr)](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
- Coverage   92.99%   92.76%   -0.23%     
==========================================
  Files          17       17              
  Lines        1128     1134       +6     
  Branches      197      198       +1     
==========================================
+ Hits         1049     1052       +3     
- Misses         65       67       +2     
- Partials       14       15       +1
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [repobee/cli.py](https://codecov.io/gh/repobee/repobee/pull/185/diff?src=pr&el=tree#diff-cmVwb2JlZS9jbGkucHk=) | `97.46% <100%> (-0.79%)` | :arrow_down: |
| [repobee/git.py](https://codecov.io/gh/repobee/repobee/pull/185/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXQucHk=) | `97.5% <100%> (-0.23%)` | :arrow_down: |
| [repobee/command.py](https://codecov.io/gh/repobee/repobee/pull/185/diff?src=pr&el=tree#diff-cmVwb2JlZS9jb21tYW5kLnB5) | `95.37% <100%> (ø)` | :arrow_up: |
| [repobee/github\_api.py](https://codecov.io/gh/repobee/repobee/pull/185/diff?src=pr&el=tree#diff-cmVwb2JlZS9naXRodWJfYXBpLnB5) | `92.2% <75%> (-0.24%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=footer). Last update [a89a3aa...ed8e9dd](https://codecov.io/gh/repobee/repobee/pull/185?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/repobee/cli.py b/repobee/cli.py
index 6fd5d48..0c7cf53 100644
--- a/repobee/cli.py
+++ b/repobee/cli.py
@@ -12,8 +12,8 @@ import argparse
 import pathlib
 import os
 import sys
+import re
 from contextlib import contextmanager
-from collections import namedtuple
 from typing import List, Iterable, Optional, Tuple
 
 import logging
@@ -48,6 +48,21 @@ daiquiri.setup(
     ),
 )
 
+
+def _filter_tokens():
+    """Filter out any secure tokens from log output."""
+    old_factory = logging.getLogRecordFactory()
+
+    def record_factory(*args, **kwargs):
+        record = old_factory(*args, **kwargs)
+        record.msg = re.sub("https://.*?@", "https://", record.msg)
+        return record
+
+    logging.setLogRecordFactory(record_factory)
+
+
+_filter_tokens()
+
 LOGGER = daiquiri.getLogger(__file__)
 SUB = "subparser"
 
@@ -144,7 +159,12 @@ def parse_args(
         # only if clone is chosen should plugins be able to hook in
         plug.manager.hook.parse_args(args=args)
 
-    api = _connect_to_api(args.github_base_url, token, args.org_name)
+    api = _connect_to_api(
+        args.github_base_url,
+        token,
+        args.org_name,
+        args.user if "user" in args else None,
+    )
 
     if "master_repo_urls" in args and args.master_repo_urls:
         master_urls = args.master_repo_urls
@@ -240,16 +260,12 @@ def dispatch_command(args: tuples.Args, api: github_api.GitHubAPI):
     if args.subparser == SETUP_PARSER:
         with _sys_exit_on_expected_error():
             command.setup_student_repos(
-                args.master_repo_urls, args.students, args.user, api
+                args.master_repo_urls, args.students, api
             )
     elif args.subparser == UPDATE_PARSER:
         with _sys_exit_on_expected_error():
             command.update_student_repos(
-                args.master_repo_urls,
-                args.students,
-                args.user,
-                api,
-                issue=args.issue,
+                args.master_repo_urls, args.students, api, issue=args.issue
             )
     elif args.subparser == OPEN_ISSUE_PARSER:
         with _sys_exit_on_expected_error():
@@ -263,7 +279,7 @@ def dispatch_command(args: tuples.Args, api: github_api.GitHubAPI):
             )
     elif args.subparser == MIGRATE_PARSER:
         with _sys_exit_on_expected_error():
-            command.migrate_repos(args.master_repo_urls, args.user, api)
+            command.migrate_repos(args.master_repo_urls, api)
     elif args.subparser == CLONE_PARSER:
         with _sys_exit_on_expected_error():
             command.clone_repos(args.master_repo_names, args.students, api)
@@ -836,11 +852,11 @@ def _extract_groups(args: argparse.Namespace) -> List[str]:
 
 
 def _connect_to_api(
-    github_base_url: str, token: str, org_name: str
+    github_base_url: str, token: str, org_name: str, user: str
 ) -> github_api.GitHubAPI:
     """Return a GitHubAPI instance connected to the specified API endpoint."""
     try:
-        api = github_api.GitHubAPI(github_base_url, token, org_name)
+        api = github_api.GitHubAPI(github_base_url, token, org_name, user)
     except exception.NotFoundError:
         # more informative message
         raise exception.NotFoundError(
diff --git a/repobee/command.py b/repobee/command.py
index 5a6964c..7a11982 100644
--- a/repobee/command.py
+++ b/repobee/command.py
@@ -42,7 +42,6 @@ MASTER_TEAM = "master_repos"
 def setup_student_repos(
     master_repo_urls: Iterable[str],
     students: Iterable[tuples.Group],
-    user: str,
     api: GitHubAPI,
 ) -> None:
     """Setup student repositories based on master repo templates. Performs three
@@ -64,21 +63,20 @@ def setup_student_repos(
         master_repo_urls: URLs to master repos. Must be in the organization
         that the api is set up for.
         students: An iterable of student GitHub usernames.
-        user: Username of the administrator that setting up the repos.
         api: A GitHubAPI instance used to interface with the GitHub instance.
     """
     urls = list(master_repo_urls)  # safe copy
 
     with tempfile.TemporaryDirectory() as tmpdir:
         LOGGER.info("cloning into master repos ...")
-        master_repo_paths = _clone_all(urls, api.token, cwd=tmpdir)
+        master_repo_paths = _clone_all(urls, cwd=tmpdir)
 
         teams = add_students_to_teams(students, api)
         repo_urls = _create_student_repos(urls, teams, api)
 
         push_tuples = _create_push_tuples(master_repo_paths, repo_urls)
         LOGGER.info("pushing files to student repos ...")
-        git.push(push_tuples, user=user, token=api.token)
+        git.push(push_tuples)
 
 
 def add_students_to_teams(
@@ -122,14 +120,13 @@ def _create_student_repos(
     return repo_urls
 
 
-def _clone_all(urls: Iterable[str], token: str, cwd: str):
+def _clone_all(urls: Iterable[str], cwd: str):
     """Attempts to clone all urls. If a repo is already present, it is skipped.
     If any one clone fails (except for fails because the repo is local),
     all cloned repos are removed
 
     Args:
         urls: HTTPS urls to git repositories.
-        token: A GitHub OATH token.
         cwd: Working directory. Use temporary directory for automatic cleanup.
     Returns:
         local paths to the cloned repos.
@@ -139,7 +136,7 @@ def _clone_all(urls: Iterable[str], token: str, cwd: str):
     try:
         for url in urls:
             LOGGER.info("cloning into {}".format(url))
-            git.clone_single(url, token, cwd=cwd)
+            git.clone_single(url, cwd=cwd)
     except exception.CloneFailedError:
         LOGGER.error("error cloning into {}, aborting ...".format(url))
         raise
@@ -151,7 +148,6 @@ def _clone_all(urls: Iterable[str], token: str, cwd: str):
 def update_student_repos(
     master_repo_urls: Iterable[str],
     students: Iterable[tuples.Group],
-    user: str,
     api: GitHubAPI,
     issue: Optional[tuples.Issue] = None,
 ) -> None:
@@ -161,7 +157,6 @@ def update_student_repos(
         master_repo_urls: URLs to master repos. Must be in the organization
             that the api is set up for.
         students: An iterable of student GitHub usernames.
-        user: Username of the administrator that setting up the repos.
         api: A GitHubAPI instance used to interface with the GitHub instance.
         issue: An optional issue to open in repos to which pushing fails.
     """
@@ -177,12 +172,12 @@ def update_student_repos(
 
     with tempfile.TemporaryDirectory() as tmpdir:
         LOGGER.info("cloning into master repos ...")
-        master_repo_paths = _clone_all(urls, api.token, tmpdir)
+        master_repo_paths = _clone_all(urls, tmpdir)
 
         push_tuples = _create_push_tuples(master_repo_paths, repo_urls)
 
         LOGGER.info("pushing files to student repos ...")
-        failed_urls = git.push(push_tuples, user=user, token=api.token)
+        failed_urls = git.push(push_tuples)
 
     if failed_urls and issue:
         LOGGER.info("Opening issue in repos to which push failed")
@@ -365,7 +360,7 @@ def clone_repos(
     repo_urls = api.get_repo_urls(repo_names)
 
     LOGGER.info("cloning into student repos ...")
-    git.clone(repo_urls, api.token)
+    git.clone(repo_urls)
 
     if (
         len(plug.manager.get_plugins()) > 1
@@ -389,16 +384,13 @@ def _execute_post_clone_hooks(repo_names: List[str], api: GitHubAPI):
     LOGGER.info("post clone hooks done")
 
 
-def migrate_repos(
-    master_repo_urls: Iterable[str], user: str, api: GitHubAPI
-) -> None:
+def migrate_repos(master_repo_urls: Iterable[str], api: GitHubAPI) -> None:
     """Migrate a repository from an arbitrary URL to the target organization.
     The new repository is added to the master_repos team, which is created if
     it does not already exist.
 
     Args:
         master_repo_urls: HTTPS URLs to the master repos to migrate.
-        user: username of the administrator performing the migration. This is
         the username that is used in the push.
         api: A GitHubAPI instance used to interface with the GitHub instance.
     """
@@ -417,7 +409,7 @@ def migrate_repos(
     ]
 
     with tempfile.TemporaryDirectory() as tmpdir:
-        _clone_all(master_repo_urls, api.token, cwd=tmpdir)
+        _clone_all(master_repo_urls, cwd=tmpdir)
         repo_urls = api.create_repos(infos)
 
         git.push(
@@ -428,9 +420,7 @@ def migrate_repos(
                     branch="master",
                 )
                 for repo_url, info in zip(repo_urls, infos)
-            ],
-            user=user,
-            token=api.token,
+            ]
         )
 
     LOGGER.info("done!")
diff --git a/repobee/git.py b/repobee/git.py
index bafb749..50c3131 100644
--- a/repobee/git.py
+++ b/repobee/git.py
@@ -39,29 +39,23 @@ def _git_init(dirpath):
     captured_run(["git", "init"], cwd=str(dirpath))
 
 
-def _pull_clone(repo_url: str, token: str, branch: str = "", cwd: str = "."):
-    """Simulate a clone with a pull to avoid writing the token to disk."""
+def _pull_clone(repo_url: str, branch: str = "", cwd: str = "."):
+    """Simulate a clone with a pull to avoid writing remotes (that could
+    include secure tokens) to disk.
+    """
     dirpath = _ensure_repo_dir_exists(repo_url, cwd)
 
-    url_with_token = _insert_token(repo_url, token)
-    pull_command = (
-        "git pull {} {}".format(url_with_token, branch).strip().split()
-    )
+    pull_command = "git pull {} {}".format(repo_url, branch).strip().split()
 
     rc, _, stderr = captured_run(pull_command, cwd=str(dirpath))
     return rc, stderr
 
 
-async def _pull_clone_async(
-    repo_url: str, token: str, branch: str = "", cwd: str = "."
-):
+async def _pull_clone_async(repo_url: str, branch: str = "", cwd: str = "."):
     """Same as _pull_clone, but asynchronously."""
     dirpath = _ensure_repo_dir_exists(repo_url, cwd)
 
-    url_with_token = _insert_token(repo_url, token)
-    pull_command = (
-        "git pull {} {}".format(url_with_token, branch).strip().split()
-    )
+    pull_command = "git pull {} {}".format(repo_url, branch).strip().split()
 
     proc = await asyncio.create_subprocess_exec(
         *pull_command,
@@ -73,38 +67,6 @@ async def _pull_clone_async(
     return proc.returncode, stderr
 
 
-def _insert_token(url: str, token: str) -> str:
-    """Insert a token into the url as described here:
-        https://blog.github.com/2012-09-21-easier-builds-and-deployments-using-git-over-https-and-oauth/
-
-    Args:
-        url: A url to a git repo.
-        token: A GitHub OAUTH token, with or without username (e.g. on the form
-        `<token>` or `<username>:<token>`)
-
-    Returns:
-        The provided url with the token inserted
-    """
-    if not token:
-        raise ValueError("invalid token, empty token not allowed")
-    return url.replace("https://", "https://{}@".format(token))
-
-
-def _insert_user_and_token(https_url: str, user: str, token: str) -> str:
-    """Insert a username and an oauth token into the https url as described here:
-        https://blog.github.com/2012-09-21-easier-builds-and-deployments-using-git-over-https-and-oauth/
-
-    Args:
-        https_url: A url on the form `https://host.topdomain`
-        user: A GitHub username.
-        token: A GitHub OAUTH token.
-
-    Returns:
-        The provided url with the username and token inserted
-    """
-    return _insert_token(https_url, "{}:{}".format(user, token))
-
-
 def captured_run(*args, **kwargs):
     """Run a subprocess and capture the output."""
     proc = subprocess.run(
@@ -113,7 +75,7 @@ def captured_run(*args, **kwargs):
     return proc.returncode, proc.stdout, proc.stderr
 
 
-def clone_single(repo_url: str, token: str, branch: str = "", cwd: str = "."):
+def clone_single(repo_url: str, branch: str = "", cwd: str = "."):
     """Clone a git repository.
 
     Args:
@@ -121,25 +83,23 @@ def clone_single(repo_url: str, token: str, branch: str = "", cwd: str = "."):
             https://<host>/<owner>/<repo>.
         branch: The branch to clone.
         cwd: Working directory. Defaults to the current directory.
-        token: A GitHub OAUTH token.
     """
-    rc, stderr = _pull_clone(repo_url, token, branch, cwd)
+    rc, stderr = _pull_clone(repo_url, branch, cwd)
     if rc != 0:
         raise exception.CloneFailedError(
             "Failed to clone", rc, stderr, repo_url
         )
 
 
-async def _clone_async(repo_url: str, token: str, branch: str = "", cwd="."):
+async def _clone_async(repo_url: str, branch: str = "", cwd="."):
     """Clone git repositories asynchronously.
 
     Args:
         repo_url: A url to clone.
         branch: Which branch to clone.
         cwd: Working directory.
-        token: A GitHub OAUTH token.
     """
-    rc, stderr = await _pull_clone_async(repo_url, token, branch, cwd)
+    rc, stderr = await _pull_clone_async(repo_url, branch, cwd)
 
     if rc != 0:
         raise exception.CloneFailedError(
@@ -152,15 +112,12 @@ async def _clone_async(repo_url: str, token: str, branch: str = "", cwd="."):
         LOGGER.info("Cloned into {}".format(repo_url))
 
 
-def clone(
-    repo_urls: Iterable[str], token: str, cwd: str = "."
-) -> List[Exception]:
+def clone(repo_urls: Iterable[str], cwd: str = ".") -> List[Exception]:
     """Clone all repos asynchronously.
 
     Args:
         repo_urls: URLs to repos to clone.
         cwd: Working directory. Defaults to the current directory.
-        token: A GitHub OAUTH token.
 
     Returns:
         URLs from which cloning failed.
@@ -168,25 +125,18 @@ def clone(
     # TODO valdate repo_urls
     return [
         exc.url
-        for exc in _batch_execution(_clone_async, repo_urls, token, cwd=cwd)
+        for exc in _batch_execution(_clone_async, repo_urls, cwd=cwd)
         if isinstance(exc, exception.CloneFailedError)
     ]
 
 
-async def _push_async(pt: Push, user: str, token: str):
+async def _push_async(pt: Push):
     """Asynchronous call to git push, pushing directly to the repo_url and branch.
 
     Args:
         pt: A Push namedtuple.
-        user: The username to use in the push.
-        token: A GitHub OAUTH token.
     """
-    command = [
-        "git",
-        "push",
-        _insert_user_and_token(pt.repo_url, user, token),
-        pt.branch,
-    ]
+    command = ["git", "push", pt.repo_url, pt.branch]
     proc = await asyncio.create_subprocess_exec(
         *command,
         cwd=os.path.abspath(pt.local_path),
@@ -208,9 +158,7 @@ async def _push_async(pt: Push, user: str, token: str):
         LOGGER.info("Pushed files to {} {}".format(pt.repo_url, pt.branch))
 
 
-def _push_no_retry(
-    push_tuples: Iterable[Push], user: str, token: str
-) -> List[str]:
+def _push_no_retry(push_tuples: Iterable[Push]) -> List[str]:
     """Push to all repos defined in push_tuples asynchronously. Amount of
     concurrent tasks is limited by CONCURRENT_TASKS.
 
@@ -218,8 +166,6 @@ def _push_no_retry(
 
     Args:
         push_tuples: Push namedtuples defining local and remote repos.
-        user: The username to put in the push.
-        token: A GitHub OAUTH token.
 
     Returns:
         urls to which pushes failed with exception.PushFailedError. Other
@@ -227,14 +173,12 @@ def _push_no_retry(
     """
     return [
         exc.url
-        for exc in _batch_execution(_push_async, push_tuples, user, token)
+        for exc in _batch_execution(_push_async, push_tuples)
         if isinstance(exc, exception.PushFailedError)
     ]
 
 
-def push(
-    push_tuples: Iterable[Push], user: str, token: str, tries: int = 3
-) -> List[str]:
+def push(push_tuples: Iterable[Push], tries: int = 3) -> List[str]:
     """Push to all repos defined in push_tuples asynchronously. Amount of
     concurrent tasks is limited by CONCURRENT_TASKS. Pushing to repos is tried
     a maximum of ``tries`` times (i.e. pushing is _retried_ ``tries - 1``
@@ -242,8 +186,6 @@ def push(
 
     Args:
         push_tuples: Push namedtuples defining local and remote repos.
-        user: The username to put in the push.
-        token: A GitHub OAUTH token.
         tries: Amount of times to try to push (including initial push).
 
     Returns:
@@ -256,7 +198,7 @@ def push(
     failed_pts = list(push_tuples)
     for i in range(tries):
         LOGGER.info("pushing, attempt {}/{}".format(i + 1, tries))
-        failed_urls = set(_push_no_retry(failed_pts, user, token))
+        failed_urls = set(_push_no_retry(failed_pts))
         failed_pts = [pt for pt in push_tuples if pt.repo_url in failed_urls]
         if not failed_pts:
             break
diff --git a/repobee/github_api.py b/repobee/github_api.py
index 6243699..d16e6e1 100644
--- a/repobee/github_api.py
+++ b/repobee/github_api.py
@@ -101,7 +101,9 @@ class GitHubAPI:
     organization.
     """
 
-    def __init__(self, base_url: str, token: str, org_name: str):
+    def __init__(
+        self, base_url: str, token: str, org_name: str, user: str = None
+    ):
         """Set up the GitHub API object.
 
         Args:
@@ -109,12 +111,14 @@ class GitHubAPI:
             https://api.github.com for GitHub or https://<HOST>/api/v3 for
             Enterprise).
             token: A GitHub OAUTH token.
+            user: Name of the current user of the API.
             org_name: Name of the target organization.
         """
         self._github = github.Github(login_or_token=token, base_url=base_url)
         self._org_name = org_name
         self._base_url = base_url
         self._token = token
+        self._user = user
         with _try_api_request():
             self._org = self._github.get_organization(self._org_name)
 
@@ -320,7 +324,7 @@ class GitHubAPI:
                     "{}/{} already exists".format(self._org_name, info.name)
                 )
 
-        return repo_urls
+        return [self._insert_auth(url) for url in repo_urls]
 
     def get_repo_urls(
         self, repo_names: Iterable[str], org_name: Optional[str] = None
@@ -344,10 +348,36 @@ class GitHubAPI:
                 else self._github.get_organization(org_name)
             )
         return [
-            "{}/{}".format(org.html_url, repo_name)
-            for repo_name in list(repo_names)
+            self._insert_auth(url)
+            for url in (
+                "{}/{}".format(org.html_url, repo_name)
+                for repo_name in list(repo_names)
+            )
         ]
 
+    def _insert_auth(self, repo_url: str):
+        """Insert an authentication token into the url.
+
+        Args:
+            repo_url: A HTTPS url to a repository.
+        Returns:
+            the input url with an authentication token inserted.
+        """
+        if not repo_url.startswith("https://"):
+            raise ValueError(
+                "unsupported protocol in '{}', please use https:// ".format(
+                    repo_url
+                )
+            )
+        # TODO in RepoBee 2.0, user will always be available
+        # user may not always be available
+        auth = (
+            self.token
+            if not self._user
+            else "{}:{}".format(self._user, self.token)
+        )
+        return repo_url.replace("https://", "https://{}@".format(auth))
+
     def get_issues(
         self,
         repo_names: Iterable[str],


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 42ed097..f237e2c 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -173,7 +173,7 @@ class TestDispatchCommand:
         cli.dispatch_command(args, api_instance_mock)
 
         command_mock.setup_student_repos.assert_called_once_with(
-            args.master_repo_urls, args.students, args.user, api_instance_mock
+            args.master_repo_urls, args.students, api_instance_mock
         )
 
     def test_update_student_repos_called_with_correct_args(
@@ -186,7 +186,6 @@ class TestDispatchCommand:
         command_mock.update_student_repos.assert_called_once_with(
             args.master_repo_urls,
             args.students,
-            args.user,
             api_instance_mock,
             issue=args.issue,
         )
@@ -227,7 +226,7 @@ class TestDispatchCommand:
         cli.dispatch_command(args, api_instance_mock)
 
         command_mock.migrate_repos.assert_called_once_with(
-            args.master_repo_urls, args.user, api_instance_mock
+            args.master_repo_urls, api_instance_mock
         )
 
     def test_clone_repos_called_with_correct_args(
@@ -626,7 +625,7 @@ def assert_base_push_args(parsed_args, api):
     assert parsed_args.master_repo_urls == [
         generate_repo_url(rn, ORG_NAME) for rn in REPO_NAMES
     ]
-    api.assert_called_once_with(GITHUB_BASE_URL, TOKEN, ORG_NAME)
+    api.assert_called_once_with(GITHUB_BASE_URL, TOKEN, ORG_NAME, USER)
 
 
 def assert_config_args(parser, parsed_args):
diff --git a/tests/test_command.py b/tests/test_command.py
index 227bd76..9fa6929 100644
--- a/tests/test_command.py
+++ b/tests/test_command.py
@@ -4,7 +4,6 @@ from unittest.mock import patch, MagicMock, call, ANY, PropertyMock
 
 import repobee
 from repobee import command
-from repobee import github_api
 from repobee import git
 from repobee import tuples
 from repobee import util
@@ -18,7 +17,6 @@ import functions
 
 from_magic_mock_issue = functions.from_magic_mock_issue
 to_magic_mock_issue = functions.to_magic_mock_issue
-User = constants.User
 TOKEN = constants.TOKEN
 
 random_date = functions.random_date
@@ -61,10 +59,8 @@ CLOSED_ISSUES = [
     )
 ]
 
-USER = "slarse"
 ORG_NAME = "test-org"
 GITHUB_BASE_URL = "https://some_enterprise_host/api/v3"
-API = github_api.GitHubAPI("bla", "bla", "bla")
 ISSUE = tuples.Issue(
     "Oops, something went wrong!", "This is the body **with some formatting**."
 )
@@ -236,7 +232,7 @@ def assert_raises_on_duplicate_master_urls(function, master_urls, students):
     master_urls.append(master_urls[0])
 
     with pytest.raises(ValueError) as exc_info:
-        function(master_urls, USER, students, ORG_NAME, GITHUB_BASE_URL)
+        function(master_urls, students, ORG_NAME, GITHUB_BASE_URL)
     assert str(exc_info.value) == "master_repo_urls contains duplicates"
 
 
@@ -250,12 +246,12 @@ class TestSetupStudentRepos:
     def test_raises_on_clone_failure(
         self, master_urls, students, git_mock, api_mock
     ):
-        git_mock.clone_single.side_effect = lambda url, tokwn, cwd: raise_(
+        git_mock.clone_single.side_effect = lambda url, cwd: raise_(
             exception.CloneFailedError("clone failed", 128, b"some error", url)
         )()
 
         with pytest.raises(exception.CloneFailedError) as exc_info:
-            command.setup_student_repos(master_urls, students, USER, api_mock)
+            command.setup_student_repos(master_urls, students, api_mock)
 
         assert exc_info.value.url == master_urls[0]
 
@@ -265,7 +261,7 @@ class TestSetupStudentRepos:
         master_urls.append(master_urls[0])
 
         with pytest.raises(ValueError) as exc_info:
-            command.setup_student_repos(master_urls, students, USER, api_mock)
+            command.setup_student_repos(master_urls, students, api_mock)
         assert str(exc_info.value) == "master_repo_urls contains duplicates"
 
     def test_happy_path(
@@ -282,22 +278,20 @@ class TestSetupStudentRepos:
     ):
         """Test that setup_student_repos makes the correct function calls."""
         expected_clone_calls = [
-            call(url, TOKEN, cwd=str(tmpdir)) for url in master_urls
+            call(url, cwd=str(tmpdir)) for url in master_urls
         ]
         expected_ensure_teams_arg = {
             str(student): [str(student)] for student in students
         }
 
-        command.setup_student_repos(master_urls, students, USER, api_mock)
+        command.setup_student_repos(master_urls, students, api_mock)
 
         git_mock.clone_single.assert_has_calls(expected_clone_calls)
         api_mock.ensure_teams_and_members.assert_called_once_with(
             expected_ensure_teams_arg
         )
         api_mock.create_repos.assert_called_once_with(repo_infos)
-        git_mock.push.assert_called_once_with(
-            push_tuples, user=USER, token=TOKEN
-        )
+        git_mock.push.assert_called_once_with(push_tuples)
 
 
 class TestUpdateStudentRepos:
@@ -313,7 +307,7 @@ class TestUpdateStudentRepos:
         master_urls.append(master_urls[0])
 
         with pytest.raises(ValueError) as exc_info:
-            command.update_student_repos(master_urls, students, USER, api_mock)
+            command.update_student_repos(master_urls, students, api_mock)
         assert str(exc_info.value) == "master_repo_urls contains duplicates"
 
     def test_happy_path(
@@ -331,19 +325,17 @@ class TestUpdateStudentRepos:
         NOTE: Ignores the git mock.
         """
         expected_clone_calls = [
-            call(url, TOKEN, cwd=str(tmpdir)) for url in master_urls
+            call(url, cwd=str(tmpdir)) for url in master_urls
         ]
 
         api_mock.get_repo_urls.side_effect = lambda repo_names: list(
             map(generate_repo_url, repo_names)
         )
 
-        command.update_student_repos(master_urls, students, USER, api_mock)
+        command.update_student_repos(master_urls, students, api_mock)
 
         git_mock.clone_single.assert_has_calls(expected_clone_calls)
-        git_mock.push.assert_called_once_with(
-            push_tuples, user=USER, token=TOKEN
-        )
+        git_mock.push.assert_called_once_with(push_tuples)
 
     @pytest.mark.nogitmock
     @pytest.mark.parametrize(
@@ -375,7 +367,7 @@ class TestUpdateStudentRepos:
             self.generate_url(name) for name in repo_names
         ]
 
-        async def raise_specific(pt, user, token):
+        async def raise_specific(pt):
             if pt.repo_url in fail_repo_urls:
                 raise exception.PushFailedError(
                     "Push failed", 128, b"some error", pt.repo_url
@@ -384,9 +376,7 @@ class TestUpdateStudentRepos:
         mocker.patch("repobee.git._push_async", side_effect=raise_specific)
         mocker.patch("repobee.git.clone_single")
 
-        command.update_student_repos(
-            master_urls, students, USER, api_mock, issue
-        )
+        command.update_student_repos(master_urls, students, api_mock, issue)
 
         if issue:  # expect issue to be opened
             call_list = api_mock.open_issue.call_args_list
@@ -425,7 +415,7 @@ class TestUpdateStudentRepos:
             self.generate_url(name) for name in repo_names
         ]
 
-        async def raise_specific(pt, token, branch):
+        async def raise_specific(pt):
             if pt.repo_url in fail_repo_urls:
                 raise exception.PushFailedError(
                     "Push failed", 128, b"some error", pt.repo_url
@@ -434,7 +424,7 @@ class TestUpdateStudentRepos:
         mocker.patch("repobee.git._push_async", side_effect=raise_specific)
         mocker.patch("repobee.git.clone_single")
 
-        command.update_student_repos(master_urls, students, USER, api_mock)
+        command.update_student_repos(master_urls, students, api_mock)
 
         assert not api_mock.open_issue.called
 
@@ -547,7 +537,7 @@ class TestCloneRepos:
         ]
         command.clone_repos(master_names, students, api_mock)
 
-        git_mock.clone.assert_called_once_with(expected_urls, TOKEN)
+        git_mock.clone.assert_called_once_with(expected_urls)
 
     def test_executes_act_hooks(
         self, api_mock, git_mock, master_names, students, act_hook_mocks
@@ -594,7 +584,7 @@ class TestMigrateRepo:
             for name, url in zip(master_names, expected_push_urls)
         ]
         expected_clone_calls = [
-            call(url, TOKEN, cwd=str(tmpdir)) for url in master_urls
+            call(url, cwd=str(tmpdir)) for url in master_urls
         ]
 
         api_mock.create_repos.side_effect = lambda infos: [
@@ -605,16 +595,14 @@ class TestMigrateRepo:
         )
         git_push_mock = mocker.patch("repobee.git.push", autospec=True)
 
-        command.migrate_repos(master_urls, USER, api_mock)
+        command.migrate_repos(master_urls, api_mock)
 
         git_clone_mock.assert_has_calls(expected_clone_calls)
         assert api_mock.create_repos.called
         api_mock.ensure_teams_and_members.assert_called_once_with(
             {command.MASTER_TEAM: []}
         )
-        git_push_mock.assert_called_once_with(
-            expected_pts, user=USER, token=TOKEN
-        )
+        git_push_mock.assert_called_once_with(expected_pts)
 
 
 class TestListIssues:
diff --git a/tests/test_exception.py b/tests/test_exception.py
index e108339..6efcc7c 100644
--- a/tests/test_exception.py
+++ b/tests/test_exception.py
@@ -1,7 +1,6 @@
 import os
 import sys
 from repobee import exception
-from repobee import git
 
 import constants
 
@@ -58,8 +57,10 @@ class TestGitError:
         """
         token = "032957238hfibwt8374"  # random garbage token
         returncode = 128
-        repo_url = "{}/some-repo".format(constants.HOST_URL)
-        repo_url_with_token = git._insert_token(repo_url, token)
+        repo_url_template = "https://{}some-host.com/some-repo"
+        repo_url = repo_url_template.format("")
+        # GitHub-style token
+        repo_url_with_token = repo_url_template.format(token + "@")
         fatal = "fatal: repo '{}' could not be found".format(
             repo_url_with_token
         )
@@ -90,9 +91,11 @@ class TestGitError:
         """
         token = "032957238hfibwt8374"  # random garbage token
         returncode = 128
-        repo_url = "{}/some-repo".format(constants.HOST_URL)
-        repo_url_with_user_and_token = git._insert_user_and_token(
-            repo_url, USER, token
+        repo_url_template = "https://{}some-host.com/some-repo"
+        repo_url = repo_url_template.format("")
+        # GitHub-style token
+        repo_url_with_user_and_token = repo_url_template.format(
+            "{}:{}@".format(USER, token)
         )
         fatal = "fatal: repo '{}' could not be found".format(
             repo_url_with_user_and_token
diff --git a/tests/test_git.py b/tests/test_git.py
index 7d849dd..9b17a2a 100644
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -11,14 +11,10 @@ from repobee import git
 from repobee import exception
 from repobee import util
 
-import constants
-from constants import TOKEN
-
 URL_TEMPLATE = "https://{}github.com/slarse/clanim"
 REPO_NAME = "clanim"
-USER = constants.USER
 
-Env = namedtuple("Env", ("expected_url", "expected_url_with_username"))
+Env = namedtuple("Env", ("expected_url",))
 
 RunTuple = namedtuple("RunTuple", ("returncode", "stdout", "stderr"))
 AioSubproc = namedtuple("AioSubproc", ("create_subprocess", "process"))
@@ -39,15 +35,7 @@ def env_setup(mocker):
     mocker.patch(
         "subprocess.run", autospec=True, return_value=RunTuple(0, b"", b"")
     )
-    # TOKEN was mocked as the environment token when repobee.git was imported
-    expected_url = URL_TEMPLATE.format(TOKEN + "@")
-    expected_url_with_username = URL_TEMPLATE.format(
-        "{}:{}@".format(USER, TOKEN)
-    )
-    return Env(
-        expected_url=expected_url,
-        expected_url_with_username=expected_url_with_username,
-    )
+    return Env(expected_url=URL_TEMPLATE.format(""))
 
 
 @pytest.fixture(scope="function")
@@ -111,33 +99,20 @@ def push_tuples():
     return tups
 
 
-def test_insert_token():
-    token = "1209487fbfuq324yfqf78b6"
-    assert git._insert_token(
-        URL_TEMPLATE.format(""), token
-    ) == URL_TEMPLATE.format(token + "@")
-
-
-def test_insert_empty_token_raises():
-    with pytest.raises(ValueError) as exc:
-        git._insert_token(URL_TEMPLATE.format(""), "")
-    assert "empty token" in str(exc)
-
-
 def test_clone_single_raises_on_non_zero_exit_from_git_pull(env_setup, mocker):
     stderr = b"This is pretty bad!"
     # already patched in env_setup fixture
     subprocess.run.return_value = RunTuple(1, "", stderr)
 
     with pytest.raises(exception.CloneFailedError) as exc:
-        git.clone_single("{}".format(URL_TEMPLATE.format("")), TOKEN)
+        git.clone_single("{}".format(URL_TEMPLATE.format("")))
     assert "Failed to clone" in str(exc.value)
 
 
 def test_clone_single_issues_correct_command_with_defaults(env_setup):
     expected_command = "git pull {}".format(env_setup.expected_url).split()
 
-    git.clone_single(URL_TEMPLATE.format(""), TOKEN)
+    git.clone_single(URL_TEMPLATE.format(""))
     subprocess.run.assert_any_call(
         expected_command,
         cwd=str(pathlib.Path(".") / REPO_NAME),
@@ -152,7 +127,7 @@ def test_clone_single_issues_correct_command_non_default_branch(env_setup):
         env_setup.expected_url, branch
     ).split()
 
-    git.clone_single(URL_TEMPLATE.format(""), TOKEN, branch=branch)
+    git.clone_single(URL_TEMPLATE.format(""), branch=branch)
 
     subprocess.run.assert_any_call(
         expected_command,
@@ -169,9 +144,7 @@ def test_clone_single_issues_correct_command_with_cwd(env_setup):
         env_setup.expected_url, branch
     ).split()
 
-    git.clone_single(
-        URL_TEMPLATE.format(""), TOKEN, branch=branch, cwd=working_dir
-    )
+    git.clone_single(URL_TEMPLATE.format(""), branch=branch, cwd=working_dir)
     subprocess.run.assert_called_once_with(
         expected_command,
         cwd=str(pathlib.Path(working_dir) / REPO_NAME),
@@ -188,7 +161,7 @@ class TestPush:
         self, env_setup, push_tuples, tries
     ):
         with pytest.raises(ValueError) as exc_info:
-            git.push(push_tuples, USER, TOKEN, tries=tries)
+            git.push(push_tuples, tries=tries)
 
         assert "tries must be larger than 0" in str(exc_info)
 
@@ -198,9 +171,7 @@ class TestPush:
         """
         expected_calls = [
             call(
-                *"git push {} {}".format(
-                    git._insert_user_and_token(url, USER, TOKEN), branch
-                ).split(),
+                *"git push {} {}".format(url, branch).split(),
                 cwd=os.path.abspath(local_repo),
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE
@@ -208,7 +179,7 @@ class TestPush:
             for local_repo, url, branch in push_tuples
         ]
 
-        failed_urls = git.push(push_tuples, USER, TOKEN)
+        failed_urls = git.push(push_tuples)
 
         assert not failed_urls
         aio_subproc.create_subprocess.assert_has_calls(expected_calls)
@@ -221,11 +192,10 @@ class TestPush:
         """
         tries = 3
         expected_calls = [
-            call(pt, USER, TOKEN)
-            for pt in sorted(push_tuples, key=lambda pt: pt.repo_url)
+            call(pt) for pt in sorted(push_tuples, key=lambda pt: pt.repo_url)
         ] * tries
 
-        async def raise_(pt, user, token):
+        async def raise_(pt):
             raise exception.PushFailedError(
                 "Push failed", 128, b"some error", pt.repo_url
             )
@@ -233,7 +203,7 @@ class TestPush:
         mocker.patch("repobee.git._push_async", side_effect=raise_)
         expected_failed_urls = [pt.repo_url for pt in push_tuples]
 
-        failed_urls = git.push(push_tuples, USER, TOKEN, tries=tries)
+        failed_urls = git.push(push_tuples, tries=tries)
 
         assert sorted(failed_urls) == sorted(expected_failed_urls)
         git._push_async.assert_has_calls(expected_calls, any_order=True)
@@ -244,7 +214,7 @@ class TestPush:
         tried = False
         fail_pt = push_tuples[1]
 
-        async def raise_once(pt, user, token):
+        async def raise_once(pt):
             nonlocal tried
             if not tried and pt == fail_pt:
                 tried = True
@@ -254,7 +224,7 @@ class TestPush:
 
         expected_num_calls = len(push_tuples) + 1  # one retry
 
-        async def raise_(pt, user, token):
+        async def raise_(pt):
             raise exception.PushFailedError(
                 "Push failed", 128, b"some error", pt.repo_url
             )
@@ -263,7 +233,7 @@ class TestPush:
             "repobee.git._push_async", side_effect=raise_once
         )
 
-        git.push(push_tuples, USER, TOKEN, tries=10)
+        git.push(push_tuples, tries=10)
 
         assert len(async_push_mock.call_args_list) == expected_num_calls
 
@@ -274,9 +244,7 @@ class TestPush:
 
         expected_calls = [
             call(
-                *"git push {}".format(
-                    git._insert_user_and_token(pt.repo_url, USER, TOKEN)
-                ).split(),
+                *"git push {}".format(pt.repo_url).split(),
                 pt.branch,
                 cwd=os.path.abspath(pt.local_path),
                 stdout=subprocess.PIPE,
@@ -285,7 +253,7 @@ class TestPush:
             for pt in push_tuples
         ]
 
-        git.push(push_tuples, USER, TOKEN)
+        git.push(push_tuples)
 
         aio_subproc.create_subprocess.assert_has_calls(expected_calls)
 
@@ -293,13 +261,12 @@ class TestPush:
 class TestClone:
     """Tests for clone."""
 
-    @pytest.mark.parametrize("token", (TOKEN, "something-else-entirely"))
-    def test_happy_path(self, env_setup, push_tuples, aio_subproc, token):
+    def test_happy_path(self, env_setup, push_tuples, aio_subproc):
         urls = [pt.repo_url for pt in push_tuples]
         working_dir = "some/working/dir"
         expected_subproc_calls = [
             call(
-                *"git pull {}".format(git._insert_token(url, token)).split(),
+                *"git pull {}".format(url).split(),
                 cwd=str(pathlib.Path(working_dir) / util.repo_name(url)),
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE
@@ -307,7 +274,7 @@ class TestClone:
             for url in urls
         ]
 
-        failed_urls = git.clone(urls, token, cwd=working_dir)
+        failed_urls = git.clone(urls, cwd=working_dir)
 
         assert not failed_urls
         aio_subproc.create_subprocess.assert_has_calls(expected_subproc_calls)
@@ -318,7 +285,7 @@ class TestClone:
         urls = [pt.repo_url for pt in push_tuples]
         fail_urls = [urls[0], urls[-1]]
 
-        expected_calls = [call(url, TOKEN, cwd=".") for url in urls]
+        expected_calls = [call(url, cwd=".") for url in urls]
 
         async def raise_(repo_url, *args, **kwargs):
             if repo_url in fail_urls:
@@ -333,7 +300,7 @@ class TestClone:
             "repobee.git._clone_async", autospec=True, side_effect=raise_
         )
 
-        failed_urls = git.clone(urls, TOKEN)
+        failed_urls = git.clone(urls)
 
         assert failed_urls == fail_urls
         clone_mock.assert_has_calls(expected_calls)
@@ -349,7 +316,7 @@ class TestClone:
 
         expected_calls = [
             call(
-                *"git pull {}".format(git._insert_token(url, TOKEN)).split(),
+                *"git pull {}".format(url).split(),
                 cwd=str(pathlib.Path(".") / util.repo_name(url)),
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE
@@ -357,7 +324,7 @@ class TestClone:
             for url in urls
         ]
 
-        failed_urls = git.clone(urls, TOKEN)
+        failed_urls = git.clone(urls)
         non_zero_aio_subproc.create_subprocess.assert_has_calls(expected_calls)
 
         assert failed_urls == urls
diff --git a/tests/test_github_api.py b/tests/test_github_api.py
index 34bbe41..c087bff 100644
--- a/tests/test_github_api.py
+++ b/tests/test_github_api.py
@@ -4,6 +4,7 @@ import pytest
 from unittest.mock import MagicMock, PropertyMock, call
 
 import github
+import repobee
 from repobee import util
 from repobee import exception
 from repobee import github_api
@@ -428,23 +429,47 @@ class TestCreateRepos:
         are validation errors.
         """
         expected_urls = [
-            generate_repo_url(info.name, ORG_NAME) for info in repo_infos
+            api._insert_auth(generate_repo_url(info.name, ORG_NAME))
+            for info in repo_infos
         ]
 
         actual_urls = api.create_repos(repo_infos)
         assert actual_urls == expected_urls
+        for url in actual_urls:
+            assert TOKEN in url
 
 
 class TestGetRepoUrls:
     """Tests for get_repo_urls."""
 
-    def test_all_repos_found(self, repos, api):
+    @pytest.mark.skipif(
+        repobee.__version__ >= "2.0.0",
+        reason="Made obsolete by including user in every CLI command",
+    )
+    def test_with_token_without_user(self, repos, api):
+        """Test getting repo urls when the api has the token, but no user. The
+        token should be in the url anyway.
+        """
+        api._user = None
+        repo_names = [repo.name for repo in repos]
+        expected_urls = [api._insert_auth(repo.html_url) for repo in repos]
+
+        urls = api.get_repo_urls(repo_names)
+
+        assert sorted(urls) == sorted(expected_urls)
+        for url in urls:
+            assert TOKEN in url
+
+    def test_with_token_and_user(self, repos, api):
         repo_names = [repo.name for repo in repos]
-        expected_urls = [repo.html_url for repo in repos]
+        api._user = USER
+        expected_urls = [api._insert_auth(repo.html_url) for repo in repos]
 
         urls = api.get_repo_urls(repo_names)
 
         assert sorted(urls) == sorted(expected_urls)
+        for url in urls:
+            assert "{}:{}".format(USER, TOKEN) in url
 
 
 class TestOpenIssue: