Skip to content

Adds PKCE auth support and login command#317

Draft
merschformann wants to merge 4 commits into
developfrom
merschformann/pkce-auth
Draft

Adds PKCE auth support and login command#317
merschformann wants to merge 4 commits into
developfrom
merschformann/pkce-auth

Conversation

@merschformann
Copy link
Copy Markdown
Member

@merschformann merschformann commented May 23, 2026

Description

Adds support for PKCE based auth (no need for an API key). Furthermore, does some restructuring (mainly around config) to better reflect the shared functionality between nextmv.cloud and nextmv.cli.

Changes

  • Added nextmv/auth.py with the full PKCE OAuth2 flow and token storage helpers.
  • Added nextmv/config.py with shared config constants and basic helpers (load_config, get_profile_type, etc.).
  • Stripped cli/configuration/config.py down to CLI-only concerns (build_*, obscure_api_key, GO_CLI_PATH).
  • Added nextmv login CLI command (cli/login.py) to run the PKCE flow for auth_flow profiles.
  • Extended configuration create to prompt for profile type and handle auth_flow profiles without an API key.
  • Extended cloud/client.py to detect auth_flow profiles and use them instead of the usual API token if detected.

Resolves ENG-7402

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces PKCE/OAuth2 (“auth_flow”) authentication as an alternative to API keys, adds a nextmv login CLI command to perform the browser-based flow and persist tokens locally, and refactors configuration helpers into a shared nextmv.config module for use across CLI and cloud SDK components.

Changes:

  • Added a new PKCE auth implementation (nextmv.auth) with token persistence helpers and supporting unit tests.
  • Added shared configuration constants/helpers (nextmv.config) and updated CLI commands to use them.
  • Updated the Cloud Client to detect auth_flow profiles and use stored/refreshed bearer tokens instead of API keys.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
nextmv/tests/test_config.py Adds unit tests for shared config helpers (get_profile_type, list_auth_flow_profiles).
nextmv/tests/test_auth.py Adds unit tests for token storage, expiry logic, and PKCE pair generation.
nextmv/nextmv/config.py Introduces shared config constants and helpers (load/save, profile-type logic).
nextmv/nextmv/cloud/client.py Adds auth-flow token loading/refresh and uses bearer token when configured.
nextmv/nextmv/cli/mcp/tools/profile.py Switches MCP profile tool to import shared config helpers/constants.
nextmv/nextmv/cli/main.py Registers nextmv login and updates shared config imports/ignored command list.
nextmv/nextmv/cli/login.py Adds nextmv login command to run PKCE flow and store tokens per profile.
nextmv/nextmv/cli/init.py Switches init command to use shared load_config.
nextmv/nextmv/cli/configuration/list.py Switches list command to use shared config constants/helpers.
nextmv/nextmv/cli/configuration/delete.py Switches delete command to use shared config load/save helpers.
nextmv/nextmv/cli/configuration/create.py Extends create to support profile_type prompting and auth_flow profiles without API keys.
nextmv/nextmv/cli/configuration/config.py Strips CLI config module down to CLI-only helpers and GO CLI path handling.
nextmv/nextmv/auth.py Implements the PKCE browser flow, token refresh, and token file storage.
Comments suppressed due to low confidence (1)

nextmv/nextmv/auth.py:507

  • The PKCE authorization request is missing a state parameter and there is no validation of state on the callback. This is an OAuth2 best-practice requirement to prevent CSRF/mix-up attacks for redirect-based flows. Please generate a cryptographically random state, include it in the authorize URL, and verify it matches in _wait_for_callback() before exchanging the code.
    params = {
        "response_type": "code",
        "client_id": CLIENT_ID,
        "redirect_uri": redirect_uri,
        "scope": SCOPES,
        "code_challenge_method": "S256",
        "code_challenge": code_challenge,
    }
    authorization_url = auth_endpoint + "?" + urllib.parse.urlencode(params)

    # Start the callback listener in a background thread so we can open the
    # browser on the main thread without blocking.
    callback_result: dict[str, str] = {}
    exc_holder: list[Exception] = []

    def _listen() -> None:
        try:
            result = _wait_for_callback(CALLBACK_PORT)
            callback_result.update(result)
        except Exception as exc:
            exc_holder.append(exc)

    listener = threading.Thread(target=_listen, daemon=True)
    listener.start()

    webbrowser.open(authorization_url)

    listener.join(timeout=_BROWSER_TIMEOUT + 5)

    if exc_holder:
        raise exc_holder[0]

    code = callback_result.get("code")
    if not code:
        raise RuntimeError("No authorization code received. Please try running [code]nextmv login[/code] again.")

    return _exchange_code_for_tokens(token_endpoint, code, code_verifier, redirect_uri)


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nextmv/nextmv/auth.py
Comment thread nextmv/nextmv/auth.py Outdated
Comment thread nextmv/nextmv/auth.py Outdated
Comment thread nextmv/nextmv/auth.py
Comment thread nextmv/nextmv/cloud/client.py
Comment thread nextmv/nextmv/cloud/client.py
Comment thread nextmv/nextmv/cli/configuration/config.py
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.

2 participants