Skip to content

Conversation

@jtcorbett
Copy link
Contributor

@jtcorbett jtcorbett commented Sep 4, 2025

Summary

Enhanced authentication system for MCP Agent Cloud CLI with improved credential management, new auth commands, and a more structured credentials storage format.

Changelist

Restructured authentication module to support richer user credentials

Added new UserCredentials model to store API keys, user info, and token expiry

Changed credentials storage path to ~/.mcp-agent/credentials.json

Added new auth commands:

whoami: Display current user identity information

logout: Clear stored credentials

Reorganized command structure with new cloud auth command group

Maintained backward compatibility with existing API key functions

Added unit tests for new authentication functionality

Test Plan

# Test login functionality
mcp-agent login

# Check current identity
mcp-agent whoami

# Test logout
mcp-agent logout

# Verify nested command structure works
mcp-agent cloud auth login
mcp-agent cloud auth whoami
mcp-agent cloud auth logout

Summary by CodeRabbit

  • New Features
    • Added whoami command to display current user details.
    • Added logout command with confirmation to clear stored credentials.
  • Improvements
    • Enhanced login: browser or manual API-key flows, --api-key and --no-open options, profile loading, reuse prompt for existing credentials.
    • Credentials persisted in a JSON-backed format with token expiry awareness and a new default storage location.
  • Refactor
    • CLI reorganized into a cloud auth subgroup while retaining root-level aliases for auth commands.

@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Warning

Rate limit exceeded

@jtcorbett has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 19 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 71daae2 and 521eca9.

📒 Files selected for processing (2)
  • src/mcp_agent/cli/auth/main.py (1 hunks)
  • src/mcp_agent/cli/auth/models.py (1 hunks)

Walkthrough

Replaces the standalone login command with a new cloud auth package exposing login, logout, and whoami. Adds JSON-backed UserCredentials, credential persistence/clear/load APIs, updates CLI wiring to register cloud/auth subgroup while preserving root aliases, and adjusts exported symbols accordingly.

Changes

Cohort / File(s) Summary
Auth package & commands
src/mcp_agent/cli/cloud/commands/auth/__init__.py, src/mcp_agent/cli/cloud/commands/auth/login/main.py, src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py, src/mcp_agent/cli/cloud/commands/auth/logout/main.py, src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py, src/mcp_agent/cli/cloud/commands/auth/whoami/main.py
Introduces auth command package and re-exports. Implements login (api_key or browser flow, API profile fetch, saves UserCredentials), logout (confirm, clear credentials), and whoami (render user info or error if missing/expired).
Commands exports refactor
src/mcp_agent/cli/cloud/commands/__init__.py
Reorganized public exports: imports login from .auth and adds logout and whoami to __all__.
Removed old login
src/mcp_agent/cli/cloud/commands/login/main.py
Deletes previous standalone login implementation; functionality replaced by new auth package.
CLI wiring / Typer groups
src/mcp_agent/cli/cloud/main.py
Adds cloud and nested cloud auth Typer groups; wires login, whoami, logout under cloud auth and re-exposes them as root-level aliases; imports updated to include new exports.
Credentials API & model
src/mcp_agent/cli/auth/__init__.py, src/mcp_agent/cli/auth/main.py, src/mcp_agent/cli/auth/models.py, src/mcp_agent/cli/auth/constants.py
Adds UserCredentials dataclass with serialization and expiry check. Replaces raw key storage with JSON-backed credentials file (DEFAULT_CREDENTIALS_PATH changed to ~/.mcp-agent/credentials.json). Adds save_credentials, load_credentials, clear_credentials, keeps load_api_key_credentials wrapper. Uses safe file write and strict permissions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as mcp-agent (Typer)
  participant Auth as cloud.auth handlers
  participant API as APIClient
  participant Store as Credential Store

  rect rgba(230,240,255,0.5)
  note over User,CLI: Login flow (api key or browser)
  User->>CLI: mcp-agent cloud auth login [--api-key|--no-open]
  CLI->>Auth: login(options)
  alt api_key provided
    Auth->>Auth: validate prefix
    Auth->>API: POST "user/get_profile" (with key)
    API-->>Auth: profile or error
    Auth->>Store: save_credentials(UserCredentials)
    Auth-->>CLI: success / return key
  else browser/manual
    Auth->>User: print/open auth URL
    User-->>Auth: provide API key
    Auth->>Auth: validate prefix
    Auth->>API: POST "user/get_profile"
    API-->>Auth: profile
    Auth->>Store: save_credentials(UserCredentials)
    Auth-->>CLI: success
  end
  end

  rect rgba(235,255,235,0.5)
  note over User,CLI: Whoami
  User->>CLI: mcp-agent cloud auth whoami
  CLI->>Auth: whoami()
  Auth->>Store: load_credentials()
  alt missing or expired
    Auth-->>CLI: CLIError (exit_code=4)
  else present
    Auth-->>CLI: render user info panel
  end
  end

  rect rgba(255,235,235,0.5)
  note over User,CLI: Logout
  User->>CLI: mcp-agent cloud auth logout
  CLI->>Auth: logout()
  Auth->>Store: load_credentials()
  alt none found
    Auth-->>CLI: "Not currently logged in."
  else found
    Auth->>User: confirm logout
    alt confirmed
      Auth->>Store: clear_credentials()
      Auth-->>CLI: "Successfully logged out."
    else cancelled
      Auth-->>CLI: "Logout cancelled."
    end
  end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Move CLI from lmai repo #392 — Prior CLI restructuring around cloud commands; closely related to this PR's auth command reorganization and export changes.

Suggested reviewers

  • saqadri

Poem

A rabbit taps the cloud with glee,
“whoami?” I ask — it shows it's me.
I hop to login, guard keys in tow,
Logout with a nod, credentials go.
New tunnels tidy, commands snug and spry. 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jcorbett-add-cloud-auth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jtcorbett jtcorbett marked this pull request as draft September 4, 2025 14:57
Copy link
Contributor Author

jtcorbett commented Sep 4, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jtcorbett jtcorbett marked this pull request as ready for review September 4, 2025 15:12
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)

47-53: Make token expiry timezone-safe (optional).

If token_expires_at may be naive or non-UTC, label “UTC” only after converting: expiry = token_expires_at.astimezone(timezone.utc); then format. Add from datetime import timezone.

src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)

84-86: Prefer “No” as default when already logged in (safer UX).

Reduces accidental re-login.

-        if not Confirm.ask("You are already logged in. Do you want to login again?"):
+        if not Confirm.ask(
+            "You are already logged in. Do you want to login again?",
+            default=False,
+        ):
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)

10-36: Optional: support non-interactive logout with --yes/-y.
For CI/scripts, add a flag to skip the prompt. This also justifies keeping the typer import.

- def logout() -> None:
+ def logout(
+     yes: bool = typer.Option(False, "--yes", "-y", help="Do not prompt for confirmation"),
+ ) -> None:
@@
-    if not Confirm.ask(f"Are you sure you want to logout {user_info}?"):
+    if not yes and not Confirm.ask(f"Are you sure you want to logout {user_info}?", default=False):
         print_info("Logout cancelled.")
         return

And re-introduce the import at the top if you take this route:

+import typer
src/mcp_agent/cli/cloud/main.py (1)

137-175: CLI grouping and aliases are well-structured.
Nested cloud auth with root-level aliases is ergonomic and backwards-compatible.

Consider de-duplicating the multi-line login help string (lines 153-155 and 169-171) into a constant to avoid drift, and capitalize “API keys”.

-Direct to the api keys page for obtaining credentials, routing through login.
+Direct to the API keys page for obtaining credentials, routing through login.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e763520 and 254037a.

📒 Files selected for processing (9)
  • src/mcp_agent/cli/cloud/commands/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/login/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/login/main.py (0 hunks)
  • src/mcp_agent/cli/cloud/main.py (2 hunks)
💤 Files with no reviewable changes (1)
  • src/mcp_agent/cli/cloud/commands/login/main.py
🧰 Additional context used
🧬 Code graph analysis (8)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
src/mcp_agent/cli/utils/ux.py (2)
  • print_info (32-49)
  • print_success (52-63)
src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
  • logout (10-37)
src/mcp_agent/cli/cloud/commands/auth/__init__.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
  • login (57-103)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
  • logout (10-37)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
  • whoami (15-57)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (2)
src/mcp_agent/cli/exceptions.py (1)
  • CLIError (4-9)
src/mcp_agent/cli/utils/ux.py (2)
  • print_error (80-91)
  • print_info (32-49)
src/mcp_agent/cli/cloud/commands/__init__.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
  • login (57-103)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
  • logout (10-37)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
  • whoami (15-57)
src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
  • whoami (15-57)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (3)
src/mcp_agent/cli/core/api_client.py (2)
  • APIClient (53-130)
  • post (75-87)
src/mcp_agent/cli/exceptions.py (1)
  • CLIError (4-9)
src/mcp_agent/cli/utils/ux.py (3)
  • print_info (32-49)
  • print_success (52-63)
  • print_warning (66-77)
src/mcp_agent/cli/cloud/main.py (3)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)
  • login (57-103)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)
  • logout (10-37)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)
  • whoami (15-57)
🪛 GitHub Actions: Pull Request Checks
src/mcp_agent/cli/cloud/commands/auth/logout/main.py

[error] 3-3: Ruff lint error: F401 'typer' imported but unused. Command: ruff check.

src/mcp_agent/cli/cloud/commands/auth/whoami/main.py

[error] 3-3: Ruff lint error: F401 'typing.Optional' imported but unused. Command: ruff check.


[error] 5-5: Ruff lint error: F401 'typer' imported but unused. Command: ruff check.


[error] 12-12: Ruff lint error: F401 'print_error' imported but unused. Command: ruff check.


[error] 12-12: Ruff lint error: F401 'print_info' imported but unused. Command: ruff check.

src/mcp_agent/cli/cloud/commands/auth/login/main.py

[error] 7-7: ModuleNotFoundError: No module named 'mcp_agent_cloud' during import in login/main.py (line 7) while running tests (command: uv run coverage run -m pytest tests -m not integration).

🔇 Additional comments (8)
src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1)

3-13: Fix incorrect imports and remove unused ones. Remove Optional, typer, print_error, print_info. Import CLIError from mcp_agent.cli.exceptions. Confirm and correct the path to load_credentials—no definition was found under mcp_agent.cli.cloud.commands.auth—before updating its import.

src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)

7-19: Keep imports pointing to mcp_agent_cloud
These imports reference the external core library (mcp_agent_cloud) that this CLI wraps—local modules under mcp_agent.cli.* are not substitutes.

Likely an incorrect or invalid review comment.

src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1)

1-5: LGTM.

Re-export is clean and conventional.

src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1)

3-5: Simple, clean re-export. LGTM.
Public surface is minimal and clear.

src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)

6-7: Do not change UX/auth imports
Cloud auth commands (login, whoami, logout) consistently import clear_credentials, load_credentials, and UX helpers from the external mcp_agent_cloud package; switching to mcp_agent.cli.utils.ux would break that pattern.

Likely an incorrect or invalid review comment.

src/mcp_agent/cli/cloud/commands/__init__.py (1)

9-11: Re-export shape looks good.
Imports from .auth and updated __all__ correctly expose login/logout/whoami.

src/mcp_agent/cli/cloud/commands/auth/__init__.py (1)

3-7: Auth package surface is tidy.
Single-point re-exports for login/logout/whoami simplify CLI wiring.

src/mcp_agent/cli/cloud/main.py (1)

16-16: Import update matches the new command layout.
Bringing logout and whoami alongside login from commands is consistent.

@jtcorbett jtcorbett force-pushed the jcorbett-add-cloud-auth branch from 00baf6f to 4de6535 Compare September 4, 2025 15:14
@jtcorbett jtcorbett force-pushed the jcorbett-add-cloud-auth branch from c36b45a to 71daae2 Compare September 4, 2025 15:22
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (3)
src/mcp_agent/cli/auth/main.py (1)

21-25: Fix double-open race and permission leak; write atomically with 0600.

Current code creates the file with default perms, then reopens it, leaving a window where credentials may be world-readable and risking incorrect final mode. Replace with atomic write to a 0600 temp file + os.replace.

Apply:

-    # Create file with restricted permissions (0600) to prevent leakage
-    with open(credentials_path, "w", encoding="utf-8") as f:
-        fd = os.open(credentials_path, os.O_WRONLY | os.O_CREAT, 0o600)
-        with os.fdopen(fd, "w") as f:
-            f.write(credentials.to_json())
+    # Atomic write with restricted permissions (0600)
+    cred_dir = os.path.dirname(credentials_path)
+    fd, tmp_path = tempfile.mkstemp(prefix="credentials.", dir=cred_dir, text=True)
+    try:
+        try:
+            os.fchmod(fd, 0o600)  # posix
+        except AttributeError:
+            os.chmod(tmp_path, 0o600)  # windows fallback
+        with os.fdopen(fd, "w", encoding="utf-8") as f:
+            f.write(credentials.to_json())
+            f.flush()
+            try:
+                os.fsync(f.fileno())
+            except (AttributeError, OSError):
+                pass
+        if os.path.islink(credentials_path):
+            raise RuntimeError("Refusing to overwrite symlink at credentials path")
+        os.replace(tmp_path, credentials_path)
+    finally:
+        try:
+            os.unlink(tmp_path)
+        except FileNotFoundError:
+            pass
src/mcp_agent/cli/auth/__init__.py (1)

6-12: Restore legacy export to keep backward compatibility (and prevent import errors).

save_api_key_credentials was removed from exports, but the PR claims BC. Provide a thin wrapper and re-export it.

Apply:

 from .main import (
     clear_credentials,
     load_api_key_credentials,
     load_credentials,
     save_credentials,
 )
 from .models import UserCredentials

+def save_api_key_credentials(api_key: str) -> None:
+    """Backward-compat wrapper; prefer save_credentials(UserCredentials(...))."""
+    save_credentials(UserCredentials(api_key=api_key))
 
 __all__ = [
     "clear_credentials",
     "load_api_key_credentials",
     "load_credentials",
     "save_credentials",
+    "save_api_key_credentials",
     "UserCredentials",
 ]

Run to confirm any legacy imports are now satisfied:

#!/bin/bash
rg -nP '\bsave_api_key_credentials\b' -C2

Also applies to: 14-20

src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)

132-147: Mask API key input and avoid returning secrets from manual prompt.

Hide input and normalize whitespace; do not return the key.

Apply:

-def _handle_manual_key_input() -> str:
+def _handle_manual_key_input() -> None:
     """Handle manual API key input.
 
-    Returns:
-        API key string
+    Returns:
+        None
     """
-    input_api_key = Prompt.ask("Please enter your API key :key:")
+    input_api_key = Prompt.ask("Please enter your API key :key:", password=True)
+    if input_api_key is not None:
+        input_api_key = input_api_key.strip()
 
     if not input_api_key:
         print_warning("No API key provided.")
         raise CLIError("Failed to set valid API key")
 
     if not _is_valid_api_key(input_api_key):
         print_warning("Invalid API key provided.")
         raise CLIError("Failed to set valid API key")
 
     credentials = _load_user_credentials(input_api_key)
 
     save_credentials(credentials)
     print_success("API key set.")
     if credentials.username:
         print_info(f"Logged in as: {credentials.username}")
 
-    return input_api_key
+    return None

Also applies to: 150-156

🧹 Nitpick comments (4)
src/mcp_agent/cli/auth/main.py (2)

1-6: Imports required for secure atomic writes and path resolution.

Add tempfile and updated constants.

Apply:

-import json
+import json
 import os
 from typing import Optional
@@
-from .constants import DEFAULT_CREDENTIALS_PATH
+import tempfile
+from .constants import DEFAULT_CREDENTIALS_PATH, DEFAULT_CREDENTIALS_ENV, LEGACY_CREDENTIALS_PATHS

51-55: Make delete robust and race-safe; optionally remove legacy files too.

Handle TOCTOU and permission errors gracefully; consider cleaning legacy files.

Apply:

-    credentials_path = os.path.expanduser(DEFAULT_CREDENTIALS_PATH)
-    if os.path.exists(credentials_path):
-        os.remove(credentials_path)
-        return True
-    return False
+    deleted = False
+    for path in [os.path.expanduser(DEFAULT_CREDENTIALS_PATH), *[os.path.expanduser(p) for p in LEGACY_CREDENTIALS_PATHS]]:
+        try:
+            os.remove(path)
+            deleted = True
+        except FileNotFoundError:
+            pass
+        except OSError:
+            pass
+    return deleted
src/mcp_agent/cli/cloud/commands/auth/login/main.py (2)

101-117: Optional: warn on non-HTTPS API base URL.

Reduce MITM risk by validating scheme.

 base_url = settings.API_BASE_URL
+if not base_url.startswith("https://"):
+    print_warning("Non-HTTPS API base URL detected; proceed with caution.")

93-99: Ensure credentials file is written atomically with 0600 perms.

Current save implementation (in auth/main.py) appears to open twice and may not enforce permissions atomically. Consider a single secure, atomic write.

Proposed implementation for save_credentials (in src/mcp_agent/cli/auth/main.py):

import os, tempfile

def save_credentials(credentials: UserCredentials) -> None:
    path = os.path.expanduser(DEFAULT_CREDENTIALS_PATH)
    os.makedirs(os.path.dirname(path), exist_ok=True)
    dir_ = os.path.dirname(path) or "."
    with tempfile.NamedTemporaryFile("w", dir=dir_, delete=False, encoding="utf-8") as tf:
        os.chmod(tf.name, 0o600)
        tf.write(credentials.to_json())
        tmp_name = tf.name
    os.replace(tmp_name, path)

Also applies to: 148-154

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 254037a and 71daae2.

📒 Files selected for processing (13)
  • src/mcp_agent/cli/auth/__init__.py (1 hunks)
  • src/mcp_agent/cli/auth/constants.py (1 hunks)
  • src/mcp_agent/cli/auth/main.py (1 hunks)
  • src/mcp_agent/cli/auth/models.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/login/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/logout/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/whoami/__init__.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/auth/whoami/main.py (1 hunks)
  • src/mcp_agent/cli/cloud/commands/login/main.py (0 hunks)
  • src/mcp_agent/cli/cloud/main.py (2 hunks)
💤 Files with no reviewable changes (1)
  • src/mcp_agent/cli/cloud/commands/login/main.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/mcp_agent/cli/cloud/commands/auth/init.py
  • src/mcp_agent/cli/cloud/commands/init.py
  • src/mcp_agent/cli/cloud/commands/auth/whoami/main.py
  • src/mcp_agent/cli/cloud/commands/auth/whoami/init.py
  • src/mcp_agent/cli/cloud/commands/auth/logout/init.py
  • src/mcp_agent/cli/cloud/main.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/utils/config.py : Configuration values such as quality_threshold, max_refinement_attempts, consolidation_interval, and evaluator_model_provider must be loaded from mcp_agent.config.yaml.

Applied to files:

  • src/mcp_agent/cli/auth/constants.py
🧬 Code graph analysis (4)
src/mcp_agent/cli/auth/__init__.py (2)
src/mcp_agent/cli/auth/main.py (4)
  • clear_credentials (45-55)
  • load_api_key_credentials (58-65)
  • load_credentials (28-42)
  • save_credentials (9-25)
src/mcp_agent/cli/auth/models.py (1)
  • UserCredentials (10-64)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (2)
src/mcp_agent/cli/auth/main.py (2)
  • clear_credentials (45-55)
  • load_credentials (28-42)
src/mcp_agent/cli/utils/ux.py (2)
  • print_info (32-49)
  • print_success (52-63)
src/mcp_agent/cli/auth/main.py (1)
src/mcp_agent/cli/auth/models.py (3)
  • UserCredentials (10-64)
  • to_json (56-58)
  • from_json (61-64)
src/mcp_agent/cli/cloud/commands/auth/login/main.py (5)
src/mcp_agent/cli/auth/models.py (2)
  • UserCredentials (10-64)
  • is_token_expired (22-26)
src/mcp_agent/cli/auth/main.py (2)
  • load_credentials (28-42)
  • save_credentials (9-25)
src/mcp_agent/cli/core/api_client.py (2)
  • APIClient (53-130)
  • post (75-87)
src/mcp_agent/cli/exceptions.py (1)
  • CLIError (4-9)
src/mcp_agent/cli/utils/ux.py (3)
  • print_info (32-49)
  • print_success (52-63)
  • print_warning (66-77)
🔇 Additional comments (3)
src/mcp_agent/cli/cloud/commands/auth/logout/main.py (1)

9-36: LGTM – safe-by-default confirmation and clear UX.

Interactive confirmation defaults to No and messages avoid exposing secrets. Looks good.

src/mcp_agent/cli/auth/main.py (1)

58-66: Backward-compat wrapper looks good.

Simple and correct delegation to the new model.

src/mcp_agent/cli/cloud/commands/auth/login/main.py (1)

57-104: Remove login() return value safely
No internal references to login()’s return value were found; changing the return type to None poses no risk.

Comment on lines +41 to +54
@classmethod
def from_dict(cls, data: dict) -> "UserCredentials":
"""Create from dictionary loaded from JSON."""

token_expires_at = None
if "token_expires_at" in data:
token_expires_at = datetime.fromisoformat(data["token_expires_at"])

return cls(
api_key=data["api_key"],
token_expires_at=token_expires_at,
username=data.get("username"),
email=data.get("email"),
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden ISO8601 parsing and normalize timezone.

datetime.fromisoformat does not accept a trailing 'Z'; also normalize naive timestamps to UTC.

Apply:

     @classmethod
     def from_dict(cls, data: dict) -> "UserCredentials":
         """Create from dictionary loaded from JSON."""
 
-        token_expires_at = None
-        if "token_expires_at" in data:
-            token_expires_at = datetime.fromisoformat(data["token_expires_at"])
+        token_expires_at = None
+        raw = data.get("token_expires_at")
+        if raw:
+            ts = raw
+            if isinstance(ts, str) and ts.endswith("Z"):
+                ts = ts[:-1] + "+00:00"
+            try:
+                token_expires_at = datetime.fromisoformat(ts)
+                if token_expires_at.tzinfo is None:
+                    token_expires_at = token_expires_at.replace(tzinfo=timezone.utc)
+            except ValueError:
+                token_expires_at = None

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/mcp_agent/cli/auth/models.py around lines 41 to 54, the ISO8601 parsing
uses datetime.fromisoformat which fails on a trailing 'Z' and leaves naive
datetimes un-normalized; change the logic to: if token_expires_at exists and is
a string, normalize a trailing 'Z' to '+00:00' before parsing with
datetime.fromisoformat, then if the parsed datetime has no tzinfo set
tzinfo=datetime.timezone.utc, otherwise convert it to UTC via
.astimezone(datetime.timezone.utc); ensure datetime.timezone is imported and
handle non-string/invalid values safely (e.g., treat parse failures as None or
re-raise as appropriate for current behavior).

Comment on lines +106 to +116
def _handle_browser_auth(base_url: str, no_open: bool) -> str:
"""Handle browser-based authentication flow.
Args:
base_url: API base URL
no_open: Whether to skip automatic browser opening
Returns:
API key string
"""
auth_url = f"{base_url}/{DEFAULT_API_AUTH_PATH}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align browser-auth helper with non-returning flow.

Make it return None and not pass secrets upward.

-def _handle_browser_auth(base_url: str, no_open: bool) -> str:
+def _handle_browser_auth(base_url: str, no_open: bool) -> None:
     """Handle browser-based authentication flow.
 
     Args:
         base_url: API base URL
         no_open: Whether to skip automatic browser opening
 
-    Returns:
-        API key string
+    Returns:
+        None
     """
     auth_url = f"{base_url}/{DEFAULT_API_AUTH_PATH}"
@@
-    return _handle_manual_key_input()
+    _handle_manual_key_input()
+    return None

Also applies to: 129-129

🤖 Prompt for AI Agents
In src/mcp_agent/cli/cloud/commands/auth/login/main.py around lines 106-116 (and
also at line 129), the browser auth helper currently returns the API key which
leaks secrets upward; change the function to return None instead of the API key,
update the docstring and type hints to indicate no return value, remove any
return statements that pass the secret back, and ensure callers treat the
browser flow as non-returning (handle None appropriately or branch to a separate
non-secret path). Also adjust any unit tests or call-sites that expect a string
to accept None or use the alternative non-returning flow.

@jtcorbett jtcorbett merged commit 157867e into main Sep 4, 2025
9 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants