diff --git a/bfabric/docs/changelog.md b/bfabric/docs/changelog.md index 0b36f222..88d5ebce 100644 --- a/bfabric/docs/changelog.md +++ b/bfabric/docs/changelog.md @@ -10,6 +10,7 @@ Minor breaking changes are still possible in `1.X.Y` but we try to announce them ## \[Unreleased\] - `ResultContainer.to_polars()` returns an empty DataFrame for an empty result set instead of raising `polars.exceptions.NoDataError`, fixing a crash in `bfabric-cli api read` when a query matched no records. +- OAuth: `Bfabric.connect_oauth` / `connect_pkce` / `connect_device_code` (and the underlying `_oauth` helpers) now require explicit `client_id` and `scope` — the core library no longer bakes in a `"CLI"` client ID or a default scope. Those defaults are now CLI policy (`bfabric-cli auth …` supplies them unchanged). `connect()` with `auth_method: oauth` now raises if the config environment has no `client_id` instead of falling back to `"CLI"`. - PKCE login: the browser callback page now renders a distinct, styled "Login failed" page showing the provider's error (e.g. a two-factor-enrollment requirement) instead of always claiming "Login successful". - OAuth token-acquisition failures (expired/revoked refresh token, unreachable token endpoint) now raise a clear `BfabricOAuthError` instead of leaking an `authlib`/`requests` traceback. diff --git a/bfabric/docs/design/oauth_integration.md b/bfabric/docs/design/oauth_integration.md index 8281d39a..efa8d125 100644 --- a/bfabric/docs/design/oauth_integration.md +++ b/bfabric/docs/design/oauth_integration.md @@ -14,7 +14,6 @@ Private module under `bfabric/src/bfabric/_oauth/` implementing all OAuth primit | File | Purpose | |------|---------| -| `_constants.py` | `DEFAULT_CLIENT_ID = "CLI"`, `DEFAULT_OAUTH_SCOPE = "api:read api:write openid profile email groups"` | | `credential_provider.py` | `OAuthCredentialProvider` — thread-safe token management with automatic refresh and disk caching. Supports both `client_credentials` and `refresh_token` grant types. | | `pkce.py` | `pkce_login()` — browser-based PKCE flow. Starts a local HTTP server, opens the browser, exchanges the authorization code for tokens. | | `device_code.py` | `device_code_login()` — RFC 8628 device authorization flow for headless environments. | @@ -23,6 +22,8 @@ Private module under `bfabric/src/bfabric/_oauth/` implementing all OAuth primit | `url_token.py` | `UrlTokenContext` + `parse_url_token()` — extracts entity context (entity_id, application_id, etc.) from B-Fabric URL token JWTs. | | `webapp_client.py` | `WebappClient` — dual-identity client bundling a `user` (from URL token) and `service` (from client credentials) `Bfabric` instance. | +The core `_oauth` API requires explicit `client_id` and `scope` arguments on all OAuth entry points. The library does not bake in a default client ID or scope. Tools like the CLI specify these values explicitly. + --- ## New: Factory methods on `Bfabric` diff --git a/bfabric/docs/design/oauth_usage_and_troubleshooting.md b/bfabric/docs/design/oauth_usage_and_troubleshooting.md index 393094c3..4c63b6b1 100644 --- a/bfabric/docs/design/oauth_usage_and_troubleshooting.md +++ b/bfabric/docs/design/oauth_usage_and_troubleshooting.md @@ -75,8 +75,14 @@ Key identity difference: **client_credentials tokens carry no user `sub` and no like `containers`.** If a resource server authorizes by *your* container membership, a service token won't do — you need a user flow (PKCE or device code). -`DEFAULT_OAUTH_SCOPE` is `"api:read api:write openid profile email groups"` — it **includes -`groups`** (the employee file-access path, see below) but not `containers` or `download`. +The core `Bfabric` OAuth methods take `client_id` and `scope` as **required** arguments — there is +no baked-in default. The `bfabric-cli` login commands (`auth login` / `auth device-code`) likewise +**require** an explicit `--scope`: pick a named preset — `read-only` (`api:read`), `read-write` +(`api:write`, which implies `api:read`), or `upload` (`api:write tus`) — or pass any scope string. +These login presets are **minimal API scopes** — they do **not** include `groups` (the employee +file-access path, see below) or the OIDC scopes, so request those explicitly when you need them. +Client/webapp registration (`auth register` / `auth register-webapp`) keeps the broader +OIDC-inclusive default. --- @@ -92,13 +98,14 @@ File/download access is authorized by **container membership**, which a token ca > **Employees: request the `groups` scope — this is the practical PKCE path.** > The **`containers` scope cannot be requested via PKCE** (it is restricted; the server silently > drops it — which is exactly why requesting `containers` in an interactive flow yields a token -> *without* the claim and no error). **`groups`, however, is requestable and is already in -> `DEFAULT_OAUTH_SCOPE`.** So an employee gets file access from a normal user flow as long as -> `groups` is in the requested scope: +> *without* the claim and no error). **`groups`, however, is requestable.** It is **not** part of +> the CLI's login presets (those are minimal API scopes), so pass it explicitly to get file access +> from a normal user flow: > > ```python > client = Bfabric.connect_pkce( > "https://fgcz-bfabric-demo.uzh.ch/bfabric", +> client_id="CLI", > scope="openid profile email api:read api:write groups", > ) > ``` diff --git a/bfabric/docs/user_guides/bfabric-cli/workunits.md b/bfabric/docs/user_guides/bfabric-cli/workunits.md index b539cc5d..97e0ba80 100644 --- a/bfabric/docs/user_guides/bfabric-cli/workunits.md +++ b/bfabric/docs/user_guides/bfabric-cli/workunits.md @@ -147,7 +147,7 @@ carries the `tus` scope. Install the extra and authenticate once: ```bash pip install 'bfabric[transfer]' -bfabric-cli auth login --scope "api:read api:write openid profile email groups tus" +bfabric-cli auth login --scope "api:write tus" ``` ### Basic Usage diff --git a/bfabric/src/bfabric/_oauth/__init__.py b/bfabric/src/bfabric/_oauth/__init__.py index e2353580..43eac085 100644 --- a/bfabric/src/bfabric/_oauth/__init__.py +++ b/bfabric/src/bfabric/_oauth/__init__.py @@ -1,6 +1,5 @@ """OAuth integration for bfabricPy.""" -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric._oauth.credential_provider import OAuthCredentialProvider from bfabric._oauth.device_code import device_code_login from bfabric._oauth.pkce import pkce_login @@ -10,8 +9,6 @@ from bfabric._oauth.webapp_client import WebappClient __all__ = [ - "DEFAULT_CLIENT_ID", - "DEFAULT_OAUTH_SCOPE", "OAuthCredentialProvider", "UrlTokenContext", "WebappClient", diff --git a/bfabric/src/bfabric/_oauth/_constants.py b/bfabric/src/bfabric/_oauth/_constants.py deleted file mode 100644 index e9ad9d48..00000000 --- a/bfabric/src/bfabric/_oauth/_constants.py +++ /dev/null @@ -1,4 +0,0 @@ -"""Shared constants for the OAuth module.""" - -DEFAULT_CLIENT_ID = "CLI" -DEFAULT_OAUTH_SCOPE = "api:read api:write openid profile email groups" diff --git a/bfabric/src/bfabric/_oauth/credential_provider.py b/bfabric/src/bfabric/_oauth/credential_provider.py index f0c8dcd7..dbc39402 100644 --- a/bfabric/src/bfabric/_oauth/credential_provider.py +++ b/bfabric/src/bfabric/_oauth/credential_provider.py @@ -27,7 +27,6 @@ from bfabric.config.bfabric_auth import OAUTH_LOGIN, BfabricAuth from bfabric.errors import BfabricOAuthError -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric._oauth.token_cache import TokenCache if TYPE_CHECKING: @@ -66,7 +65,7 @@ def __init__( client_secret: str, token_url: str, *, - scope: str = DEFAULT_OAUTH_SCOPE, + scope: str, token: dict[str, object] | None = None, grant_type: str = "client_credentials", token_cache_path: Path | None = None, diff --git a/bfabric/src/bfabric/_oauth/device_code.py b/bfabric/src/bfabric/_oauth/device_code.py index 89f03bc7..71132374 100644 --- a/bfabric/src/bfabric/_oauth/device_code.py +++ b/bfabric/src/bfabric/_oauth/device_code.py @@ -18,7 +18,6 @@ import httpx from loguru import logger -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric.errors import BfabricOAuthError @@ -139,8 +138,8 @@ def _poll_for_token( def device_code_login( base_url: str, *, - client_id: str = DEFAULT_CLIENT_ID, - scope: str = DEFAULT_OAUTH_SCOPE, + client_id: str, + scope: str, timeout: float = 600.0, ) -> dict[str, object]: """Perform an OAuth 2.0 Device Authorization Grant flow (RFC 8628). diff --git a/bfabric/src/bfabric/_oauth/pkce.py b/bfabric/src/bfabric/_oauth/pkce.py index 05e71a99..cbf7b52b 100644 --- a/bfabric/src/bfabric/_oauth/pkce.py +++ b/bfabric/src/bfabric/_oauth/pkce.py @@ -26,7 +26,6 @@ import httpx from loguru import logger -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric.errors import BfabricOAuthError @@ -180,8 +179,8 @@ def _exchange_code( def pkce_login( base_url: str, *, - client_id: str = DEFAULT_CLIENT_ID, - scope: str = DEFAULT_OAUTH_SCOPE, + client_id: str, + scope: str, port: int = 0, open_browser: bool = True, timeout: float = 120.0, diff --git a/bfabric/src/bfabric/_oauth/registration.py b/bfabric/src/bfabric/_oauth/registration.py index 70cf6360..c44412ce 100644 --- a/bfabric/src/bfabric/_oauth/registration.py +++ b/bfabric/src/bfabric/_oauth/registration.py @@ -7,8 +7,6 @@ import httpx from loguru import logger -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE - if TYPE_CHECKING: from bfabric.bfabric import Bfabric from bfabric.results.result_container import ResultContainer @@ -39,8 +37,8 @@ def register_client( client_name: str, redirect_uri: str, *, + scope: str, service_user: str | None = None, - scope: str = DEFAULT_OAUTH_SCOPE, grant_types: list[str] | None = None, ) -> dict[str, object]: """Register a new OAuth client with the B-Fabric server. @@ -55,8 +53,8 @@ def register_client( :param token: Employee Bearer token for authorization :param client_name: Human-readable name for the client :param redirect_uri: OAuth redirect URI for the client - :param service_user: Optional service user login to enable ``client_credentials`` grant :param scope: OAuth scope string + :param service_user: Optional service user login to enable ``client_credentials`` grant :param grant_types: Explicit list of grant types to request (overrides the default) :returns: Registration response containing ``client_id``, ``client_secret``, etc. """ @@ -90,8 +88,8 @@ def register_webapp( app_name: str, web_url: str, *, + scope: str, service_user: str | None = None, - scope: str = DEFAULT_OAUTH_SCOPE, application_id: int | None = None, technology_id: int | None = None, description: str | None = None, @@ -108,8 +106,8 @@ def register_webapp( :param token: Employee Bearer token for the OAuth registration endpoint :param app_name: Human-readable name for both the OAuth client and the application :param web_url: The webapp URL (used as both OAuth redirect URI and application ``weburl``) - :param service_user: Optional service user login to enable ``client_credentials`` grant :param scope: OAuth scope string + :param service_user: Optional service user login to enable ``client_credentials`` grant :param application_id: Existing application ID to update (omit to create a new application) :param technology_id: Technology ID for the application :param description: Application description diff --git a/bfabric/src/bfabric/_oauth/webapp_client.py b/bfabric/src/bfabric/_oauth/webapp_client.py index 34d510fd..7cf35a55 100644 --- a/bfabric/src/bfabric/_oauth/webapp_client.py +++ b/bfabric/src/bfabric/_oauth/webapp_client.py @@ -5,8 +5,6 @@ from dataclasses import dataclass from typing import TYPE_CHECKING -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE - if TYPE_CHECKING: from pathlib import Path @@ -35,7 +33,7 @@ def create( *, client_id: str, client_secret: str, - scope: str = DEFAULT_OAUTH_SCOPE, + scope: str, user_token_cache_path: Path | None = None, service_token_cache_path: Path | None = None, ) -> WebappClient: @@ -80,6 +78,7 @@ def create( client_id=client_id, client_secret=client_secret, token_url=token_url, + scope="", token=token_dict, grant_type="refresh_token", token_cache_path=user_token_cache_path, diff --git a/bfabric/src/bfabric/bfabric.py b/bfabric/src/bfabric/bfabric.py index 0bc60d38..85727ec8 100644 --- a/bfabric/src/bfabric/bfabric.py +++ b/bfabric/src/bfabric/bfabric.py @@ -28,7 +28,6 @@ from loguru import logger from rich.console import Console -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric.config import DEFAULT_CONFIG_FILE, BfabricAuth, BfabricClientConfig from bfabric.config.bfabric_client_config import BfabricAPIEngineType from bfabric.config.config_data import ConfigData, load_config_data @@ -128,7 +127,12 @@ def _connect_oauth_from_config(cls, config_data: ConfigData) -> Bfabric: from bfabric._oauth.token_cache import TokenCache, compute_token_cache_path base_url = config_data.client.base_url.rstrip("/") - client_id = config_data.client_id or DEFAULT_CLIENT_ID + if not config_data.client_id: + raise ValueError( + "OAuth config is missing 'client_id'. Set it in the config environment " + "(e.g. re-run 'bfabric-cli auth login' or 'bfabric-cli auth device-code')." + ) + client_id = config_data.client_id env_name = config_data.env_name or "default" cache_path = compute_token_cache_path(base_url, client_id, env_name).expanduser() if not TokenCache(cache_path).load(): @@ -137,6 +141,7 @@ def _connect_oauth_from_config(cls, config_data: ConfigData) -> Bfabric: client_id=client_id, client_secret="", token_url=f"{base_url}/rest/oauth/token", + scope="", grant_type="refresh_token", token_cache_path=cache_path, ) @@ -230,7 +235,7 @@ def connect_oauth( client_secret: str, base_url: str, *, - scope: str = DEFAULT_OAUTH_SCOPE, + scope: str, token_cache_path: Path | None = None, ) -> Bfabric: """Returns a new Bfabric instance that authenticates via OAuth 2.0 client credentials. @@ -266,8 +271,8 @@ def connect_pkce( cls, base_url: str, *, - client_id: str = DEFAULT_CLIENT_ID, - scope: str = DEFAULT_OAUTH_SCOPE, + client_id: str, + scope: str, port: int = 0, open_browser: bool = True, timeout: float = 120.0, @@ -318,8 +323,8 @@ def connect_device_code( cls, base_url: str, *, - client_id: str = DEFAULT_CLIENT_ID, - scope: str = DEFAULT_OAUTH_SCOPE, + client_id: str, + scope: str, timeout: float = 600.0, token_cache_path: Path | None = None, ) -> Bfabric: diff --git a/bfabric/src/bfabric/examples/prove_tus_resume.py b/bfabric/src/bfabric/examples/prove_tus_resume.py index 66feba16..583b0e43 100644 --- a/bfabric/src/bfabric/examples/prove_tus_resume.py +++ b/bfabric/src/bfabric/examples/prove_tus_resume.py @@ -25,7 +25,7 @@ Prerequisites: an OAuth-backed config env with the ``tus`` scope. Authenticate once with:: - bfabric-cli auth login --scope "api:read api:write openid profile email groups tus" + bfabric-cli auth login --scope "api:write tus" then run like the equivalent CLI upload:: diff --git a/bfabric/src/bfabric/transfer/tokens.py b/bfabric/src/bfabric/transfer/tokens.py index 0264c0eb..c4f0bdc7 100644 --- a/bfabric/src/bfabric/transfer/tokens.py +++ b/bfabric/src/bfabric/transfer/tokens.py @@ -25,8 +25,10 @@ from bfabric import Bfabric from bfabric.config import BfabricAuth -# Scopes the CLI is pre-registered for but that DEFAULT_OAUTH_SCOPE does not request by default. -_PKCE_SCOPE_HINT = 'bfabric-cli auth login --scope "api:read api:write openid profile email groups {scope}"' +# Re-auth hint shown when a required transfer scope (``tus`` for upload, ``containers`` for download) +# is missing from the token: request read-write API access (``api:write`` implies ``api:read``) plus +# the missing scope. ``{scope}`` is filled with whatever the failed operation needed. +_PKCE_SCOPE_HINT = 'bfabric-cli auth login --scope "api:write {scope}"' def _safe_auth(client: Bfabric) -> BfabricAuth | None: diff --git a/bfabric_scripts/docs/changelog.md b/bfabric_scripts/docs/changelog.md index 4150e26a..089784a8 100644 --- a/bfabric_scripts/docs/changelog.md +++ b/bfabric_scripts/docs/changelog.md @@ -10,6 +10,8 @@ Versioning currently follows `X.Y.Z` semantic versioning, independent of the `bf ## \[Unreleased\] +- `auth login` / `auth device-code` now **require** an explicit `--scope` (no default). Named presets document the useful sets — `read-only` (`api:read`), `read-write` (`api:write`), `upload` (`api:write tus`) — or pass any scope string. Client/webapp registration (`auth register` / `auth register-webapp`) keeps the OIDC-inclusive default, which webapps need for user-identity claims. + ## \[1.16.0rc2\] - 2026-07-15 - `bfabric-cli auth` command group for OAuth authentication and client management: diff --git a/bfabric_scripts/example/test_webapp_flow.py b/bfabric_scripts/example/test_webapp_flow.py index 8a96a5d2..0465acc6 100644 --- a/bfabric_scripts/example/test_webapp_flow.py +++ b/bfabric_scripts/example/test_webapp_flow.py @@ -26,6 +26,7 @@ from bfabric._oauth.registration import register_webapp from bfabric._oauth.webapp_client import WebappClient from bfabric.entities.core.uri import EntityUri +from bfabric_scripts.cli.login._constants import DEFAULT_REGISTRATION_SCOPE PORT = 19876 REDIRECT_PATH = "/callback" @@ -56,6 +57,7 @@ def main() -> None: app_name=app_name, web_url=web_url, service_user="itfeeder", + scope=DEFAULT_REGISTRATION_SCOPE, hidden=True, ) oauth_info = result["oauth"] @@ -149,6 +151,7 @@ def serve() -> None: launch_token=jwt, client_id=oauth_client_id, client_secret=oauth_client_secret, + scope=DEFAULT_REGISTRATION_SCOPE, ) except Exception as e: print(f"\nERROR: Token exchange failed: {e}", file=sys.stderr) diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/_constants.py b/bfabric_scripts/src/bfabric_scripts/cli/login/_constants.py new file mode 100644 index 00000000..3cce90fa --- /dev/null +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/_constants.py @@ -0,0 +1,37 @@ +"""Default OAuth client ID and scope policy for the CLI login commands. + +These are CLI-level policy: ``"CLI"`` is the client identity the CLI registers +as, and the scopes below are the permission sets the CLI requests. The core +``bfabric`` library deliberately does not bake in these defaults — its OAuth +API requires callers to state ``client_id`` and ``scope`` explicitly. +""" + +from __future__ import annotations + +from typing import NamedTuple + +DEFAULT_CLIENT_ID = "CLI" + +# Scope requested when *registering* an OAuth client/webapp (``auth register`` / +# ``auth register-webapp``). Broad and OIDC-inclusive on purpose: a registered webapp +# needs ``openid profile email groups`` for the user-identity claims it reads from the +# URL-token exchange, which the login presets below intentionally omit. +DEFAULT_REGISTRATION_SCOPE = "api:read api:write openid profile email groups" + + +class ScopePreset(NamedTuple): + """A named, use-case-oriented OAuth scope set offered at interactive login.""" + + name: str + scope: str + description: str + + +# Scope presets for interactive *login* (``auth login`` / ``auth device-code``), ordered +# by increasing capability. ``api:write`` implies ``api:read`` server-side, so the +# read-write set lists only ``api:write``; ``tus`` adds file-upload permission on top. +SCOPE_PRESETS: tuple[ScopePreset, ...] = ( + ScopePreset("read-only", "api:read", "Read from the API"), + ScopePreset("read-write", "api:write", "Write to the API (includes read from the API)"), + ScopePreset("upload", "api:write tus", "Upload files to the API (includes read and write API)"), +) diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/device_code.py b/bfabric_scripts/src/bfabric_scripts/cli/login/device_code.py index fa8412ad..a72e5fde 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/device_code.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/device_code.py @@ -7,21 +7,21 @@ import cyclopts -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric._oauth.credential_provider import OAuthCredentialProvider from bfabric._oauth.device_code import device_code_login from bfabric._oauth.token_cache import compute_token_cache_path from bfabric.config import DEFAULT_CONFIG_FILE from bfabric.config.config_writer import write_environment_to_config +from bfabric_scripts.cli.login._constants import DEFAULT_CLIENT_ID def cmd_login_device_code( base_url: Annotated[str, cyclopts.Parameter(help="B-Fabric instance URL.")], *, + scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")], client_id: Annotated[str, cyclopts.Parameter(help="OAuth client ID.")] = DEFAULT_CLIENT_ID, config_env: Annotated[str, cyclopts.Parameter(help="Environment name in the config file.")] = "PRODUCTION", config_file: Annotated[Path, cyclopts.Parameter(help="Path to the config file.")] = DEFAULT_CONFIG_FILE, - scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_OAUTH_SCOPE, timeout: Annotated[float, cyclopts.Parameter(help="Seconds to wait for authorization.")] = 600.0, set_default: Annotated[ bool, cyclopts.Parameter(help="Set this environment as the default in the config file.") diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/logout.py b/bfabric_scripts/src/bfabric_scripts/cli/login/logout.py index 62607474..d55a3876 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/logout.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/logout.py @@ -9,11 +9,10 @@ import cyclopts import yaml -from bfabric._oauth._constants import DEFAULT_CLIENT_ID - from bfabric.config import DEFAULT_CONFIG_FILE from bfabric.config.config_file import ConfigFile from bfabric._oauth.token_cache import TokenCache, compute_token_cache_path +from bfabric_scripts.cli.login._constants import DEFAULT_CLIENT_ID def cmd_login_logout( diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/pkce.py b/bfabric_scripts/src/bfabric_scripts/cli/login/pkce.py index c1627e57..ce33a68c 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/pkce.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/pkce.py @@ -7,21 +7,21 @@ import cyclopts -from bfabric._oauth._constants import DEFAULT_CLIENT_ID, DEFAULT_OAUTH_SCOPE from bfabric._oauth.credential_provider import OAuthCredentialProvider from bfabric._oauth.pkce import pkce_login from bfabric._oauth.token_cache import compute_token_cache_path from bfabric.config import DEFAULT_CONFIG_FILE from bfabric.config.config_writer import write_environment_to_config +from bfabric_scripts.cli.login._constants import DEFAULT_CLIENT_ID def cmd_auth_login( base_url: Annotated[str, cyclopts.Parameter(help="B-Fabric instance URL.")], *, + scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")], client_id: Annotated[str, cyclopts.Parameter(help="OAuth client ID.")] = DEFAULT_CLIENT_ID, config_env: Annotated[str, cyclopts.Parameter(help="Environment name in the config file.")] = "PRODUCTION", config_file: Annotated[Path, cyclopts.Parameter(help="Path to the config file.")] = DEFAULT_CONFIG_FILE, - scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_OAUTH_SCOPE, port: Annotated[int, cyclopts.Parameter(help="Local port for callback (0 = auto).")] = 0, timeout: Annotated[float, cyclopts.Parameter(help="Seconds to wait for login.")] = 120.0, set_default: Annotated[ diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/register.py b/bfabric_scripts/src/bfabric_scripts/cli/login/register.py index 2f2cf777..95e1b8cf 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/register.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/register.py @@ -10,9 +10,9 @@ import cyclopts -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric._oauth.registration import register_client from bfabric.config import DEFAULT_CONFIG_FILE +from bfabric_scripts.cli.login._constants import DEFAULT_CLIENT_ID, DEFAULT_REGISTRATION_SCOPE def _resolve_token_from_config(config_env: str, config_file: Path) -> tuple[str, str]: @@ -23,7 +23,6 @@ def _resolve_token_from_config(config_env: str, config_file: Path) -> tuple[str, """ import yaml - from bfabric._oauth._constants import DEFAULT_CLIENT_ID from bfabric._oauth.credential_provider import OAuthCredentialProvider from bfabric._oauth.token_cache import compute_token_cache_path from bfabric.config.config_file import ConfigFile @@ -54,6 +53,7 @@ def _resolve_token_from_config(config_env: str, config_file: Path) -> tuple[str, client_id=client_id, client_secret="", token_url=token_url, + scope="", grant_type="refresh_token", token_cache_path=cache_path, ) @@ -80,7 +80,7 @@ def cmd_login_register( service_user: Annotated[ str | None, cyclopts.Parameter(help="Service user login (enables client_credentials grant).") ] = None, - scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_OAUTH_SCOPE, + scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_REGISTRATION_SCOPE, grant_types: Annotated[ list[str] | None, cyclopts.Parameter(help="Grant types to request (overrides default webapp grants)."), diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/register_webapp.py b/bfabric_scripts/src/bfabric_scripts/cli/login/register_webapp.py index 49df473e..0d623764 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/register_webapp.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/register_webapp.py @@ -9,28 +9,24 @@ import cyclopts -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric.config import DEFAULT_CONFIG_FILE +from bfabric_scripts.cli.login._constants import DEFAULT_REGISTRATION_SCOPE def cmd_login_register_webapp( app_name: Annotated[str, cyclopts.Parameter(help="Human-readable name for the webapp.")], web_url: Annotated[str, cyclopts.Parameter(help="The webapp URL (used as redirect URI and application weburl).")], *, - config_env: Annotated[ - str | None, cyclopts.Parameter(help="Config environment to use.") - ] = None, + config_env: Annotated[str | None, cyclopts.Parameter(help="Config environment to use.")] = None, config_file: Annotated[Path, cyclopts.Parameter(help="Path to the config file.")] = DEFAULT_CONFIG_FILE, service_user: Annotated[ str | None, cyclopts.Parameter(help="Service user login (enables client_credentials grant).") ] = None, - scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_OAUTH_SCOPE, + scope: Annotated[str, cyclopts.Parameter(help="OAuth scope.")] = DEFAULT_REGISTRATION_SCOPE, application_id: Annotated[ int | None, cyclopts.Parameter(help="Existing application ID to update (omit to create new).") ] = None, - technology_id: Annotated[ - int | None, cyclopts.Parameter(help="Technology ID for the application.") - ] = None, + technology_id: Annotated[int | None, cyclopts.Parameter(help="Technology ID for the application.")] = None, description: Annotated[str | None, cyclopts.Parameter(help="Application description.")] = None, ) -> None: """Register a new OAuth webapp: create OAuth client and B-Fabric application. @@ -73,4 +69,4 @@ def cmd_login_register_webapp( print( f"\nApplication saved. OAuth client id={oauth_info['id']}, client_id={oauth_info['client_id']}", file=sys.stderr, - ) \ No newline at end of file + ) diff --git a/bfabric_scripts/src/bfabric_scripts/cli/login/status.py b/bfabric_scripts/src/bfabric_scripts/cli/login/status.py index c5d8588d..955b5cf5 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/login/status.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/login/status.py @@ -9,11 +9,10 @@ import cyclopts import yaml -from bfabric._oauth._constants import DEFAULT_CLIENT_ID - from bfabric.config import DEFAULT_CONFIG_FILE from bfabric.config.config_file import ConfigFile from bfabric._oauth.token_cache import TokenCache, compute_token_cache_path +from bfabric_scripts.cli.login._constants import DEFAULT_CLIENT_ID def cmd_login_status( diff --git a/bfabric_scripts/src/bfabric_scripts/cli/workunit/upload.py b/bfabric_scripts/src/bfabric_scripts/cli/workunit/upload.py index 01132af2..31b632dd 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/workunit/upload.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/workunit/upload.py @@ -75,7 +75,7 @@ def cmd_workunit_upload(params: UploadParams, *, client: Bfabric) -> None: Creates a new workunit, or uploads into an existing one with ``--workunit-id``. Requires an OAuth-backed client with the ``tus`` scope; authenticate with - ``bfabric-cli auth login --scope "api:read api:write openid profile email groups tus"``. + ``bfabric-cli auth login --scope "api:write tus"``. """ with _upload_progress(enabled=_progress_enabled(requested=params.progress)) as reporter: summary = upload_files( diff --git a/tests/bfabric/oauth/test_credential_provider.py b/tests/bfabric/oauth/test_credential_provider.py index 5c68b4d2..c4f40efc 100644 --- a/tests/bfabric/oauth/test_credential_provider.py +++ b/tests/bfabric/oauth/test_credential_provider.py @@ -28,6 +28,7 @@ def provider(mock_oauth2_session): client_id="test-id", client_secret="test-secret", token_url="https://example.com/rest/oauth/token", + scope="", ) @@ -38,6 +39,7 @@ def test_client_credentials_requires_secret(self, mock_oauth2_session): client_id="test-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="client_credentials", ) @@ -48,6 +50,7 @@ def test_refresh_token_allows_empty_secret(self, mock_oauth2_session): client_id="test-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "t", "refresh_token": "rt", "expires_at": 9999999999}, ) @@ -101,6 +104,7 @@ def slow_fetch(*args, **kwargs): client_id="id", client_secret="secret", token_url="https://example.com/rest/oauth/token", + scope="", ) threads = [threading.Thread(target=provider.get_auth) for _ in range(5)] @@ -168,6 +172,7 @@ def test_refresh_token_preserved_for_refresh_grant(self, mock_oauth2_session): client_id="app-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "jwt", "refresh_token": "keep_me", "expires_at": time.time() + 3600}, ) @@ -190,6 +195,7 @@ def test_seeded_token_with_refresh(self, mock_oauth2_session): client_id="app-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", token=token, grant_type="refresh_token", ) @@ -209,6 +215,7 @@ def test_session_configured_for_refresh(self, mock_oauth2_session, mocker): client_id="app-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "jwt", "refresh_token": "rt", "expires_at": time.time() + 3600}, ) @@ -230,6 +237,7 @@ def test_refresh_failure_raises_bfabric_oauth_error(self, mock_oauth2_session): client_id="app-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "stale", "refresh_token": "rt", "expires_at": 1}, ) @@ -260,6 +268,7 @@ def test_transport_failure_raises_bfabric_oauth_error(self, mock_oauth2_session) client_id="app-id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "stale", "refresh_token": "rt", "expires_at": 1}, ) @@ -282,6 +291,7 @@ def test_uses_disk_cache_on_init(self, tmp_path, mock_oauth2_session): client_id="id", client_secret="secret", token_url="https://example.com/rest/oauth/token", + scope="", token_cache_path=cache_path, ) @@ -300,6 +310,7 @@ def test_saves_to_disk_via_update_token_callback(self, tmp_path, mock_oauth2_ses client_id="id", client_secret="secret", token_url="https://example.com/rest/oauth/token", + scope="", token_cache_path=cache_path, ) # Extract the update_token callback that was passed to OAuth2Session @@ -328,6 +339,7 @@ def do_fetch(*args, **kwargs): client_id="id", client_secret="secret", token_url="https://example.com/rest/oauth/token", + scope="", token_cache_path=cache_path, ) provider.get_auth() @@ -348,6 +360,7 @@ def test_supplied_token_preferred_over_disk_cache(self, tmp_path, mock_oauth2_se client_id="id", client_secret="", token_url="https://example.com/rest/oauth/token", + scope="", token=supplied, grant_type="refresh_token", token_cache_path=cache_path, diff --git a/tests/bfabric/oauth/test_device_code.py b/tests/bfabric/oauth/test_device_code.py index 87053c7e..0454d38d 100644 --- a/tests/bfabric/oauth/test_device_code.py +++ b/tests/bfabric/oauth/test_device_code.py @@ -3,7 +3,6 @@ import httpx import pytest -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric._oauth.device_code import ( _poll_for_token, _request_device_code, @@ -290,7 +289,7 @@ def test_prints_verification_uri_complete(self, mocker, capsys): mocker.patch("bfabric._oauth.device_code._request_device_code", return_value=device_response) mocker.patch("bfabric._oauth.device_code._poll_for_token", return_value={"access_token": "at"}) - device_code_login("https://example.com/bfabric") + device_code_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") captured = capsys.readouterr() assert "https://example.com/device?user_code=WXYZ-9876" in captured.err @@ -311,12 +310,12 @@ def test_strips_trailing_slash(self, mocker): "bfabric._oauth.device_code._poll_for_token", return_value={"access_token": "at"}, ) - device_code_login("https://example.com/bfabric///") + device_code_login("https://example.com/bfabric///", client_id="test-cli", scope="api:read") mock_request.assert_called_once_with( "https://example.com/bfabric", - client_id="CLI", - scope=DEFAULT_OAUTH_SCOPE, + client_id="test-cli", + scope="api:read", ) assert mock_poll.call_args[0][0] == "https://example.com/bfabric" @@ -332,6 +331,6 @@ def test_default_interval_when_missing(self, mocker): "bfabric._oauth.device_code._poll_for_token", return_value={"access_token": "at"}, ) - device_code_login("https://example.com/bfabric") + device_code_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") assert mock_poll.call_args[1]["interval"] == 5.0 diff --git a/tests/bfabric/oauth/test_pkce.py b/tests/bfabric/oauth/test_pkce.py index 80e3635f..3f573e1e 100644 --- a/tests/bfabric/oauth/test_pkce.py +++ b/tests/bfabric/oauth/test_pkce.py @@ -206,7 +206,7 @@ def test_happy_path(self, mocker): mock_server.serve_forever = mocker.MagicMock() mock_server_cls.return_value = mock_server - result = pkce_login("https://example.com/bfabric", client_id="test-cli") + result = pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") assert result == token_dict mock_exchange.assert_called_once_with( @@ -231,7 +231,7 @@ def test_state_mismatch_raises(self, mocker): mock_server_cls.return_value = mock_server with pytest.raises(BfabricOAuthError, match="CSRF state mismatch") as exc_info: - pkce_login("https://example.com/bfabric") + pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") # State values must NOT appear in the error message (security) assert "expected_state" not in str(exc_info.value) assert "wrong_state" not in str(exc_info.value) @@ -254,7 +254,7 @@ def test_error_from_server_raises(self, mocker): mock_server_cls.return_value = mock_server with pytest.raises(RuntimeError, match="Authorization error: access_denied"): - pkce_login("https://example.com/bfabric") + pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") def test_timeout_raises(self, mocker): mock_server_cls = mocker.patch("bfabric._oauth.pkce._CallbackServer") @@ -273,7 +273,7 @@ def test_timeout_raises(self, mocker): mock_thread_cls.return_value = mock_thread with pytest.raises(RuntimeError, match="timed out"): - pkce_login("https://example.com/bfabric", timeout=0.1) + pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read", timeout=0.1) def test_browser_fallback_prints_url(self, mocker, capsys): mock_server_cls = mocker.patch("bfabric._oauth.pkce._CallbackServer") @@ -289,7 +289,7 @@ def test_browser_fallback_prints_url(self, mocker, capsys): mock_server.serve_forever = mocker.MagicMock() mock_server_cls.return_value = mock_server - pkce_login("https://example.com/bfabric") + pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read") captured = capsys.readouterr() assert "Open this URL to log in:" in captured.err @@ -308,7 +308,7 @@ def test_open_browser_false_prints_url(self, mocker, capsys): mock_server.serve_forever = mocker.MagicMock() mock_server_cls.return_value = mock_server - pkce_login("https://example.com/bfabric", open_browser=False) + pkce_login("https://example.com/bfabric", client_id="test-cli", scope="api:read", open_browser=False) captured = capsys.readouterr() assert "Open this URL to log in:" in captured.err diff --git a/tests/bfabric/oauth/test_registration.py b/tests/bfabric/oauth/test_registration.py index 0aec4bcb..b540a9d1 100644 --- a/tests/bfabric/oauth/test_registration.py +++ b/tests/bfabric/oauth/test_registration.py @@ -2,9 +2,11 @@ import pytest -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric._oauth.registration import register_client, register_webapp +# register_client/register_webapp now require an explicit scope; tests pass this. +_TEST_SCOPE = "api:read api:write" + @pytest.fixture def mock_httpx_post(mocker): @@ -29,6 +31,7 @@ def test_basic_registration(self, mock_httpx_post): token="bearer-token", client_name="my-app", redirect_uri="http://localhost:8050/callback", + scope=_TEST_SCOPE, ) mock_httpx_post.assert_called_once_with( @@ -37,7 +40,7 @@ def test_basic_registration(self, mock_httpx_post): "client_name": "my-app", "redirect_uris": ["http://localhost:8050/callback"], "grant_types": [_TOKEN_EXCHANGE, "refresh_token", "authorization_code"], - "scope": DEFAULT_OAUTH_SCOPE, + "scope": _TEST_SCOPE, }, headers={"Authorization": "Bearer bearer-token"}, timeout=30, @@ -52,6 +55,7 @@ def test_with_service_user_adds_client_credentials_grant(self, mock_httpx_post): client_name="app", redirect_uri="http://localhost/cb", service_user="gfeeder", + scope=_TEST_SCOPE, ) call_body = mock_httpx_post.call_args[1]["json"] @@ -69,6 +73,7 @@ def test_without_service_user_no_client_credentials_grant(self, mock_httpx_post) token="tok", client_name="app", redirect_uri="http://localhost/cb", + scope=_TEST_SCOPE, ) call_body = mock_httpx_post.call_args[1]["json"] @@ -94,6 +99,7 @@ def test_explicit_grant_types_override(self, mock_httpx_post): client_name="app", redirect_uri="http://localhost/cb", grant_types=["authorization_code", "refresh_token"], + scope=_TEST_SCOPE, ) call_body = mock_httpx_post.call_args[1]["json"] @@ -105,11 +111,12 @@ def test_without_optional_params(self, mock_httpx_post): token="tok", client_name="app", redirect_uri="http://localhost/cb", + scope=_TEST_SCOPE, ) call_body = mock_httpx_post.call_args[1]["json"] assert "service_user_login" not in call_body - assert call_body["scope"] == DEFAULT_OAUTH_SCOPE + assert call_body["scope"] == _TEST_SCOPE def test_normalizes_trailing_slash(self, mock_httpx_post): register_client( @@ -117,6 +124,7 @@ def test_normalizes_trailing_slash(self, mock_httpx_post): token="tok", client_name="app", redirect_uri="http://localhost/cb", + scope=_TEST_SCOPE, ) url = mock_httpx_post.call_args[0][0] @@ -138,6 +146,7 @@ def test_raises_on_http_error(self, mocker): token="bad-token", client_name="app", redirect_uri="http://localhost/cb", + scope=_TEST_SCOPE, ) @@ -168,6 +177,7 @@ def test_calls_register_client_and_saves_application(self, mock_client, mock_reg token="bearer-tok", app_name="My Webapp", web_url="https://myapp.example.com/", + scope=_TEST_SCOPE, ) mock_register.assert_called_once_with( @@ -176,7 +186,7 @@ def test_calls_register_client_and_saves_application(self, mock_client, mock_reg client_name="My Webapp", redirect_uri="https://myapp.example.com/", service_user=None, - scope=DEFAULT_OAUTH_SCOPE, + scope=_TEST_SCOPE, ) mock_client.save.assert_called_once_with( "application", @@ -197,6 +207,7 @@ def test_forwards_service_user(self, mock_client, mock_register): app_name="app", web_url="https://app.example.com/", service_user="svc", + scope=_TEST_SCOPE, ) assert mock_register.call_args[1]["service_user"] == "svc" @@ -219,6 +230,7 @@ def test_updates_existing_application(self, mock_client, mock_register): app_name="app", web_url="https://app.example.com/", application_id=42, + scope=_TEST_SCOPE, ) save_obj = mock_client.save.call_args[0][1] @@ -232,6 +244,7 @@ def test_sets_optional_fields(self, mock_client, mock_register): web_url="https://app.example.com/", technology_id=7, description="A test app", + scope=_TEST_SCOPE, ) save_obj = mock_client.save.call_args[0][1] @@ -244,6 +257,7 @@ def test_omits_optional_fields_when_not_provided(self, mock_client, mock_registe token="tok", app_name="app", web_url="https://app.example.com/", + scope=_TEST_SCOPE, ) save_obj = mock_client.save.call_args[0][1] diff --git a/tests/bfabric/oauth/test_webapp_client.py b/tests/bfabric/oauth/test_webapp_client.py index b6a01306..394c3609 100644 --- a/tests/bfabric/oauth/test_webapp_client.py +++ b/tests/bfabric/oauth/test_webapp_client.py @@ -4,7 +4,6 @@ import pytest -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric._oauth.url_token import UrlTokenContext from bfabric._oauth.webapp_client import WebappClient @@ -52,6 +51,7 @@ def test_returns_correct_user_service_context(self, mocker, mock_token_dict, moc launch_token="short.lived.jwt", client_id="app-id", client_secret="app-secret", + scope="api:read", ) assert wc.service is mock_service_client @@ -71,6 +71,7 @@ def test_forwards_parameters_to_exchange_token(self, mocker, mock_token_dict): launch_token="my.launch.jwt", client_id="cid", client_secret="csecret", + scope="api:read", ) mock_exchange.assert_called_once_with( @@ -92,6 +93,7 @@ def test_verifies_exchanged_access_token_jwt(self, mocker, mock_token_dict): launch_token="jwt", client_id="cid", client_secret="csecret", + scope="api:read", ) mock_verify.assert_called_once_with( @@ -111,6 +113,7 @@ def test_creates_user_provider_with_refresh_token(self, mocker, mock_token_dict) launch_token="jwt", client_id="cid", client_secret="csecret", + scope="api:read", user_token_cache_path="/tmp/user_cache", ) @@ -118,6 +121,7 @@ def test_creates_user_provider_with_refresh_token(self, mocker, mock_token_dict) client_id="cid", client_secret="csecret", token_url="https://bfabric.example.com/bfabric/rest/oauth/token", + scope="", token=mock_token_dict, grant_type="refresh_token", token_cache_path="/tmp/user_cache", @@ -147,22 +151,6 @@ def test_forwards_parameters_to_connect_oauth(self, mocker, mock_token_dict): token_cache_path="/tmp/svc_cache", ) - def test_default_scope(self, mocker, mock_token_dict): - mocker.patch(_PATCH_EXCHANGE, return_value=mock_token_dict) - mocker.patch(_PATCH_VERIFY_JWT, return_value=dict(SAMPLE_CLAIMS)) - mocker.patch(_PATCH_PROVIDER) - mock_connect_oauth = mocker.patch(_PATCH_CONNECT_OAUTH, return_value=mocker.MagicMock()) - mocker.patch(_PATCH_LOG) - - WebappClient.create( - base_url="https://bfabric.example.com/bfabric", - launch_token="jwt", - client_id="cid", - client_secret="csecret", - ) - - assert mock_connect_oauth.call_args.kwargs["scope"] == DEFAULT_OAUTH_SCOPE - class TestWebappClientFrozen: @pytest.fixture @@ -207,6 +195,7 @@ def test_user_and_service_are_independent(self, mocker): launch_token="jwt", client_id="cid", client_secret="csecret", + scope="api:read", ) assert wc.user is not wc.service diff --git a/tests/bfabric/test_bfabric.py b/tests/bfabric/test_bfabric.py index 96360787..64419915 100644 --- a/tests/bfabric/test_bfabric.py +++ b/tests/bfabric/test_bfabric.py @@ -6,13 +6,15 @@ from pydantic import SecretStr from bfabric import Bfabric, BfabricAPIEngineType, BfabricClientConfig, BfabricAuth -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE from bfabric.config import DEFAULT_CONFIG_FILE from bfabric.config.bfabric_auth import OAUTH_LOGIN from bfabric.config.config_data import ConfigData from bfabric.engine.engine_suds import EngineSUDS from bfabric.entities.core.entity_reader import EntityReader +# The core OAuth API no longer bakes in a default scope; tests pass it explicitly. +_TEST_SCOPE = "api:read api:write" + @pytest.fixture def mock_config(): @@ -449,6 +451,19 @@ def test_repr(bfabric_instance, variant): ) +class TestConnectOAuthFromConfig: + def test_raises_when_client_id_missing(self, mocker): + mocker.patch.object(Bfabric, "_log_version_message") + config_data = ConfigData( + client=BfabricClientConfig(base_url="https://example.com/bfabric"), + auth=None, + auth_method="oauth", + client_id=None, + ) + with pytest.raises(ValueError, match="missing 'client_id'"): + Bfabric._connect_oauth_from_config(config_data) + + class TestConnectOAuth: def test_creates_instance_with_provider(self, mocker): mocker.patch.object(Bfabric, "_log_version_message") @@ -458,13 +473,14 @@ def test_creates_instance_with_provider(self, mocker): client_id="my-id", client_secret="my-secret", base_url="https://example.com/bfabric", + scope=_TEST_SCOPE, ) mock_provider_cls.assert_called_once_with( client_id="my-id", client_secret="my-secret", token_url="https://example.com/bfabric/rest/oauth/token", - scope=DEFAULT_OAUTH_SCOPE, + scope=_TEST_SCOPE, grant_type="client_credentials", token_cache_path=None, ) @@ -496,6 +512,7 @@ def test_strips_trailing_slash(self, mocker): client_id="id", client_secret="secret", base_url="https://example.com/bfabric/", + scope=_TEST_SCOPE, ) assert client.config.base_url == "https://example.com/bfabric/" @@ -559,23 +576,25 @@ def test_creates_instance_with_provider(self, mocker): client = Bfabric.connect_pkce( base_url="https://example.com/bfabric", + client_id="my-cli", + scope=_TEST_SCOPE, ) mock_pkce_login.assert_called_once_with( "https://example.com/bfabric", - client_id="CLI", - scope=DEFAULT_OAUTH_SCOPE, + client_id="my-cli", + scope=_TEST_SCOPE, port=0, open_browser=True, timeout=120.0, ) mock_provider_cls.assert_called_once_with( - client_id="CLI", + client_id="my-cli", client_secret="", token_url="https://example.com/bfabric/rest/oauth/token", token=mock_pkce_login.return_value, grant_type="refresh_token", - scope=DEFAULT_OAUTH_SCOPE, + scope=_TEST_SCOPE, token_cache_path=None, ) assert client._credential_provider == mock_provider_cls.return_value @@ -620,6 +639,8 @@ def test_strips_trailing_slash(self, mocker): client = Bfabric.connect_pkce( base_url="https://example.com/bfabric///", + client_id="my-cli", + scope=_TEST_SCOPE, ) assert client.config.base_url == "https://example.com/bfabric/" @@ -640,21 +661,23 @@ def test_creates_instance_with_provider(self, mocker): client = Bfabric.connect_device_code( base_url="https://example.com/bfabric", + client_id="my-cli", + scope=_TEST_SCOPE, ) mock_device_code_login.assert_called_once_with( "https://example.com/bfabric", - client_id="CLI", - scope=DEFAULT_OAUTH_SCOPE, + client_id="my-cli", + scope=_TEST_SCOPE, timeout=600.0, ) mock_provider_cls.assert_called_once_with( - client_id="CLI", + client_id="my-cli", client_secret="", token_url="https://example.com/bfabric/rest/oauth/token", token=mock_device_code_login.return_value, grant_type="refresh_token", - scope=DEFAULT_OAUTH_SCOPE, + scope=_TEST_SCOPE, token_cache_path=None, ) assert client._credential_provider == mock_provider_cls.return_value @@ -695,6 +718,8 @@ def test_strips_trailing_slash(self, mocker): client = Bfabric.connect_device_code( base_url="https://example.com/bfabric///", + client_id="my-cli", + scope=_TEST_SCOPE, ) assert client.config.base_url == "https://example.com/bfabric/" @@ -761,6 +786,7 @@ def test_oauth_only_client_survives_pickle(self, mocker): client_id="cid", client_secret="", token_url="https://example.com/bfabric/rest/oauth/token", + scope="", grant_type="refresh_token", token={"access_token": "tok-abc", "token_type": "Bearer", "expires_at": 9999999999}, ) diff --git a/tests/bfabric_scripts/cli/login/test_cmd_auth_login.py b/tests/bfabric_scripts/cli/login/test_cmd_auth_login.py index 59b8e78d..9a8c708b 100644 --- a/tests/bfabric_scripts/cli/login/test_cmd_auth_login.py +++ b/tests/bfabric_scripts/cli/login/test_cmd_auth_login.py @@ -25,6 +25,7 @@ def test_writes_config_and_caches_token(self, tmp_path, mocker): mocker.patch("bfabric._oauth.credential_provider.OAuth2Session", return_value=mock_session) cmd_auth_login( base_url="https://example.com/bfabric", + scope="api:read", client_id="test-client", config_env="PROD", config_file=config_file, @@ -52,6 +53,7 @@ def test_set_default_false_does_not_set_default(self, tmp_path, mocker): mocker.patch("bfabric._oauth.credential_provider.OAuth2Session", return_value=mock_session) cmd_auth_login( base_url="https://example.com/bfabric", + scope="api:read", client_id="test-client", config_env="PROD", config_file=config_file, diff --git a/tests/bfabric_scripts/cli/login/test_cmd_login_device_code.py b/tests/bfabric_scripts/cli/login/test_cmd_login_device_code.py index b389131f..62520d0d 100644 --- a/tests/bfabric_scripts/cli/login/test_cmd_login_device_code.py +++ b/tests/bfabric_scripts/cli/login/test_cmd_login_device_code.py @@ -25,6 +25,7 @@ def test_writes_config_and_caches_token(self, tmp_path, mocker): mocker.patch("bfabric._oauth.credential_provider.OAuth2Session", return_value=mock_session) cmd_login_device_code( base_url="https://example.com/bfabric", + scope="api:read", client_id="test-client", config_env="PROD", config_file=config_file, @@ -52,6 +53,7 @@ def test_set_default_false_does_not_set_default(self, tmp_path, mocker): mocker.patch("bfabric._oauth.credential_provider.OAuth2Session", return_value=mock_session) cmd_login_device_code( base_url="https://example.com/bfabric", + scope="api:read", client_id="test-client", config_env="PROD", config_file=config_file, diff --git a/tests/bfabric_scripts/cli/login/test_cmd_login_register.py b/tests/bfabric_scripts/cli/login/test_cmd_login_register.py index 599eec42..a170230d 100644 --- a/tests/bfabric_scripts/cli/login/test_cmd_login_register.py +++ b/tests/bfabric_scripts/cli/login/test_cmd_login_register.py @@ -5,7 +5,7 @@ import pytest import yaml -from bfabric._oauth._constants import DEFAULT_OAUTH_SCOPE +from bfabric_scripts.cli.login._constants import DEFAULT_REGISTRATION_SCOPE from bfabric_scripts.cli.login.register import cmd_login_register @@ -107,7 +107,7 @@ def test_uses_cached_token_from_config_env(self, tmp_path, mocker, capsys): client_name="My App", redirect_uri="http://localhost/callback", service_user=None, - scope=DEFAULT_OAUTH_SCOPE, + scope=DEFAULT_REGISTRATION_SCOPE, grant_types=None, ) output = capsys.readouterr() @@ -154,7 +154,7 @@ def test_explicit_base_url_overrides_config(self, tmp_path, mocker, capsys): client_name="My App", redirect_uri="http://localhost/callback", service_user=None, - scope=DEFAULT_OAUTH_SCOPE, + scope=DEFAULT_REGISTRATION_SCOPE, grant_types=None, ) diff --git a/tests/bfabric_scripts/cli/login/test_login_constants.py b/tests/bfabric_scripts/cli/login/test_login_constants.py new file mode 100644 index 00000000..9dbef193 --- /dev/null +++ b/tests/bfabric_scripts/cli/login/test_login_constants.py @@ -0,0 +1,18 @@ +from __future__ import annotations + +from bfabric_scripts.cli.login._constants import SCOPE_PRESETS + + +class TestScopePresets: + def test_presets_are_minimal_use_case_sets(self): + # Login presets deliberately omit the OIDC/groups scopes (those live in + # DEFAULT_REGISTRATION_SCOPE for registration), and api:write implies api:read + # server-side so read-write lists only api:write. + assert [(p.name, p.scope) for p in SCOPE_PRESETS] == [ + ("read-only", "api:read"), + ("read-write", "api:write"), + ("upload", "api:write tus"), + ] + + def test_every_preset_has_a_description(self): + assert all(preset.description for preset in SCOPE_PRESETS)