Adds PKCE auth support and login command#317
Draft
merschformann wants to merge 4 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
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
Clientto detectauth_flowprofiles 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
stateparameter and there is no validation ofstateon the callback. This is an OAuth2 best-practice requirement to prevent CSRF/mix-up attacks for redirect-based flows. Please generate a cryptographically randomstate, 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.cloudandnextmv.cli.Changes
nextmv/auth.pywith the full PKCE OAuth2 flow and token storage helpers.nextmv/config.pywith shared config constants and basic helpers (load_config,get_profile_type, etc.).cli/configuration/config.pydown to CLI-only concerns (build_*,obscure_api_key,GO_CLI_PATH).nextmv loginCLI command (cli/login.py) to run the PKCE flow for auth_flow profiles.cloud/client.pyto detect auth_flow profiles and use them instead of the usual API token if detected.Resolves ENG-7402