Repository navigation
feat(provider): support plugin-managed OAuth PKCE login - #10266
LIghtJUNction wants to merge 5 commits into
Conversation
Expose OAuth sessions with owner-bound PKCE login, serialized refresh, private persistence callbacks and scoped HTTPX request authentication. Add identity-checked adapter cleanup, a plugin page example, bilingual guides and network-free OAuth and provider registration tests.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="examples/astrbot_plugin_oauth_provider/main.py" line_range="101" />
<code_context>
+ self.http = httpx.AsyncClient(proxy=self.config.get("proxy") or None)
</code_context>
<issue_to_address>
**issue (bug_risk):** If construction of the plugin-owned `httpx.AsyncClient` raises, `self.http` remains `None`, but the initialization cleanup unconditionally executes `await self.http.aclose()`, raising `AttributeError` and masking the actual configuration or proxy error.
**Triggers:** When `httpx.AsyncClient(proxy=...)` fails during plugin initialization.
**Suggested fix:** Guard the cleanup with `if self.http is not None:` as done for the other owned resources.
```suggestion
if self.http is not None:
await self.http.aclose()
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this adds a server-side OAuth credential flow and a Bearer-token request hook; a validation or lifecycle mistake could expose provider access or send credentials to an unintended API, and reverting would not recall tokens already transmitted. It also introduces a shared-account disconnect endpoint that any authenticated Dashboard user can invoke, so an authorization mistake could delete another user's local credentials.
Blocking findings: examples/astrbot_plugin_oauth_provider/main.py:101
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | 44140dc | Commit Preview URL Branch Preview URL |
Oct 09 2026, 06:13 PM |
Apply the exact Ruff 0.15.22 CI formatting diff and defensively guard optional HTTP client cleanup. Restore the unmodified CI workflow after diagnostics; keep the pull request focused on provider OAuth support.
|
Follow-up in
Please use checks on the latest commit for the final CI result; earlier formatting failures have been addressed rather than disabled. |
Replace four credential-write paths with one fail-closed persistence method. Clear the in-memory token before awaiting storage so failed or cancelled writes cannot reuse an old account or an already-rotated refresh credential. Reuse terminate() for partial initialization cleanup. Use AsyncExitStack to attempt all owned provider shutdowns, adapter unregistration and HTTP closure even when an earlier cleanup raises. Close auth before inspecting the current provider instances, retaining the original reload ordering and class checks. Consolidate duplicate endpoint/path validation tests, remove unused test store history, and add parameterized storage/lifecycle regressions. Isolate the example subclass registration during test imports. Validation against original PR files with verified Git blob hashes: - Original OAuth tests: 59 passed; SDK test blocked by missing openai. - Persistence regressions reproduced 3 failures on the original code. - Initial cleanup regressions reproduced 3 failures on the original code. - Updated OAuth tests: 67 passed, 1 SDK test deselected locally. - Example lifecycle tests: 8 passed, including replacement during auth close, with unrelated AstrBot runtime imports stubbed by a local-only runner. This is not a full AstrBot runtime integration test. - Python compilation, Python 3.10 grammar and git diff --check passed. - Ruff, the SDK test, full suite and real supplier login remain unverified locally due to unavailable dependencies/network. No CI rules weakened. Relative to 444ddff: production +37/-39 (net -2); tests +136/-27. Public API, existing security validation and dependencies are unchanged.
Expand the review beyond the newly added OAuth implementation into the existing provider/platform managers, their registries, provider base classes, and core shutdown caller. Replace 44 provider and 17 platform import branches with explicit module maps. Keep imports lazy, retain custom plugin registration, and never derive an import path from arbitrary configuration. Use one provider-category table for type checks, instance-list updates, legacy default fields, reload fallback and teardown. Remove the five copied initialization paths. Read the configured TTS default from TTS settings, not chat settings. Close unpublished instances after initialization failure or cancellation. Detach instances before awaiting termination so failed closes do not leave stale registrations or remove a newly registered replacement. Include embedding and rerank lists in teardown, and close all provider types at shutdown while continuing after an ordinary adapter cleanup error. Deep-copy the completed model/source merge so nested source keys and headers are not shared between model instances. Keep model overrides and model IDs. Validation (exact original Git blob hashes checked locally): - New lifecycle regressions on 39a45d2: 26 failed, 3 passed. - Updated lifecycle and import tests: 39 passed (29 + 10). - All 44 provider and 17 platform module mappings match the original AST. - Python compilation, Python 3.10 grammar and git diff --check passed. - Local tests execute the real manager code with a local-only bootstrap that stubs surrounding runtime modules and provider base classes. Adapter I/O is mocked. The committed tests use normal AstrBot imports; these results are not a full dependency-complete AstrBot or supplier integration run. - Full suite and Ruff could not run locally: clone/package installation is blocked by DNS/network access. CI rules, dependencies and skips unchanged. Relative to 39a45d2: existing production code +194/-416 (net -222), tests +247/-0, total +441/-416 (net +25). These deletions are in existing core files, not only in files introduced by this OAuth PR.
|
Expanded the requested quality review beyond the new OAuth files in commit 44140dc. The existing provider and platform managers now use explicit lazy-import tables instead of 44 provider and 17 platform import branches. All 61 type-to-module mappings were checked against the original source; custom plugin registration remains supported. Provider loading, reload fallback and removal share one category table. This also fixes concrete existing defects: embedding/rerank instances remaining in their lists after removal; shutdown closing only chat providers; failed initialization not closing acquired resources; failed termination leaving stale registrations; removal of a replacement registered while an old instance closes; and TTS defaults being read from chat settings. Model/source configuration is now copied after merging so nested source keys and headers are not shared between models. Change size relative to 39a45d2: existing production code +194/-416, net -222 lines; regression tests +247/-0; total +441/-416, net +25. The production deletions are in pre-existing core files, not merely reductions to files introduced by this PR. Validation: the lifecycle tests gave 26 failures and 3 passes on the previous implementation (test cases, not 26 distinct defects). The updated lifecycle and import tests give 39 passes. The local runner executes the actual manager source but substitutes surrounding runtime modules and provider base classes; adapter I/O is mocked. The committed tests use normal AstrBot imports. Python compilation, Python 3.10 grammar checks and |
Motivation
Allow a plugin to register an AI supplier and implement OAuth login without adding a vendor-specific authentication implementation to AstrBot core. This is an opt-in public-client authorization-code/PKCE extension, not an integration with a particular supplier or consumer subscription.
Modifications
Expose
OAuth2Session,OAuth2Token, andOAuth2Errorthroughastrbot.api.provider.oauth.Implement owner-bound, expiring, single-use authorization attempts with PKCE S256, exact redirect/state validation, cancellation, refresh-token rotation, serialized refresh, and plugin-owned asynchronous persistence callbacks.
Provide an HTTPX request hook that attaches the current Bearer token only within the configured inference API origin/path. Token requests do not follow redirects; arbitrary resource destinations are rejected. Do not mutate shared SDK API keys or replay inference automatically after a 401.
Export provider registration and add identity-checked
unregister_provider_adapterfor plugin teardown/reload. Existing adapters and API-key behavior are unchanged.Add
examples/astrbot_plugin_oauth_provider/: an OpenAI-compatible adapter, authenticated Plugin Page actions, encrypted plugin-private KV storage using an externally supplied Fernet key, and explicit saved-model activation after hot reload.Add English/Chinese developer guides and focused tests, including an actual OpenAI SDK transport test with mocked HTTP responses for discovery, chat, and SSE.
This is NOT a breaking change.
Scope and security boundaries
The example uses a manual full callback-URL paste through an authenticated Plugin Page. It does not open a loopback listener or introduce anonymous Dashboard routes. The supplier must permit the exact registered redirect and explicitly support the client's Bearer tokens for inference. Device authorization, confidential clients, OIDC identity verification, supplier-specific protocols, and automatic callback hosting are not included.
The example shares one account across its models. Storage is scoped to supplier/client settings and scopes; encryption keys remain outside provider/plugin configuration.
disconnect()deletes local credentials; it does not claim supplier-side revocation. Plugins remain trusted server-side Python, not a credential-isolation sandbox. Multi-process refresh coordination is outside this helper's scope.Screenshots or Test Results
Executed in an isolated Python 3.13 environment with HTTPX mock transports:
The three registration/unregistration tests also passed with the registry's unrelated logger/metadata/tool-manager imports stubbed. This is an isolated registry-logic check, not a full AstrBot runtime test.
Python compilation and Python 3.10 grammar checks passed for the changed Python files. The example page's JavaScript passed
node --input-type=module --check; JSON configuration parsed successfully.Not yet verified locally: the OpenAI SDK transport test, the complete AstrBot test suite, Ruff, and a real supplier/browser login. The local environment lacks the SDK/Ruff and package/network access. The SDK test is included without a skip so the normal dependency-complete CI exercises it. No real supplier credentials or private client IDs are present in this PR. CI results should be evaluated separately from the focused local results.
Verification Steps
uv run pytest tests/test_provider_oauth.py tests/test_provider_oauth_registration.py -q, then the repository's regular tests and Ruff checks.docs/en/dev/star/guides/provider-oauth.md(or the Chinese guide) to configure a legitimate registered public client and install the example plugin.oauthPage, authorize in the browser, paste the exact callback URL, and complete it as the same Dashboard user.Checklist
Summary by Sourcery
Enable trusted plugins to provide secure OAuth PKCE authentication for AI providers while improving adapter lifecycle management and documenting the integration.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: