Repository navigation
chore(oauth): upgrade oauth2 to 5 - #618
Draft
hogan-yuan wants to merge 3 commits into
Draft
hogan-yuan wants to merge 3 commits into
hogan-yuan wants to merge 3 commits into
Conversation
Ports #615 (by @Tickwhiz) onto #617: - oauth2 4.4 -> 5: typestate BasicClient (LongbridgeClient alias), TokenResponse bound, async requests through a reqwest 0.12 client that refuses redirects like the previous transport - revoke_with takes any oauth2::AsyncHttpClient; the #617 timeout and lock-holding tests use Fn clients as oauth2 5 requires - loopback regression tests for authorization, denial, CSRF, refresh rotation/omission, HTTP and storage failures, reauthorization, redirect refusal, default expiry, blocking cached-token loading and endpoint selection (task-local test endpoint override) - the separate reqwest 0.11 / hyper 0.14 / rustls 0.21 chain is gone
The 5s per-flow timeout only guards against hangs; 30s avoids spurious failures on slow CI runners.
- CI runs clippy (all targets, -D warnings) and tests for longbridge-oauth; only the longbridge crate was tested, so the loopback regression suite never ran in CI - the endpoint-selection child skips instead of panicking when run without its parent (e.g. --include-ignored) - outside TEST_OAUTH_BASE_URL.scope, tests fail closed instead of silently reaching the real OAuth endpoints; the revoke-without-token test no longer probes the region over the network - the reqwest client has connect/total timeouts, so the authorization code exchange is bounded too
This branch has not been deployed
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
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.
Summary
Carries #615 by @Tickwhiz forward. That PR's branch does not allow maintainer edits, and it conflicts with #617, so it is ported here with the original authorship kept on the main commit. Stacked on #617. The base will be switched to
main(via rebase) once #617 is merged.From #615
oauth24.4 → 5. The typestateBasicClientis aliased asLongbridgeClient, and theTokenResponsebound is updated.Token requests go through a
reqwest0.12 client withredirect::Policy::none(), which preserves the old transport's refusal to follow redirects.Regression tests on loopback servers cover:
A
cfg(test)task-local overrides the endpoint for these tests.The separate
reqwest0.11 /hyper0.14 /rustls0.21 chain is gone.cargo treenow shows a singlereqwest0.12.28, and nohyper0.14.Adaptations to #617
revoke_withnow takes anyoauth2::AsyncHttpClientinstead of an oauth2 4FnOnceclient.Fnclients, as oauth2 5 requires.TOKEN_REQUEST_TIMEOUTstill wraps refresh and revoke.Review notes
reqwesthasdefault-features = false, and the feature graph only enablesrustls-tls-webpki-roots. OAuth therefore stays on rustls with webpki roots, as before, and does not switch to native-tls.LONGBRIDGE_REGIONis honored bylongbridge_geo::is_cn, so they make no network calls.Test plan
cargo test -p longbridge-oauth(47 passed, 1 ignored child) and--all-features(48 + 3 doctests)cargo clippy -p longbridge-oauth --all-features --all-targets -- -D warningscargo checkoflongbridge,longbridge-c,longbridge-java,longbridge-python,longbridge-nodejs(all features)cargo test -p longbridge --lib(81)cargo +nightly fmt --allCloses #615 once merged.
🤖 Generated with Claude Code