diff --git a/.github/workflows/cloudinary-cli-test.yml b/.github/workflows/cloudinary-cli-test.yml index 9d0664f..dfef328 100644 --- a/.github/workflows/cloudinary-cli-test.yml +++ b/.github/workflows/cloudinary-cli-test.yml @@ -9,11 +9,25 @@ on: jobs: build: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: - python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"] + # Full Python matrix on Linux; macOS and Windows get a single latest-Python smoke job each + # (enough to catch platform-specific regressions without 3x the runners and test clouds). + os: [ubuntu-latest] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + include: + - os: macos-latest + python-version: "3.14" + - os: windows-latest + python-version: "3.14" + + # Git Bash ships on the GitHub Windows runners, so a single bash shell keeps every step + # identical across Linux, macOS, and Windows (no PowerShell variants to maintain). + defaults: + run: + shell: bash steps: - uses: actions/checkout@v4 @@ -35,7 +49,7 @@ jobs: - name: Get test cloud run: echo "CLOUDINARY_URL=$(bash tools/get_test_cloud.sh)" >> $GITHUB_ENV - name: Show test cloud - run: echo $CLOUDINARY_URL | cut -d'@' -f2 + run: echo "$CLOUDINARY_URL" | cut -d'@' -f2 - name: Test with pytest run: | pytest diff --git a/.gitignore b/.gitignore index 796c08f..413b071 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ venv .cld-sync .cld-settings +.cld-config .venv diff --git a/README.md b/README.md index 3769a80..876d3f7 100644 --- a/README.md +++ b/README.md @@ -12,12 +12,22 @@ It is fully documented at [https://cloudinary.com/documentation/cloudinary_cli]( ## Requirements Your own Cloudinary account. If you don't already have one, sign up at [https://cloudinary.com/users/register/free](https://cloudinary.com/users/register/free). -Python 3.6 or later. You can install Python from [https://www.python.org/](https://www.python.org/). Note that the Python Package Installer (pip) is installed with it. +Python 3.8 or later. You can install Python from [https://www.python.org/](https://www.python.org/). Note that the Python Package Installer (pip) is installed with it. ## Setup and Installation 1. To install this package, run: `pip3 install cloudinary-cli` -2. To make all your `cld` commands point to your Cloudinary account, set up your CLOUDINARY\_URL environment variable. For example: +2. Point your `cld` commands at a Cloudinary account using **either** of the following: + + **Option A — Log in with OAuth (recommended).** Run: + + ``` + cld login + ``` + + This opens your browser to authorize the CLI, then saves the login as a configuration (named after the cloud) and sets it as the default. The CLI refreshes the token automatically, and you can remove the login at any time with `cld logout`. + + **Option B — Set your CLOUDINARY\_URL environment variable.** For example: * On Mac or Linux:
`export CLOUDINARY_URL=cloudinary://123456789012345:abcdefghijklmnopqrstuvwxyzA@cloud_name` * On Windows (cmd.exe):
`set CLOUDINARY_URL=cloudinary://123456789012345:abcdefghijklmnopqrstuvwxyzA@cloud_name` * On Windows (PowerShell):
`$Env:CLOUDINARY_URL="cloudinary://123456789012345:abcdefghijklmnopqrstuvwxyzA@cloud_name"` @@ -47,6 +57,8 @@ Usage: cld [cli options] [command] [command options] [method] [method parameters ``` cld --help # Lists available commands. +cld login # Logs in to a Cloudinary account via OAuth in your browser. +cld logout # Revokes and removes a saved OAuth login. cld search --help # Shows usage for the Search API. cld admin # Lists Admin API methods. cld uploader # Lists Upload API methods. @@ -243,7 +255,7 @@ Whereas using the saved configuration "accountx": cld -C accountx admin usage ``` -_**Caution:** Creating a saved configuration may put your API secret at risk as it is stored in a local plain text file._ +_**Caution:** Creating a saved configuration may put your credentials at risk as they are stored in a local plain text file. This applies to both API-key configurations and OAuth logins._ You can create, delete and list saved configurations using the `config` command. @@ -252,3 +264,42 @@ cld config [options] ``` For details, see the [Cloudinary CLI documentation](https://cloudinary.com/documentation/cloudinary_cli#config). + +### Logging in with OAuth + +Instead of saving an API key and secret, you can log in to a Cloudinary account through your browser. The CLI saves the resulting session as a named configuration and refreshes its token automatically. + +``` +cld login # Log in and save the configuration (named after the cloud). +cld login my-account # Save the login under a specific name. +cld logout # Choose a saved OAuth login to log out of. +cld logout my-account # Log out of a specific saved OAuth login. +``` + +The first login becomes the default automatically. When other configurations already exist, the new login is saved but not made the default; `cld login` tells you so and prints the command to make it the default. Once saved, an OAuth login is selected with `-C ` just like any other saved configuration. + +`cld logout` revokes the login's token at the server and removes the saved configuration. If the token cannot be revoked (for example, you are offline), the saved configuration is still removed. + +### Choosing a default configuration + +The default configuration is used when no `-c`/`-C` option is given and no `CLOUDINARY_URL` environment variable is set. The first OAuth login becomes the default automatically; you can change it at any time. + +``` +cld config -d # Set an existing saved configuration as the default. +cld config --unset-default # Clear the stored default. +cld config -ls # List saved configurations, marking the default and the active one. +``` + +When creating a configuration with `-n` or `--from_url`, add `--set-default` to make it the default in the same step. Resolution precedence is: `-c` (inline URL) > `-C` (saved name) > stored default > `CLOUDINARY_URL` environment variable. + +### Refreshing OAuth tokens + +OAuth tokens are refreshed automatically as needed, but you can refresh them manually. + +``` +cld config --refresh # Refresh a saved OAuth configuration's token. +cld config --refresh-all # Refresh every saved OAuth configuration whose token is stale. +cld config --refresh --force # Refresh even if the token is still fresh. +``` + +If a token can no longer be refreshed (for example, the login was revoked), the CLI reports the configuration and the `cld login` command to use to log in again. diff --git a/cloudinary_cli/auth/__init__.py b/cloudinary_cli/auth/__init__.py new file mode 100644 index 0000000..0dfb3e6 --- /dev/null +++ b/cloudinary_cli/auth/__init__.py @@ -0,0 +1,244 @@ +"""OAuth login façade: runs the PKCE loopback flow, persists each login as a named +`cloudinary://` entry in `config.json`, and refreshes tokens when a saved login is selected.""" +import secrets +import webbrowser + +import requests + +from cloudinary_cli.auth import flow +from cloudinary_cli.auth.loopback_server import start_callback_server, wait_for_callback +from cloudinary_cli.auth.session import ( + Session, + to_cloudinary_url, + from_cloudinary_url, + is_oauth_url, +) +from cloudinary_cli.defaults import logger, normalize_region, DEFAULT_REGION, CLOUDINARY_REGION +from cloudinary_cli.utils.config_utils import ( + load_config, + update_config, + remove_config_keys, + config_lock, + user_config_names, + get_default_config_name, + set_default_config, + is_reserved_config_name, + is_env_configured, +) +from cloudinary_cli.utils.utils import log_exception, is_interactive + +# Names already warned about a failed background refresh, so a bulk run (many workers, each reading +# the token) logs the re-login hint once per config instead of once per asset. Mutated only under +# config_lock, so no extra synchronization is needed. +_refresh_warned = set() + + +def login(region=None, name=None, set_default=False): + """ + Run the interactive browser login and persist the resulting session as a named config entry. + + Returns (config_name, is_default), where is_default is True when this login was made the default + configuration (explicitly with set_default, or automatically as the sole login). + """ + if name and is_reserved_config_name(name): + raise RuntimeError(f"'{name}' is a reserved configuration name.") + region = normalize_region(region or CLOUDINARY_REGION) + session = _run_browser_flow(region) + if not session.cloud_name: + raise RuntimeError("Login token did not include a cloud name; cannot save this login.") + config_name = name or _derive_config_name(session.cloud_name, region) + update_config({config_name: to_cloudinary_url(session)}) + + is_default = bool(set_default or _should_auto_default(config_name)) + if is_default: + set_default_config(config_name) + return config_name, is_default + + +def _should_auto_default(name): + """ + True when the just-saved login should become the default without an explicit flag: it is the + only saved config, the environment configures nothing, and no default is already stored. + + A stored default outranks the environment, so auto-defaulting is suppressed when an env config + is present: a single `cld login` must not silently override a user's CLOUDINARY_URL. They can + still opt in with `--set-default`. + """ + cfg = load_config() + return ( + user_config_names(cfg) == [name] + and not is_env_configured() + and not get_default_config_name() + ) + + +def logout(name): + """ + Log out of a saved OAuth login by name: revoke its refresh token at the authorization server, + then remove the saved configuration. The local entry is always removed even if revocation fails + (offline, server error), so logout never leaves a stale entry behind. + + Returns "removed" (revoked and removed), "revoke_failed" (removed locally but the token could not + be revoked), "not_found", or "not_oauth". + """ + saved = load_config() + if name not in saved: + return "not_found" + if not is_oauth_url(saved[name]): + return "not_oauth" + + revoked = _revoke_login(name, saved[name]) + remove_config_keys(name) + return "removed" if revoked else "revoke_failed" + + +def _revoke_login(name, url): + """Best-effort revocation of a saved login's refresh token. Returns True on success (or when + there is nothing to revoke), False if the revoke request failed.""" + session = from_cloudinary_url(url) + if not session.refresh_token: + return True + try: + flow.revoke(session.refresh_token, session.region) + return True + except requests.RequestException as e: + log_exception(e, debug_message=f"Could not revoke the OAuth token for '{name}'") + return False + + +def refresh_url_if_stale(name, url, force=False): + """ + Given a saved config value, refresh it if it is a stale OAuth login (rewriting the stored + URL on token rotation). Non-OAuth and still-fresh URLs are returned unchanged. With force=True + a still-fresh token is refreshed too (used by the explicit `config --refresh --force`). + + The refresh consumes a single-use refresh token, so the whole read-refresh-write runs under + a cross-process lock with the freshness re-checked inside it: a peer that refreshed while we + waited leaves a fresh token we adopt instead of refreshing (and burning) it again. + """ + if not is_oauth_url(url): + return url + + session = from_cloudinary_url(url) + if (session.is_fresh() and not force) or not session.refresh_token: + return url + + with config_lock(): + url = load_config().get(name, url) # re-read: a peer may have refreshed while we waited + session = from_cloudinary_url(url) + if (session.is_fresh() and not force) or not session.refresh_token: + return url + + try: + token_response = flow.refresh(session.refresh_token, session.region) + except requests.RequestException as e: + # Serve the stale token (a bulk run survives a transient blip) but make the failure + # visible once per config, not a silent debug line followed by a bare downstream 401. + log_exception(e, debug_message="OAuth token refresh failed") + if name not in _refresh_warned: + _refresh_warned.add(name) + logger.warning(f"Could not refresh the OAuth token for '{name}'; using the existing " + f"token, which may be expired. Re-login with `{relogin_command(name)}`.") + return url + + _refresh_warned.discard(name) # a later success re-arms the warning for this config + + # The authorization server rotates refresh tokens; keep the old one only if a new one was not returned. + token_response.setdefault("refresh_token", session.refresh_token) + refreshed_url = to_cloudinary_url(session.updated_from(token_response)) + update_config({name: refreshed_url}) + return refreshed_url + + +def refresh_config(name, force=False): + """ + Refresh a single saved OAuth config by name and report the outcome. Returns one of: + "not_found", "not_oauth", "fresh" (skipped, still valid), "refreshed", or "failed" + ("failed" = stale/forced but no refresh token, or the network refresh did not rotate it). + """ + cfg = load_config() + if name not in user_config_names(cfg): + return "not_found" + url = cfg[name] + if not is_oauth_url(url): + return "not_oauth" + + session = from_cloudinary_url(url) + if session.is_fresh() and not force: + return "fresh" + if not session.refresh_token: + return "failed" + + new_url = refresh_url_if_stale(name, url, force=force) + return "refreshed" if new_url != url else "failed" + + +def refresh_configs(force=False): + """Refresh every saved OAuth config. Returns {name: outcome} (see refresh_config).""" + return {name: refresh_config(name, force=force) for name in list_oauth_login_names()} + + +def relogin_command(name): + """ + Build the `cld login` command to re-authenticate a saved OAuth config, preserving its region + (a non-default region must be passed explicitly so the right OAuth host is used). + """ + cmd = f"cld login {name}" + url = load_config().get(name) + region = from_cloudinary_url(url).region if url and is_oauth_url(url) else None + if region and region != DEFAULT_REGION: + cmd += f" --region {region}" + return cmd + + +def list_oauth_login_names(): + """Return the names of all saved OAuth logins.""" + cfg = load_config() + return [name for name in user_config_names(cfg) if is_oauth_url(cfg[name])] + + +def _run_browser_flow(region): + verifier, challenge = flow.generate_pkce_pair() + state = secrets.token_urlsafe(16) + httpd, redirect_uri = start_callback_server() + + authorize_url = flow.build_authorize_url(challenge, state, redirect_uri, region) + logger.info("Opening browser to log in to Cloudinary...") + opened = webbrowser.open(authorize_url) + if not opened and not is_interactive(): + # No browser and no interactive terminal: nobody can complete the redirect, so fail fast + # instead of blocking until the callback times out. Headless runs use a pre-set config. + httpd.server_close() + raise RuntimeError( + "cld login needs an interactive browser session, but no browser could be opened and " + "this is not an interactive terminal. For headless/CI use, configure credentials with " + "an API-key URL instead: `cld -c cloudinary://:@ ` or save " + "one with `cld config -n ` and select it via `-C `." + ) + if not opened: + logger.info(f"Could not open a browser. Visit this URL to log in:\n{authorize_url}") + else: + logger.info(f"If it doesn't open automatically, visit:\n{authorize_url}") + + auth_code, returned_state = wait_for_callback(httpd) + if returned_state != state: + raise RuntimeError("State mismatch - possible CSRF, aborting.") + + token_response = flow.exchange_code(auth_code, verifier, redirect_uri, region) + return Session.from_token_response(token_response, region=region) + + +def _derive_config_name(cloud_name, region): + """ + Build the saved name: cloud_name + region geo suffix (when not default) + auth-type suffix + only when the base name collides with a DIFFERENT auth type (re-login overwrites in place). + """ + base = cloud_name + if region != DEFAULT_REGION: + base = f"{base}-{region[len('api-'):]}" # api-eu -> "-eu" + + config = load_config() + existing = config.get(base) + if existing is None or is_oauth_url(existing): + return base # free, or same (oauth) type -> overwrite in place + return f"{base}-oauth" # taken by an api-key config -> suffix the new oauth entry diff --git a/cloudinary_cli/auth/flow.py b/cloudinary_cli/auth/flow.py new file mode 100644 index 0000000..a10444e --- /dev/null +++ b/cloudinary_cli/auth/flow.py @@ -0,0 +1,72 @@ +"""OAuth 2.0 Authorization Code + PKCE protocol helpers (RFC 8252): build the authorize URL, +exchange a code, refresh a token. Pure protocol, no file I/O or global state.""" +import base64 +import hashlib +import secrets +import urllib.parse + +import requests + +from cloudinary_cli.defaults import ( + oauth_authorize_url_for_region, + oauth_token_url_for_region, + oauth_revoke_url_for_region, + OAUTH_CLIENT_ID, + OAUTH_SCOPES, + OAUTH_HTTP_TIMEOUT_SECONDS, +) + + +def generate_pkce_pair(): + """Return (code_verifier, code_challenge) for the S256 PKCE method.""" + verifier = base64.urlsafe_b64encode(secrets.token_bytes(32)).rstrip(b"=").decode("ascii") + digest = hashlib.sha256(verifier.encode("ascii")).digest() + challenge = base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii") + return verifier, challenge + + +def build_authorize_url(challenge, state, redirect_uri, region): + query = urllib.parse.urlencode({ + "client_id": OAUTH_CLIENT_ID, + "response_type": "code", + "scope": OAUTH_SCOPES, + "redirect_uri": redirect_uri, + "state": state, + "code_challenge": challenge, + "code_challenge_method": "S256", + }) + return f"{oauth_authorize_url_for_region(region)}?{query}" + + +def exchange_code(auth_code, verifier, redirect_uri, region): + """Exchange the authorization code for tokens. Public PKCE client - no client_secret.""" + resp = requests.post(oauth_token_url_for_region(region), data={ + "grant_type": "authorization_code", + "code": auth_code, + "redirect_uri": redirect_uri, + "client_id": OAUTH_CLIENT_ID, + "code_verifier": verifier, + }, timeout=OAUTH_HTTP_TIMEOUT_SECONDS) + resp.raise_for_status() + return resp.json() + + +def refresh(refresh_token, region): + resp = requests.post(oauth_token_url_for_region(region), data={ + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": OAUTH_CLIENT_ID, + }, timeout=OAUTH_HTTP_TIMEOUT_SECONDS) + resp.raise_for_status() + return resp.json() + + +def revoke(token, region, token_type_hint="refresh_token"): + """Revoke a token at the authorization server (RFC 7009). Revoking the refresh token ends the + offline-access grant so it can no longer mint new access tokens.""" + resp = requests.post(oauth_revoke_url_for_region(region), data={ + "token": token, + "token_type_hint": token_type_hint, + "client_id": OAUTH_CLIENT_ID, + }, timeout=OAUTH_HTTP_TIMEOUT_SECONDS) + resp.raise_for_status() diff --git a/cloudinary_cli/auth/loopback_server.py b/cloudinary_cli/auth/loopback_server.py new file mode 100644 index 0000000..873b5fc --- /dev/null +++ b/cloudinary_cli/auth/loopback_server.py @@ -0,0 +1,79 @@ +"""Single-shot loopback HTTP server that captures the OAuth redirect: binds a localhost port, +serves until the `?code=&state=` (or `?error=`) redirect arrives or it times out.""" +import time +import urllib.parse +from http.server import BaseHTTPRequestHandler, HTTPServer + +from cloudinary_cli.defaults import ( + OAUTH_REDIRECT_HOST, + OAUTH_REDIRECT_PORT, + OAUTH_CALLBACK_PATH, + OAUTH_CALLBACK_TIMEOUT_SECONDS, +) + + +class _CallbackHandler(BaseHTTPRequestHandler): + """Captures the ?code=&state= redirect from the authorization server.""" + + def do_GET(self): # noqa: N802 (http.server API) + parsed = urllib.parse.urlparse(self.path) + params = urllib.parse.parse_qs(parsed.query) + + # Ignore stray requests (e.g. /favicon.ico, wrong path) so they don't consume the wait. + if parsed.path != OAUTH_CALLBACK_PATH or ("code" not in params and "error" not in params): + self.send_response(404) + self.end_headers() + return + + self.server.auth_code = params.get("code", [None])[0] + self.server.auth_state = params.get("state", [None])[0] + self.server.auth_error = params.get("error", [None])[0] + + self.send_response(200) + self.send_header("Content-Type", "text/html; charset=utf-8") + self.end_headers() + if self.server.auth_error: + body = f"

Login failed

{self.server.auth_error}

" + else: + body = "

Login successful

You can close this tab and return to the terminal.

" + self.wfile.write(f"{body}".encode("utf-8")) + + def log_message(self, *args): + pass # silence the default stderr request logging + + +def start_callback_server(): + """Bind the loopback server and return (httpd, redirect_uri).""" + try: + httpd = HTTPServer((OAUTH_REDIRECT_HOST, OAUTH_REDIRECT_PORT), _CallbackHandler) + except OSError as e: + raise RuntimeError( + f"Could not start the local login server on {OAUTH_REDIRECT_HOST}:{OAUTH_REDIRECT_PORT} " + f"({e.strerror or e}). Another login may be in progress, or the port is in use. " + f"Close it and retry." + ) from e + httpd.auth_code = httpd.auth_state = httpd.auth_error = None + httpd.timeout = OAUTH_CALLBACK_TIMEOUT_SECONDS + redirect_uri = f"http://{OAUTH_REDIRECT_HOST}:{OAUTH_REDIRECT_PORT}{OAUTH_CALLBACK_PATH}" + return httpd, redirect_uri + + +def wait_for_callback(httpd): + """ + Serve requests until the redirect arrives (ignoring favicon/etc.) or the timeout elapses. + Returns (auth_code, auth_state); raises on error or timeout. + """ + deadline = time.monotonic() + OAUTH_CALLBACK_TIMEOUT_SECONDS + try: + while httpd.auth_code is None and httpd.auth_error is None: + if time.monotonic() > deadline: + break + httpd.handle_request() + finally: + httpd.server_close() + + if httpd.auth_error: + raise RuntimeError(f"Authorization failed: {httpd.auth_error}") + if not httpd.auth_code: + raise RuntimeError("Timed out waiting for the authorization redirect.") + return httpd.auth_code, httpd.auth_state diff --git a/cloudinary_cli/auth/oauth_config.py b/cloudinary_cli/auth/oauth_config.py new file mode 100644 index 0000000..8456c2b --- /dev/null +++ b/cloudinary_cli/auth/oauth_config.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +import cloudinary + +from cloudinary_cli.auth.session import is_oauth_url, from_cloudinary_url + + +class OAuthConfig(cloudinary.Config): + """ + A Cloudinary config whose `oauth_token` refreshes itself on read, at the moment the SDK builds + a request. Presence/type checks read `has_oauth` instead and never touch the network, so offline + paths (`config -ls`, `config -s`, the group-level validity check) stay offline. + + The raw access token is kept in `__dict__["oauth_token"]` so serialization (config_to_dict, + masking) still sees it; the class-level property shadows it on attribute *read* to + refresh-if-stale. A parsed Session (`_session`) carries expiry/refresh-token so a still-fresh + token short-circuits with no disk read and no lock — only a stale token reads config + refreshes. + """ + + def bind_saved(self, name, url): + # name: the saved-config name this maps to (None for env / inline -c -> never refreshes). + # url: the full cloudinary:// URL, kept parsed so we know expiry without re-reading disk. + self._saved_name = name + self._session = from_cloudinary_url(url) if (name and url and is_oauth_url(url)) else None + + @classmethod + def from_env(cls): + """An OAuthConfig populated from the environment (CLOUDINARY_URL/CLOUDINARY_*). Static: it is + not bound to a saved name, so reading its oauth_token never refreshes.""" + cfg = cls() # the base Config constructor loads the environment + cfg._saved_name = None + cfg._session = None + return cfg + + @classmethod + def from_url(cls, url): + """An OAuthConfig populated from a cloudinary:// URL, not bound to a saved name (static).""" + cfg = cls() + cfg._load_from_url(url) + cfg._saved_name = None + cfg._session = None + return cfg + + @property + def has_oauth(self): + """True if this config carries an OAuth token. Cheap, never refreshes.""" + return bool(self.__dict__.get("oauth_token")) + + @property + def oauth_token(self): + session = getattr(self, "_session", None) + if session is None: + return self.__dict__.get("oauth_token") # env / -c / api-key: static, no refresh + if session.is_fresh() or not session.refresh_token: + return self.__dict__.get("oauth_token") # still valid (or unrefreshable): no I/O + + # Stale: read the saved URL (a peer may already have refreshed it) and refresh under lock. + from cloudinary_cli.auth import refresh_url_if_stale + from cloudinary_cli.utils.config_utils import load_config + + url = load_config().get(self._saved_name) + if not url: + return self.__dict__.get("oauth_token") # config removed underneath us; serve what we have + fresh_url = refresh_url_if_stale(self._saved_name, url) + self._session = from_cloudinary_url(fresh_url) + self.__dict__["oauth_token"] = self._session.access_token + return self.__dict__["oauth_token"] + + @oauth_token.setter + def oauth_token(self, value): + self.__dict__["oauth_token"] = value + + +def install_oauth_config(cloudinary_url, saved_name=None): + """ + Load `cloudinary_url` and install it as the active SDK config. The installed object is always an + OAuthConfig (so every active config exposes `has_oauth`); it self-refreshes only when bound to a + saved OAuth `saved_name`, and is static for api-key / inline `-c` URLs. The single seam that + swaps the global config object. + """ + cloudinary.reset_config() + cfg = OAuthConfig() + cfg._load_from_url(cloudinary_url) + cfg.bind_saved(saved_name, cloudinary_url) + cloudinary._config = cfg + return cfg + + +def install_env_config(): + """Install the environment config as a (static) OAuthConfig, so the active global is always an + OAuthConfig and exposes has_oauth without a refresh. Used for the env fallback branch.""" + cfg = OAuthConfig.from_env() + cloudinary._config = cfg + return cfg diff --git a/cloudinary_cli/auth/session.py b/cloudinary_cli/auth/session.py new file mode 100644 index 0000000..b126ecb --- /dev/null +++ b/cloudinary_cli/auth/session.py @@ -0,0 +1,113 @@ +"""The OAuth session and its `cloudinary://` URL codec. A login is persisted as a `cloudinary://` +URL so it flows through the SDK parser and existing config machinery unchanged; the `Session` +dataclass is the in-memory form, `to_cloudinary_url`/`from_cloudinary_url` the persisted one.""" +import base64 +import json +import time +import urllib.parse +from dataclasses import dataclass + +from cloudinary_cli.defaults import ( + logger, + OAUTH_EXPIRY_SKEW_SECONDS, + OAUTH_FALLBACK_EXPIRES_IN_SECONDS, + api_host_for_region, +) + +# Query-string keys that carry the OAuth session inside a cloudinary:// URL. +_OAUTH_MARKER = "oauth_token" + +_OAUTH_INTERNAL_KEYS = frozenset({"refresh_token", "expires_at", "region", "issuer"}) + + +def strip_oauth_internal_keys(config_dict): + return {k: v for k, v in config_dict.items() if k not in _OAUTH_INTERNAL_KEYS} + + +@dataclass +class Session: + cloud_name: str + access_token: str + refresh_token: str = None + expires_at: int = 0 + region: str = "api" + issuer: str = None + + def is_fresh(self, skew=OAUTH_EXPIRY_SKEW_SECONDS): + return int(self.expires_at or 0) - skew > int(time.time()) + + @classmethod + def from_token_response(cls, token_response, cloud_name=None, region="api", issuer=None): + access_token = token_response["access_token"] + expires_in = int(token_response.get("expires_in") or 0) or OAUTH_FALLBACK_EXPIRES_IN_SECONDS + return cls( + cloud_name=cloud_name or decode_cloud_name(access_token), + access_token=access_token, + refresh_token=token_response.get("refresh_token"), + expires_at=int(time.time()) + expires_in, + region=region, + issuer=decode_issuer(access_token), + ) + + def updated_from(self, token_response): + """Return a new Session with refreshed tokens, preserving cloud_name/region.""" + return Session.from_token_response( + token_response, cloud_name=self.cloud_name, region=self.region) + + +def to_cloudinary_url(session): + """Encode a Session as a key-less cloudinary:// URL (Bearer auth, region-derived host).""" + params = { + "oauth_token": session.access_token, + "refresh_token": session.refresh_token or "", + "expires_at": session.expires_at, + "region": session.region, + "issuer": session.issuer or "", + "upload_prefix": api_host_for_region(session.region), + } + return f"cloudinary://{session.cloud_name}?{urllib.parse.urlencode(params)}" + + +def from_cloudinary_url(url): + """Parse an OAuth cloudinary:// URL back into a Session.""" + parsed = urllib.parse.urlparse(url) + q = {k: v[0] for k, v in urllib.parse.parse_qs(parsed.query).items()} + return Session( + cloud_name=parsed.hostname, + access_token=q.get("oauth_token"), + refresh_token=q.get("refresh_token") or None, + expires_at=int(q.get("expires_at", 0) or 0), + region=q.get("region", "api"), + issuer=q.get("issuer") or None, + ) + + +def is_oauth_url(url): + if not isinstance(url, str): + return False + query = urllib.parse.urlparse(url).query + return _OAUTH_MARKER in urllib.parse.parse_qs(query) + + +def _decode_jwt_payload(access_token): + payload_b64 = access_token.split(".")[1] + payload_b64 += "=" * (-len(payload_b64) % 4) # pad to a multiple of 4 + return json.loads(base64.urlsafe_b64decode(payload_b64)) + + +def decode_cloud_name(access_token): + """Best-effort extraction of cloud_name from the JWT's `ext` claim.""" + try: + payload = _decode_jwt_payload(access_token) + return (payload.get("ext") or {}).get("cloud_name") or payload.get("cloud_name") + except Exception as e: + logger.debug(f"Could not decode cloud_name from token: {e}") + return None + + +def decode_issuer(access_token): + try: + return _decode_jwt_payload(access_token).get("iss") + except Exception as e: + logger.debug(f"Could not decode issuer from token: {e}") + return None diff --git a/cloudinary_cli/cli_group.py b/cloudinary_cli/cli_group.py index 9cd4147..a1b5301 100644 --- a/cloudinary_cli/cli_group.py +++ b/cloudinary_cli/cli_group.py @@ -7,8 +7,7 @@ import cloudinary from cloudinary_cli.defaults import logger -from cloudinary_cli.utils.config_utils import load_config, refresh_cloudinary_config, \ - is_valid_cloudinary_config +from cloudinary_cli.utils.config_resolver import resolve_cli_config from cloudinary_cli.version import __version__ as cli_version CONTEXT_SETTINGS = dict(max_content_width=shutil.get_terminal_size()[0], terminal_width=shutil.get_terminal_size()[0]) @@ -29,19 +28,8 @@ @click_log.simple_verbosity_option(logger) @click.pass_context def cli(ctx, config, config_saved): - if config: - refresh_cloudinary_config(config) - elif config_saved: - config = load_config() - if config_saved not in config: - raise Exception(f"Config {config_saved} does not exist") + resolve_cli_config(config, config_saved) - refresh_cloudinary_config(config[config_saved]) - - if not is_valid_cloudinary_config(): - logger.warning("No Cloudinary configuration found.") - - # If no subcommand was invoked, show help and exit with code 0 if ctx.invoked_subcommand is None: click.echo(ctx.get_help()) ctx.exit(0) diff --git a/cloudinary_cli/core/__init__.py b/cloudinary_cli/core/__init__.py index 7646e1c..8e2edeb 100644 --- a/cloudinary_cli/core/__init__.py +++ b/cloudinary_cli/core/__init__.py @@ -1,7 +1,8 @@ import click from cloudinary_cli.core.admin import admin -from cloudinary_cli.core.config import config +from cloudinary_cli.core.auth import login, logout +from cloudinary_cli.core.config import config_command from cloudinary_cli.core.search import search, search_folders from cloudinary_cli.core.uploader import uploader from cloudinary_cli.core.provisioning import provisioning @@ -11,7 +12,9 @@ setattr(click.Group, "resolve_command", resolve_command) commands = [ - config, + config_command, + login, + logout, search, search_folders, admin, diff --git a/cloudinary_cli/core/auth.py b/cloudinary_cli/core/auth.py new file mode 100644 index 0000000..9d6b533 --- /dev/null +++ b/cloudinary_cli/core/auth.py @@ -0,0 +1,88 @@ +from click import command, argument, option, echo + +from cloudinary_cli.auth import login as run_login, logout as run_logout, list_oauth_login_names +from cloudinary_cli.defaults import logger +from cloudinary_cli.utils.utils import log_exception, prompt_user + + +@command("login", help="Log in to Cloudinary via OAuth (opens a browser). The session is saved " + "as a named configuration you can select with `-C`.") +@argument("name", required=False) +@option("--region", + help="Cloudinary region to log in to (e.g. eu, ap, or api-eu). Defaults to the " + "global region (api).") +@option("--set-default", "set_default", is_flag=True, + help="Set this login as the default configuration used when no -c/-C and no environment " + "config is given.") +def login(name, region, set_default): + try: + config_name, is_default = run_login(region=region, name=name, set_default=set_default) + except Exception as e: + log_exception(e, "Login failed") + return False + + logger.info(f"Logged in. Saved as '{config_name}'.") + if is_default: + logger.info(f"This is now the default configuration. Run `cld ` to use it, " + f"or `cld -C {config_name} ` to select it explicitly.") + else: + logger.info(f"Run `cld -C {config_name} ` to use it, " + f"or make it the default with `cld config -d {config_name}`.") + return True + + +@command("logout", help="Log out: revoke a saved OAuth login's token and remove its configuration. " + "Run without a name to choose from the saved logins.") +@argument("name", required=False) +def logout(name): + if not name: + action, name = _select_oauth_login() + if action == "invalid": + return False + if action != "selected": + return True + + status = run_logout(name) + if status == "removed": + logger.info(f"Logged out of '{name}'. Its token was revoked and the saved login removed.") + elif status == "revoke_failed": + logger.warning(f"Removed '{name}', but could not revoke its token at the server " + f"(it may still be valid until it expires).") + elif status == "not_oauth": + logger.error(f"'{name}' is not an OAuth login; refusing to remove it. " + f"Use `config -rm {name}` to delete a saved configuration.") + return False + else: + logger.info(f"No saved OAuth configuration named '{name}'.") + return True + + +def _select_oauth_login(): + """ + Prompt the user to pick a saved OAuth login by number. + + Returns ("selected", name), ("cancelled", None), ("none", None), or ("invalid", None). + """ + names = list_oauth_login_names() + if not names: + logger.info("No saved OAuth logins to log out of.") + return "none", None + + echo("Saved OAuth logins:") + for i, name in enumerate(names, start=1): + echo(f" {i}) {name}") + + # The selection needs real input that no flag replaces, so on non-interactive stdin prompt_user + # returns None (after logging the hint) and we report it as an invalid (non-zero) outcome. + choice = prompt_user( + f"Select a login to log out of [1-{len(names)}] (or Enter to cancel): ", + noninteractive_hint="Pass the configuration name directly: `cld logout `.") + if choice is None: + return "invalid", None + choice = choice.strip() + if not choice: + return "cancelled", None + if not (choice.isdigit() and 1 <= int(choice) <= len(names)): + logger.error(f"Invalid selection '{choice}'. Expected a number between 1 and {len(names)}.") + return "invalid", None + return "selected", names[int(choice) - 1] diff --git a/cloudinary_cli/core/config.py b/cloudinary_cli/core/config.py index c558e17..258e118 100644 --- a/cloudinary_cli/core/config.py +++ b/cloudinary_cli/core/config.py @@ -1,25 +1,80 @@ import cloudinary -from click import command, option, echo, BadParameter +from click import command, option, echo, BadParameter, UsageError -from cloudinary_cli.defaults import logger -from cloudinary_cli.utils.config_utils import load_config, verify_cloudinary_url, update_config, remove_config_keys, \ - show_cloudinary_config +from cloudinary_cli.defaults import logger, DEFAULT_CONFIG_KEY +from cloudinary_cli.utils.config_utils import ( + load_config, + verify_cloudinary_url, + update_config, + remove_config_keys, + show_cloudinary_config, + is_valid_cloudinary_config, + user_config_names, + get_default_config_name, + set_default_config, + clear_default_config, + is_reserved_config_name, + config_type, +) +from cloudinary_cli.utils.utils import ConfigurationError +from cloudinary_cli.utils.json_utils import print_json +from cloudinary_cli.utils.config_resolver import active_config_name, active_config_is_url +from cloudinary_cli.auth import refresh_config, refresh_configs, relogin_command +from cloudinary_cli.utils.config_listing import ( + list_configs, + render_config_table, + config_meta, + active_config_meta, + config_type_label, + SYNTHETIC_NAMES, +) @command("config", help="Display the current configuration, and manage additional configurations.") @option("-n", "--new", help="""\b Create and name a configuration from a Cloudinary account environment variable. e.g. cld config -n """, nargs=2) @option("-ls", "--ls", help="List all saved configurations.", is_flag=True) +@option("-j", "--json", "as_json", + help="Output as JSON (with -ls, -s, or the bare config view).", is_flag=True) @option("-s", "--show", help="Show details of a specified configuration.", nargs=1) @option("-rm", "--rm", help="Delete a specified configuration.", nargs=1) @option("-url", "--from_url", help="Create a configuration from a Cloudinary account environment variable. " "The configuration name is the cloud name.", nargs=1) -def config(new, ls, show, rm, from_url): +@option("-d", "--default", "default", nargs=1, + help="Set the named saved configuration as the default.") +@option("--set-default", "set_default", is_flag=True, + help="Set the configuration created by this command (-n / --from_url) as the default.") +@option("-ud", "--unset-default", "unset_default", is_flag=True, + help="Clear the stored default configuration.") +@option("-r", "--refresh", "refresh", nargs=1, + help="Refresh the OAuth token of a saved configuration (use the active config if no name).", + is_flag=False, flag_value="") +@option("-ra", "--refresh-all", "refresh_all", is_flag=True, + help="Refresh every saved OAuth configuration whose token is stale.") +@option("-f", "--force", "force", is_flag=True, + help="With --refresh/--refresh-all, refresh even tokens that are still fresh.") +def config_command(new, ls, as_json, show, rm, from_url, default, set_default, unset_default, + refresh, refresh_all, force): + if set_default and not (new or from_url): + raise UsageError("--set-default requires -n or --from_url; " + "to default an existing config use -d .") + + if force and refresh is None and not refresh_all: + raise UsageError("--force only applies to --refresh or --refresh-all.") + + if refresh_all: + return _refresh_all(force) + if refresh is not None: + return _refresh_one(refresh, force) + if new or from_url: config_name, cloudinary_url = new or [None, from_url] + if config_name and is_reserved_config_name(config_name): + raise BadParameter(f"'{config_name}' is a reserved configuration name.") + if not verify_cloudinary_url(cloudinary_url): return False @@ -29,22 +84,111 @@ def config(new, ls, show, rm, from_url): logger.info("Config '{}' saved!".format(config_name)) logger.info("Example usage: cld -C {} ".format(config_name)) + + if set_default: + set_default_config(config_name) + logger.info(f"Default set to '{config_name}'.") + elif default: + if default not in user_config_names(load_config()): + raise BadParameter(f"Configuration {default} does not exist, " + f"use -ls to list available configurations.") + set_default_config(default) + logger.info(f"Default set to '{default}'.") + elif unset_default: + clear_default_config() + logger.info("Default configuration cleared.") elif rm: if remove_config_keys(rm): logger.warning(f"Configuration '{rm}' not found.") else: + if get_default_config_name() == rm: + clear_default_config() logger.info(f"Configuration '{rm}' deleted.") elif ls: - echo("\n".join(load_config().keys())) + rows = list_configs() + if as_json: + print_json(rows) + else: + echo(render_config_table(rows)) elif show: curr_config = load_config() - if show not in curr_config: + if show not in user_config_names(curr_config): raise BadParameter(f"Configuration {show} does not exist, use -ls to list available configurations.") config_obj = cloudinary.Config() # noinspection PyProtectedMember - config_obj._setup_from_parsed_url(config_obj._parse_cloudinary_url(load_config()[show])) + config_obj._setup_from_parsed_url(config_obj._parse_cloudinary_url(curr_config[show])) + + if as_json: + return print_json(config_meta(show, curr_config, config_obj)) + _show_config_header(show, curr_config) return show_cloudinary_config(config_obj) else: + if not is_valid_cloudinary_config(): + raise ConfigurationError("No Cloudinary configuration found.") + if as_json: + return print_json(active_config_meta(cloudinary.config())) + _show_active_header() return show_cloudinary_config(cloudinary.config()) + + +_REFRESH_MESSAGES = { + "not_oauth": ("info", "'{name}' is an api-key config; nothing to refresh."), + "fresh": ("info", "'{name}' token is still fresh; nothing to refresh (use --force to refresh anyway)."), + "refreshed": ("info", "Refreshed '{name}'."), + "failed": ("error", "Could not refresh '{name}'. Please re-login with `{relogin}`."), +} + + +def _report_refresh(name, outcome): + """Log the outcome of a single refresh. Returns True on success (or a benign no-op).""" + level, template = _REFRESH_MESSAGES[outcome] + # The re-login hint must carry the config's region so the right OAuth host is used. + relogin = relogin_command(name) if outcome == "failed" else None + getattr(logger, level)(template.format(name=name, relogin=relogin)) + return outcome != "failed" + + +def _refresh_one(name, force): + name = name or active_config_name() + if not name: + raise UsageError("No active saved configuration to refresh; pass a name: " + "cld config --refresh .") + outcome = refresh_config(name, force=force) + if outcome == "not_found": + raise BadParameter(f"Configuration {name} does not exist, use -ls to list available configurations.") + return _report_refresh(name, outcome) + + +def _refresh_all(force): + results = refresh_configs(force=force) + if not results: + logger.info("No saved OAuth configurations to refresh.") + return True + ok = True + for name, outcome in results.items(): + ok = _report_refresh(name, outcome) and ok + return ok + + +def _show_config_header(name, cfg): + flags = [] + if cfg.get(DEFAULT_CONFIG_KEY) == name: + flags.append("default") + if active_config_name() == name: + flags.append("active") + suffix = f" [{', '.join(flags)}]" if flags else "" + echo(f"name: {name} ({config_type(cfg[name])}){suffix}\n") + + +def _show_active_header(): + """Header for bare `cld config`: identify the active config (saved name, -c URL, or env).""" + name = active_config_name() + if name is not None: + _show_config_header(name, load_config()) + return + active = cloudinary.config() + type_label = config_type_label(active) + label = SYNTHETIC_NAMES["url"] if active_config_is_url() else SYNTHETIC_NAMES["env"] + echo(f"name: {label} ({type_label}) [active]\n") diff --git a/cloudinary_cli/defaults.py b/cloudinary_cli/defaults.py index eb41905..5dcfc6d 100644 --- a/cloudinary_cli/defaults.py +++ b/cloudinary_cli/defaults.py @@ -24,6 +24,69 @@ CLOUDINARY_CLI_CONFIG_FILE = abspath(path_join(CLOUDINARY_HOME, 'config.json')) +# Reserved key inside config.json that names the default saved configuration. Double-underscore +# names are rejected as user config names, so this can't collide with a saved config. +DEFAULT_CONFIG_KEY = "__default__" + +# OAuth configuration for `cld login`. The region string derives both the API and +# OAuth hosts; an unknown region simply fails to resolve. +DEFAULT_REGION = 'api' + + +def normalize_region(region): + # Bare geo codes ('eu') become 'api-'; 'api' and 'api-*' pass through. + region = (region or DEFAULT_REGION).strip() + return region if region.startswith('api') else f'api-{region}' + + +def _oauth_host_for(region): + # Short suffixes (geo codes) use the central authz server; longer ones route to oauth-. + _, _, suffix = region.partition('-') + return 'oauth.cloudinary.com' if len(suffix) <= 2 else f'oauth-{suffix}.cloudinary.com' + + +def api_host_for_region(region): + return f'https://{normalize_region(region)}.cloudinary.com' + + +def oauth_base_url_for_region(region): + return f'https://{_oauth_host_for(normalize_region(region))}' + + +def oauth_authorize_url_for_region(region): + return f'{oauth_base_url_for_region(region)}/oauth2/auth' + + +def oauth_token_url_for_region(region): + return f'{oauth_base_url_for_region(region)}/oauth2/token' + + +def oauth_revoke_url_for_region(region): + return f'{oauth_base_url_for_region(region)}/oauth2/revoke' + + +CLOUDINARY_REGION = normalize_region(os.environ.get('CLOUDINARY_REGION')) + +# Public PKCE client (no secret). Overridable for testing against a non-prod authorization server +# registered with a different client; production uses the single registered client below. +OAUTH_DEFAULT_CLIENT_ID = 'a920ea9c-531b-4613-9783-1d4f4cc10655' +OAUTH_CLIENT_ID = os.environ.get('CLOUDINARY_OAUTH_CLIENT_ID', OAUTH_DEFAULT_CLIENT_ID) +OAUTH_DEFAULT_SCOPES = 'openid offline_access asset_management upload' +OAUTH_SCOPES = os.environ.get('CLOUDINARY_OAUTH_SCOPES', OAUTH_DEFAULT_SCOPES) + +# The authorization server requires an exact redirect match, so the port is fixed and must match the registered client. +OAUTH_DEFAULT_REDIRECT_HOST = '127.0.0.1' +OAUTH_REDIRECT_HOST = os.environ.get('CLOUDINARY_OAUTH_REDIRECT_HOST', OAUTH_DEFAULT_REDIRECT_HOST) +OAUTH_DEFAULT_REDIRECT_PORT = 49421 +OAUTH_REDIRECT_PORT = int(os.environ.get('CLOUDINARY_OAUTH_REDIRECT_PORT', OAUTH_DEFAULT_REDIRECT_PORT)) +OAUTH_CALLBACK_PATH = '/callback' + +OAUTH_CALLBACK_TIMEOUT_SECONDS = 300 +OAUTH_EXPIRY_SKEW_SECONDS = 30 +OAUTH_HTTP_TIMEOUT_SECONDS = 30 +# Fallback when the token response omits expires_in, so it can't pin expires_at to "now". +OAUTH_FALLBACK_EXPIRES_IN_SECONDS = 3600 + TEMPLATE_FOLDER_NAME = 'templates' CLOUDINARY_CLI_ROOT = dirname(__file__) TEMPLATE_FOLDER = path_join(CLOUDINARY_CLI_ROOT, TEMPLATE_FOLDER_NAME) diff --git a/cloudinary_cli/modules/clone.py b/cloudinary_cli/modules/clone.py index 44bc856..c93b8d1 100644 --- a/cloudinary_cli/modules/clone.py +++ b/cloudinary_cli/modules/clone.py @@ -4,7 +4,7 @@ from cloudinary.auth_token import _digest from cloudinary_cli.utils.utils import run_tasks_concurrently from cloudinary_cli.utils.api_utils import upload_file -from cloudinary_cli.utils.config_utils import get_cloudinary_config, config_to_dict +from cloudinary_cli.utils.config_resolver import get_cloudinary_config, config_to_api_kwargs from cloudinary_cli.defaults import logger from cloudinary_cli.core.search import execute_single_request, handle_auto_pagination import time @@ -115,7 +115,7 @@ def _prepare_upload_list(source_assets, target_config, overwrite, async_, notification_url, auth_token, url_expiry, normalize_list_params(fields)) - updated_options.update(config_to_dict(target_config)) + updated_options.update(config_to_api_kwargs(target_config)) upload_list.append((asset_url, {**updated_options})) return upload_list diff --git a/cloudinary_cli/modules/sync.py b/cloudinary_cli/modules/sync.py index 8e82c82..483ac4c 100644 --- a/cloudinary_cli/modules/sync.py +++ b/cloudinary_cli/modules/sync.py @@ -336,7 +336,7 @@ def _save_sync_meta_file(self, upload_results): current_diverse_files.update(diverse_filenames) try: logger.debug(f"Updating '{self.sync_meta_file}' file") - write_json_to_file(current_diverse_files, self.sync_meta_file) + write_json_to_file(current_diverse_files, self.sync_meta_file, atomic=True) logger.debug(f"Updated '{self.sync_meta_file}' file") except Exception as e: # Meta file is not critical for the sync itself, in case we cannot write it, we just log a warning diff --git a/cloudinary_cli/utils/config_listing.py b/cloudinary_cli/utils/config_listing.py new file mode 100644 index 0000000..1c138e7 --- /dev/null +++ b/cloudinary_cli/utils/config_listing.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +"""Presentation of the saved-config inventory: the rows behind `config -ls`, the table renderer, +and the per-config metadata used for `config`/`config -s` (text headers and JSON).""" +import cloudinary + +from cloudinary_cli.auth.oauth_config import OAuthConfig +from cloudinary_cli.defaults import DEFAULT_CONFIG_KEY +from cloudinary_cli.utils.config_utils import ( + load_config, + user_config_names, + cloud_name_from_url, + config_type, + cloudinary_config_details, + is_env_configured, +) +from cloudinary_cli.utils.config_resolver import ( + active_config_name, + active_config_is_env, + active_config_is_url, +) + +# Display names for the synthetic (non-saved) configs. Parenthesized so they read as a source +# label, not a saved config name, in both the table and JSON. +SYNTHETIC_NAMES = {"env": "(environment)", "url": "(command-line)"} + + +def config_type_label(config_obj): + """oauth/api_key for a config OBJECT. Every active config the CLI installs is an OAuthConfig, so + presence is read via has_oauth (refresh-free). (config_utils.config_type classifies a URL str.)""" + return "oauth" if config_obj.has_oauth else "api_key" + +_TABLE_COLUMNS = [("name", "NAME"), ("cloud_name", "CLOUD"), ("type", "TYPE"), + ("default", "DEFAULT"), ("active", "ACTIVE")] + + +def list_configs(): + cfg = load_config() + # "default" is the persistent user choice (-d); "active" is the config this very invocation + # resolved to (honoring -c/-C/default/env precedence), as recorded by the resolver. + default = cfg.get(DEFAULT_CONFIG_KEY) + active_name = active_config_name() + + rows = [] + if active_config_is_url(): + rows.append(_url_row()) # an inline -c URL: not a saved config, but it is what's active now + if is_env_configured(): + rows.append(_env_row(env_active=active_config_is_env())) + rows += [ + { + "name": name, + "cloud_name": cloud_name_from_url(cfg[name]), + "type": config_type(cfg[name]), + "source": "saved", + "default": name == default, + "active": name == active_name, + } + for name in user_config_names(cfg) + ] + return rows + + +def config_meta(name, cfg, config_obj): + """JSON view of a named saved config: header metadata plus the masked detail fields.""" + return { + "name": name, + "source": "saved", + "type": config_type(cfg[name]), + "default": cfg.get(DEFAULT_CONFIG_KEY) == name, + "active": active_config_name() == name, + **cloudinary_config_details(config_obj), + } + + +def active_config_meta(config_obj): + """JSON view of the active config for bare `cld config` (saved name, -c URL, or env).""" + name = active_config_name() + if name is not None: + return config_meta(name, load_config(), config_obj) + source = "url" if active_config_is_url() else "env" + return { + "name": SYNTHETIC_NAMES[source], + "source": source, + "type": config_type_label(config_obj), + "default": False, + "active": True, + **cloudinary_config_details(config_obj), + } + + +def render_config_table(rows): + headers = [title for _, title in _TABLE_COLUMNS] + cells = [[_cell(row, key) for key, _ in _TABLE_COLUMNS] for row in rows] + widths = [max(len(headers[i]), *(len(r[i]) for r in cells)) if cells else len(headers[i]) + for i in range(len(headers))] + line = lambda values: " ".join(v.ljust(widths[i]) for i, v in enumerate(values)).rstrip() + return "\n".join([line(headers)] + [line(r) for r in cells]) + + +def _url_row(): + active = cloudinary.config() # the CLI global, which the resolver loaded from the -c URL + return { + "name": SYNTHETIC_NAMES["url"], + "cloud_name": active.cloud_name or "", + "type": config_type_label(active), + "source": "url", + "default": False, # an inline URL is never the stored default + "active": True, # it outranks everything else for this invocation + } + + +def _env_row(env_active): + env_config = OAuthConfig.from_env() # constructed fresh from the environment, not the CLI global + return { + "name": SYNTHETIC_NAMES["env"], + "cloud_name": env_config.cloud_name or "", + "type": config_type_label(env_config), + "source": "env", + "default": False, # the environment is never the *stored* default + "active": env_active, # active only when no stored default outranks it + } + + +def _cell(row, key): + if key in ("default", "active"): + return "*" if row[key] else "" + return str(row.get(key) or "") diff --git a/cloudinary_cli/utils/config_resolver.py b/cloudinary_cli/utils/config_resolver.py new file mode 100644 index 0000000..24fc308 --- /dev/null +++ b/cloudinary_cli/utils/config_resolver.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +import cloudinary +from click import UsageError + +from cloudinary_cli.auth import refresh_url_if_stale +from cloudinary_cli.auth.session import strip_oauth_internal_keys +from cloudinary_cli.defaults import logger, DEFAULT_CONFIG_KEY +from cloudinary_cli.utils.config_utils import ( + load_config, + config_to_dict, + ping_cloudinary, + refresh_cloudinary_config, + is_valid_cloudinary_config, + is_env_configured, + user_config_names, +) + +_UNCONFIGURED_MESSAGE = ( + "No Cloudinary configuration found.\n" + " - Log in with OAuth: cld login\n" + " - Add an API-key config: cld config -n " + "cloudinary://:@ --set-default\n" + " - Set an existing config\n" + " as the default: cld config -d " +) + +# What the last resolve_cli_config selected, by precedence. One of: +# "url" -> an inline -c CLOUDINARY_URL +# "env" -> the environment fallback +# None -> nothing configured +# plus _active_name, the saved-config name when a -C/default saved entry was selected (else None), +# read by `config -ls` to mark the row active for this invocation. Token freshness is no longer +# handled here: a saved OAuth config installs a self-refreshing OAuthConfig that refreshes lazily +# when the SDK reads its oauth_token at request time. +_active_name = None +_active_source = None + + +def resolve_cli_config(config=None, config_saved=None): + """Select a config by precedence and load it into the SDK global. No network I/O.""" + global _active_name, _active_source + _active_name = None + _active_source = None + + if config and config_saved: + raise UsageError("-c/--config and -C/--config_saved are mutually exclusive; pass only one.") + + if config: + _active_source = "url" + refresh_cloudinary_config(config) + return _format_ok() + + cfg = load_config() + + if config_saved: + if config_saved not in user_config_names(cfg): + raise Exception(f"Config {config_saved} does not exist") + _active_name = config_saved + _active_source = "saved" + refresh_cloudinary_config(cfg[config_saved], saved_name=config_saved) + return _format_ok() + + default = cfg.get(DEFAULT_CONFIG_KEY) + if default and default in cfg: + _active_name = default + _active_source = "saved" + refresh_cloudinary_config(cfg[default], saved_name=default) + return _format_ok() + + # No stored default: fall back to the environment. Install it as an OAuthConfig (static, no + # saved name -> never refreshes) so the active global is always an OAuthConfig and exposes + # has_oauth uniformly; if nothing is configured, _format_ok warns. + if is_env_configured(): + _active_source = "env" + from cloudinary_cli.auth.oauth_config import install_env_config + install_env_config() + return _format_ok() + + +def active_config_name(): + """The saved-config name selected by the last resolution, or None for -c/env/unconfigured.""" + return _active_name + + +def active_config_is_env(): + """True when the last resolution fell through to the environment fallback.""" + return _active_source == "env" + + +def active_config_is_url(): + """True when the last resolution loaded an inline -c CLOUDINARY_URL.""" + return _active_source == "url" + + +def _format_ok(): + """Format-only check: is a usable-SHAPED config loaded? Does NOT contact the network.""" + if not is_valid_cloudinary_config(): + logger.warning(_UNCONFIGURED_MESSAGE) + return False + return True + + +def get_cloudinary_config(target): + target_config = cloudinary.Config() + if target.startswith("cloudinary://"): + parsed_url = target_config._parse_cloudinary_url(target) + elif target in load_config(): + url = refresh_url_if_stale(target, load_config().get(target)) + parsed_url = target_config._parse_cloudinary_url(url) + else: + return False + + target_config._setup_from_parsed_url(parsed_url) + + if not ping_cloudinary(**config_to_api_kwargs(target_config)): + logger.error(f"Invalid Cloudinary config: {target}") + return False + + return target_config + + +def config_to_api_kwargs(config): + return strip_oauth_internal_keys(config_to_dict(config)) diff --git a/cloudinary_cli/utils/config_utils.py b/cloudinary_cli/utils/config_utils.py index 7b5732a..b47b786 100644 --- a/cloudinary_cli/utils/config_utils.py +++ b/cloudinary_cli/utils/config_utils.py @@ -1,45 +1,123 @@ #!/usr/bin/env python3 import os +import re +import time +from datetime import datetime, timezone import cloudinary from click import echo from cloudinary import api - -from cloudinary_cli.defaults import CLOUDINARY_CLI_CONFIG_FILE, OLD_CLOUDINARY_CLI_CONFIG_FILE, logger +from filelock import FileLock + +from cloudinary_cli.defaults import ( + CLOUDINARY_CLI_CONFIG_FILE, + OLD_CLOUDINARY_CLI_CONFIG_FILE, + DEFAULT_CONFIG_KEY, + logger, +) from cloudinary_cli.utils.json_utils import write_json_to_file, read_json_from_file from cloudinary_cli.utils.utils import log_exception +# Cross-process lock guarding read-modify-write of the config file. Reentrant within a process, +# so callers may hold it across a multi-step update (e.g. token refresh) without deadlocking. +_config_lock = FileLock(CLOUDINARY_CLI_CONFIG_FILE + ".lock") + + +def config_lock(): + # The lock file lives in the config dir, which may not exist yet on a fresh install. + _verify_file_path(CLOUDINARY_CLI_CONFIG_FILE) + return _config_lock + + +# Parsed-config cache keyed on the file's (mtime_ns, size). The config file is read on nearly every +# code path; caching skips the re-read + JSON parse when it has not changed on disk (including +# changes written by a peer process, which os.replace stamps with a new mtime). +_config_cache = None +_config_cache_stat = None + + +def _config_stat(): + try: + st = os.stat(CLOUDINARY_CLI_CONFIG_FILE) + return st.st_mtime_ns, st.st_size + except FileNotFoundError: + return None + + +def _invalidate_config_cache(): + global _config_cache, _config_cache_stat + _config_cache = None + _config_cache_stat = None + def load_config(): - return read_json_from_file(CLOUDINARY_CLI_CONFIG_FILE, does_not_exist_ok=True) + global _config_cache, _config_cache_stat + stat = _config_stat() + if stat is not None and stat == _config_cache_stat and _config_cache is not None: + return dict(_config_cache) # copy: callers mutate the result in place (e.g. cfg.update(...)) + cfg = read_json_from_file(CLOUDINARY_CLI_CONFIG_FILE, does_not_exist_ok=True) + _config_cache, _config_cache_stat = cfg, stat + return dict(cfg) def save_config(config): + # 0600 from the start: the config file holds secrets (api_secret, account_url, OAuth tokens), + # and writing the temp file 0600 before the atomic replace means it is never momentarily + # world-readable (unlike a chmod applied after the replace). _verify_file_path(CLOUDINARY_CLI_CONFIG_FILE) - write_json_to_file(config, CLOUDINARY_CLI_CONFIG_FILE) + write_json_to_file(config, CLOUDINARY_CLI_CONFIG_FILE, atomic=True, mode=0o600) + _invalidate_config_cache() # next load_config re-stats and reloads our own write def update_config(new_config): - curr_config = load_config() - curr_config.update(new_config) - save_config(curr_config) + with config_lock(): + curr_config = load_config() + curr_config.update(new_config) + save_config(curr_config) def remove_config_keys(*keys): - curr_config = load_config() - not_found = [] - for key in keys: - if not curr_config.pop(key, None): - not_found.append(key) + with config_lock(): + curr_config = load_config() + not_found = [] + for key in keys: + if not curr_config.pop(key, None): + not_found.append(key) - save_config(curr_config) + save_config(curr_config) return not_found -def refresh_cloudinary_config(cloudinary_url): - os.environ.update({'CLOUDINARY_URL': cloudinary_url}) - cloudinary.reset_config() +def get_default_config_name(): + """Return the stored default config name, or None if none is set.""" + return load_config().get(DEFAULT_CONFIG_KEY) + + +def set_default_config(name): + update_config({DEFAULT_CONFIG_KEY: name}) + + +def clear_default_config(): + remove_config_keys(DEFAULT_CONFIG_KEY) + + +def user_config_names(cfg=None): + """Saved config names with the reserved default key filtered out.""" + cfg = cfg if cfg is not None else load_config() + return [k for k in cfg if k != DEFAULT_CONFIG_KEY] + + +def is_reserved_config_name(name): + """Names wrapped in double underscores are reserved for internal keys (e.g. the default).""" + return name.startswith("__") and name.endswith("__") + + +def refresh_cloudinary_config(cloudinary_url, saved_name=None): + """Install cloudinary_url as the active config. OAuth URLs install a self-refreshing config + bound to saved_name (so token rotations persist); other URLs use the plain SDK config.""" + from cloudinary_cli.auth.oauth_config import install_oauth_config + install_oauth_config(cloudinary_url, saved_name=saved_name) def verify_cloudinary_url(cloudinary_url): @@ -47,42 +125,171 @@ def verify_cloudinary_url(cloudinary_url): return ping_cloudinary() -def get_cloudinary_config(target): - target_config = cloudinary.Config() - if target.startswith("cloudinary://"): - parsed_url = target_config._parse_cloudinary_url(target) - elif target in load_config(): - parsed_url = target_config._parse_cloudinary_url(load_config().get(target)) - else: - return False +def config_to_dict(config): + return {k: v for k, v in config.__dict__.items() if not k.startswith("_")} - target_config._setup_from_parsed_url(parsed_url) - if not ping_cloudinary(**config_to_dict(target_config)): - logger.error(f"Invalid Cloudinary config: {target}") - return False +def cloud_name_from_url(url): + """Parse a saved cloudinary:// URL and return its cloud name, or "" if it cannot be parsed.""" + config_obj = cloudinary.Config() + try: + # noinspection PyProtectedMember + config_obj._setup_from_parsed_url(config_obj._parse_cloudinary_url(url)) + except Exception: + return "" + return config_obj.cloud_name or "" - return target_config -def config_to_dict(config): - return {k: v for k, v in config.__dict__.items() if not k.startswith("_")} +def config_type(url): + """Classify a saved config URL as "oauth" or "api_key".""" + from cloudinary_cli.auth.session import is_oauth_url + return "oauth" if is_oauth_url(url) else "api_key" + + +_SECRET_KEYS = {"api_secret", "oauth_token", "refresh_token"} +_URL_SECRET_KEYS = {"account_url"} +# Fixed mask width so a long secret (e.g. an OAuth JWT) does not print a wall of asterisks and the +# real length is not leaked. The last 4 chars are kept as a fingerprint to identify the value. +_MASK_PREFIX = "****" + + +def _mask_secret(value): + value = str(value) + return _MASK_PREFIX + value[-4:] if len(value) > 4 else "*" * len(value) + + +def _mask_url_secret(url): + # Mask the password between `:` and `@` in scheme://user:secret@host. + return re.sub(r'(://[^:/?#]+:)([^@]+)(@)', + lambda m: m.group(1) + _mask_secret(m.group(2)) + m.group(3), str(url)) + + +# account://:@ +_ACCOUNT_URL_RE = re.compile(r'^account://([^:/?#]+):([^@]+)@(.+)$') + + +def _format_account_url(url): + """Render the provisioning account URL as a labeled, secret-masked block (or None if unparsable).""" + match = _ACCOUNT_URL_RE.match(str(url)) + if not match: + return None + api_key, api_secret, account_id = match.groups() + fields = { + "account_id": account_id, + "provisioning_api_key": api_key, + "provisioning_api_secret": _mask_secret(api_secret), + } + width = len(max(fields, key=len)) + 1 + template = "{0:" + str(width) + "} {1}" + return "\n".join(template.format(f"{k}:", v) for k, v in fields.items()) + + +def _format_expires_at(value): + # OAuth token expiry: show the raw epoch plus a human-readable UTC time and live/expired state. + try: + epoch = int(value) + except (TypeError, ValueError): + return value + human = datetime.fromtimestamp(epoch, tz=timezone.utc).strftime("%Y-%m-%d %H:%M:%S UTC") + state = "expired" if epoch <= int(time.time()) else "valid" + return f"{epoch} ({human}, {state})" + def show_cloudinary_config(cloudinary_config): obfuscated_config = config_to_dict(cloudinary_config) - if "api_secret" in obfuscated_config: - api_secret = obfuscated_config["api_secret"] - obfuscated_config["api_secret"] = "*" * (len(api_secret) - 4) + f"{api_secret[-4:]}" - # omit default signature algorithm if obfuscated_config.get("signature_algorithm", None) == cloudinary.utils.SIGNATURE_SHA1: obfuscated_config.pop("signature_algorithm") - if not obfuscated_config: + # The account URL is long and structurally distinct, so it is shown in its own section below. + account_url = obfuscated_config.pop("account_url", None) + + obfuscated_config = { + key: _display_value(key, value) + for key, value in obfuscated_config.items() + if value not in (None, "") # drop empty/None fields (e.g. api_key on an OAuth config) + } + + if not obfuscated_config and not account_url: return False - template = "{0:" + str(len(max(obfuscated_config, key=len)) + 1) + "} {1}" # Gets the maximal length of the keys. - echo('\n'.join([template.format(f"{k}:", v) for k, v in obfuscated_config.items()])) + if obfuscated_config: + width = len(max(obfuscated_config, key=len)) + 1 + template = "{0:" + str(width) + "} {1}" + echo('\n'.join([template.format(f"{k}:", v) for k, v in obfuscated_config.items()])) + + if account_url: + structured = _format_account_url(account_url) + if structured is not None: + echo(f"\nAccount (provisioning) API:\n{structured}") + else: + echo(f"\naccount_url: {_mask_url_secret(account_url)}") + + +def cloudinary_config_details(cloudinary_config): + """ + JSON-friendly, secret-masked view of a Cloudinary config: the same fields shown by + show_cloudinary_config, with secrets masked, empties dropped, expires_at expanded into a + structured object, and account_url decomposed into a nested `account` object. + """ + raw = config_to_dict(cloudinary_config) + + if raw.get("signature_algorithm", None) == cloudinary.utils.SIGNATURE_SHA1: + raw.pop("signature_algorithm") + + account_url = raw.pop("account_url", None) + + details = {} + for key, value in raw.items(): + if value in (None, ""): + continue + if key in _SECRET_KEYS: + details[key] = _mask_secret(value) + elif key == "expires_at": + details[key] = _expires_at_details(value) + else: + details[key] = value + + account = _account_url_details(account_url) if account_url else None + if account is not None: + details["account"] = account + elif account_url: + details["account_url"] = _mask_url_secret(account_url) + + return details + + +def _expires_at_details(value): + try: + epoch = int(value) + except (TypeError, ValueError): + return value + return { + "epoch": epoch, + "utc": datetime.fromtimestamp(epoch, tz=timezone.utc).strftime("%Y-%m-%d %H:%M:%S UTC"), + "expired": epoch <= int(time.time()), + } + + +def _account_url_details(url): + match = _ACCOUNT_URL_RE.match(str(url)) + if not match: + return None + api_key, api_secret, account_id = match.groups() + return { + "account_id": account_id, + "provisioning_api_key": api_key, + "provisioning_api_secret": _mask_secret(api_secret), + } + + +def _display_value(key, value): + if key in _SECRET_KEYS: + return _mask_secret(value) + if key == "expires_at": + return _format_expires_at(value) + return value def migrate_old_config(): @@ -105,7 +312,17 @@ def migrate_old_config(): def is_valid_cloudinary_config(): - return None not in [cloudinary.config().cloud_name, cloudinary.config().api_key, cloudinary.config().api_secret] + config = cloudinary.config() + # has_oauth reports token presence without triggering OAuthConfig's refresh-on-read. Fall back + # to a refresh-free __dict__ read for a plain SDK Config (e.g. before any config is installed). + has_oauth = config.has_oauth if hasattr(config, "has_oauth") else bool(config.__dict__.get("oauth_token")) + if config.cloud_name and has_oauth: + return True + return None not in [config.cloud_name, config.api_key, config.api_secret] + + +def is_env_configured(): + return bool(cloudinary.Config().cloud_name) def initialize(): diff --git a/cloudinary_cli/utils/file_utils.py b/cloudinary_cli/utils/file_utils.py index 561d686..a13a62e 100644 --- a/cloudinary_cli/utils/file_utils.py +++ b/cloudinary_cli/utils/file_utils.py @@ -1,5 +1,6 @@ import os import stat +import tempfile from os import walk, path, listdir, rmdir, sep from os.path import split, relpath, abspath from pathlib import PurePath @@ -39,6 +40,49 @@ } +def atomic_write(filename, write_fn, mode=None): + """ + Writes via a temp file in the same directory, then atomically replaces the target, so a + concurrent reader never sees a half-written file and an interleaved write can't truncate it. + + :param filename: The destination file path. + :param write_fn: Callable receiving the open temp file object; performs the actual write. + :param mode: Final permission bits to set on the file. When given, the temp file is set to + this mode before the replace, so the destination is never momentarily wider + (mkstemp creates it 0600, so a secret file is never world-readable mid-write). + When omitted, normalize to the process umask default like a plain open(). + """ + directory = path.dirname(filename) or "." + fd, tmp_path = tempfile.mkstemp(dir=directory, prefix=".tmp-") + try: + with os.fdopen(fd, 'w') as file: + write_fn(file) + if mode is not None: + os.chmod(tmp_path, mode) + else: + _apply_umask_permissions(tmp_path) + os.replace(tmp_path, filename) + except BaseException: + try: + os.remove(tmp_path) + except OSError: + pass + raise + + +def _apply_umask_permissions(file): + # mkstemp creates the temp file as 0600, and os.replace preserves that mode onto the + # destination. Normalize to the process umask default so output files keep the same + # permissions a plain open() would have produced; callers needing 0600 (e.g. the config + # file) tighten it explicitly afterwards. + current_umask = os.umask(0) + os.umask(current_umask) + try: + os.chmod(file, 0o666 & ~current_umask) + except OSError as e: + logger.debug(f"Could not normalize permissions on {file}: {e}") + + def walk_dir(root_dir, include_hidden=False): all_files = {} for root, dirs, files in walk(root_dir): diff --git a/cloudinary_cli/utils/json_utils.py b/cloudinary_cli/utils/json_utils.py index 7f90869..b976c34 100644 --- a/cloudinary_cli/utils/json_utils.py +++ b/cloudinary_cli/utils/json_utils.py @@ -1,9 +1,11 @@ import json -from platform import system +import sys from os import path import click from pygments import highlight, lexers, formatters +from cloudinary_cli.utils.file_utils import atomic_write + def read_json_from_file(filename, does_not_exist_ok=False): if does_not_exist_ok and (not path.exists(filename) or path.getsize(filename) < 1): @@ -13,21 +15,29 @@ def read_json_from_file(filename, does_not_exist_ok=False): return json.loads(file.read() or "{}") -def write_json_to_file(json_obj, filename, indent=2, sort_keys=False): - with open(filename, 'w') as file: +def write_json_to_file(json_obj, filename, indent=2, sort_keys=False, atomic=False, mode=None): + def dump(file): json.dump(json_obj, file, indent=indent, sort_keys=sort_keys) + if atomic: + atomic_write(filename, dump, mode=mode) + else: + with open(filename, 'w') as file: + dump(file) + -def update_json_file(json_obj, filename, indent=2, sort_keys=False): +def update_json_file(json_obj, filename, indent=2, sort_keys=False, atomic=False): curr_obj = read_json_from_file(filename, True) curr_obj.update(json_obj) - write_json_to_file(curr_obj, filename, indent, sort_keys) + write_json_to_file(curr_obj, filename, indent, sort_keys, atomic) def print_json(res): res_str = json.dumps(res, indent=2) - if system() != "Windows": + # Colorize only for an interactive terminal. When stdout is piped/redirected/captured (e.g. an + # LLM agent or `| jq`), emit plain JSON so ANSI escapes never corrupt the parsed output. + if sys.stdout.isatty(): res_str = highlight(res_str.encode('UTF-8'), lexers.JsonLexer(), formatters.TerminalFormatter()).strip() click.echo(res_str) diff --git a/cloudinary_cli/utils/utils.py b/cloudinary_cli/utils/utils.py index e0c69a4..8d282a2 100644 --- a/cloudinary_cli/utils/utils.py +++ b/cloudinary_cli/utils/utils.py @@ -2,6 +2,7 @@ import builtins import json import os +import sys from collections import OrderedDict from csv import DictWriter from functools import reduce @@ -198,6 +199,26 @@ def run_tasks_concurrently(func, tasks, concurrent_workers): thread_pool.starmap(func, tasks) +def is_interactive(): + """True when we can prompt the user (stdin is an interactive terminal). The single home for the + interactivity check, so flow code never pokes sys.stdin directly.""" + return sys.stdin.isatty() + + +def prompt_user(message, noninteractive_hint=None): + """ + Read a line of user input. The single place that calls input(): returns None when no input can + be read (closed/non-interactive stdin), logging noninteractive_hint (if given) so the caller's + decision is never a silent no-op. + """ + try: + return input(message) + except EOFError: + if noninteractive_hint: + logger.warning(f"No input available (non-interactive terminal). {noninteractive_hint}") + return None + + def confirm_action(message="Continue? (y/N)"): """ Confirms whether the user wants to continue. @@ -208,10 +229,12 @@ def confirm_action(message="Continue? (y/N)"): :return: Boolean indicating whether user wants to continue. :rtype bool """ - return get_user_action(message, {"y": True, "default": False}) + return get_user_action( + message, {"y": True, "default": False}, + noninteractive_hint="Pass --force (-F) to skip this confirmation in non-interactive runs.") -def get_user_action(message, options): +def get_user_action(message, options, noninteractive_hint=None): """ Reads user input and returns value specified in options. @@ -222,10 +245,14 @@ def get_user_action(message, options): :type message: string :param options: Options mapping. :type options: dict + :param noninteractive_hint: Logged when no input can be read (closed/non-interactive stdin), to + point the user at the flag or piped input that replaces this prompt. The default option is + then applied. :return: Value according to the user selection. """ - r = input(message).lower() + r = prompt_user(message, noninteractive_hint) + r = r.lower() if r is not None else "" # no input -> apply the default option return options.get(r, options.get("default")) diff --git a/requirements.txt b/requirements.txt index a0ab04f..7576a44 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,7 @@ pygments jinja2 click click-log +filelock requests docstring-parser urllib3>=2.2.2 # not directly required, pinned by Snyk to avoid a vulnerability diff --git a/setup.py b/setup.py index d6f1c5a..b2644d7 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,7 @@ keywords='cloudinary cli pycloudinary image video digital asset management command line interface transformation ' 'friendly easy flexible', license="MIT", - python_requires='>=3.6.0', + python_requires='>=3.8.0', setup_requires=["pytest-runner"], tests_require=["pytest", "mock", "urllib3"], install_requires=requirements, diff --git a/test/helper_test.py b/test/helper_test.py index 0a3fb06..ce2f118 100644 --- a/test/helper_test.py +++ b/test/helper_test.py @@ -5,12 +5,18 @@ from functools import wraps from pathlib import Path +import cloudinary import cloudinary.api from cloudinary import logger from cloudinary_cli.utils.api_utils import query_cld_folder from urllib3 import HTTPResponse, disable_warnings from urllib3._collections import HTTPHeaderDict +# Many CLI tests mock the HTTP layer but still need a resolvable config to run; without one the +# command exits "No Cloudinary configuration found". Gate those tests on a config being present. +CONFIG_PRESENT = bool(cloudinary.config().cloud_name) +REQUIRES_CONFIG = "Requires a Cloudinary configuration (set CLOUDINARY_URL or a saved config)" + SUFFIX = os.environ.get('TRAVIS_JOB_ID') or random.randint(10000, 99999) RESOURCES_DIR = Path.joinpath(Path(__file__).resolve().parent, "resources") diff --git a/test/test_auth_flow.py b/test/test_auth_flow.py new file mode 100644 index 0000000..d84edf8 --- /dev/null +++ b/test/test_auth_flow.py @@ -0,0 +1,69 @@ +import base64 +import hashlib +import unittest +from unittest.mock import patch, MagicMock +from urllib.parse import urlparse, parse_qs + +from cloudinary_cli.auth import flow + + +class TestAuthFlow(unittest.TestCase): + def test_pkce_pair_s256_no_padding(self): + verifier, challenge = flow.generate_pkce_pair() + self.assertNotIn("=", verifier) + self.assertNotIn("=", challenge) + # challenge must be the S256 of the verifier + expected = base64.urlsafe_b64encode( + hashlib.sha256(verifier.encode("ascii")).digest()).rstrip(b"=").decode("ascii") + self.assertEqual(expected, challenge) + + def test_build_authorize_url(self): + url = flow.build_authorize_url("the_challenge", "the_state", "http://127.0.0.1:49421/callback", "api") + q = parse_qs(urlparse(url).query) + self.assertTrue(url.startswith("https://oauth.cloudinary.com/oauth2/auth?")) + self.assertEqual("code", q["response_type"][0]) + self.assertEqual("S256", q["code_challenge_method"][0]) + self.assertEqual("the_challenge", q["code_challenge"][0]) + self.assertEqual("the_state", q["state"][0]) + self.assertEqual("http://127.0.0.1:49421/callback", q["redirect_uri"][0]) + self.assertIn("client_id", q) + + def test_build_authorize_url_region_drives_host(self): + url = flow.build_authorize_url("c", "s", "http://127.0.0.1:49421/callback", "test") + self.assertTrue(url.startswith("https://oauth-test.cloudinary.com/oauth2/auth?")) + + def test_exchange_code_posts_pkce_no_secret(self): + resp = MagicMock() + resp.json.return_value = {"access_token": "tok"} + with patch("cloudinary_cli.auth.flow.requests.post", return_value=resp) as post: + flow.exchange_code("the_code", "the_verifier", "http://127.0.0.1:49421/callback", "test") + self.assertEqual("https://oauth-test.cloudinary.com/oauth2/token", post.call_args.args[0]) + data = post.call_args.kwargs["data"] + self.assertEqual("authorization_code", data["grant_type"]) + self.assertEqual("the_code", data["code"]) + self.assertEqual("the_verifier", data["code_verifier"]) + self.assertNotIn("client_secret", data) + self.assertIn("timeout", post.call_args.kwargs) + + def test_refresh_posts_refresh_token(self): + resp = MagicMock() + resp.json.return_value = {"access_token": "tok2"} + with patch("cloudinary_cli.auth.flow.requests.post", return_value=resp) as post: + flow.refresh("rt_abc", "api-eu") + self.assertEqual("https://oauth.cloudinary.com/oauth2/token", post.call_args.args[0]) + data = post.call_args.kwargs["data"] + self.assertEqual("refresh_token", data["grant_type"]) + self.assertEqual("rt_abc", data["refresh_token"]) + self.assertIn("timeout", post.call_args.kwargs) + + def test_revoke_posts_token_to_revoke_endpoint(self): + resp = MagicMock() + with patch("cloudinary_cli.auth.flow.requests.post", return_value=resp) as post: + flow.revoke("rt_abc", "api-eu") + self.assertEqual("https://oauth.cloudinary.com/oauth2/revoke", post.call_args.args[0]) + data = post.call_args.kwargs["data"] + self.assertEqual("rt_abc", data["token"]) + self.assertEqual("refresh_token", data["token_type_hint"]) + self.assertIn("client_id", data) + self.assertIn("timeout", post.call_args.kwargs) + resp.raise_for_status.assert_called_once() diff --git a/test/test_auth_loopback.py b/test/test_auth_loopback.py new file mode 100644 index 0000000..8985e0e --- /dev/null +++ b/test/test_auth_loopback.py @@ -0,0 +1,83 @@ +import threading +import unittest +import urllib.request +from http.server import HTTPServer +from unittest.mock import patch + +from cloudinary_cli.auth.loopback_server import _CallbackHandler, start_callback_server, wait_for_callback + + +class TestLoopbackServer(unittest.TestCase): + def setUp(self): + # Bind an OS-assigned port on loopback so tests don't collide with the real default. + self.httpd = HTTPServer(("127.0.0.1", 0), _CallbackHandler) + self.httpd.auth_code = self.httpd.auth_state = self.httpd.auth_error = None + self.httpd.timeout = 5 + self.port = self.httpd.server_address[1] + + def tearDown(self): + try: + self.httpd.server_close() + except Exception: + pass + + def _get(self, path): + try: + urllib.request.urlopen(f"http://127.0.0.1:{self.port}{path}", timeout=5).read() + except Exception: + pass + + def test_captures_code_and_state(self): + waiter = threading.Thread(target=lambda: setattr(self, "result", wait_for_callback(self.httpd))) + waiter.start() + self._get("/callback?code=the_code&state=the_state") + waiter.join(timeout=5) + self.assertEqual(("the_code", "the_state"), self.result) + + def test_ignores_favicon_then_captures(self): + waiter = threading.Thread(target=lambda: setattr(self, "result", wait_for_callback(self.httpd))) + waiter.start() + self._get("/favicon.ico") # must NOT end the wait + self._get("/callback?code=c2&state=s2") + waiter.join(timeout=5) + self.assertEqual(("c2", "s2"), self.result) + + def test_ignores_code_on_wrong_path_then_captures(self): + waiter = threading.Thread(target=lambda: setattr(self, "result", wait_for_callback(self.httpd))) + waiter.start() + self._get("/anything?code=stray&state=s") # wrong path must NOT end the wait + self._get("/callback?code=real&state=s3") + waiter.join(timeout=5) + self.assertEqual(("real", "s3"), self.result) + + def test_error_raises(self): + error = {} + + def run(): + try: + wait_for_callback(self.httpd) + except Exception as e: + error["e"] = e + + waiter = threading.Thread(target=run) + waiter.start() + self._get("/callback?error=access_denied") + waiter.join(timeout=5) + self.assertIsInstance(error.get("e"), RuntimeError) + self.assertIn("access_denied", str(error["e"])) + + +class TestStartCallbackServerPortBusy(unittest.TestCase): + """A2: a failed bind (e.g. busy redirect port) must surface a clear RuntimeError, not a raw + OSError. The bind is mocked to fail so the test is deterministic across OSes (Windows does not + raise on a double-bind the way POSIX does).""" + + def test_bind_failure_raises_friendly_error(self): + with patch("cloudinary_cli.auth.loopback_server.HTTPServer", + side_effect=OSError(48, "Address already in use")): + with self.assertRaises(RuntimeError) as ctx: + start_callback_server() + msg = str(ctx.exception) + self.assertIn("local login server", msg) + self.assertIn("in use", msg) + self.assertIsInstance(ctx.exception.__cause__, OSError) # chains the original diff --git a/test/test_auth_region.py b/test/test_auth_region.py new file mode 100644 index 0000000..c8ac3c7 --- /dev/null +++ b/test/test_auth_region.py @@ -0,0 +1,65 @@ +import importlib +import unittest +from unittest.mock import patch + +import cloudinary_cli.defaults as defaults +from cloudinary_cli.defaults import normalize_region, _oauth_host_for, api_host_for_region + + +class TestAuthRegion(unittest.TestCase): + def test_normalize_region(self): + self.assertEqual('api', normalize_region(None)) + self.assertEqual('api', normalize_region('')) + self.assertEqual('api', normalize_region('api')) + self.assertEqual('api-eu', normalize_region('eu')) + self.assertEqual('api-ap', normalize_region('ap')) + self.assertEqual('api-eu', normalize_region('api-eu')) + self.assertEqual('api-eu', normalize_region(' api-eu ')) + self.assertEqual('api-test', normalize_region('test')) + self.assertEqual('api-test', normalize_region('api-test')) + + def test_api_host_for_region(self): + self.assertEqual('https://api.cloudinary.com', api_host_for_region('api')) + self.assertEqual('https://api-eu.cloudinary.com', api_host_for_region('api-eu')) + # short codes are normalized first + self.assertEqual('https://api-ap.cloudinary.com', api_host_for_region('ap')) + self.assertEqual('https://api-test.cloudinary.com', api_host_for_region('test')) + self.assertEqual('https://api-test.cloudinary.com', api_host_for_region('api-test')) + + def test_oauth_host_central_for_geo_regions(self): + # <= 2-char suffixes (and bare 'api') use the central authz server + self.assertEqual('oauth.cloudinary.com', _oauth_host_for('api')) + self.assertEqual('oauth.cloudinary.com', _oauth_host_for('api-eu')) + self.assertEqual('oauth.cloudinary.com', _oauth_host_for('api-ap')) + + def test_oauth_host_dedicated_for_long_suffix(self): + # longer suffixes route to their own oauth- host + self.assertEqual('oauth-test.cloudinary.com', _oauth_host_for('api-test')) + + +class TestOAuthClientConfig(unittest.TestCase): + """OAUTH_CLIENT_ID / OAUTH_SCOPES are env-overridable (resolved at module import).""" + + def _reload(self, env): + # The values are read at import time, so reload defaults under the patched environment. + with patch.dict("os.environ", env, clear=False): + for key in ("CLOUDINARY_OAUTH_CLIENT_ID", "CLOUDINARY_OAUTH_SCOPES"): + if key not in env: + __import__("os").environ.pop(key, None) + return importlib.reload(defaults) + + def tearDown(self): + importlib.reload(defaults) # restore the unpatched module for other tests + + def test_defaults_when_unset(self): + d = self._reload({}) + self.assertEqual('a920ea9c-531b-4613-9783-1d4f4cc10655', d.OAUTH_CLIENT_ID) + self.assertEqual('openid offline_access asset_management upload', d.OAUTH_SCOPES) + + def test_client_id_override(self): + d = self._reload({"CLOUDINARY_OAUTH_CLIENT_ID": "non-prod-client"}) + self.assertEqual("non-prod-client", d.OAUTH_CLIENT_ID) + + def test_scopes_override(self): + d = self._reload({"CLOUDINARY_OAUTH_SCOPES": "openid upload"}) + self.assertEqual("openid upload", d.OAUTH_SCOPES) diff --git a/test/test_auth_session.py b/test/test_auth_session.py new file mode 100644 index 0000000..ee9a484 --- /dev/null +++ b/test/test_auth_session.py @@ -0,0 +1,328 @@ +import time +import unittest +from unittest import mock +from unittest.mock import patch + +import cloudinary + +from cloudinary_cli.auth import ( + login, + refresh_url_if_stale, + refresh_config, + refresh_configs, + _derive_config_name, +) +from cloudinary_cli.auth.session import ( + Session, + to_cloudinary_url, + from_cloudinary_url, + is_oauth_url, + strip_oauth_internal_keys, +) + + +def _session(**overrides): + base = dict(cloud_name="eu-cloud", access_token="eyJ.aaa.bbb", refresh_token="rt_123", + expires_at=int(time.time()) + 300, region="api-eu", + issuer="https://oauth.cloudinary.com/") + base.update(overrides) + return Session(**base) + + +class TestSessionCodec(unittest.TestCase): + def test_round_trip(self): + s = _session() + parsed = from_cloudinary_url(to_cloudinary_url(s)) + self.assertEqual(s.cloud_name, parsed.cloud_name) + self.assertEqual(s.access_token, parsed.access_token) + self.assertEqual(s.refresh_token, parsed.refresh_token) + self.assertEqual(s.region, parsed.region) + self.assertEqual(s.issuer, parsed.issuer) + self.assertEqual(s.expires_at, parsed.expires_at) + self.assertIsInstance(parsed.expires_at, int) + + def test_parses_through_sdk_as_bearer(self): + url = to_cloudinary_url(_session()) + config = cloudinary.Config() + config._setup_from_parsed_url(config._parse_cloudinary_url(url)) + self.assertEqual("eu-cloud", config.cloud_name) + self.assertEqual("eyJ.aaa.bbb", config.oauth_token) + self.assertIsNone(config.api_key) + self.assertIsNone(config.api_secret) + self.assertEqual("https://api-eu.cloudinary.com", config.upload_prefix) + + def test_is_oauth_url(self): + self.assertTrue(is_oauth_url(to_cloudinary_url(_session()))) + self.assertFalse(is_oauth_url("cloudinary://key:secret@cloud")) + self.assertFalse(is_oauth_url(None)) + # substring 'oauth_token' outside the query key must not match + self.assertFalse(is_oauth_url("cloudinary://key:secret@oauth_token.example.com")) + self.assertFalse(is_oauth_url("cloudinary://key:secret@cloud?cname=oauth_token.io")) + + def test_is_fresh(self): + self.assertTrue(_session().is_fresh()) + self.assertFalse(_session(expires_at=int(time.time()) - 10).is_fresh()) + + def test_missing_expires_in_falls_back_to_fresh(self): + s = Session.from_token_response({"access_token": "eyJ.aaa.bbb"}, cloud_name="c") + self.assertGreater(s.expires_at, int(time.time())) + self.assertTrue(s.is_fresh()) + + def test_zero_expires_in_falls_back_to_fresh(self): + s = Session.from_token_response( + {"access_token": "eyJ.aaa.bbb", "expires_in": 0}, cloud_name="c") + self.assertTrue(s.is_fresh()) + + +class TestStripOAuthInternalKeys(unittest.TestCase): + def test_drops_bookkeeping_keeps_auth_and_host(self): + url = to_cloudinary_url(_session()) + config = cloudinary.Config() + config._setup_from_parsed_url(config._parse_cloudinary_url(url)) + full = {k: v for k, v in config.__dict__.items() if not k.startswith("_")} + self.assertEqual({"refresh_token", "expires_at", "region", "issuer"}, full.keys() & + {"refresh_token", "expires_at", "region", "issuer"}) + + sanitized = strip_oauth_internal_keys(full) + for leaked in ("refresh_token", "expires_at", "region", "issuer"): + self.assertNotIn(leaked, sanitized) + self.assertEqual("eyJ.aaa.bbb", sanitized["oauth_token"]) + self.assertEqual("https://api-eu.cloudinary.com", sanitized["upload_prefix"]) + self.assertEqual("eu-cloud", sanitized["cloud_name"]) + + def test_noop_on_api_key_config(self): + full = {"cloud_name": "c", "api_key": "k", "api_secret": "s"} + self.assertEqual(full, strip_oauth_internal_keys(full)) + + +class TestRefreshUrlIfStale(unittest.TestCase): + def test_non_oauth_passthrough(self): + url = "cloudinary://key:secret@cloud" + self.assertEqual(url, refresh_url_if_stale("c", url)) + + def test_fresh_unchanged(self): + url = to_cloudinary_url(_session()) + with patch("cloudinary_cli.auth.flow.refresh") as refresh: + self.assertEqual(url, refresh_url_if_stale("eu-cloud", url)) + refresh.assert_not_called() + + def test_force_refreshes_fresh_token(self): + url = to_cloudinary_url(_session()) # fresh + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": url}), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response) as refresh, \ + patch("cloudinary_cli.auth.update_config"): + new_url = refresh_url_if_stale("eu-cloud", url, force=True) + refresh.assert_called_once() + self.assertIn("oauth_token=eyJ.new.tok", new_url) + + def test_stale_refreshes_and_rewrites(self): + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": stale_url}), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config") as update_config: + new_url = refresh_url_if_stale("eu-cloud", stale_url) + self.assertIn("oauth_token=eyJ.new.tok", new_url) + self.assertIn("refresh_token=rt_new", new_url) + update_config.assert_called_once() + + def test_no_refresh_token_returns_unchanged(self): + url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10, refresh_token=None)) + self.assertEqual(url, refresh_url_if_stale("eu-cloud", url)) + + def test_refresh_timeout_returns_stale_url(self): + import requests + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": stale_url}), \ + patch("cloudinary_cli.auth.flow.refresh", side_effect=requests.Timeout()), \ + patch("cloudinary_cli.auth.update_config") as update_config: + self.assertEqual(stale_url, refresh_url_if_stale("eu-cloud", stale_url)) + update_config.assert_not_called() + + def test_refresh_failure_warns_once_per_config(self): + # A3a: a failed background refresh must surface a re-login hint (not just a debug line), but + # only once per config so a bulk run does not log it per asset. + import requests + import cloudinary_cli.auth as auth + auth._refresh_warned.discard("eu-cloud") + self.addCleanup(auth._refresh_warned.discard, "eu-cloud") + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": stale_url}), \ + patch("cloudinary_cli.auth.flow.refresh", side_effect=requests.ConnectionError()), \ + patch("cloudinary_cli.auth.update_config"), \ + patch("cloudinary_cli.auth.logger.warning") as warn: + refresh_url_if_stale("eu-cloud", stale_url) + refresh_url_if_stale("eu-cloud", stale_url) # second stale read in the same run + warn.assert_called_once() + self.assertIn("cld login eu-cloud", warn.call_args[0][0]) + + def test_refresh_success_rearms_the_warning(self): + # After a successful refresh the warning is re-armed, so a later failure warns again. + import cloudinary_cli.auth as auth + auth._refresh_warned.add("eu-cloud") + self.addCleanup(auth._refresh_warned.discard, "eu-cloud") + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": stale_url}), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + refresh_url_if_stale("eu-cloud", stale_url) + self.assertNotIn("eu-cloud", auth._refresh_warned) + + def test_adopts_peer_refresh_without_calling_refresh(self): + # Peer already rewrote the saved URL to a fresh token while we waited for the lock. + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + peer_fresh_url = to_cloudinary_url(_session( + access_token="eyJ.peer.tok", expires_at=int(time.time()) + 300)) + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": peer_fresh_url}), \ + patch("cloudinary_cli.auth.flow.refresh") as refresh, \ + patch("cloudinary_cli.auth.update_config") as update_config: + result = refresh_url_if_stale("eu-cloud", stale_url) + self.assertEqual(peer_fresh_url, result) + refresh.assert_not_called() # we did not burn the (already-rotated) refresh token + update_config.assert_not_called() + + def test_refreshes_when_peer_value_still_stale(self): + stale_url = to_cloudinary_url(_session(expires_at=int(time.time()) - 10)) + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value={"eu-cloud": stale_url}), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response) as refresh, \ + patch("cloudinary_cli.auth.update_config") as update_config: + result = refresh_url_if_stale("eu-cloud", stale_url) + self.assertIn("oauth_token=eyJ.new.tok", result) + refresh.assert_called_once() + update_config.assert_called_once() + + +class TestRefreshConfig(unittest.TestCase): + def _cfg(self, **extra): + cfg = { + "stale": to_cloudinary_url(_session(cloud_name="stale", expires_at=int(time.time()) - 10)), + "fresh": to_cloudinary_url(_session(cloud_name="fresh")), + "key": "cloudinary://k:s@kc", + } + cfg.update(extra) + return cfg + + def test_not_found(self): + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()): + self.assertEqual("not_found", refresh_config("ghost")) + + def test_not_oauth(self): + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()): + self.assertEqual("not_oauth", refresh_config("key")) + + def test_fresh_skipped(self): + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()), \ + patch("cloudinary_cli.auth.flow.refresh") as refresh: + self.assertEqual("fresh", refresh_config("fresh")) + refresh.assert_not_called() + + def test_stale_refreshed(self): + token_response = {"access_token": "new", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + self.assertEqual("refreshed", refresh_config("stale")) + + def test_force_refreshes_fresh(self): + token_response = {"access_token": "new", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response) as refresh, \ + patch("cloudinary_cli.auth.update_config"): + self.assertEqual("refreshed", refresh_config("fresh", force=True)) + refresh.assert_called_once() + + def test_failed_when_no_refresh_token(self): + cfg = self._cfg(stale=to_cloudinary_url(_session( + cloud_name="stale", expires_at=int(time.time()) - 10, refresh_token=None))) + with patch("cloudinary_cli.auth.load_config", return_value=cfg): + self.assertEqual("failed", refresh_config("stale")) + + def test_relogin_command_includes_non_default_region(self): + from cloudinary_cli.auth import relogin_command + cfg = { + "global": to_cloudinary_url(_session(cloud_name="global", region="api")), + "stg": to_cloudinary_url(_session(cloud_name="stg", region="api-staging")), + "key": "cloudinary://k:s@kc", + } + with patch("cloudinary_cli.auth.load_config", return_value=cfg): + self.assertEqual("cld login global", relogin_command("global")) + self.assertEqual("cld login stg --region api-staging", relogin_command("stg")) + self.assertEqual("cld login key", relogin_command("key")) # non-oauth: no region + + def test_refresh_configs_sweeps_oauth_only(self): + token_response = {"access_token": "new", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.auth.load_config", return_value=self._cfg()), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + results = refresh_configs() + self.assertEqual({"stale": "refreshed", "fresh": "fresh"}, results) # "key" not swept + + +class TestLoginGuards(unittest.TestCase): + def test_missing_cloud_name_raises_and_saves_nothing(self): + session = _session(cloud_name=None) + with patch("cloudinary_cli.auth._run_browser_flow", return_value=session), \ + patch("cloudinary_cli.auth.update_config") as update_config: + with self.assertRaises(RuntimeError): + login(region="api-eu") + update_config.assert_not_called() + + +class TestBrowserFlowNonInteractive(unittest.TestCase): + """No browser + no TTY: _run_browser_flow must fail fast with a headless-usage hint, never block + in wait_for_callback until the callback times out.""" + + def test_no_browser_no_tty_fails_fast_without_waiting(self): + from cloudinary_cli.auth import _run_browser_flow + fake_httpd = mock.Mock() + with patch("cloudinary_cli.auth.start_callback_server", + return_value=(fake_httpd, "http://127.0.0.1:49421/callback")), \ + patch("cloudinary_cli.auth.webbrowser.open", return_value=False), \ + patch("cloudinary_cli.auth.is_interactive", return_value=False), \ + patch("cloudinary_cli.auth.wait_for_callback") as wait: + with self.assertRaises(RuntimeError) as ctx: + _run_browser_flow("api-eu") + wait.assert_not_called() # fails fast: no 5-minute callback wait + fake_httpd.server_close.assert_called_once() # releases the bound port + self.assertIn("-c", str(ctx.exception)) # points at the headless API-key alternative + + def test_no_browser_but_tty_still_waits(self): + # A human at a TTY can paste the printed URL, so we must NOT fail fast here. + from cloudinary_cli.auth import _run_browser_flow + with patch("cloudinary_cli.auth.start_callback_server", + return_value=(mock.Mock(), "http://127.0.0.1:49421/callback")), \ + patch("cloudinary_cli.auth.webbrowser.open", return_value=False), \ + patch("cloudinary_cli.auth.is_interactive", return_value=True), \ + patch("cloudinary_cli.auth.wait_for_callback", return_value=("code", "st")) as wait, \ + patch("cloudinary_cli.auth.flow.exchange_code", return_value={"access_token": "x"}): + # state mismatch is irrelevant here; we only assert it reached the wait (did not fast-fail) + with patch("cloudinary_cli.auth.secrets.token_urlsafe", return_value="st"): + _run_browser_flow("api-eu") + wait.assert_called_once() + + +class TestDeriveConfigName(unittest.TestCase): + def _derive(self, cloud, region, config): + with patch("cloudinary_cli.auth.load_config", return_value=config): + return _derive_config_name(cloud, region) + + def test_default_region_bare(self): + self.assertEqual("my_cloud", self._derive("my_cloud", "api", {})) + + def test_region_suffix(self): + self.assertEqual("my_cloud-eu", self._derive("my_cloud", "api-eu", {})) + + def test_relogin_overwrites_same_type(self): + existing = {"my_cloud-eu": "cloudinary://my_cloud-eu?oauth_token=x"} + self.assertEqual("my_cloud-eu", self._derive("my_cloud", "api-eu", existing)) + + def test_cross_type_collision_gets_oauth_suffix(self): + existing = {"my_cloud": "cloudinary://key:secret@my_cloud"} + self.assertEqual("my_cloud-oauth", self._derive("my_cloud", "api", existing)) + + def test_cross_type_collision_with_region(self): + existing = {"my_cloud-eu": "cloudinary://key:secret@my_cloud"} + self.assertEqual("my_cloud-eu-oauth", self._derive("my_cloud", "api-eu", existing)) diff --git a/test/test_cli.py b/test/test_cli.py index ca8251d..87e1208 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -11,6 +11,8 @@ class TestCLI(unittest.TestCase): COMMANDS = [ 'admin', 'config', + 'login', + 'logout', 'make', 'migrate', 'provisioning', diff --git a/test/test_cli_api.py b/test/test_cli_api.py index d18361e..1ec464c 100644 --- a/test/test_cli_api.py +++ b/test/test_cli_api.py @@ -6,7 +6,8 @@ from click.testing import CliRunner from cloudinary_cli.cli import cli -from test.helper_test import api_response_mock, uploader_response_mock, URLLIB3_REQUEST +from test.helper_test import api_response_mock, uploader_response_mock, URLLIB3_REQUEST, \ + CONFIG_PRESENT, REQUIRES_CONFIG API_MOCK_RESPONSE = api_response_mock() UPLOAD_MOCK_RESPONSE = uploader_response_mock() @@ -17,6 +18,7 @@ class TestCLIApi(unittest.TestCase): runner = CliRunner() + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(URLLIB3_REQUEST) def test_admin(self, mocker): mocker.return_value = API_MOCK_RESPONSE @@ -25,6 +27,7 @@ def test_admin(self, mocker): self.assertEqual(0, result.exit_code, result.output) self.assertIn('"foo": "bar"', result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(URLLIB3_REQUEST) def test_upload(self, mocker): mocker.return_value = UPLOAD_MOCK_RESPONSE @@ -56,6 +59,7 @@ def test_delete_all_resources_decline_skips_call(self, http_mock, confirm_mock): confirm_mock.assert_called_once() self.assertFalse(http_mock.called, "SDK should not be called when user declines") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=True) @patch(URLLIB3_REQUEST) def test_delete_all_resources_accept_calls_sdk(self, http_mock, confirm_mock): @@ -66,6 +70,7 @@ def test_delete_all_resources_accept_calls_sdk(self, http_mock, confirm_mock): confirm_mock.assert_called_once() self.assertTrue(http_mock.called, "SDK should be called when user accepts") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=False) @patch(URLLIB3_REQUEST) def test_delete_all_resources_force_skips_prompt(self, http_mock, confirm_mock): @@ -86,6 +91,7 @@ def test_delete_resources_by_tag_decline_skips_call(self, http_mock, confirm_moc confirm_mock.assert_called_once() self.assertFalse(http_mock.called, "SDK should not be called when user declines") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=False) @patch(URLLIB3_REQUEST) def test_delete_resources_explicit_ids_no_prompt(self, http_mock, confirm_mock): @@ -96,6 +102,7 @@ def test_delete_resources_explicit_ids_no_prompt(self, http_mock, confirm_mock): self.assertFalse(confirm_mock.called, "Explicit-ID delete must not prompt") self.assertTrue(http_mock.called, "SDK should be called for explicit-ID delete") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=False) @patch(URLLIB3_REQUEST) def test_uploader_add_tag_no_prompt(self, http_mock, confirm_mock): @@ -106,6 +113,7 @@ def test_uploader_add_tag_no_prompt(self, http_mock, confirm_mock): self.assertFalse(confirm_mock.called, "Non-destructive bulk methods must not prompt") self.assertTrue(http_mock.called, "SDK should be called for non-destructive bulk methods") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=False) @patch(URLLIB3_REQUEST) def test_admin_resources_read_no_prompt(self, http_mock, confirm_mock): @@ -116,6 +124,7 @@ def test_admin_resources_read_no_prompt(self, http_mock, confirm_mock): self.assertFalse(confirm_mock.called, "Read commands must not prompt") self.assertTrue(http_mock.called, "SDK should be called for read commands") + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(CONFIRM_ACTION_PATCH, return_value=False) @patch(URLLIB3_REQUEST) def test_admin_resources_read_with_force_no_prompt(self, http_mock, confirm_mock): diff --git a/test/test_cli_config.py b/test/test_cli_config.py index fee18bf..fac0964 100644 --- a/test/test_cli_config.py +++ b/test/test_cli_config.py @@ -1,4 +1,6 @@ +import os import unittest +from unittest.mock import patch import cloudinary from click.testing import CliRunner @@ -6,6 +8,10 @@ from cloudinary_cli.cli import cli +def _env_without_cloudinary_vars(): + return {k: v for k, v in os.environ.items() if not k.startswith("CLOUDINARY_")} + + def _get_real_cloudinary_url(): cfg = cloudinary.config() @@ -88,9 +94,13 @@ def test_cli_config_show(self): @unittest.skipUnless(cloudinary.config().api_secret, "Requires api_key/api_secret") def test_cli_config_show_default_no_config(self): - self.runner.invoke(cli, ['config', '--from_url', self.EMPTY_CLOUDINARY_URL]) + # This asserts the "nothing configured" path, so it must run with no environment config: + # otherwise the resolver legitimately falls back to CLOUDINARY_URL and the config is valid. + with patch.dict(os.environ, _env_without_cloudinary_vars(), clear=True): + cloudinary.reset_config() + self.runner.invoke(cli, ['config', '--from_url', self.EMPTY_CLOUDINARY_URL]) - result = self.runner.invoke(cli, ['config']) + result = self.runner.invoke(cli, ['config']) self.assertEqual(1, result.exit_code) diff --git a/test/test_cli_config_oauth.py b/test/test_cli_config_oauth.py new file mode 100644 index 0000000..9fb775c --- /dev/null +++ b/test/test_cli_config_oauth.py @@ -0,0 +1,863 @@ +import json +import os +import time +import unittest +from unittest.mock import patch + +import cloudinary +from click.testing import CliRunner + +from cloudinary_cli.auth.session import Session, to_cloudinary_url +from cloudinary_cli.cli import cli +from cloudinary_cli.utils.config_resolver import config_to_api_kwargs, get_cloudinary_config +from cloudinary_cli.utils.config_utils import config_to_dict, show_cloudinary_config + + +def _oauth_url(cloud="eu-cloud", region="api-eu"): + return to_cloudinary_url(Session( + cloud_name=cloud, access_token="eyJ.secret_access.tok", refresh_token="rt_secret_value", + expires_at=int(time.time()) + 300, region=region, issuer="https://oauth.cloudinary.com/")) + + +class _RestoresSdkConfig(unittest.TestCase): + def setUp(self): + self._env_snapshot = dict(os.environ) + # Strip ambient CLOUDINARY_* so a bare cloudinary.Config() built in a test is not polluted by + # the developer's env (e.g. a real account_url leaking into masking/display assertions). + for key in [k for k in os.environ if k.startswith("CLOUDINARY_")]: + del os.environ[key] + cloudinary.reset_config() + self.addCleanup(self._restore_sdk_config) + + def _restore_sdk_config(self): + os.environ.clear() + os.environ.update(self._env_snapshot) + cloudinary.reset_config() + + +class TestLogoutScope(unittest.TestCase): + """logout must only remove OAuth logins, never plain saved configs.""" + + def test_removes_oauth_login(self): + from cloudinary_cli.auth import logout + saved = {"eu-cloud": _oauth_url()} + with patch("cloudinary_cli.auth.load_config", return_value=saved), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove, \ + patch("cloudinary_cli.auth.flow.revoke") as revoke: + self.assertEqual("removed", logout("eu-cloud")) + remove.assert_called_once_with("eu-cloud") + revoke.assert_called_once_with("rt_secret_value", "api-eu") + + def test_revoke_failure_still_removes_locally(self): + import requests + from cloudinary_cli.auth import logout + saved = {"eu-cloud": _oauth_url()} + with patch("cloudinary_cli.auth.load_config", return_value=saved), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove, \ + patch("cloudinary_cli.auth.flow.revoke", side_effect=requests.ConnectionError()): + self.assertEqual("revoke_failed", logout("eu-cloud")) + remove.assert_called_once_with("eu-cloud") # local entry removed despite revoke failure + + def test_refuses_non_oauth_config(self): + from cloudinary_cli.auth import logout + saved = {"mykey": "cloudinary://key:secret@cloud"} + with patch("cloudinary_cli.auth.load_config", return_value=saved), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + self.assertEqual("not_oauth", logout("mykey")) + remove.assert_not_called() + + def test_missing_name(self): + from cloudinary_cli.auth import logout + with patch("cloudinary_cli.auth.load_config", return_value={}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + self.assertEqual("not_found", logout("nope")) + remove.assert_not_called() + + +class TestLogoutInteractiveSelect(unittest.TestCase): + """`cld logout` with no name lists OAuth logins and removes the chosen one.""" + + runner = CliRunner() + + def test_lists_only_oauth_and_removes_selected(self): + saved = {"mykey": "cloudinary://key:secret@cloud", + "cloud-a": _oauth_url("cloud-a"), "cloud-b": _oauth_url("cloud-b")} + with patch("cloudinary_cli.auth.load_config", return_value=saved), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove, \ + patch("cloudinary_cli.auth.flow.revoke"): + result = self.runner.invoke(cli, ["logout"], input="2\n") + self.assertIn("cloud-a", result.output) + self.assertIn("cloud-b", result.output) + self.assertNotIn("mykey", result.output) # non-oauth not offered + remove.assert_called_once_with("cloud-b") + + def test_no_oauth_logins(self): + with patch("cloudinary_cli.auth.load_config", + return_value={"mykey": "cloudinary://key:secret@cloud"}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + result = self.runner.invoke(cli, ["logout"], input="\n") + self.assertIn("No saved OAuth logins", result.output) + remove.assert_not_called() + + def test_cancel_on_empty_input(self): + with patch("cloudinary_cli.auth.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + result = self.runner.invoke(cli, ["logout"], input="\n") + remove.assert_not_called() + self.assertEqual(0, result.exit_code) + + def test_invalid_non_numeric_errors(self): + with patch("cloudinary_cli.auth.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + result = self.runner.invoke(cli, ["logout"], input="sdfdsf\n", standalone_mode=False) + self.assertIn("Invalid selection", result.output) + self.assertFalse(result.return_value) # main() maps falsy -> exit 1 + remove.assert_not_called() + + def test_out_of_range_errors(self): + with patch("cloudinary_cli.auth.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove: + result = self.runner.invoke(cli, ["logout"], input="5\n", standalone_mode=False) + self.assertIn("Invalid selection", result.output) + self.assertFalse(result.return_value) + remove.assert_not_called() + + def test_noninteractive_stdin_errors_with_hint(self): + # Closed stdin (no input at all): the selection cannot be made, so error with the + # non-interactive form (`cld logout `) and exit non-zero, not a silent no-op. + import builtins + with patch("cloudinary_cli.auth.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \ + patch("cloudinary_cli.auth.remove_config_keys") as remove, \ + patch.object(builtins, "input", side_effect=EOFError()): + result = self.runner.invoke(cli, ["logout"], standalone_mode=False) + self.assertIn("cld logout ", result.output) + self.assertFalse(result.return_value) # main() maps falsy -> exit 1 + remove.assert_not_called() + + +class TestLoginSetDefault(unittest.TestCase): + """`login` sets the default explicitly with --set-default and auto-defaults a sole login.""" + + def _patches(self, saved): + session = Session(cloud_name="eu-cloud", access_token="a", refresh_token="r", + expires_at=int(time.time()) + 300, region="api-eu", + issuer="https://oauth.cloudinary.com/") + return patch.multiple( + "cloudinary_cli.auth", + _run_browser_flow=lambda region: session, + load_config=lambda: dict(saved), + update_config=lambda *a, **k: None, + is_env_configured=lambda: False, + ) + + def test_set_default_flag_marks_default(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url(), "other": _oauth_url("other")}), \ + patch("cloudinary_cli.auth.set_default_config") as set_default, \ + patch("cloudinary_cli.auth.get_default_config_name", return_value=None): + auth.login(region="eu", name="eu-cloud", set_default=True) + set_default.assert_called_once_with("eu-cloud") + + def test_auto_default_when_sole_config_no_env_no_default(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url()}), \ + patch("cloudinary_cli.auth.set_default_config") as set_default, \ + patch("cloudinary_cli.auth.get_default_config_name", return_value=None): + name, is_default = auth.login(region="eu", name="eu-cloud") + set_default.assert_called_once_with("eu-cloud") + self.assertEqual(("eu-cloud", True), (name, is_default)) + + def test_returns_not_default_when_other_configs_exist(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url(), "other": _oauth_url("other")}), \ + patch("cloudinary_cli.auth.set_default_config"), \ + patch("cloudinary_cli.auth.get_default_config_name", return_value=None): + name, is_default = auth.login(region="eu", name="eu-cloud") + self.assertEqual(("eu-cloud", False), (name, is_default)) + + def test_no_auto_default_when_other_configs_exist(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url(), "other": _oauth_url("other")}), \ + patch("cloudinary_cli.auth.set_default_config") as set_default, \ + patch("cloudinary_cli.auth.get_default_config_name", return_value=None): + auth.login(region="eu", name="eu-cloud") + set_default.assert_not_called() + + def test_no_auto_default_when_env_configured(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url()}), \ + patch("cloudinary_cli.auth.is_env_configured", return_value=True), \ + patch("cloudinary_cli.auth.set_default_config") as set_default, \ + patch("cloudinary_cli.auth.get_default_config_name", return_value=None): + auth.login(region="eu", name="eu-cloud") + set_default.assert_not_called() + + def test_no_auto_default_when_default_already_stored(self): + from cloudinary_cli import auth + with self._patches({"eu-cloud": _oauth_url()}), \ + patch("cloudinary_cli.auth.set_default_config") as set_default, \ + patch("cloudinary_cli.auth.get_default_config_name", return_value="something"): + auth.login(region="eu", name="eu-cloud") + set_default.assert_not_called() + + def test_reserved_name_rejected(self): + from cloudinary_cli import auth + with patch("cloudinary_cli.auth._run_browser_flow"): + with self.assertRaises(RuntimeError): + auth.login(region="eu", name="__default__") + + def test_cli_message_when_default(self): + with patch("cloudinary_cli.core.auth.run_login", return_value=("tttt", True)): + result = CliRunner().invoke(cli, ["login", "tttt"]) + self.assertIn("default configuration", result.output) + + def test_cli_message_when_not_default_shows_how_to_default(self): + with patch("cloudinary_cli.core.auth.run_login", return_value=("tttt", False)): + result = CliRunner().invoke(cli, ["login", "tttt"]) + self.assertIn("cld -C tttt", result.output) + self.assertIn("cld config -d tttt", result.output) # how to make it default + + +class TestConfigSecretMasking(_RestoresSdkConfig): + """show_cloudinary_config must never print a secret in the clear.""" + + def test_masks_api_secret(self): + config = cloudinary.Config() + config.update(cloud_name="c", api_key="k", api_secret="abcdefghIJKLMNOP") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + out = echo.call_args[0][0] + self.assertNotIn("abcdefghIJKLMNOP", out) + self.assertIn("MNOP", out) # last 4 kept + + def test_masks_account_url_password(self): + config = cloudinary.Config() + config.update(account_url="account://acc_key:SUPERSECRETPASSWORD@account_id") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + out = echo.call_args[0][0] + self.assertNotIn("SUPERSECRETPASSWORD", out) + self.assertIn("acc_key", out) # identifier kept + self.assertIn("account_id", out) # host kept + + def test_masks_oauth_and_refresh_tokens(self): + config = cloudinary.Config() + config.update(cloud_name="c", oauth_token="eyJ.secret_access.tok", + refresh_token="rt_secret_value") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + out = echo.call_args[0][0] + self.assertNotIn("eyJ.secret_access.tok", out) + self.assertNotIn("rt_secret_value", out) + + def test_mask_is_fixed_width_for_long_secret(self): + config = cloudinary.Config() + config.update(cloud_name="c", oauth_token="eyJ" + "A" * 2000 + "N2dQ") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + out = echo.call_args[0][0] + self.assertIn("****N2dQ", out) # fixed prefix + last 4 + self.assertNotIn("*" * 8, out) # never a wall of asterisks + + def test_hides_empty_fields(self): + config = cloudinary.Config() + config.update(cloud_name="c", oauth_token="eyJ.tok", api_key=None, api_secret=None) + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + out = echo.call_args[0][0] + self.assertNotIn("api_key", out) + self.assertNotIn("api_secret", out) + self.assertNotIn("None", out) + self.assertIn("cloud_name", out) + + def test_expires_at_human_readable_and_state(self): + future = cloudinary.Config() + future.update(cloud_name="c", oauth_token="eyJ.tok", expires_at=int(time.time()) + 3600) + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(future) + out = echo.call_args[0][0] + self.assertIn("UTC", out) + self.assertIn("valid", out) + + past = cloudinary.Config() + past.update(cloud_name="c", oauth_token="eyJ.tok", expires_at=1782310673) + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(past) + out = echo.call_args[0][0] + self.assertIn("1782310673", out) # raw epoch kept + self.assertIn("2026-06-24", out) # human-readable date + self.assertIn("expired", out) + + def test_account_url_shown_as_structured_section(self): + config = cloudinary.Config() + config.update(cloud_name="c", api_key="k", api_secret="abcdefghIJKLMNOP", + account_url="account://acc_key:SUPERSECRETPASSWORD@account_id") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + # two echo calls: the main block, then the account (provisioning) section + self.assertEqual(2, echo.call_count) + main = echo.call_args_list[0][0][0] + account = echo.call_args_list[1][0][0] + self.assertNotIn("account://", main) + # the account URL is decomposed into labeled fields, secret masked + self.assertIn("account_id:", account) + self.assertIn("provisioning_api_key:", account) + self.assertIn("provisioning_api_secret:", account) + self.assertIn("acc_key", account) + self.assertIn("account_id", account) + self.assertNotIn("SUPERSECRETPASSWORD", account) + self.assertNotIn("account://", account) # no raw URL string + + def test_malformed_account_url_falls_back_to_raw_line(self): + config = cloudinary.Config() + config.update(cloud_name="c", account_url="account://garbage") + with patch("cloudinary_cli.utils.config_utils.echo") as echo: + show_cloudinary_config(config) + account = echo.call_args_list[-1][0][0] + self.assertIn("account_url: account://garbage", account) + + +class TestOAuthConfigCoexistence(_RestoresSdkConfig): + runner = CliRunner() + + CONFIG = { + "prod-account": "cloudinary://key:secret@prod_cloud", + "eu-cloud": _oauth_url(), + } + + def test_ls_shows_both(self): + with patch("cloudinary_cli.utils.config_listing.load_config", return_value=dict(self.CONFIG)): + result = self.runner.invoke(cli, ['config', '--ls']) + self.assertEqual(0, result.exit_code) + self.assertIn("prod-account", result.output) + self.assertIn("eu-cloud", result.output) + + def test_show_oauth_masks_token(self): + with patch("cloudinary_cli.core.config.load_config", return_value=dict(self.CONFIG)): + result = self.runner.invoke(cli, ['config', '--show', 'eu-cloud']) + self.assertEqual(0, result.exit_code) + self.assertIn("eu-cloud", result.output) + self.assertNotIn("eyJ.secret_access.tok", result.output) + self.assertNotIn("rt_secret_value", result.output) + + def test_select_oauth_login_configures_sdk(self): + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(self.CONFIG)): + result = self.runner.invoke(cli, ['-C', 'eu-cloud', 'url', 'sample']) + self.assertEqual(0, result.exit_code, result.output) + self.assertIn("eu-cloud", result.output) + + def test_show_header_includes_name_type_and_flags(self): + cfg = {"__default__": "eu-cloud", "prod-account": "cloudinary://key:secret@prod_cloud", + "eu-cloud": _oauth_url()} + with patch("cloudinary_cli.core.config.load_config", return_value=dict(cfg)), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(cfg)), \ + patch.dict("os.environ", {}, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + default_active = self.runner.invoke(cli, ['config', '-s', 'eu-cloud']) + plain = self.runner.invoke(cli, ['config', '-s', 'prod-account']) + self.assertIn("name: eu-cloud (oauth) [default, active]", default_active.output) + self.assertIn("name: prod-account (api_key)\n", plain.output) # no flag bracket + + def test_bare_config_header_matches_active(self): + cfg = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + with patch("cloudinary_cli.core.config.load_config", return_value=dict(cfg)), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(cfg)), \ + patch.dict("os.environ", {}, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + bare = self.runner.invoke(cli, ['config']) + shown = self.runner.invoke(cli, ['config', '-s', 'eu-cloud']) + # bare `config` identifies the active config the same way `-s ` does + self.assertIn("name: eu-cloud (oauth) [default, active]", bare.output) + self.assertIn("name: eu-cloud (oauth) [default, active]", shown.output) + + def test_bare_config_header_for_command_line_url(self): + cfg = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + with patch("cloudinary_cli.core.config.load_config", return_value=dict(cfg)), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(cfg)), \ + patch.dict("os.environ", {}, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + result = self.runner.invoke(cli, ['-c', 'cloudinary://a:b@cmdcloud', 'config']) + self.assertIn("name: (command-line) (api_key) [active]", result.output) + + def _show_json(self, args, cfg): + with patch("cloudinary_cli.core.config.load_config", return_value=dict(cfg)), \ + patch("cloudinary_cli.utils.config_listing.load_config", return_value=dict(cfg)), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(cfg)), \ + patch.dict("os.environ", {}, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + result = self.runner.invoke(cli, args) + self.assertEqual(0, result.exit_code, result.output) + return json.loads(result.output[result.output.index("{"):]) + + def test_show_json_includes_meta_and_masks_secrets(self): + cfg = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + data = self._show_json(['config', '-s', 'eu-cloud', '--json'], cfg) + self.assertEqual("eu-cloud", data["name"]) + self.assertEqual("saved", data["source"]) + self.assertEqual("oauth", data["type"]) + self.assertTrue(data["default"]) + self.assertTrue(data["active"]) + self.assertEqual("eu-cloud", data["cloud_name"]) + # secrets masked, never in the clear + self.assertNotIn("eyJ.secret_access.tok", json.dumps(data)) + self.assertNotIn("rt_secret_value", json.dumps(data)) + # expires_at expanded into a structured object + self.assertIn("epoch", data["expires_at"]) + self.assertIn("expired", data["expires_at"]) + + def test_bare_config_json_matches_show_json(self): + cfg = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + bare = self._show_json(['config', '--json'], cfg) + shown = self._show_json(['config', '-s', 'eu-cloud', '--json'], cfg) + self.assertEqual(shown, bare) + + def test_bare_config_json_env_carries_source(self): + # a synthetic active source is disambiguated by `source`, matching the -ls -j rows + with patch("cloudinary_cli.core.config.load_config", return_value={}), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value={}), \ + patch.dict("os.environ", {"CLOUDINARY_URL": "cloudinary://ek:es@env_cloud"}, clear=False): + cloudinary.reset_config() + result = self.runner.invoke(cli, ['config', '--json']) + data = json.loads(result.output[result.output.index("{"):]) + self.assertEqual("(environment)", data["name"]) + self.assertEqual("env", data["source"]) + + def test_config_details_decomposes_account_url(self): + from cloudinary_cli.utils.config_utils import cloudinary_config_details + config = cloudinary.Config() + config.update(cloud_name="c", account_url="account://pk:SUPERSECRETxyz@acc_id") + details = cloudinary_config_details(config) + self.assertEqual("acc_id", details["account"]["account_id"]) + self.assertEqual("pk", details["account"]["provisioning_api_key"]) + self.assertNotIn("SUPERSECRETxyz", json.dumps(details)) + self.assertTrue(details["account"]["provisioning_api_secret"].endswith("txyz") + or details["account"]["provisioning_api_secret"].startswith("****")) + self.assertNotIn("account_url", details) # decomposed, not raw + + +class TestDefaultConfigResolution(_RestoresSdkConfig): + """Resolution precedence: -c > -C > stored default > environment > unconfigured.""" + + runner = CliRunner() + + def _invoke(self, args, saved, env=None): + env = dict(env or {}) + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch.dict("os.environ", env, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + if key not in env: + os.environ.pop(key, None) + cloudinary.reset_config() + return self.runner.invoke(cli, args) + + def test_stored_default_applies_when_no_explicit_config(self): + saved = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + result = self._invoke(['url', 'sample'], saved=saved) + self.assertEqual(0, result.exit_code, result.output) + self.assertEqual("eu-cloud", cloudinary.config().cloud_name) + + def test_no_implicit_sole_login_without_default(self): + # A single saved login with no stored default no longer auto-applies. + saved = {"eu-cloud": _oauth_url()} + result = self._invoke(['url', 'sample'], saved=saved) + self.assertIn("No Cloudinary configuration found.", result.output) + self.assertIsNone(cloudinary.config().cloud_name) + + def test_stored_default_beats_env(self): + saved = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + self._invoke(['url', 'sample'], saved=saved, + env={"CLOUDINARY_URL": "cloudinary://key:secret@env_cloud"}) + self.assertEqual("eu-cloud", cloudinary.config().cloud_name) + + def test_env_applies_when_no_stored_default(self): + saved = {"eu-cloud": _oauth_url()} + self._invoke(['url', 'sample'], saved=saved, + env={"CLOUDINARY_URL": "cloudinary://key:secret@env_cloud"}) + self.assertEqual("env_cloud", cloudinary.config().cloud_name) + self.assertIsNone(cloudinary.config().oauth_token) + + def test_explicit_minus_C_overrides_default(self): + saved = {"__default__": "eu-cloud", "eu-cloud": _oauth_url(), + "other": "cloudinary://key:secret@other_cloud"} + result = self._invoke(['-C', 'other', 'url', 'sample'], saved=saved) + self.assertEqual(0, result.exit_code, result.output) + self.assertEqual("other_cloud", cloudinary.config().cloud_name) + self.assertIsNone(cloudinary.config().oauth_token) + + def test_default_pointing_at_deleted_config_is_ignored(self): + saved = {"__default__": "gone"} + result = self._invoke(['url', 'sample'], saved=saved) + self.assertIn("No Cloudinary configuration found.", result.output) + + def test_inline_url_and_saved_together_errors(self): + # A1: -c and -C are mutually exclusive; passing both must error, not silently drop one. + saved = {"eu-cloud": _oauth_url()} + result = self._invoke( + ['-c', 'cloudinary://a:b@inline', '-C', 'eu-cloud', 'url', 'sample'], saved=saved) + self.assertEqual(2, result.exit_code) + self.assertIn("mutually exclusive", result.output) + + +class TestResolverNoNetworkIO(_RestoresSdkConfig): + """Finding 1 regression: resolution never refreshes a stale OAuth token (no network I/O).""" + + runner = CliRunner() + + def _stale_url(self): + return to_cloudinary_url(Session( + cloud_name="eu-cloud", access_token="eyJ.old.tok", refresh_token="rt_old", + expires_at=int(time.time()) - 10, region="api-eu", + issuer="https://oauth.cloudinary.com/")) + + def test_resolve_does_not_call_flow_refresh(self): + saved = {"__default__": "eu-cloud", "eu-cloud": self._stale_url()} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh") as refresh, \ + patch.dict("os.environ", {}, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + from cloudinary_cli.utils.config_resolver import resolve_cli_config + resolve_cli_config() + refresh.assert_not_called() + # The stale token is loaded as-is (presence check is refresh-free), awaiting a lazy refresh + # only when the SDK reads oauth_token at request time. + self.assertTrue(cloudinary.config().has_oauth) + self.assertEqual("eyJ.old.tok", cloudinary.config().__dict__.get("oauth_token")) + + def test_help_does_not_reach_phase_b(self): + with patch("cloudinary_cli.auth.flow.refresh") as refresh: + self.runner.invoke(cli, ['--help']) + refresh.assert_not_called() + + +class TestSelfRefreshingOAuthToken(_RestoresSdkConfig): + """The active OAuth config refreshes its token lazily when the SDK reads oauth_token at request + time; presence/type checks (has_oauth) never trigger a refresh.""" + + def _stale_url(self): + return to_cloudinary_url(Session( + cloud_name="eu-cloud", access_token="eyJ.old.tok", refresh_token="rt_old", + expires_at=int(time.time()) - 10, region="api-eu", + issuer="https://oauth.cloudinary.com/")) + + def test_reading_oauth_token_refreshes_stale_active_login(self): + import cloudinary_cli.utils.config_resolver as resolver + saved = {"eu-cloud": self._stale_url()} + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + resolver.resolve_cli_config(config_saved="eu-cloud") + # The read of oauth_token is what triggers the refresh (as the SDK does per request). + self.assertEqual("eyJ.new.tok", cloudinary.config().oauth_token) + + def test_presence_check_does_not_refresh(self): + """has_oauth (used by type/validity/-ls) must NOT touch the network on a stale token.""" + import cloudinary_cli.utils.config_resolver as resolver + saved = {"eu-cloud": self._stale_url()} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh") as refresh: + resolver.resolve_cli_config(config_saved="eu-cloud") + self.assertTrue(cloudinary.config().has_oauth) + refresh.assert_not_called() + + def test_noop_for_inline_url(self): + import cloudinary_cli.utils.config_resolver as resolver + with patch("cloudinary_cli.auth.flow.refresh") as refresh: + resolver.resolve_cli_config(config="cloudinary://key:secret@cloud") + _ = cloudinary.config().oauth_token + refresh.assert_not_called() + + def test_noop_for_api_key_config(self): + import cloudinary_cli.utils.config_resolver as resolver + saved = {"mykey": "cloudinary://key:secret@cloud"} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh") as refresh: + resolver.resolve_cli_config(config_saved="mykey") + _ = cloudinary.config().oauth_token + refresh.assert_not_called() + + +class TestConfigDefaultCommands(_RestoresSdkConfig): + """`cld config` default management: -d, --set-default, --unset-default, -ls marker, -rm cleanup.""" + + runner = CliRunner() + + def test_d_marks_existing(self): + saved = {"prod": "cloudinary://k:s@prod", "eu-cloud": _oauth_url()} + with patch("cloudinary_cli.core.config.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.core.config.set_default_config") as set_default: + result = self.runner.invoke(cli, ['config', '-d', 'prod']) + self.assertEqual(0, result.exit_code, result.output) + set_default.assert_called_once_with("prod") + self.assertIn("Default set to 'prod'", result.output) + + def test_d_nonexistent_errors(self): + with patch("cloudinary_cli.core.config.load_config", return_value={"prod": "cloudinary://k:s@prod"}): + result = self.runner.invoke(cli, ['config', '-d', 'nope']) + self.assertEqual(2, result.exit_code) + self.assertIn("does not exist", result.output) + + def test_set_default_without_create_flag_errors(self): + result = self.runner.invoke(cli, ['config', '--set-default']) + self.assertEqual(2, result.exit_code) + self.assertIn("requires -n or --from_url", result.output) + + def test_new_with_set_default(self): + with patch("cloudinary_cli.core.config.verify_cloudinary_url", return_value=True), \ + patch("cloudinary_cli.core.config.update_config"), \ + patch("cloudinary_cli.core.config.set_default_config") as set_default: + result = self.runner.invoke( + cli, ['config', '-n', 'prod', 'cloudinary://k:s@prod', '--set-default']) + self.assertEqual(0, result.exit_code, result.output) + set_default.assert_called_once_with("prod") + self.assertIn("Default set to 'prod'", result.output) + + def test_set_default_on_failing_url_neither_saves_nor_defaults(self): + with patch("cloudinary_cli.core.config.verify_cloudinary_url", return_value=False), \ + patch("cloudinary_cli.core.config.update_config") as update, \ + patch("cloudinary_cli.core.config.set_default_config") as set_default: + self.runner.invoke( + cli, ['config', '-n', 'prod', 'cloudinary://bad', '--set-default']) + update.assert_not_called() + set_default.assert_not_called() + + def test_unset_default(self): + with patch("cloudinary_cli.core.config.load_config", return_value={}), \ + patch("cloudinary_cli.core.config.clear_default_config") as clear: + result = self.runner.invoke(cli, ['config', '--unset-default']) + self.assertEqual(0, result.exit_code, result.output) + clear.assert_called_once() + self.assertIn("cleared", result.output) + + def _run_ls(self, args, saved, env=None): + # Both the resolver (Phase A, which records the active config) and the config command read + # the same saved dict; env is controlled via os.environ so is_env_configured() is genuine. + env = dict(env or {}) + with patch("cloudinary_cli.core.config.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.utils.config_listing.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)), \ + patch.dict("os.environ", env, clear=False): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + if key not in env: + os.environ.pop(key, None) + cloudinary.reset_config() + return self.runner.invoke(cli, args) + + def _ls_json(self, saved, env=None): + result = self._run_ls(['config', '-ls', '--json'], saved, env) + self.assertEqual(0, result.exit_code, result.output) + return {r["name"]: r for r in json.loads(result.output[result.output.index("["):])} + + def test_ls_table_marks_default(self): + saved = {"__default__": "eu-cloud", "prod": "cloudinary://k:s@prod", "eu-cloud": _oauth_url()} + result = self._run_ls(['config', '-ls'], saved) + self.assertEqual(0, result.exit_code, result.output) + for header in ("NAME", "CLOUD", "TYPE", "DEFAULT", "ACTIVE"): + self.assertIn(header, result.output) + self.assertIn("prod", result.output) + self.assertIn("oauth", result.output) + self.assertIn("api_key", result.output) + # with no env, the stored default is both default and active (two markers) + rows = {line.split()[0]: line for line in result.output.splitlines() if line.startswith(("prod", "eu-cloud"))} + self.assertEqual(2, rows["eu-cloud"].count("*")) + self.assertEqual(0, rows["prod"].count("*")) + self.assertNotIn("__default__", result.output) + + def test_ls_json(self): + saved = {"__default__": "eu-cloud", "prod": "cloudinary://k:s@prodcloud", "eu-cloud": _oauth_url()} + by_name = self._ls_json(saved) + self.assertNotIn("__default__", by_name) + self.assertEqual("oauth", by_name["eu-cloud"]["type"]) + self.assertEqual("saved", by_name["eu-cloud"]["source"]) + self.assertTrue(by_name["eu-cloud"]["default"]) + self.assertTrue(by_name["eu-cloud"]["active"]) # no env -> stored default is active + self.assertEqual("api_key", by_name["prod"]["type"]) + self.assertEqual("prodcloud", by_name["prod"]["cloud_name"]) + self.assertFalse(by_name["prod"]["default"]) + self.assertFalse(by_name["prod"]["active"]) + + def test_ls_minus_C_marks_selected_active(self): + saved = {"__default__": "eu-cloud", "eu-cloud": _oauth_url(), + "test": "cloudinary://k:s@test_cloud"} + # an explicit -C selects the active config for this invocation, overriding the default + result = self._run_ls(['-C', 'test', 'config', '-ls', '--json'], saved) + self.assertEqual(0, result.exit_code, result.output) + by_name = {r["name"]: r for r in json.loads(result.output[result.output.index("["):])} + self.assertTrue(by_name["test"]["active"]) + self.assertFalse(by_name["eu-cloud"]["active"]) # not active, but still the stored default + self.assertTrue(by_name["eu-cloud"]["default"]) + + def test_ls_inline_url_shown_as_active_command_line_row(self): + saved = {"__default__": "eu-cloud", "eu-cloud": _oauth_url()} + # an inline -c URL is not a saved config, but it is what's active for this invocation + result = self._run_ls(['-c', 'cloudinary://a:b@inline_cloud', 'config', '-ls', '--json'], saved) + self.assertEqual(0, result.exit_code, result.output) + by_name = {r["name"]: r for r in json.loads(result.output[result.output.index("["):])} + cmd = by_name["(command-line)"] # synthetic source: parenthesized in both table and JSON + self.assertEqual("url", cmd["source"]) + self.assertEqual("inline_cloud", cmd["cloud_name"]) + self.assertTrue(cmd["active"]) + self.assertFalse(cmd["default"]) + # the stored default is still recorded, but it is not active while -c wins + self.assertTrue(by_name["eu-cloud"]["default"]) + self.assertFalse(by_name["eu-cloud"]["active"]) + + _ENV = {"CLOUDINARY_URL": "cloudinary://k:s@env_cloud"} + + def test_ls_env_row_active_when_no_default(self): + by_name = self._ls_json({"eu-cloud": _oauth_url()}, env=self._ENV) + env = by_name["(environment)"] + self.assertEqual("env", env["source"]) + self.assertEqual("env_cloud", env["cloud_name"]) + self.assertFalse(env["default"]) # the environment is never the *stored* default + self.assertTrue(env["active"]) # active because nothing outranks it + self.assertFalse(by_name["eu-cloud"]["active"]) + + def test_ls_stored_default_outranks_env_row(self): + by_name = self._ls_json({"__default__": "eu-cloud", "eu-cloud": _oauth_url()}, env=self._ENV) + env = by_name["(environment)"] + self.assertFalse(env["active"]) # stored default outranks the environment + # the stored default is both recorded and active + self.assertTrue(by_name["eu-cloud"]["default"]) + self.assertTrue(by_name["eu-cloud"]["active"]) + + def test_synthetic_row_name_parenthesized_in_table_and_json(self): + # synthetic (environment / command-line) configs read as parenthesized in both views + result = self._run_ls(['config', '-ls'], {"eu-cloud": _oauth_url()}, env=dict(self._ENV)) + self.assertIn("(environment)", result.output) + by_name = self._ls_json({"eu-cloud": _oauth_url()}, env=dict(self._ENV)) + self.assertIn("(environment)", by_name) + + def test_rm_of_default_clears_it(self): + with patch("cloudinary_cli.core.config.remove_config_keys", return_value=[]), \ + patch("cloudinary_cli.core.config.get_default_config_name", return_value="prod"), \ + patch("cloudinary_cli.core.config.clear_default_config") as clear: + result = self.runner.invoke(cli, ['config', '-rm', 'prod']) + self.assertEqual(0, result.exit_code, result.output) + clear.assert_called_once() + + def test_reserved_name_rejected_on_new(self): + result = self.runner.invoke( + cli, ['config', '-n', '__default__', 'cloudinary://k:s@c']) + self.assertEqual(2, result.exit_code) + self.assertIn("reserved", result.output) + + def test_refresh_named_delegates_to_refresh_config(self): + with patch("cloudinary_cli.core.config.refresh_config", return_value="refreshed") as rc: + result = self.runner.invoke(cli, ['config', '--refresh', 'eu-cloud']) + self.assertEqual(0, result.exit_code, result.output) + rc.assert_called_once_with("eu-cloud", force=False) + self.assertIn("Refreshed 'eu-cloud'", result.output) + + def test_refresh_force_passes_flag(self): + with patch("cloudinary_cli.core.config.refresh_config", return_value="refreshed") as rc: + self.runner.invoke(cli, ['config', '--refresh', 'eu-cloud', '--force']) + rc.assert_called_once_with("eu-cloud", force=True) + + def test_refresh_no_name_uses_active_config(self): + with patch("cloudinary_cli.core.config.active_config_name", return_value="active-one"), \ + patch("cloudinary_cli.core.config.refresh_config", return_value="refreshed") as rc: + self.runner.invoke(cli, ['config', '--refresh']) + rc.assert_called_once_with("active-one", force=False) + + def test_refresh_unknown_name_errors(self): + with patch("cloudinary_cli.core.config.refresh_config", return_value="not_found"): + result = self.runner.invoke(cli, ['config', '--refresh', 'ghost']) + self.assertEqual(2, result.exit_code) + self.assertIn("does not exist", result.output) + + def test_refresh_failed_reports_relogin_with_region(self): + with patch("cloudinary_cli.core.config.refresh_config", return_value="failed"), \ + patch("cloudinary_cli.core.config.relogin_command", + return_value="cld login eu-cloud --region api-eu") as relogin: + result = self.runner.invoke(cli, ['config', '--refresh', 'eu-cloud'], standalone_mode=False) + self.assertFalse(result.return_value) # main() maps falsy -> exit 1 + relogin.assert_called_once_with("eu-cloud") + self.assertIn("cld login eu-cloud --region api-eu", result.output) + + def test_refresh_all_reports_each(self): + with patch("cloudinary_cli.core.config.refresh_configs", + return_value={"a": "refreshed", "b": "fresh"}) as rc: + result = self.runner.invoke(cli, ['config', '--refresh-all']) + self.assertEqual(0, result.exit_code, result.output) + rc.assert_called_once_with(force=False) + self.assertIn("Refreshed 'a'", result.output) + self.assertIn("'b' token is still fresh", result.output) + + def test_force_without_refresh_errors(self): + result = self.runner.invoke(cli, ['config', '--force']) + self.assertEqual(2, result.exit_code) + self.assertIn("--force only applies", result.output) + + +class TestConfigToApiKwargs(unittest.TestCase): + def _oauth_config(self): + config = cloudinary.Config() + config._setup_from_parsed_url(config._parse_cloudinary_url(_oauth_url())) + return config + + def test_drops_oauth_bookkeeping(self): + config = self._oauth_config() + kwargs = config_to_api_kwargs(config) + for leaked in ("refresh_token", "expires_at", "region", "issuer"): + self.assertNotIn(leaked, kwargs) + self.assertEqual("eyJ.secret_access.tok", kwargs["oauth_token"]) + self.assertEqual("eu-cloud", kwargs["cloud_name"]) + + def test_config_to_dict_still_faithful(self): + full = config_to_dict(self._oauth_config()) + self.assertIn("refresh_token", full) + self.assertIn("region", full) + + +class TestGetCloudinaryConfigOAuth(_RestoresSdkConfig): + def _stale_url(self): + return to_cloudinary_url(Session( + cloud_name="eu-cloud", access_token="eyJ.old.tok", refresh_token="rt_old", + expires_at=int(time.time()) - 10, region="api-eu", + issuer="https://oauth.cloudinary.com/")) + + def test_refreshes_stale_target_before_use(self): + config = {"eu-cloud": self._stale_url()} + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=config), \ + patch("cloudinary_cli.auth.load_config", return_value=config), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"), \ + patch("cloudinary_cli.utils.config_resolver.ping_cloudinary", return_value=True): + target_config = get_cloudinary_config("eu-cloud") + self.assertTrue(target_config) + self.assertEqual("eyJ.new.tok", target_config.oauth_token) + + def test_ping_receives_sanitized_config(self): + config = {"eu-cloud": _oauth_url()} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=config), \ + patch("cloudinary_cli.auth.load_config", return_value=config), \ + patch("cloudinary_cli.utils.config_resolver.ping_cloudinary", return_value=True) as ping: + get_cloudinary_config("eu-cloud") + ping_kwargs = ping.call_args.kwargs + for leaked in ("refresh_token", "expires_at", "region", "issuer"): + self.assertNotIn(leaked, ping_kwargs) + self.assertEqual("eyJ.secret_access.tok", ping_kwargs["oauth_token"]) diff --git a/test/test_cli_search_api.py b/test/test_cli_search_api.py index c48e775..a1e282d 100644 --- a/test/test_cli_search_api.py +++ b/test/test_cli_search_api.py @@ -4,7 +4,8 @@ from click.testing import CliRunner from cloudinary_cli.cli import cli -from test.helper_test import api_response_mock, uploader_response_mock, URLLIB3_REQUEST +from test.helper_test import api_response_mock, uploader_response_mock, URLLIB3_REQUEST, \ + CONFIG_PRESENT, REQUIRES_CONFIG API_MOCK_RESPONSE = api_response_mock() UPLOAD_MOCK_RESPONSE = uploader_response_mock() @@ -13,6 +14,7 @@ class TestCLISearchApi(unittest.TestCase): runner = CliRunner() + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(URLLIB3_REQUEST) def test_search(self, mocker): mocker.return_value = API_MOCK_RESPONSE @@ -27,6 +29,7 @@ def test_search_fields(self): self.assertEqual(0, result.exit_code) self.assertIn('"fields": [\n "url",\n "tags",\n "context"\n ]', result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) def test_search_url(self): result = self.runner.invoke(cli, ['search', 'cat', '-c', 'NEXT_CURSOR', '--ttl', '1000', '--url']) @@ -37,6 +40,7 @@ def test_search_url(self): self.assertIn('1000', result.output) self.assertIn('NEXT_CURSOR', result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) @patch(URLLIB3_REQUEST) def test_search_folders(self, mocker): mocker.return_value = API_MOCK_RESPONSE diff --git a/test/test_cli_url.py b/test/test_cli_url.py index 08bff9f..8d6511a 100644 --- a/test/test_cli_url.py +++ b/test/test_cli_url.py @@ -3,6 +3,7 @@ from click.testing import CliRunner from cloudinary_cli.cli import cli +from test.helper_test import CONFIG_PRESENT, REQUIRES_CONFIG class TestCLIURL(unittest.TestCase): @@ -14,18 +15,21 @@ def test_url_no_public_id(self): self.assertEqual(2, result.exit_code) self.assertIn("Error: Missing argument 'PUBLIC_ID'", result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) def test_url(self): result = self.runner.invoke(cli, ['url', 'sample']) self.assertEqual(0, result.exit_code) self.assertIn('image/upload/sample', result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) def test_url_list(self): result = self.runner.invoke(cli, ['url', 'sample', '--type', 'list']) self.assertEqual(0, result.exit_code) self.assertIn('image/list/sample.json', result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) def test_url_authenticated(self): result = self.runner.invoke(cli, ['url', 'sample', '--type', 'authenticated']) diff --git a/test/test_cli_utils.py b/test/test_cli_utils.py index 77dcfac..5db4e5b 100644 --- a/test/test_cli_utils.py +++ b/test/test_cli_utils.py @@ -3,6 +3,7 @@ from click.testing import CliRunner from cloudinary_cli.cli import cli +from test.helper_test import CONFIG_PRESENT, REQUIRES_CONFIG class TestCLIUtils(unittest.TestCase): @@ -26,6 +27,7 @@ def test_list_utils(self): for util in self.UTILS: self.assertIn(util, result.output) + @unittest.skipUnless(CONFIG_PRESENT, REQUIRES_CONFIG) def test_utils_cloudinary_url(self): result = self.runner.invoke(cli, ['utils', 'cloudinary_url', 'sample']) diff --git a/test/test_config_cache.py b/test/test_config_cache.py new file mode 100644 index 0000000..b411875 --- /dev/null +++ b/test/test_config_cache.py @@ -0,0 +1,62 @@ +"""The parsed-config cache in load_config(): it skips the re-read+parse when the file is unchanged +on disk, but must return a fresh copy each call (callers mutate in place), must invalidate on our +own save, and must reload when a peer rewrites the file (os.replace stamps a new mtime).""" +import os +import tempfile +import unittest +from unittest.mock import patch + +import cloudinary_cli.utils.config_utils as cu + + +class TestLoadConfigCache(unittest.TestCase): + def setUp(self): + self._dir = tempfile.mkdtemp() + self._path = os.path.join(self._dir, "config.json") + self._patch = patch.object(cu, "CLOUDINARY_CLI_CONFIG_FILE", self._path) + self._patch.start() + cu._invalidate_config_cache() + self.addCleanup(self._patch.stop) + self.addCleanup(cu._invalidate_config_cache) + + def _write(self, text): + with open(self._path, "w") as f: + f.write(text) + + def test_returns_fresh_copy_so_caller_mutation_does_not_leak(self): + self._write('{"a": "cloudinary://k:s@a"}') + first = cu.load_config() + first["injected"] = "mutated" # callers do cfg.update(...) on the result + second = cu.load_config() + self.assertNotIn("injected", second) # the cache was not poisoned by the caller's mutation + + def test_cache_hit_skips_reparse_when_unchanged(self): + self._write('{"a": "cloudinary://k:s@a"}') + cu.load_config() # populates the cache + with patch.object(cu, "read_json_from_file") as read: + cu.load_config() + read.assert_not_called() # served from cache: no second read+parse + + def test_reloads_when_file_changes_on_disk(self): + self._write('{"a": "cloudinary://k:s@a"}') + self.assertIn("a", cu.load_config()) + # A peer rewrite changes mtime/size; os.utime forces a distinct mtime even on a fast disk. + self._write('{"a": "cloudinary://k:s@a", "b": "cloudinary://k:s@b"}') + os.utime(self._path, (1, 1)) + self.assertIn("b", cu.load_config()) + + def test_save_config_invalidates_cache(self): + self._write('{"a": "cloudinary://k:s@a"}') + cu.load_config() # warm the cache + cu.save_config({"a": "cloudinary://k:s@a", "c": "cloudinary://k:s@c"}) + self.assertIn("c", cu.load_config()) # invalidated -> reloaded our own write + + def test_missing_file_caches_empty_without_error(self): + self.assertEqual({}, cu.load_config()) # no file: empty dict, no exception + self._write('{"a": "cloudinary://k:s@a"}') + os.utime(self._path, (2, 2)) + self.assertIn("a", cu.load_config()) # appears once the file is created + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_config_concurrency.py b/test/test_config_concurrency.py new file mode 100644 index 0000000..12bce31 --- /dev/null +++ b/test/test_config_concurrency.py @@ -0,0 +1,46 @@ +import json +import os +import subprocess +import sys +import tempfile +import unittest + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +# Each worker refreshes config under the cross-process lock; with the lock the read-modify-write +# is serialized, so disjoint keys from concurrent writers must all survive (no last-writer-wins). +_WORKER = """ +import os, sys, time +sys.path.insert(0, {repo!r}) +import cloudinary, cloudinary.api # noqa +from cloudinary_cli.utils.config_utils import update_config +key = sys.argv[1] +update_config({{key: "cloudinary://k:s@" + key}}) +""" + + +class TestConfigConcurrency(unittest.TestCase): + def test_concurrent_writers_lose_no_keys(self): + tmp = tempfile.mkdtemp() + env = dict(os.environ, CLOUDINARY_HOME=tmp) + worker = _WORKER.format(repo=REPO_ROOT) + + n = 12 + procs = [ + subprocess.Popen([sys.executable, "-c", worker, f"cloud{i}"], + env=env, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) + for i in range(n) + ] + for p in procs: + _, err = p.communicate(timeout=60) + self.assertEqual(0, p.returncode, err.decode()) + + with open(os.path.join(tmp, "config.json")) as f: + config = json.load(f) # must be valid JSON (never half-written) + + for i in range(n): + self.assertIn(f"cloud{i}", config) # every concurrent write survived + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_config_permissions.py b/test/test_config_permissions.py new file mode 100644 index 0000000..f1df01a --- /dev/null +++ b/test/test_config_permissions.py @@ -0,0 +1,45 @@ +import json +import os +import stat +import subprocess +import sys +import tempfile +import unittest + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +# save_config resolves CLOUDINARY_CLI_CONFIG_FILE from CLOUDINARY_HOME at import time, so the +# write must happen in a subprocess with CLOUDINARY_HOME pointed at a temp dir. +_WRITER = """ +import sys +sys.path.insert(0, {repo!r}) +import cloudinary, cloudinary.api # noqa +from cloudinary_cli.utils.config_utils import save_config +save_config({{"cloud": "cloudinary://key:secret@cloud?oauth_token=tok&refresh_token=r"}}) +""" + + +@unittest.skipIf(sys.platform == "win32", "POSIX file modes not applicable on Windows") +class TestConfigPermissions(unittest.TestCase): + def test_saved_config_is_owner_only(self): + tmp = tempfile.mkdtemp() + env = dict(os.environ, CLOUDINARY_HOME=tmp) + + proc = subprocess.run( + [sys.executable, "-c", _WRITER.format(repo=REPO_ROOT)], + env=env, capture_output=True, + ) + self.assertEqual(0, proc.returncode, proc.stderr.decode()) + + config_file = os.path.join(tmp, "config.json") + # The file holds api_secret + OAuth tokens, so it must not be group/world readable. + mode = stat.S_IMODE(os.stat(config_file).st_mode) + self.assertEqual(0o600, mode, f"expected 0600, got {oct(mode)}") + + # Sanity: it's still valid JSON carrying the secret-bearing value. + with open(config_file) as f: + self.assertIn("oauth_token", json.load(f)["cloud"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_default_config.py b/test/test_default_config.py new file mode 100644 index 0000000..5cafc89 --- /dev/null +++ b/test/test_default_config.py @@ -0,0 +1,57 @@ +import unittest +from contextlib import contextmanager +from unittest.mock import patch + +import cloudinary_cli.utils.config_utils as config_utils + + +@contextmanager +def _in_memory_config(initial=None): + """Back load_config/save_config with an in-memory dict (no real config.json or lock).""" + store = {"cfg": dict(initial or {})} + + def _load(): + return dict(store["cfg"]) + + def _save(cfg): + store["cfg"] = dict(cfg) + + @contextmanager + def _noop_lock(): + yield + + with patch("cloudinary_cli.utils.config_utils.load_config", side_effect=_load), \ + patch("cloudinary_cli.utils.config_utils.save_config", side_effect=_save), \ + patch("cloudinary_cli.utils.config_utils.config_lock", _noop_lock): + yield store + + +class TestDefaultConfigStorage(unittest.TestCase): + def test_get_set_clear_round_trip(self): + with _in_memory_config({"prod": "cloudinary://k:s@prod"}): + self.assertIsNone(config_utils.get_default_config_name()) + config_utils.set_default_config("prod") + self.assertEqual("prod", config_utils.get_default_config_name()) + config_utils.clear_default_config() + self.assertIsNone(config_utils.get_default_config_name()) + + def test_user_config_names_filters_reserved_key(self): + with _in_memory_config({"prod": "cloudinary://k:s@prod"}): + config_utils.set_default_config("prod") + self.assertEqual(["prod"], config_utils.user_config_names()) + + def test_default_key_present_in_raw_dict_only(self): + with _in_memory_config({"a": "cloudinary://k:s@a", "b": "cloudinary://k:s@b"}): + config_utils.set_default_config("b") + self.assertIn("__default__", config_utils.load_config()) + self.assertNotIn("__default__", config_utils.user_config_names()) + + def test_is_reserved_config_name(self): + self.assertTrue(config_utils.is_reserved_config_name("__default__")) + self.assertTrue(config_utils.is_reserved_config_name("__foo__")) + self.assertFalse(config_utils.is_reserved_config_name("prod")) + self.assertFalse(config_utils.is_reserved_config_name("__prod")) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_file_utils.py b/test/test_file_utils.py index 7dd8f60..d663005 100644 --- a/test/test_file_utils.py +++ b/test/test_file_utils.py @@ -1,7 +1,17 @@ +import os +import stat +import sys +import tempfile import unittest from pathlib import Path +from unittest.mock import patch -from cloudinary_cli.utils.file_utils import get_destination_folder, walk_dir, normalize_file_extension +from cloudinary_cli.utils.file_utils import ( + get_destination_folder, + walk_dir, + normalize_file_extension, + atomic_write, +) from test.helper_test import RESOURCES_DIR @@ -33,3 +43,142 @@ def test_normalize_file_extension(self): "SAMPLE.JPEG": "SAMPLE.jpg", }.items(): self.assertEqual(expected, normalize_file_extension(value)) + + +class AtomicWriteTest(unittest.TestCase): + def setUp(self): + self.dir = tempfile.mkdtemp() + self.path = os.path.join(self.dir, "out.txt") + + def _leftover(self): + return [f for f in os.listdir(self.dir) if f != os.path.basename(self.path)] + + def test_writes_content(self): + atomic_write(self.path, lambda f: f.write("hello")) + with open(self.path) as f: + self.assertEqual("hello", f.read()) + + def test_overwrite_replaces_contents(self): + atomic_write(self.path, lambda f: f.write("old")) + atomic_write(self.path, lambda f: f.write("new")) + with open(self.path) as f: + self.assertEqual("new", f.read()) + + def test_leaves_no_temp_files(self): + atomic_write(self.path, lambda f: f.write("x")) + self.assertEqual([], self._leftover()) + + def test_failed_write_removes_temp_and_keeps_original(self): + atomic_write(self.path, lambda f: f.write("keep")) + + def boom(f): + f.write("partial") + raise ValueError("write failed") + + with self.assertRaises(ValueError): + atomic_write(self.path, boom) + + with open(self.path) as f: + self.assertEqual("keep", f.read()) + self.assertEqual([], self._leftover()) + + def test_missing_target_is_not_created_on_failure(self): + with self.assertRaises(ValueError): + atomic_write(self.path, lambda f: (_ for _ in ()).throw(ValueError())) + self.assertFalse(os.path.exists(self.path)) + self.assertEqual([], os.listdir(self.dir)) + + @unittest.skipIf(sys.platform == "win32", "POSIX file modes not applicable on Windows") + def test_normalizes_to_umask_mode(self): + # mkstemp creates the temp as 0600; atomic_write must relax it to the umask default + # so output files are not silently owner-only. + old_umask = os.umask(0o022) + try: + atomic_write(self.path, lambda f: f.write("x")) + finally: + os.umask(old_umask) + mode = stat.S_IMODE(os.stat(self.path).st_mode) + self.assertEqual(0o644, mode) + + @unittest.skipIf(sys.platform == "win32", "POSIX file modes not applicable on Windows") + def test_respects_restrictive_umask(self): + old_umask = os.umask(0o077) + try: + atomic_write(self.path, lambda f: f.write("x")) + finally: + os.umask(old_umask) + mode = stat.S_IMODE(os.stat(self.path).st_mode) + self.assertEqual(0o600, mode) + + @unittest.skipIf(sys.platform == "win32", "POSIX permission bits") + def test_explicit_mode_overrides_umask(self): + # A4: with an explicit mode the result is that mode regardless of a permissive umask, so the + # config file is never widened to the umask default. + old_umask = os.umask(0o000) + try: + atomic_write(self.path, lambda f: f.write("x"), mode=0o600) + finally: + os.umask(old_umask) + self.assertEqual(0o600, stat.S_IMODE(os.stat(self.path).st_mode)) + + @unittest.skipIf(sys.platform == "win32", "POSIX permission bits") + def test_explicit_mode_temp_file_never_wider_during_write(self): + # The temp file must already carry the final mode before the replace, so there is no instant + # at which the destination is world-readable. Capture the temp file's mode at replace time. + seen = {} + real_replace = os.replace + + def capturing_replace(src, dst): + seen["mode"] = stat.S_IMODE(os.stat(src).st_mode) + return real_replace(src, dst) + + old_umask = os.umask(0o000) + try: + with patch("cloudinary_cli.utils.file_utils.os.replace", side_effect=capturing_replace): + atomic_write(self.path, lambda f: f.write("x"), mode=0o600) + finally: + os.umask(old_umask) + self.assertEqual(0o600, seen["mode"]) # 0600 on the temp file, before it becomes the target + + def test_writes_to_filename_in_cwd_without_dir(self): + # path.dirname("") is "" -> must fall back to "." rather than failing. + old_cwd = os.getcwd() + os.chdir(self.dir) + try: + atomic_write("bare.txt", lambda f: f.write("x")) + with open("bare.txt") as f: + self.assertEqual("x", f.read()) + finally: + os.chdir(old_cwd) + + @unittest.skipIf(sys.platform == "win32", "POSIX directory modes not applicable on Windows") + @unittest.skipIf(hasattr(os, "geteuid") and os.geteuid() == 0, "root bypasses permission bits") + def test_readonly_directory_raises_and_leaves_nothing(self): + # mkstemp needs to create the temp inside the directory, so a read-only directory must + # fail loudly rather than silently writing nothing, and must not leave a temp file behind. + os.chmod(self.dir, 0o500) + try: + with self.assertRaises(OSError): + atomic_write(self.path, lambda f: f.write("x")) + self.assertFalse(os.path.exists(self.path)) + self.assertEqual([], os.listdir(self.dir)) + finally: + os.chmod(self.dir, 0o700) + + @unittest.skipIf(sys.platform == "win32", "POSIX file modes not applicable on Windows") + @unittest.skipIf(hasattr(os, "geteuid") and os.geteuid() == 0, "root bypasses permission bits") + def test_overwrites_readonly_target_in_writable_dir(self): + # os.replace only needs write permission on the directory, not the target, so atomic_write + # can replace a read-only file (where a plain open(file, 'w') would fail) and normalizes + # the result to the umask default. + old_umask = os.umask(0o022) + try: + atomic_write(self.path, lambda f: f.write("old")) + os.chmod(self.path, 0o400) + atomic_write(self.path, lambda f: f.write("new")) + finally: + os.umask(old_umask) + with open(self.path) as f: + self.assertEqual("new", f.read()) + self.assertEqual(0o644, stat.S_IMODE(os.stat(self.path).st_mode)) + self.assertEqual([], self._leftover()) diff --git a/test/test_json_utils.py b/test/test_json_utils.py new file mode 100644 index 0000000..381a126 --- /dev/null +++ b/test/test_json_utils.py @@ -0,0 +1,145 @@ +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +from cloudinary_cli.utils.json_utils import ( + write_json_to_file, + read_json_from_file, + update_json_file, + print_json, +) + + +class WriteJsonToFileTest(unittest.TestCase): + def setUp(self): + self.dir = tempfile.mkdtemp() + self.path = os.path.join(self.dir, "config.json") + + def _leftover(self): + return [f for f in os.listdir(self.dir) if f != "config.json"] + + @unittest.skipIf(sys.platform == "win32", "POSIX directory modes not applicable on Windows") + @unittest.skipIf(hasattr(os, "geteuid") and os.geteuid() == 0, "root bypasses permission bits") + def test_readonly_directory_raises_in_both_modes(self): + os.chmod(self.dir, 0o500) + try: + with self.assertRaises(OSError): + write_json_to_file({"a": 1}, self.path, atomic=True) + with self.assertRaises(OSError): + write_json_to_file({"a": 1}, self.path, atomic=False) + self.assertFalse(os.path.exists(self.path)) + self.assertEqual([], os.listdir(self.dir)) + finally: + os.chmod(self.dir, 0o700) + + def test_writes_valid_json(self): + write_json_to_file({"a": 1, "b": "two"}, self.path) + self.assertEqual({"a": 1, "b": "two"}, read_json_from_file(self.path)) + + def test_overwrite_replaces_contents(self): + write_json_to_file({"old": True}, self.path) + write_json_to_file({"new": True}, self.path) + self.assertEqual({"new": True}, read_json_from_file(self.path)) + + def test_respects_indent_and_sort_keys(self): + write_json_to_file({"b": 1, "a": 2}, self.path, indent=2, sort_keys=True) + with open(self.path) as f: + content = f.read() + self.assertEqual('{\n "a": 2,\n "b": 1\n}', content) + + def test_atomic_writes_valid_json(self): + write_json_to_file({"a": 1}, self.path, atomic=True) + self.assertEqual({"a": 1}, read_json_from_file(self.path)) + + def test_atomic_leaves_no_temp_files(self): + write_json_to_file({"a": 1}, self.path, atomic=True) + self.assertEqual([], self._leftover()) + + def test_atomic_failed_write_removes_temp_and_keeps_original(self): + write_json_to_file({"keep": True}, self.path, atomic=True) + # An unserializable object makes json.dump raise mid-write. + with self.assertRaises(TypeError): + write_json_to_file({"bad": object()}, self.path, atomic=True) + self.assertEqual({"keep": True}, read_json_from_file(self.path)) + self.assertEqual([], self._leftover()) + + def test_non_atomic_is_default(self): + # The non-atomic path writes in place: open('w') truncates the target up front, so a + # mid-write failure leaves a corrupted file. This documents why atomic=True exists and + # must be opted into for files that matter (config, sync meta). + write_json_to_file({"keep": True}, self.path) + with self.assertRaises(TypeError): + write_json_to_file({"bad": object()}, self.path) + with self.assertRaises(ValueError): # JSONDecodeError on the partial write + read_json_from_file(self.path) + + +class UpdateJsonFileTest(unittest.TestCase): + def setUp(self): + self.dir = tempfile.mkdtemp() + self.path = os.path.join(self.dir, "data.json") + + def test_creates_file_when_missing(self): + update_json_file({"a": 1}, self.path) + self.assertEqual({"a": 1}, read_json_from_file(self.path)) + + def test_merges_into_existing(self): + write_json_to_file({"a": 1, "b": 2}, self.path) + update_json_file({"b": 20, "c": 3}, self.path) + self.assertEqual({"a": 1, "b": 20, "c": 3}, read_json_from_file(self.path)) + + def test_atomic_flag_merges_and_leaves_no_temp(self): + write_json_to_file({"a": 1}, self.path) + update_json_file({"b": 2}, self.path, atomic=True) + self.assertEqual({"a": 1, "b": 2}, read_json_from_file(self.path)) + leftover = [f for f in os.listdir(self.dir) if f != "data.json"] + self.assertEqual([], leftover) + + +class PrintJsonColorTest(unittest.TestCase): + """print_json must emit plain (parseable) JSON when stdout is not a terminal, so piped/captured + output (LLM agents, `| jq`, redirects) is never corrupted by ANSI color escapes.""" + + DATA = {"a": 1, "b": "two", "nested": {"c": True}} + + def _captured(self): + with patch("cloudinary_cli.utils.json_utils.click.echo") as echo: + print_json(self.DATA) + return echo.call_args[0][0] + + def test_non_tty_is_plain_parseable_json(self): + with patch("cloudinary_cli.utils.json_utils.sys.stdout.isatty", return_value=False): + out = self._captured() + self.assertNotIn("\x1b[", out) # no ANSI escapes + self.assertEqual(self.DATA, json.loads(out)) # round-trips cleanly + + def test_tty_is_colorized(self): + with patch("cloudinary_cli.utils.json_utils.sys.stdout.isatty", return_value=True): + out = self._captured() + self.assertIn("\x1b[", out) # colorized for an interactive terminal + + def test_non_tty_is_plain_on_windows_too(self): + # The automation guarantee must hold on every OS: a non-terminal stdout yields plain JSON + # regardless of platform (the old code special-cased Windows; the branch is OS-independent now). + with patch("cloudinary_cli.utils.json_utils.sys.platform", "win32"), \ + patch("cloudinary_cli.utils.json_utils.sys.stdout.isatty", return_value=False): + out = self._captured() + self.assertNotIn("\x1b[", out) + self.assertEqual(self.DATA, json.loads(out)) + + def test_colorization_is_decided_by_isatty_not_os(self): + # Same isatty() value -> same colorization decision under any reported platform, so Windows + # interactive consoles get color (click.echo translates ANSI) and Windows pipes stay plain. + for plat in ("win32", "darwin", "linux"): + with patch("cloudinary_cli.utils.json_utils.sys.platform", plat): + with patch("cloudinary_cli.utils.json_utils.sys.stdout.isatty", return_value=True): + self.assertIn("\x1b[", self._captured(), f"expected color on tty ({plat})") + with patch("cloudinary_cli.utils.json_utils.sys.stdout.isatty", return_value=False): + self.assertNotIn("\x1b[", self._captured(), f"expected plain off tty ({plat})") + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_modules/test_cli_clone.py b/test/test_modules/test_cli_clone.py index a4843b1..7abe383 100644 --- a/test/test_modules/test_cli_clone.py +++ b/test/test_modules/test_cli_clone.py @@ -313,5 +313,38 @@ def test_process_metadata_restricted_raw_asset_with_auth_token(self, mock_cloudi self.assertEqual(url, ('https://res.cloudinary.com/demo/raw/upload/s--XyZaBcDeF--/sample_document', {})) +class TestCloneOAuthTarget(unittest.TestCase): + def _oauth_target_config(self): + import cloudinary + from cloudinary_cli.auth.session import Session, to_cloudinary_url + import time + session = Session( + cloud_name="target_cloud", access_token="eyJ.access.tok", + refresh_token="rt", expires_at=int(time.time()) + 3600, + region="api-eu", issuer="https://oauth.cloudinary.com/") + config = cloudinary.Config() + config._setup_from_parsed_url(config._parse_cloudinary_url(to_cloudinary_url(session))) + return config + + def test_upload_list_drops_oauth_bookkeeping(self): + source_assets = { + 'resources': [{ + 'public_id': 'sample', 'type': 'upload', 'resource_type': 'image', + 'format': 'jpg', + 'secure_url': 'https://res.cloudinary.com/demo/image/upload/v1/sample.jpg', + }] + } + upload_list = clone_module._prepare_upload_list( + source_assets, self._oauth_target_config(), overwrite=False, + async_=False, notification_url=None, auth_token=None, + url_expiry=3600, fields=()) + + _, options = upload_list[0] + for leaked in ("refresh_token", "expires_at", "region", "issuer"): + self.assertNotIn(leaked, options) + self.assertEqual("eyJ.access.tok", options["oauth_token"]) + self.assertEqual("target_cloud", options["cloud_name"]) + + if __name__ == '__main__': unittest.main() diff --git a/test/test_oauth_token_seam.py b/test/test_oauth_token_seam.py new file mode 100644 index 0000000..b3a1558 --- /dev/null +++ b/test/test_oauth_token_seam.py @@ -0,0 +1,144 @@ +"""The redesign's central premise: the active OAuth config refreshes its token through the single +SDK seam, `cloudinary.config().oauth_token`, read at request build time. These tests exercise that +seam the way the SDK does (call_api / uploader) rather than reading oauth_token directly, and pin +the post-resolve invariant that the active config is always an OAuthConfig.""" +import time +import unittest +from unittest.mock import patch + +import cloudinary + +from cloudinary_cli.auth.oauth_config import OAuthConfig, install_oauth_config, install_env_config +from cloudinary_cli.auth.session import Session, to_cloudinary_url + + +def _url(cloud="eu-cloud", token="eyJ.tok", refresh="rt", region="api-eu", expires_delta=300): + return to_cloudinary_url(Session( + cloud_name=cloud, access_token=token, refresh_token=refresh, + expires_at=int(time.time()) + expires_delta, region=region, + issuer="https://oauth.cloudinary.com/")) + + +class _RestoresSdkConfig(unittest.TestCase): + def setUp(self): + import os + self._env_snapshot = dict(os.environ) + for key in [k for k in os.environ if k.startswith("CLOUDINARY_")]: + del os.environ[key] + cloudinary.reset_config() + self.addCleanup(self._restore) + + def _restore(self): + import os + os.environ.clear() + os.environ.update(self._env_snapshot) + cloudinary.reset_config() + + +class TestSdkSeamTriggersRefresh(_RestoresSdkConfig): + """The SDK reads cloudinary.config().oauth_token to build the Authorization header; that read + (not any CLI-side probe) is what refreshes a stale token.""" + + def _saved_stale(self): + return {"eu-cloud": _url(token="eyJ.old.tok", refresh="rt_old", expires_delta=-10)} + + def test_call_api_authorize_path_refreshes_stale_token(self): + saved = self._saved_stale() + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + install_oauth_config(saved["eu-cloud"], saved_name="eu-cloud") + # Reproduce verbatim the read cloudinary.api_client.call_api performs at request build + # time (call_api.py:63): options.pop("oauth_token", cloudinary.config().oauth_token). + options = {} + oauth_token = options.pop("oauth_token", cloudinary.config().oauth_token) + self.assertEqual("eyJ.new.tok", oauth_token) + + def test_uploader_header_path_refreshes_stale_token(self): + saved = self._saved_stale() + token_response = {"access_token": "eyJ.fresh.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response), \ + patch("cloudinary_cli.auth.update_config"): + install_oauth_config(saved["eu-cloud"], saved_name="eu-cloud") + # The uploader reads the same attribute to set the Bearer header (uploader.py:877): + # oauth_token = options.get("oauth_token", cloudinary.config().oauth_token). + import cloudinary.uploader # noqa: F401 (ensures the seam module is importable) + options = {} + token = options.get("oauth_token", cloudinary.config().oauth_token) + self.assertEqual("eyJ.fresh.tok", token) + + def test_seam_read_refreshes_only_once_then_serves_cached(self): + saved = self._saved_stale() + token_response = {"access_token": "eyJ.new.tok", "refresh_token": "rt_new", "expires_in": 300} + with patch("cloudinary_cli.utils.config_utils.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.load_config", return_value=dict(saved)), \ + patch("cloudinary_cli.auth.flow.refresh", return_value=token_response) as refresh, \ + patch("cloudinary_cli.auth.update_config"): + install_oauth_config(saved["eu-cloud"], saved_name="eu-cloud") + first = cloudinary.config().oauth_token + second = cloudinary.config().oauth_token + self.assertEqual("eyJ.new.tok", first) + self.assertEqual("eyJ.new.tok", second) + refresh.assert_called_once() # the now-fresh _session short-circuits the second read + + +class TestPostResolveInvariant(_RestoresSdkConfig): + """Not-done item #5 / Caveat B: every install seam leaves an OAuthConfig as the active global, so + has_oauth is universal and self-refresh is never silently disabled by a plain Config swap.""" + + runner = None + + def test_saved_oauth_install_is_oauthconfig(self): + install_oauth_config(_url(), saved_name="eu-cloud") + self.assertIsInstance(cloudinary.config(), OAuthConfig) + + def test_inline_url_install_is_oauthconfig(self): + install_oauth_config("cloudinary://key:secret@cloud", saved_name=None) + self.assertIsInstance(cloudinary.config(), OAuthConfig) + + def test_env_install_is_oauthconfig(self): + import os + os.environ["CLOUDINARY_URL"] = "cloudinary://k:s@env_cloud" + cloudinary.reset_config() + install_env_config() + self.assertIsInstance(cloudinary.config(), OAuthConfig) + + def test_resolver_leaves_oauthconfig_for_every_branch(self): + from click.testing import CliRunner + from cloudinary_cli.cli import cli + import os + saved = {"__default__": "eu-cloud", "eu-cloud": _url()} + with patch("cloudinary_cli.utils.config_resolver.load_config", return_value=dict(saved)): + for key in ("CLOUDINARY_URL", "CLOUDINARY_CLOUD_NAME", "CLOUDINARY_API_KEY", + "CLOUDINARY_API_SECRET"): + os.environ.pop(key, None) + cloudinary.reset_config() + # default branch + CliRunner().invoke(cli, ['url', 'sample']) + self.assertIsInstance(cloudinary.config(), OAuthConfig) + + +class TestEnvConfigStatic(_RestoresSdkConfig): + """An env-installed OAuthConfig is static: it has no saved name, so reading oauth_token never + refreshes even if the token is expired (it cannot rotate an env-supplied token).""" + + def test_env_oauth_token_never_refreshes_even_when_stale(self): + import os + os.environ["CLOUDINARY_URL"] = ( + "cloudinary://env_cloud?oauth_token=eyJ.env.tok&refresh_token=rt&" + f"expires_at={int(time.time()) - 10}®ion=api") + cloudinary.reset_config() + with patch("cloudinary_cli.auth.flow.refresh") as refresh: + cfg = install_env_config() + token = cfg.oauth_token + self.assertEqual("eyJ.env.tok", token) + refresh.assert_not_called() + self.assertIsNone(getattr(cfg, "_session")) # static: no parsed session to drive a refresh + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_utils.py b/test/test_utils.py index b77b3fe..d186d2d 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -1,7 +1,57 @@ +import builtins import unittest +from unittest.mock import patch from cloudinary_cli.utils.utils import parse_option_value, parse_args_kwargs, whitelist_keys, merge_responses, \ - normalize_list_params, chunker, group_params + normalize_list_params, chunker, group_params, confirm_action, get_user_action, prompt_user, is_interactive + + +class NonInteractiveInputTest(unittest.TestCase): + """A confirmation/selection prompt on closed (non-interactive) stdin must apply the default and + surface a hint, not raise EOFError up to a blank 'Command execution failed' with exit 0.""" + + def _eof(self, *args): + raise EOFError("EOF when reading a line") + + def test_confirm_action_defaults_to_no_on_eof(self): + with patch.object(builtins, "input", self._eof), \ + patch("cloudinary_cli.utils.utils.logger.warning") as warn: + self.assertFalse(confirm_action()) + warn.assert_called_once() + self.assertIn("--force", warn.call_args[0][0]) + + def test_get_user_action_returns_default_on_eof(self): + with patch.object(builtins, "input", self._eof): + self.assertEqual("fallback", + get_user_action("pick: ", {"y": True, "default": "fallback"})) + + def test_get_user_action_no_hint_when_not_provided(self): + with patch.object(builtins, "input", self._eof), \ + patch("cloudinary_cli.utils.utils.logger.warning") as warn: + self.assertIsNone(get_user_action("pick: ", {"y": True})) + warn.assert_not_called() + + def test_empty_line_still_uses_default(self): + # An empty line (piped) is distinct from EOF and already used the default; keep that intact. + with patch.object(builtins, "input", lambda *a: ""): + self.assertFalse(confirm_action()) + + def test_prompt_user_returns_line_when_available(self): + with patch.object(builtins, "input", lambda *a: " 2 "): + self.assertEqual(" 2 ", prompt_user("pick: ")) + + def test_prompt_user_returns_none_and_hints_on_eof(self): + with patch.object(builtins, "input", self._eof), \ + patch("cloudinary_cli.utils.utils.logger.warning") as warn: + self.assertIsNone(prompt_user("pick: ", noninteractive_hint="do X instead")) + warn.assert_called_once() + self.assertIn("do X instead", warn.call_args[0][0]) + + def test_is_interactive_reflects_stdin_isatty(self): + with patch("cloudinary_cli.utils.utils.sys.stdin.isatty", return_value=True): + self.assertTrue(is_interactive()) + with patch("cloudinary_cli.utils.utils.sys.stdin.isatty", return_value=False): + self.assertFalse(is_interactive()) class UtilsTest(unittest.TestCase):