refactor(oauth): require explicit client_id and scope#562
Merged
Conversation
The core bfabric OAuth entry points no longer bake in a "CLI" client ID or a default scope. connect_oauth/connect_pkce/connect_device_code and the _oauth helpers now require client_id and scope explicitly. The CLI-facing defaults move to bfabric_scripts (cli/login/_constants.py). connect() with auth_method: oauth now raises if the config lacks client_id instead of falling back to "CLI".
…it-client-id-scope # Conflicts: # bfabric/docs/changelog.md # bfabric/src/bfabric/_oauth/credential_provider.py
Model login scope presets in cli/login/_constants.py as ScopePreset named tuples (name, scope, description), oriented by use case: read-only (api:read), read-write (api:write, which implies api:read), and upload (api:write tus). `auth login` / `auth device-code` now default to the read-write preset instead of the broad OIDC-inclusive scope. Client/webapp registration keeps DEFAULT_OAUTH_SCOPE (webapps need the OIDC claims). The presets + by-name index + default live in _constants.py for the interactive scope picker to consume as a follow-up.
The CLI login default is now the read-write preset (api:write), not the broad OIDC scope, so the troubleshooting doc no longer claims groups is in the default (employees must request it explicitly for file access). Rename the CLI registration-scope constant DEFAULT_OAUTH_SCOPE -> DEFAULT_REGISTRATION_SCOPE to say what it is, and fix a stale comment in transfer/tokens.py that referenced the removed core constant.
…ng scope
The re-auth hint (_PKCE_SCOPE_HINT) and the upload examples requested the full
OIDC scope set, which the operation never needs. Reduce them to "api:write
{scope}" (api:write implies api:read) — for upload that is exactly the `upload`
preset (api:write tus). Also fix the changelog preset name (upload, not
read-write-upload).
…moval Reverts the CLI scope presets (7f15ffd), the DEFAULT_OAUTH_SCOPE rename + troubleshooting-doc edits (58d4de1), and the upload/hint scope changes (48cd276). #562 stays scoped to removing baked-in OAuth defaults from the core library; the CLI keeps its original DEFAULT_OAUTH_SCOPE / DEFAULT_CLIENT_ID and behavior unchanged. Scope presets belong in the interactive-picker follow-up.
leoschwarz
marked this pull request as ready for review
July 15, 2026 14:32
leoschwarz
added a commit
that referenced
this pull request
Jul 16, 2026
Integrates the core-defaults removal (#562) and the auth login rename (#561) into the interactive auth command group. Conflict/breakage resolution: - Relocate DEFAULT_CLIENT_ID import from the deleted bfabric._oauth._constants to bfabric_scripts.cli.login._constants (pkce, device_code, logout, status). - Apply the #561 rename: cmd_login_pkce -> cmd_auth_login (auth login); test_cmd_login_pkce.py -> test_cmd_auth_login.py; changelog auth pkce -> auth login. - Adopt #562's use-case scope presets (read-only / read-write / upload) in _constants.py (ScopePreset) and consume them from _common.py, replacing the OIDC-inclusive presets #558 had built on the (now CLI-owned) DEFAULT_OAUTH_SCOPE. - Repoint bfabric_scripts login tests at _constants; update preset expansion assertions (upload = "api:write tus"; login default = DEFAULT_LOGIN_SCOPE).
…tration scope Move the CLI OAuth scope policy into #562 (was reverted in b4175b5 as an interactive-picker follow-up; now homed here so #562 owns scope policy and #558 owns only the interactive picker). - cli/login/_constants.py: add use-case-oriented scope presets (read-only=api:read, read-write=api:write, upload=api:write tus) with a by-name index and read-write default; rename the broad registration-scope constant DEFAULT_OAUTH_SCOPE -> DEFAULT_REGISTRATION_SCOPE to say what it is now that login differs. - auth login / auth device-code default to the minimal read-write scope (api:write) instead of the broad OIDC set; registration keeps DEFAULT_REGISTRATION_SCOPE. - Simplify upload/re-auth scope hints (_PKCE_SCOPE_HINT, upload examples/docs) to 'api:write {scope}' since api:write implies api:read.
Drop the ScopePreset NamedTuple and SCOPE_PRESETS_BY_NAME index in favour of a
plain {name: scope} dict — shorter, and it removes the three same-typed positional
args that made preset construction easy to get wrong. Human-readable descriptions
are UI copy and move to the interactive picker (bfabric_scripts _common).
In register_client / register_webapp the required 'scope' sat after the defaulted 'service_user'. Legal for keyword-only args, but list the required one first for consistency; docstrings reordered to match. No call-site impact (keyword-only).
Revert the name->scope dict switch (8c089b1). The NamedTuple keeps each preset's human-readable description co-located with its name and scope — which the interactive picker (bfabric_scripts _common) consumes — instead of splitting descriptions into a parallel map. Restores ScopePreset, SCOPE_PRESETS, and SCOPE_PRESETS_BY_NAME.
…LOGIN_SCOPE) Make --scope required on `auth login` / `auth device-code` and delete the DEFAULT_LOGIN_SCOPE constant, so the CLI no longer bakes in a default scope (matching the core library). Removes the [default: ...] hint from --help. SCOPE_PRESETS stays as the documented preset catalog.
leoschwarz
added a commit
that referenced
this pull request
Jul 17, 2026
Same reconciliation as the previous merge: main's #562 ("require explicit client_id and scope", no CLI default) vs this branch's interactive scope picker. Keep the picker. - pkce / device_code: keep --scope OPTIONAL (picker runs when omitted); discard main's required-scope signature. - _constants: keep SCOPE_PRESETS_BY_NAME (needed by the picker); our file is main's plus that one binding. - logout / status: keep our feature imports; main's DEFAULT_CLIENT_ID relocation is already present in our version. - changelog: keep the picker + no-default entry; drop main's "require --scope" line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves client_id and scope defaults to bfabric-cli rather than the core Bfabric class.
🤖 Prepared with assistance from Claude Opus 4.8 via Claude Code.