tests: e2e tests for PAT usage with CLI

Author: RMCrean

tests/e2e_integration/cli_commands/conftest.py

@@ -110,8 +110,15 @@ def factory(CONSTANTS):
         cli_settings.CONFIG_PATH.unlink(missing_ok=True)
         # tokens can either be stored in device keyring (or in a fallback file if e.g. keyring not available - likely for CI or disabled for a test)
         with contextlib.suppress(KeyringError):
-            keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_USERNAME)
-        cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+            keyring.delete_password(
+                service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_TOKENS_USERNAME
+            )
+        cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
+        with contextlib.suppress(KeyringError):
+            keyring.delete_password(
+                service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_PATS_USERNAME
+            )
+        cli_settings.PATS_FALLBACK_PATH.unlink(missing_ok=True)
 
         # running any cmd that requires the config file will create it
         for project in CONSTANTS["PROJECT_TO_BUCKET_MAP"]:
@@ -139,7 +146,14 @@ def factory(CONSTANTS):
         cli_settings.CONFIG_PATH.unlink(missing_ok=True)
         # tokens can either be stored in device keyring (or in a fallback file if e.g. keyring not available - likely for CI or disabled for a test)
         with contextlib.suppress(KeyringError):
-            keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_USERNAME)
-        cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+            keyring.delete_password(
+                service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_TOKENS_USERNAME
+            )
+        cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
+        with contextlib.suppress(KeyringError):
+            keyring.delete_password(
+                service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_PATS_USERNAME
+            )
+        cli_settings.PATS_FALLBACK_PATH.unlink(missing_ok=True)
 
     return factory

tests/e2e_integration/cli_commands/test_auth_cli.py

@@ -3,9 +3,11 @@
 """
 
 import shutil
+from datetime import datetime, timedelta
 
 import keyring
 import pytest
+from click.testing import Result
 from keyring.errors import NoKeyringError
 from pydantic import SecretStr
 from typer.testing import CliRunner
@@ -39,9 +41,9 @@ def make_tokens_expired(access: bool = False, refresh: bool = False):
     Helper function to make either the access token or refresh token expired in the tokens file.
     Sets the expiry time to 1970 (unix time stamp)
     """
-    with open(cli_settings.TOKENS_PATH, "r") as token_file:
+    with open(cli_settings.TOKENS_FALLBACK_PATH, "r") as token_file:
         lines = token_file.readlines()
-    with open(cli_settings.TOKENS_PATH, "w") as token_file:
+    with open(cli_settings.TOKENS_FALLBACK_PATH, "w") as token_file:
         for line in lines:
             if access and line.startswith("access_token_expires_at:"):
                 token_file.write("access_token_expires_at: 1\n")
@@ -220,12 +222,12 @@ def test_using_revoked_refresh_token_fails(tmp_path, disable_keyring_backend):
     log_in_as_user()
 
     shutil.copy(cli_settings.CONFIG_PATH, tmp_path / "config_backup.yaml")
-    shutil.copy(cli_settings.TOKENS_PATH, tmp_path / "tokens_backup.yaml")
+    shutil.copy(cli_settings.TOKENS_FALLBACK_PATH, tmp_path / "tokens_backup.yaml")
 
     result = runner.invoke(app=app, args=logout_command)
     assert result.exit_code == 0
 
-    shutil.copy(tmp_path / "tokens_backup.yaml", cli_settings.TOKENS_PATH)
+    shutil.copy(tmp_path / "tokens_backup.yaml", cli_settings.TOKENS_FALLBACK_PATH)
     shutil.copy(tmp_path / "config_backup.yaml", cli_settings.CONFIG_PATH)
     make_tokens_expired(access=True)
 
@@ -282,3 +284,121 @@ def test_jwt_session_takes_priority_over_pat(disable_keyring_backend, monkeypatc
     result = runner.invoke(app=app, args="auth whoami")
     assert result.exit_code == 0
     assert USER_EMAIL in result.stdout
+
+
+_FAKE_PAT_NAME = "work-laptop"
+_FAKE_PAT = "divbase_pat_fakefakefakefakefakefakefake"
+
+
+def run_add_pat_cmd(
+    name: str = _FAKE_PAT_NAME,
+    expires: int | None = 9999999999,
+    pat: str = _FAKE_PAT,
+    overwrite: bool = False,
+) -> Result:
+    """Helper fn to run the add-pat command."""
+    args = f"auth add-pat {name}"
+    if expires is not None:
+        args += f" --expires {expires}"
+    if overwrite:
+        args += " --overwrite-existing"
+    return runner.invoke(
+        app=app,
+        args=args,
+        input=f"{pat}\n",
+    )
+
+
+@pytest.fixture
+def cleanup_stored_cli_pat():
+    """Remove any possibly stored PAT from a add-pat command in a test"""
+    yield
+    # rm-pat is idempotent, so doesn't matter if there is no PAT.
+    result = runner.invoke(app=app, args="auth rm-pat")
+    assert result.exit_code == 0
+
+
+def test_add_pat_stores_pat(cleanup_stored_cli_pat):
+    """add-pat should store the PAT and pat-info should now show the PATs is stored"""
+    result = run_add_pat_cmd()
+    assert result.exit_code == 0
+    assert _FAKE_PAT_NAME in result.stdout
+
+    info = runner.invoke(app=app, args="auth pat-info")
+    assert info.exit_code == 0
+    assert _FAKE_PAT_NAME in info.stdout
+    assert _FAKE_PAT not in info.stdout  # PAT should not be printed
+
+
+def test_add_pat_rejects_invalid_pat_prefix(cleanup_stored_cli_pat):
+    """add-pat should fail if the token doesn't start with the PAT prefix."""
+    result = run_add_pat_cmd(pat="not_a_real_pat")
+    assert result.exit_code != 0
+    assert "not a valid personal access token" in result.stdout
+
+
+def test_add_pat_rejects_already_expired_pat(cleanup_stored_cli_pat):
+    """add-pat should fail if the token doesn't start with the PAT prefix."""
+    one_day_ago = datetime.now() - timedelta(days=1)
+    result = run_add_pat_cmd(expires=int(one_day_ago.timestamp()))
+    assert result.exit_code != 0
+    assert "expiry" in result.stdout
+
+
+def test_add_pat_with_no_expiry(cleanup_stored_cli_pat):
+    """add-pat without --expires set works and stores a PAT."""
+    result = run_add_pat_cmd(expires=None)
+    assert result.exit_code == 0
+    assert _FAKE_PAT_NAME in result.stdout
+
+    info = runner.invoke(app=app, args="auth pat-info")
+    assert info.exit_code == 0
+    assert _FAKE_PAT_NAME in info.stdout
+    assert "Never" in info.stdout  # expiry info
+    assert _FAKE_PAT not in info.stdout  # PAT should not be printed
+
+
+def test_cannot_add_pat_without_overwrite_flag(cleanup_stored_cli_pat):
+    """add-pat must refuse to overwrite an existing PAT unless --overwrite-existing is passed."""
+    result = run_add_pat_cmd()
+    assert result.exit_code == 0
+    assert _FAKE_PAT_NAME in result.stdout
+
+    result = run_add_pat_cmd()
+    assert result.exit_code != 0
+    assert "already stored" in result.stdout
+    assert "--overwrite-existing" in result.stdout
+
+    result = run_add_pat_cmd(overwrite=True)
+    assert result.exit_code == 0
+    assert _FAKE_PAT_NAME in result.stdout
+
+    result = run_add_pat_cmd(name="a-new-pat", overwrite=True)
+    assert result.exit_code == 0
+    assert "a-new-pat" in result.stdout
+
+
+def test_rm_pat_removes_stored_pat(cleanup_stored_cli_pat):
+    """rm-pat should remove the stored PAT so that pat-info shows nothing."""
+    result = run_add_pat_cmd()
+    assert result.exit_code == 0
+
+    result = runner.invoke(app=app, args="auth rm-pat")
+    assert result.exit_code == 0
+
+    info = runner.invoke(app=app, args="auth pat-info")
+    assert info.exit_code == 0
+    assert "No personal access token" in info.stdout
+
+
+def test_rm_pat_with_no_pat_is_ok(cleanup_stored_cli_pat):
+    """rm-pat should succeed even if no PAT is currently stored."""
+    result = runner.invoke(app=app, args="auth rm-pat")
+    assert result.exit_code == 0
+
+
+def test_pat_info_when_no_pat_stored(cleanup_stored_cli_pat):
+    """pat-info should report nothing stored when no PAT has been added."""
+    info = runner.invoke(app=app, args="auth pat-info")
+    assert info.exit_code == 0
+    assert "No personal access token" in info.stdout

tests/e2e_integration/cli_commands/test_personal_access_tokens.py

@@ -9,6 +9,14 @@
 get_project_member in deps.py. Other more special cases like auth whoami and task-history user have their logic.
 
 So below we cover a representative cmd for project scope (files ls) and then the special cases.
+
+### Why mostly just env var tests
+A user can provide a PAT via an env var or from storing it with the add-pat command.
+
+Logic to validate those cli cmds is in test_auth_cli.py.
+
+We have 1 e2e test here that covers using the PAT stored by the add-pat command, otherwise we use the env var approach for convienance.
+These tests primarly focus on PATs being scoped correctly etc...
 """
 
 from datetime import datetime, timedelta, timezone
@@ -264,11 +272,32 @@ def test_expired_pat_rejected(CONSTANTS, logged_out_user_with_existing_config, p
     assert_401_error(result)
 
 
-def test_pat_does_not_work_on_frontend_endpoints(
-    CONSTANTS, logged_out_user_with_existing_config, pat_factory, monkeypatch
-):
+def test_pat_stored_via_add_pat_cmd_works(CONSTANTS, logged_out_user_with_existing_config, pat_factory):
+    """Validate that a PAT stored via the add-pat cmd works for authentication and respects scopes."""
+    raw_token: SecretStr = pat_factory(user_email=EDIT_USER_EMAIL, permissions=NO_SCOPE_PERMISSIONS)
+    try:
+        result = runner.invoke(
+            app=app,
+            args="auth add-pat my-pat",
+            input=f"{raw_token.get_secret_value()}\n",
+        )
+        assert result.exit_code == 0
+
+        result = runner.invoke(app, "auth whoami")
+        assert result.exit_code == 0
+        assert EDIT_USER_EMAIL in result.output
+
+        result = runner.invoke(app, f"files ls --project {CONSTANTS['DEFAULT_PROJECT']}")
+        assert_403_error(result)
+    finally:
+        # clean up the stored PAT so it doesn't interfere with other tests, even if above fails
+        result = runner.invoke(app=app, args="auth rm-pat")
+        assert result.exit_code == 0
+
+
+def test_pat_does_not_work_on_frontend_endpoints(CONSTANTS, logged_out_user_with_existing_config, pat_factory):
     """PATs should not work on frontend endpoints"""
-    raw_token = pat_factory(user_email=EDIT_USER_EMAIL, permissions=FULL_ACCESS_PAT_PERMISSIONS)
+    raw_token: SecretStr = pat_factory(user_email=EDIT_USER_EMAIL, permissions=FULL_ACCESS_PAT_PERMISSIONS)
 
     home_url = "http://localhost:8001/"
     # technically don't need the token here

tests/e2e_integration/conftest.py

@@ -52,16 +52,26 @@ def clean_tmp_config_and_tokens_between_tests():
     cli_settings.CONFIG_PATH.unlink(missing_ok=True)
     # tokens can either be stored in device keyring (or in a fallback file if e.g. keyring not available - likely for CI or disabled for a test)
     with contextlib.suppress(KeyringError):
-        keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_USERNAME)
-    cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+        keyring.delete_password(
+            service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_TOKENS_USERNAME
+        )
+    cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
+    with contextlib.suppress(KeyringError):
+        keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_PATS_USERNAME)
+    cli_settings.PATS_FALLBACK_PATH.unlink(missing_ok=True)
 
     yield
 
     cli_settings.CONFIG_PATH.unlink(missing_ok=True)
     # tokens can either be stored in device keyring (or in a fallback file if e.g. keyring not available - likely for CI or disabled for a test)
     with contextlib.suppress(KeyringError):
-        keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_USERNAME)
-    cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+        keyring.delete_password(
+            service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_TOKENS_USERNAME
+        )
+    cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
+    with contextlib.suppress(KeyringError):
+        keyring.delete_password(service_name=cli_settings.KEYRING_SERVICE, username=cli_settings.KEYRING_PATS_USERNAME)
+    cli_settings.PATS_FALLBACK_PATH.unlink(missing_ok=True)
 
 
 @pytest.fixture(scope="session")
@@ -268,7 +278,7 @@ def logged_in_edit_user_with_existing_config(CONSTANTS):
     """Shared fixture: logged-in edit user with existing CLI config."""
     # ensure no config or tokens file exist before test
     cli_settings.CONFIG_PATH.unlink(missing_ok=True)
-    cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+    cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
 
     for project in CONSTANTS["PROJECT_TO_BUCKET_MAP"]:
         add_command = f"config add {project}"
@@ -287,7 +297,7 @@ def logged_in_edit_user_with_existing_config(CONSTANTS):
     yield
 
     cli_settings.CONFIG_PATH.unlink(missing_ok=True)
-    cli_settings.TOKENS_PATH.unlink(missing_ok=True)
+    cli_settings.TOKENS_FALLBACK_PATH.unlink(missing_ok=True)
 
 
 @pytest.fixture