Add OAuth login (cld login) with auto-refreshing tokens and saveable default configs#116
Open
const-cloudinary wants to merge 14 commits into
Open
Add OAuth login (cld login) with auto-refreshing tokens and saveable default configs#116const-cloudinary wants to merge 14 commits into
const-cloudinary wants to merge 14 commits into
Conversation
Add `cld login` / `cld logout` backed by an OAuth 2.0 Authorization Code + PKCE loopback flow (RFC 8252). A login is persisted as a named `cloudinary://` entry in config.json so it flows through the SDK parser and existing config machinery; tokens refresh transparently when a saved login is selected. Highlights: - auth/: protocol helpers (flow), session codec + JWT decode (session), single-shot loopback server (loopback_server), and the login façade. - Region drives both the API host and the OAuth host; `--region` on the login subcommand, falling back to CLOUDINARY_REGION. - config_resolver: explicit-config guard so a sole OAuth login never hijacks an explicitly chosen `-c`/`-C` account; strips OAuth bookkeeping keys before they reach the API/upload kwargs. - Hardening: request timeouts on token calls, broadened refresh except, cloud_name guard, loopback path check, key-based is_oauth_url, and an expires_in fallback. - logout is OAuth-only and, with no name, lists saved logins for a validated numbered selection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add atomic_write primitive (temp file + os.replace) so an interrupted or interleaved write can never truncate/corrupt the config or sync meta file. write_json_to_file gains an opt-in atomic= flag (default False); enabled for save_config and the .cld-sync meta write. atomic_write normalizes the temp file's mkstemp 0600 mode to the umask default so non-config output files keep their usual permissions. Add a reentrant cross-process FileLock around config read-modify-write (update_config / remove_config_keys), and use it in refresh_url_if_stale to re-read and re-check token freshness inside the lock. This prevents two concurrent processes from both refreshing — and thus burning — Hydra's single-use rotated refresh token; a peer's fresh token is adopted instead. Fix OAUTH_CLIENT_ID to the registered Hydra client. Add filelock dependency. Add OAUTH_ATOMIC_CONFIG_REVIEW.md for the next reviewer. Tests: atomic write (incl. read-only dir/target, umask), both json modes, config 0600 + concurrency, peer-refresh adoption vs re-refresh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n refresh Replace the implicit "sole OAuth login auto-applies" rule with an explicit, stored default configuration, and split config handling into an offline format-check (group level) and a lazy network refresh at point-of-use. - Stored default in config.json under reserved key `__default__`; settable via `cld login --set-default`, `cld config -d/--set-default/--unset-default`, and auto-set when a login is the only config with no env/default. - Resolution precedence: -c > -C > stored default > environment > unconfigured. The default outranks env; env is the fallback when no default is set. - Resolver does selection + load only (no network). Stale OAuth tokens refresh lazily at the API chokepoints (call_api + search .execute()), closing the eager-refresh hang (Finding 1). - `cld config -ls` table + `--json`; `cld config`/`-s` gain a header and `--json` with masked secrets, structured expires_at, decomposed account_url, and an active/default/source view. Synthetic (environment)/(command-line) rows. - `cld config --refresh [name] / --refresh-all [--force]` to refresh OAuth tokens; failure hint preserves the config's region in the re-login command. - Fixed-width secret masking (no asterisk walls / length leak); empty fields hidden. - Extract config inventory presentation into utils/config_listing.py; move cloud_name_from_url / config_type into config_utils. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace ensure_active_config_fresh() and its four hand-maintained API chokepoints with a single self-refreshing oauth_token on the active config object. Access tokens are valid ~5 minutes, so refreshes are frequent; the SDK reads cloudinary.config().oauth_token per request, which is the one seam that needs a fresh token. - New cloudinary_cli/auth/oauth_config.py: OAuthConfig(cloudinary.Config). oauth_token is a class-level property that refreshes-if-stale on read (reusing refresh_url_if_stale's lock + double-check + atomic persist) and short-circuits with no I/O once the in-object session is fresh. It never calls reset_config() in the getter (avoids the global-swap thread race). has_oauth reports token presence without refreshing. - Every active config the CLI installs is now an OAuthConfig (saved, env-fallback, inline -c) via install_oauth_config / install_env_config, so has_oauth is universal and offline paths stay offline. - Delete ensure_active_config_fresh and its call sites in core/search.py and utils/api_utils.py; refresh_cloudinary_config delegates to the install seam. - load_config() gains an (mtime_ns, size) cache (copy-on-return, invalidated on save) to cut redundant reads on the remaining hot paths. - Type/validity/-ls classifiers read has_oauth (presence), never the refreshing property, so config -ls / -s / the group-level check do no network even on a stale token. Test isolation fixes (pre-existing bugs surfaced by the refactor): - test_auth_session: patch load_config so refresh tests never read/write the real ~/.cloudinary-cli/config.json (this had poisoned a real entry). - test_cli_config show_default_no_config: clear CLOUDINARY_* so it actually exercises the unconfigured path. - test_cli_config_oauth TestConfigSecretMasking: strip CLOUDINARY_* in setUp so a dev env's account_url does not leak into masking assertions (fixed 4 failures on account-enabled machines). - Rewrite TestEnsureActiveConfigFresh -> TestSelfRefreshingOAuthToken for the new model, with a presence-does-not-refresh regression guard. - Add CONFIG_PRESENT/REQUIRES_CONFIG skip predicate so the 13 mocked-HTTP tests that still need a resolvable config skip cleanly on a bare machine. See OAUTH_LAZY_TOKEN_HANDOFF.md for the full writeup and the remaining follow-ups (thread-local refresh lock, refresh-on-401 retry, transactional multi-step config ops). Builds on OAUTH_DEFAULT_CONFIG_IMPLEMENTATION.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sh visibility
A1: reject -c and -C together with a UsageError instead of silently ignoring -C.
A2: surface a clear error when the loopback login port is in use, not a raw OSError.
A4: write config.json 0600 via the atomic temp file so it is never momentarily
world-readable mid-write (drops the post-replace chmod that left a window).
A3a: on a failed background token refresh, warn once per config with a re-login
hint instead of a silent debug line followed by a bare downstream 401.
Also adds the previously-uncommitted tests for the load_config mtime cache and the
SDK oauth_token refresh seam.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously a closed/non-interactive stdin raised a bare EOFError from input(), surfacing as a blank "Command execution failed" with exit 0 — so a destructive bulk op or a name-less `cld logout` silently no-op'd while reporting success. Separately, colored --json output was only suppressed on Windows, so it relied on click.echo's implicit ANSI stripping for piped output. Interactivity is now centralized in cloudinary_cli/utils/utils.py: - is_interactive(): the single sys.stdin.isatty() check. - prompt_user(): the single input() call; returns None on EOF, logging an optional hint so the caller's decision is never a silent no-op. - get_user_action / confirm_action delegate to prompt_user; on no input they apply the default and hint at --force (-F). Behavior: - `cld logout` with no name: on EOF, error with the `cld logout <name>` form and exit non-zero (the selection has no flag substitute). - `cld login`: when no browser opens and stdin is not a TTY, fail fast with a headless-usage hint (-c/-C) instead of blocking until the 300s callback timeout. - print_json: colorize only when stdout is an interactive TTY (drops the Windows-only guard), so piped/captured JSON (LLM agents, `| jq`, redirects) is never corrupted by ANSI escapes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- CI matrix: full Python (3.10-3.14) on Linux, plus one latest-Python (3.14) smoke job each on macos-latest and windows-latest. A single bash shell runs every step identically on all three (Git Bash ships on the Windows runner). - Guard the remaining POSIX-mode assertions (umask/file-mode) with skipIf(win32); os.chmod only honors the read-only flag on Windows, so exact-mode checks do not apply there (the code still runs, the hardening is just POSIX-only). - Rewrite the loopback port-busy test to mock HTTPServer raising OSError instead of relying on a real double-bind, which Windows does not reject like POSIX does. - Add print_json tests asserting non-TTY output stays plain on Windows too and that colorization is decided by isatty(), not the OS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…shadowing The `config` command function shadowed the `cloudinary_cli.core.config` submodule as a package attribute, so on Python 3.10 mock.patch resolved "cloudinary_cli.core.config.<name>" to the click.Command (no such attribute) instead of the module. Python 3.11+ prefers the real submodule, so only the 3.10 CI job failed. Rename the function to config_command (CLI command name "config" is unchanged) so the submodule is never shadowed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `cld login`/`logout` as the recommended setup path (no API secret on disk), and document choosing a default configuration, resolution precedence, and manual OAuth token refresh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These are working notes, not part of the published package; keep them local. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add CLOUDINARY_OAUTH_CLIENT_ID and CLOUDINARY_OAUTH_SCOPES overrides (mirroring the redirect host/port pattern) for testing against a non-prod authorization server; production defaults are unchanged. Also drop implementation-specific server naming from comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CLI imports dataclasses (3.7+), and the pinned urllib3>=2.2.2 / zipp>=3.19.1 security floors require Python 3.8+, so 3.6 and 3.7 cannot install or run it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
logout now revokes the refresh token at the authorization server (RFC 7009) before removing the saved configuration; the local entry is removed even if revocation fails. login reports whether the new login became the default and, when it did not, prints the command to make it the default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Brief Summary of Changes
Adds OAuth-based authentication to the CLI and configuration management around it.
New commands
cld login [name] [--set-default]— browser-based OAuth login (Authorization Code + PKCE, loopback redirect on127.0.0.1:49421). The session is saved as a namedcloudinary://config entry; no API secret is stored on disk. Reports whether the login became the default, and if not, how to make it one.cld logout [name]— revoke a saved OAuth login's token at the server and remove its configuration (interactive picker when no name is given). The local entry is removed even if revocation fails. Refuses to remove non-OAuth configs.cld configadditions-d/--default <name>,--set-default,-ud/--unset-default— manage a stored default configuration.--refresh [name],--refresh-all,-f/--force— refresh saved OAuth tokens.-ls/-s/bareconfiggain--jsonoutput and an inventory view marking the default and active config; secrets (api_secret, oauth_token, refresh_token, account_url password) are masked.Token handling
oauth_tokenat request-build time. Presence/type checks (has_oauth) never touch the network.Config resolution precedence:
-c(inline URL) >-C(saved name) > stored default >CLOUDINARY_URLenv > unconfigured.Config storage
0600(POSIX).Env-overridable settings (defaults unchanged):
CLOUDINARY_OAUTH_CLIENT_ID,CLOUDINARY_OAUTH_SCOPES,CLOUDINARY_OAUTH_REDIRECT_HOST,CLOUDINARY_OAUTH_REDIRECT_PORT.Other
cld loginfast-fails when headless,print_jsoncolorizes only on a TTY.filelock.What does this PR address?
Are tests included?
Reviewer, please note:
cloudinary_cli/auth/oauth_config.py): every active config is installed as anOAuthConfigsooauth_tokenreads can refresh. Confirm no code path swaps in a plainConfigand silently disables refresh.auth/__init__.py::refresh_url_if_stale): the single-use refresh token is consumed under a cross-process lock with a freshness re-check; a peer refresh is adopted rather than burning the token again.config_utils.py): verify no config view prints a token/secret in the clear.configcommand rename: the command function isconfig_command(CLI name stillconfig) to avoid shadowing thecore.configsubmodule on Python 3.10.filelockadded torequirements.txt.Checklist: