fix(auth): report why credentials failed, and as whom (0.22.3)

dt/__init__.py

@@ -1,3 +1,3 @@
 """DVC Tools - Convenient tools for working with DVC in HPC environments."""
 
-__version__ = "0.22.2"
+__version__ = "0.22.3"

dt/auth/credentials.py

@@ -17,6 +17,7 @@
 import os
 import re
 import subprocess
+import sys
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Dict, List, Optional, Set, Tuple
@@ -30,6 +31,50 @@
 # DVC config helpers
 # =============================================================================
 
+def describe_secret_identity(backend) -> Optional[str]:
+    """Lines naming the identity and project a secret read will use.
+
+    Returns None for backends that have no notion of an identity, so callers
+    can print unconditionally without special-casing.
+    """
+    from ..secrets import GCPSecretBackend
+
+    if not isinstance(backend, GCPSecretBackend):
+        return None
+    ident = backend.identity()
+    lines = [
+        f"Authenticating to GCP as: {ident.describe()}",
+        f"Reading secrets from project: {backend.project}",
+    ]
+    if ident.caveat:
+        lines.append(f"  ⚠ {ident.caveat}")
+    return '\n'.join(lines)
+
+
+def _no_credentials_message(reasons: Dict[str, str], identity: Optional[str]) -> str:
+    """Summary error that names every cause, rather than implying one.
+
+    The per-repo reasons are printed as they happen, but stdout is easily
+    lost -- piped, scrolled past, or reported second-hand by a collaborator
+    who only pastes the last line. Repeating them in the exception means the
+    error alone is enough to diagnose from.
+    """
+    lines = ["No credentials were installed."]
+    for name, reason in reasons.items():
+        # Reasons are often multi-line (the secret-denied hint is several
+        # lines); indent continuations so the repo names stay scannable.
+        first, *rest = str(reason).splitlines() or ['']
+        lines.append(f"  {name}: {first}")
+        lines.extend(f"    {line}" for line in rest)
+    if identity:
+        lines.append("")
+        lines.append(identity)
+    lines.append("")
+    lines.append("To see which secrets this identity can read:  "
+                 "dt auth credentials list")
+    return '\n'.join(lines)
+
+
 def _get_secret_backend():
     """Get the configured secret backend.
 
@@ -65,14 +110,19 @@ def _get_secret_backend():
             )
         prefix = cfg.get_value('secrets.prefix') or 'dvc-remote-'
         # Quick auth check — fail fast instead of hanging on GCP calls
+        account = None
         if not GCPSecretBackend._has_adc_credentials():
-            status, _account = GCPSecretBackend.gcloud_auth_status()
+            status, account = GCPSecretBackend.gcloud_auth_status()
             if status != 'ok':
                 raise AuthError(
                     "GCP authentication unavailable.\n"
                     + GCPSecretBackend.gcloud_auth_hint(status)
                 )
-        return GCPSecretBackend(project=project, prefix=prefix)
+        backend = GCPSecretBackend(project=project, prefix=prefix)
+        # Hand over what this check already established, so reporting the
+        # identity later does not pay for another gcloud round trip.
+        backend._cli_account = account
+        return backend
     else:
         raise AuthError(
             f"Unknown secret backend: {backend_type}\n"
@@ -670,35 +720,44 @@ def install_credentials(
 
     backend = _get_secret_backend()
 
+    # Say who we are about to authenticate as, before anything can fail.
+    # A denial is uninterpretable without it: "permission denied" and "I
+    # already granted her access" are both true when the caller is a service
+    # account picked up from GOOGLE_APPLICATION_CREDENTIALS.
+    identity = describe_secret_identity(backend)
+    if identity:
+        print(identity)
+
     results: Dict[str, bool] = {}
     legacy_repos: List[str] = []
+    # Why each repo failed, so the summary error can say rather than imply.
+    reasons: Dict[str, str] = {}
+
+    def _failed(name: str, reason: str) -> None:
+        results[name] = False
+        reasons[name] = reason
+        # Unconditional, and on stderr: a failure reason is not verbose-level
+        # information, and it must survive the output being piped elsewhere.
+        print(f"  \u26a0 {name}: {reason}", file=sys.stderr)
 
     for repo_info in repos:
         try:
             raw = backend.get_raw_config(repo_info.name)
         except SecretError as exc:
-            results[repo_info.name] = False
-            if verbose:
-                print(f"  \u26a0 {repo_info.name}: {exc}")
+            _failed(repo_info.name, str(exc))
             continue
 
         if not raw or not raw.strip():
-            results[repo_info.name] = False
-            if verbose:
-                print(f"  \u26a0 {repo_info.name}: empty secret")
+            _failed(repo_info.name, "secret exists but is empty")
             continue
 
         fmt = _detect_secret_format(raw)
         if fmt == 'dvc':
             legacy_repos.append(repo_info.name)
-            results[repo_info.name] = False
-            if verbose:
-                print(f"  \u26a0 {repo_info.name}: legacy DVC-INI format")
+            _failed(repo_info.name, "secret is in legacy DVC-INI format")
             continue
         if fmt == 'unknown':
-            results[repo_info.name] = False
-            if verbose:
-                print(f"  \u26a0 {repo_info.name}: unrecognised secret format")
+            _failed(repo_info.name, "secret is not in a recognised AWS-INI format")
             continue
 
         access_key, secret_key = _parse_aws_secret(raw, repo_info.name)
@@ -720,7 +779,7 @@ def install_credentials(
         )
 
     if not any(results.values()):
-        raise AuthError("No credentials were installed (no usable secrets found).")
+        raise AuthError(_no_credentials_message(reasons, identity))
 
     # Strip now-redundant credential keys from DVC global config, scoped to
     # remotes in the current project.

dt/auth/setup.py

@@ -115,6 +115,11 @@ class SetupReport:
     credentials_installed: Dict[str, bool] = field(default_factory=dict)
     skipped_ssh: bool = False
     skipped_credentials: bool = False
+    #: Why credential install was skipped, when it was not simply that the
+    #: project has no S3 endpoints. Reporting "No S3 endpoints" for an auth
+    #: failure told users their repo had no S3 remote when it plainly did,
+    #: and invited them to dismiss the real error printed underneath.
+    credentials_skip_reason: Optional[str] = None
     errors: List[str] = field(default_factory=list)
 
 
@@ -186,9 +191,10 @@ def auth_setup(
             if status != 'ok':
                 hint = GCPSecretBackend.gcloud_auth_hint(status)
                 report.errors.append(f"GCP authentication unavailable. {hint}")
-                if verbose:
-                    print(f"\n\u26a0 {hint}")
+                print(f"\n\u26a0 GCP authentication unavailable. {hint}")
                 has_s3 = False  # skip credential install
+                report.credentials_skip_reason = (
+                    "GCP authentication unavailable \u2014 see the error below")
 
     # -- 3. SSH setup (if needed) ------------------------------------------
     if has_ssh_or_git:
@@ -235,7 +241,11 @@ def auth_setup(
     else:
         report.skipped_credentials = True
         if verbose:
-            print("\nNo S3 endpoints found — skipping credential install.")
+            if report.credentials_skip_reason:
+                print(f"\nSkipping credential install: "
+                      f"{report.credentials_skip_reason}")
+            else:
+                print("\nNo S3 endpoints found — skipping credential install.")
 
     return report
 
@@ -478,7 +488,11 @@ def format_setup_report(report: SetupReport) -> str:
             icon = click.style("\u2713", fg='green') if ok else click.style("\u2717", fg='red')
             lines.append(f"  {icon} {repo}")
     elif report.skipped_credentials:
-        lines.append("\nNo S3 endpoints — credential install skipped.")
+        if report.credentials_skip_reason:
+            lines.append(
+                f"\nCredential install skipped: {report.credentials_skip_reason}")
+        else:
+            lines.append("\nNo S3 endpoints — credential install skipped.")
 
     if report.errors:
         lines.append(click.style("\nErrors:", fg='red', bold=True))

dt/cli.py

@@ -2952,11 +2952,19 @@ def auth_credentials_list():
     except auth_mod.AuthError as e:
         raise click.ClickException(str(e))
 
+    # Name the project. Two people on one machine can have different
+    # secrets.gcp.project (system vs user vs repo scope), so a bare list is
+    # ambiguous about which project it came from -- and that ambiguity is
+    # exactly what makes a misdirected access grant so hard to spot.
+    project = cfg.get_value('secrets.gcp.project') or '<not configured>'
+
     if not names:
-        click.echo("No secrets found (or prefix matches nothing).")
+        click.echo(f"No secrets visible in project '{project}'.")
+        click.echo("Either it holds none with the configured prefix, or your "
+                   "account cannot see them.")
         return
 
-    click.echo(f"Secrets ({len(names)}):")
+    click.echo(f"Secrets readable in project '{project}' ({len(names)}):")
     for name in names:
         click.echo(f"  {name}")
 
@@ -2975,9 +2983,16 @@ def auth_credentials_check(repo_name):
     info = auth_mod.check_secret(repo_name)
 
     if not info.exists:
-        msg = f"✗ '{repo_name}': secret not found"
+        # An error here means we could not determine existence, which is not
+        # the same as having determined it is absent. GCP returns
+        # PERMISSION_DENIED for both, so claiming "not found" for a denial
+        # sends people off to create a secret that already exists.
         if info.error:
-            msg += f"\n  {info.error}"
+            msg = (f"✗ '{repo_name}': could not confirm the secret exists\n"
+                   f"  {info.error}")
+        else:
+            msg = (f"✗ '{repo_name}': secret not found\n"
+                   f"  See what does exist: dt auth credentials list")
         raise click.ClickException(msg)
 
     if not info.accessible:
@@ -3026,7 +3041,11 @@ def auth_credentials_install(repo_name, verbose):
     if ok:
         click.echo(click.style(f"✓ Installed: {', '.join(ok)}", fg='green'))
     if failed:
-        click.echo(click.style(f"⚠ Not found: {', '.join(failed)}", fg='yellow'))
+        # Not necessarily "not found" — could be denied, empty, or the wrong
+        # format. The reason was printed to stderr as each one failed.
+        click.echo(click.style(f"⚠ Failed: {', '.join(failed)}", fg='yellow'))
+        click.echo("  Reasons above. To see what you can read: "
+                   "dt auth credentials list")
 
 
 @auth_credentials.command('uninstall')

dt/secrets/__init__.py

@@ -6,10 +6,11 @@
 """
 
 from .base import SecretBackend, SecretError
-from .gcp import GCPSecretBackend
+from .gcp import GCPIdentity, GCPSecretBackend
 
 __all__ = [
     'SecretBackend',
     'SecretError',
     'GCPSecretBackend',
+    'GCPIdentity',
 ]