diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3375cb9f0..6608cfade2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,10 +48,14 @@ jobs: cache-bin: false - name: Check with clippy - run: cargo clippy -p longbridge --all-features + run: | + cargo clippy -p longbridge --all-features + cargo clippy -p longbridge-oauth --all-features --all-targets -- -D warnings - name: Test - run: cargo test -p longbridge --all-features + run: | + cargo test -p longbridge --all-features + cargo test -p longbridge-oauth --all-features check-python-sdk: needs: diff --git a/CHANGELOG.md b/CHANGELOG.md index 7928c97e43..29bb2f2ebb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,17 +8,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **All SDKs:** `OAuth.revoke()` revokes the current token on the server through the existing RFC 7009 endpoint (`POST /oauth2/revoke`): the refresh token when present, otherwise the access token. It only contacts the server; the in-memory and cached tokens are left untouched. On a client created with `from_token` (no client ID) it fails with a `RevokeFailed` error instead of calling the server. +- **All SDKs:** OAuth errors carry a stable numeric code (`OAuthError::code()` in Rust), exposed wherever the error kind is `OAuth`: `lb_error_code` (C), `Status::code()` (C++), `OpenApiException.getCode()` (Java) and `OpenApiException.code` (Python). Callers can tell failures apart without parsing messages, e.g. `1` = no token available, `3` = refresh failed, `4` = revoke failed, `16` = token file delete failed (full table on `OAuthError::code`). A request that fails because the access token cannot be obtained is reported the same way (kind `OAuth` with the code) instead of kind `Other`. The Node.js binding still reports errors as plain messages. +- **All SDKs:** `OAuth.revoke()` revokes the current token on the server through the existing RFC 7009 endpoint (`POST /oauth2/revoke`): the refresh token when present, otherwise the access token. It only contacts the server; the in-memory and cached tokens are left untouched. On a client created with `from_token` (no client ID) it fails with a `RevokeFailed` error instead of calling the server. The token lock is held for the whole request, so a concurrent token refresh cannot rotate the refresh token in between (which would revoke the stale token and leave the new one valid). While it runs, `access_token()` and every request that needs a token wait for it; the request is bounded by a 30-second timeout (`RevokeFailed` on expiry). - **All SDKs:** `OAuth.clear()` drops the token locally: it clears the in-memory copy and deletes the cached token file (`~/.longbridge/openapi/tokens/`; a missing file is not an error). It does not contact the server, so call `revoke()` first to disconnect the application completely. After `clear()`, `access_token()` fails with `NoTokenAvailable`. The persisted token is deleted first and the in-memory token is cleared only when that succeeds, so a failed delete leaves the handle usable and the error is returned. - - Rust: `OAuth::revoke` / `OAuth::clear` (async), `OAuthError::RevokeFailed`, `OAuthError::TokenFileDelete`. C: `lb_oauth_revoke` / `lb_oauth_clear`. C++: `OAuth::revoke` / `OAuth::clear`. Java: `OAuth.revoke()` / `OAuth.clear()` returning `CompletableFuture`. Node.js: `oauth.revoke()` / `oauth.clear()` returning promises. Python: `OAuth.revoke()` / `OAuth.clear()` plus `revoke_async()` / `clear_async()`; the `openapi.pyi` stub is updated. The blocking Python calls release the GIL while waiting. In C/C++, a NULL handle (e.g. a moved-from C++ `OAuth`) is reported through the callback as an error instead of crashing. + - Rust: `OAuth::revoke` / `OAuth::clear` (async), `OAuthError::RevokeFailed`, `OAuthError::TokenFileDelete`. C: `lb_oauth_revoke` / `lb_oauth_clear`. C++: `OAuth::revoke` / `OAuth::clear`. Java: `OAuth.revoke()` / `OAuth.clear()` returning `CompletableFuture`, which fails with `OpenApiException` like other SDK calls; both are `synchronized` with `close()`, so a concurrent `close()` cannot free the native handle while it is in use (a closed handle throws `IllegalStateException`). Node.js: `oauth.revoke()` / `oauth.clear()` returning promises. Python: `OAuth.revoke()` / `OAuth.clear()` plus `revoke_async()` / `clear_async()`; the `openapi.pyi` stub is updated. The blocking Python calls release the GIL while waiting. In C/C++, a NULL handle (e.g. a moved-from C++ `OAuth`) is reported through the callback as an error instead of crashing. - **Rust:** `TokenStorage::delete(client_id)` removes a persisted token. It has a default implementation that returns an error, so existing custom storage backends keep compiling but `OAuth::clear` fails loudly (and keeps the in-memory token) instead of reporting success without wiping anything; override it to support `clear`. `FileTokenStorage` deletes the token file. +### Changed + +- **Rust:** the internal OAuth dependency is upgraded from `oauth2` 4.4 to 5, which uses the workspace's `reqwest` 0.12 stack instead of a separate `reqwest` 0.11 / `hyper` 0.14 / `rustls` 0.21 chain. The public OAuth API, token persistence and redirect protection (token requests do not follow redirects) are unchanged; error text coming from the HTTP layer may differ. + ### Breaking changes +- **Rust:** `Error::OAuth` now holds the typed `longbridge::oauth::OAuthError` instead of a `String` (it is also `From`), `SimpleError::OAuth` becomes `SimpleError::OAuth { code, message }`, and `HttpClientError::OAuth` holds an `OAuthError`. Code that constructs or destructures these variants needs updating; `Display` output is unchanged. +- **Java:** the future returned by `OAuthBuilder.build()` now completes exceptionally with `OpenApiException` (kind `OAuth`, with the code above) instead of a bare `RuntimeException`, like `revoke()` / `clear()` and other SDK calls. `build().join()` / `get()` still throw `CompletionException` / `ExecutionException`; only code that inspects the cause (`getCause() instanceof RuntimeException`, a cast in `exceptionally`) needs updating. +- **Bindings:** because OAuth errors now carry a code, the exception text of the bindings changes where it includes the code: Python `str(OpenApiException)` shows `(kind=..., code=N, ...)` and Java `OpenApiException.toString()` shows `code=N` instead of `null`. A request that fails because the token cannot be obtained now reports kind `OAuth` with the bare OAuth message (e.g. `no token available`) instead of kind `Other` with `oauth error: no token available`. - **Rust:** `OAuthError` is now `#[non_exhaustive]` and gains `RevokeFailed` and `TokenFileDelete`. Exhaustive `match`es on `OAuthError` outside this crate need a wildcard arm; adding variants will no longer be a breaking change. ### Fixed +- **All SDKs:** an OAuth token refresh is bounded by a 30-second timeout (`RefreshTokenFailed: timed out after 30s`). It runs under the token lock, so a hung refresh previously blocked every request needing a token, and `revoke()` / `clear()`, indefinitely; a hung refresh while building the client now falls back to the browser authorization after 30 s instead of never returning. Trade-off: if the server rotated the refresh token but its response arrives after the timeout, the new token is lost and the next refresh fails with `invalid_grant`, requiring a new authorization. The authorization code exchange is bounded too (10 s to connect, 35 s in total). +- **All SDKs:** OAuth transport failures report their cause (e.g. `Request failed: client error: error sending request ...: tcp connect error: ...`) instead of a bare `Request failed`. +- **Java:** `GridContext.create` / `FundContext.create` read the `Config` native handle inside `synchronized (config)`, like every other context, so a concurrent `Config.close()` can no longer free it while it is being used. - **All SDKs:** the OAuth browser authorization flow now uses PKCE (RFC 7636, `S256`): the authorization request carries a `code_challenge` and the token exchange sends the matching `code_verifier`. The server requires PKCE for `public` clients, so previously such clients (e.g. those created through dynamic client registration) could not authorize through the SDK and failed with `OAuth authorization failed: server_error`. Clients that don't require PKCE are unaffected. ## [5.3.0] - 2026-10-23 diff --git a/Cargo.toml b/Cargo.toml index ee2c61a03c..d74cd07720 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,6 +70,6 @@ darling = "0.21.2" convert_case = "0.8.0" Inflector = "0.11.4" cbindgen = "0.29.2" -oauth2 = "4.4" +oauth2 = "5" open = "5" dirs = "6" diff --git a/c/csrc/include/longbridge.h b/c/csrc/include/longbridge.h index 8b35cf19b7..76a7e6597f 100644 --- a/c/csrc/include/longbridge.h +++ b/c/csrc/include/longbridge.h @@ -14762,6 +14762,37 @@ void lb_error_free(struct lb_error_t *error); const char *lb_error_message(const struct lb_error_t *error); +/** + * Returns the error code. + * + * For `ErrorKindHttp` it is the HTTP status, for + * `ErrorKindOpenApi` the OpenAPI error code. For `ErrorKindOAuth` + * it is a stable code identifying the failure: + * + * | Code | Meaning | + * |------|---------| + * | 0 | other (e.g. NULL OAuth handle) | + * | 1 | no token available | + * | 2 | no refresh token | + * | 3 | token refresh failed | + * | 4 | token revocation failed | + * | 5 | authorization failed | + * | 6 | authorization timeout | + * | 7 | authorization code exchange failed | + * | 8 | CSRF token mismatch | + * | 9 | failed to bind the callback server | + * | 10 | failed to get the local address | + * | 11 | callback channel closed | + * | 12 | cannot determine the home directory | + * | 13 | failed to read the token file | + * | 14 | failed to parse the token file | + * | 15 | failed to write the token file | + * | 16 | failed to delete the token file | + * | 17 | failed to create the token directory | + * | 18 | failed to serialize the token | + * + * It is 0 when the error carries no code; check `lb_error_kind` first. + */ int64_t lb_error_code(const struct lb_error_t *error); enum lb_error_kind_t lb_error_kind(const struct lb_error_t *error); diff --git a/c/src/error.rs b/c/src/error.rs index c4c5b0046f..2d44d5d5ba 100644 --- a/c/src/error.rs +++ b/c/src/error.rs @@ -62,6 +62,35 @@ pub unsafe extern "C" fn lb_error_message(error: *const CError) -> *const c_char (*error).message.to_ffi_type() } +/// Returns the error code. +/// +/// For `ErrorKindHttp` it is the HTTP status, for +/// `ErrorKindOpenApi` the OpenAPI error code. For `ErrorKindOAuth` +/// it is a stable code identifying the failure: +/// +/// | Code | Meaning | +/// |------|---------| +/// | 0 | other (e.g. NULL OAuth handle) | +/// | 1 | no token available | +/// | 2 | no refresh token | +/// | 3 | token refresh failed | +/// | 4 | token revocation failed | +/// | 5 | authorization failed | +/// | 6 | authorization timeout | +/// | 7 | authorization code exchange failed | +/// | 8 | CSRF token mismatch | +/// | 9 | failed to bind the callback server | +/// | 10 | failed to get the local address | +/// | 11 | callback channel closed | +/// | 12 | cannot determine the home directory | +/// | 13 | failed to read the token file | +/// | 14 | failed to parse the token file | +/// | 15 | failed to write the token file | +/// | 16 | failed to delete the token file | +/// | 17 | failed to create the token directory | +/// | 18 | failed to serialize the token | +/// +/// It is 0 when the error carries no code; check `lb_error_kind` first. #[unsafe(no_mangle)] pub unsafe extern "C" fn lb_error_code(error: *const CError) -> i64 { (*error).code diff --git a/c/src/oauth.rs b/c/src/oauth.rs index d27d650a7c..81fd41c5ec 100644 --- a/c/src/oauth.rs +++ b/c/src/oauth.rs @@ -55,7 +55,7 @@ pub unsafe extern "C" fn lb_oauth_new( open_url_callback(c_url.as_ptr(), open_url_userdata_usize as *mut c_void); }) .await - .map_err(|e| longbridge::Error::OAuth(e.to_string()))?; + .map_err(longbridge::Error::OAuth)?; Ok(Box::into_raw(Box::new(COAuth { inner: oauth }))) }); } @@ -86,7 +86,9 @@ unsafe fn clone_inner(oauth: *const COAuth) -> Option { } fn null_oauth_error() -> longbridge::Error { - longbridge::Error::OAuth("OAuth client is NULL".to_string()) + longbridge::Error::OAuth(longbridge::oauth::OAuthError::Other( + "OAuth client is NULL".to_string(), + )) } /// Asynchronously revoke the current token on the server (RFC 7009) @@ -110,7 +112,7 @@ pub unsafe extern "C" fn lb_oauth_revoke( .ok_or_else(null_oauth_error)? .revoke() .await - .map_err(|e| longbridge::Error::OAuth(e.to_string()))?; + .map_err(longbridge::Error::OAuth)?; Ok(()) }); } @@ -136,7 +138,7 @@ pub unsafe extern "C" fn lb_oauth_clear( .ok_or_else(null_oauth_error)? .clear() .await - .map_err(|e| longbridge::Error::OAuth(e.to_string()))?; + .map_err(longbridge::Error::OAuth)?; Ok(()) }); } diff --git a/cpp/include/status.hpp b/cpp/include/status.hpp index 6e6e1689b5..8461f3edd6 100644 --- a/cpp/include/status.hpp +++ b/cpp/include/status.hpp @@ -40,7 +40,9 @@ class Status /// Returns the error kind if an error occurs std::optional kind() const; - /// Returns the error code if an error occurs + /// Returns the error code if an error occurs: the HTTP status, the OpenAPI + /// error code, or for `ErrorKind::OAuth` a stable code identifying the + /// failure (see `lb_error_code` in longbridge.h for the table) std::optional code() const; /// Returns the error message if an error occurs diff --git a/java/javasrc/src/main/java/com/longbridge/OAuth.java b/java/javasrc/src/main/java/com/longbridge/OAuth.java index 787637bc5c..3538e65e2a 100644 --- a/java/javasrc/src/main/java/com/longbridge/OAuth.java +++ b/java/javasrc/src/main/java/com/longbridge/OAuth.java @@ -57,8 +57,9 @@ long getRaw() { * (see {@link #clear()}). * * @return A Future that completes when the token has been revoked + * @throws IllegalStateException If the handle has already been closed */ - public CompletableFuture revoke() { + public synchronized CompletableFuture revoke() { long r = getRaw(); return AsyncCallback.executeTask((callback) -> { SdkNative.oauthRevoke(r, callback); @@ -74,8 +75,9 @@ public CompletableFuture revoke() { * contacted (see {@link #revoke()}). * * @return A Future that completes when the local token has been cleared + * @throws IllegalStateException If the handle has already been closed */ - public CompletableFuture clear() { + public synchronized CompletableFuture clear() { long r = getRaw(); return AsyncCallback.executeTask((callback) -> { SdkNative.oauthClear(r, callback); diff --git a/java/javasrc/src/main/java/com/longbridge/OAuthBuilder.java b/java/javasrc/src/main/java/com/longbridge/OAuthBuilder.java index 6965310958..ff34aa4595 100644 --- a/java/javasrc/src/main/java/com/longbridge/OAuthBuilder.java +++ b/java/javasrc/src/main/java/com/longbridge/OAuthBuilder.java @@ -69,7 +69,9 @@ public OAuthBuilder setCallbackPort(int port) { * * @param onOpenUrl Called with the authorization URL; open it in a browser * or print it to the console - * @return CompletableFuture that resolves to a new {@link OAuth} handle + * @return CompletableFuture that resolves to a new {@link OAuth} handle; on + * failure it completes exceptionally with an + * {@link OpenApiException} of kind {@link ErrorKind#OAuth} */ public CompletableFuture build(Consumer onOpenUrl) { return AsyncCallback.executeTask((callback) -> { diff --git a/java/javasrc/src/main/java/com/longbridge/OpenApiException.java b/java/javasrc/src/main/java/com/longbridge/OpenApiException.java index 7e040265d2..cc5492de69 100644 --- a/java/javasrc/src/main/java/com/longbridge/OpenApiException.java +++ b/java/javasrc/src/main/java/com/longbridge/OpenApiException.java @@ -7,8 +7,9 @@ * pieces of diagnostic information: *
    *
  • {@link #getKind()} – broad category of the error (HTTP, OpenAPI, OAuth, Other)
  • - *
  • {@link #getCode()} – numeric error code returned by the server, or {@code null} for - * client-side errors
  • + *
  • {@link #getCode()} – numeric error code: the HTTP status or OpenAPI code returned + * by the server, a stable OAuth error code for {@link ErrorKind#OAuth} (see + * {@link #getCode()}), or {@code null} for other client-side errors
  • *
  • {@link #getMessage()} – human-readable error description
  • *
*/ @@ -40,9 +41,13 @@ public ErrorKind getKind() { } /** - * Returns the numeric error code returned by the server. + * Returns the numeric error code. * - * @return error code, or {@code null} for client-side errors + *

+ * For {@link ErrorKind#OAuth} errors this is a stable code identifying the + * failure: 1 no token available, 2 no refresh token, 3 refresh failed, 4 revoke failed, 5 authorization failed, 6 authorization timeout, 7 code exchange failed, 8 CSRF mismatch, 9 callback bind failed, 10 local address failed, 11 callback channel closed, 12 no home dir, 13 token file read, 14 token file parse, 15 token file write, 16 token file delete, 17 create dir failed, 18 serialize token, 0 other. + * + * @return error code, or {@code null} for client-side errors other than OAuth */ public Long getCode() { return code; diff --git a/java/javasrc/src/main/java/com/longbridge/fund/FundContext.java b/java/javasrc/src/main/java/com/longbridge/fund/FundContext.java index 14597d95e9..040c363930 100644 --- a/java/javasrc/src/main/java/com/longbridge/fund/FundContext.java +++ b/java/javasrc/src/main/java/com/longbridge/fund/FundContext.java @@ -26,7 +26,7 @@ private long raw() { */ public static FundContext create(Config config) { FundContext ctx = new FundContext(); - ctx.raw = SdkNative.newFundContext(config.getRaw()); + synchronized (config) { ctx.raw = SdkNative.newFundContext(config.getRaw()); } return ctx; } diff --git a/java/javasrc/src/main/java/com/longbridge/grid/GridContext.java b/java/javasrc/src/main/java/com/longbridge/grid/GridContext.java index 42f29f37d6..fd8445a108 100644 --- a/java/javasrc/src/main/java/com/longbridge/grid/GridContext.java +++ b/java/javasrc/src/main/java/com/longbridge/grid/GridContext.java @@ -26,7 +26,7 @@ private long raw() { */ public static GridContext create(Config config) { GridContext ctx = new GridContext(); - ctx.raw = SdkNative.newGridContext(config.getRaw()); + synchronized (config) { ctx.raw = SdkNative.newGridContext(config.getRaw()); } return ctx; } diff --git a/java/src/error.rs b/java/src/error.rs index 0e8b3d43b8..3f0302c7e9 100644 --- a/java/src/error.rs +++ b/java/src/error.rs @@ -29,7 +29,7 @@ impl From for JniError { impl From for JniError { fn from(e: longbridge::oauth::OAuthError) -> Self { - JniError::Other(e.to_string()) + JniError::OpenApi(Box::new(longbridge::Error::OAuth(e))) } } diff --git a/java/src/oauth.rs b/java/src/oauth.rs index 66d75e5188..a80a4d804d 100644 --- a/java/src/oauth.rs +++ b/java/src/oauth.rs @@ -50,7 +50,7 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_oauthBuild( } }) .await - .map_err(|e| crate::error::JniError::Other(e.to_string()))?; + .map_err(crate::error::JniError::from)?; Ok(JavaLong::from(Box::into_raw(Box::new(oauth)) as jlong)) })?; Ok(()) @@ -81,7 +81,7 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_oauthRevoke( oauth .revoke() .await - .map_err(|e| crate::error::JniError::Other(e.to_string())) + .map_err(|e| longbridge::Error::OAuth(e).into()) })?; Ok(()) }) @@ -101,7 +101,7 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_oauthClear( oauth .clear() .await - .map_err(|e| crate::error::JniError::Other(e.to_string())) + .map_err(|e| longbridge::Error::OAuth(e).into()) })?; Ok(()) }) diff --git a/python/pysrc/longbridge/openapi.pyi b/python/pysrc/longbridge/openapi.pyi index 11e6258a30..e4d583e598 100644 --- a/python/pysrc/longbridge/openapi.pyi +++ b/python/pysrc/longbridge/openapi.pyi @@ -33,6 +33,11 @@ class ErrorKind: Other error """ + class OAuth(ErrorKind): + """ + OAuth error + """ + class OpenApiException(Exception): """ OpenAPI exception @@ -43,9 +48,16 @@ class OpenApiException(Exception): Error kind """ - code: int + code: Optional[int] + """ + Error code: the HTTP status or OpenAPI code returned by the server, or for + ``ErrorKind.OAuth`` a stable code identifying the failure (1 no token available, 2 no refresh token, 3 refresh failed, 4 revoke failed, 5 authorization failed, 6 authorization timeout, 7 code exchange failed, 8 CSRF mismatch, 9 callback bind failed, 10 local address failed, 11 callback channel closed, 12 no home dir, 13 token file read, 14 token file parse, 15 token file write, 16 token file delete, 17 create dir failed, 18 serialize token, 0 other). + `None` for other client-side errors. """ - Error code + + trace_id: Optional[str] + """ + Trace ID of the request, if any """ message: str @@ -53,7 +65,13 @@ class OpenApiException(Exception): Error message """ - def __init__(self, code: int, message: str) -> None: ... + def __init__( + self, + kind: ErrorKind, + code: Optional[int], + trace_id: Optional[str], + message: str, + ) -> None: ... class HttpClient: """ diff --git a/python/src/oauth.rs b/python/src/oauth.rs index f29f7ea0a6..e847a14e6f 100644 --- a/python/src/oauth.rs +++ b/python/src/oauth.rs @@ -4,7 +4,7 @@ use pyo3::prelude::*; use crate::error::ErrorNewType; fn oauth_err(e: OAuthError) -> PyErr { - ErrorNewType(longbridge::Error::OAuth(e.to_string())).into() + ErrorNewType(longbridge::Error::OAuth(e)).into() } /// OAuth 2.0 client handle for Longbridge OpenAPI diff --git a/rust/crates/httpclient/src/error.rs b/rust/crates/httpclient/src/error.rs index e19a27ec15..cc12e2f43d 100644 --- a/rust/crates/httpclient/src/error.rs +++ b/rust/crates/httpclient/src/error.rs @@ -106,9 +106,9 @@ pub enum HttpClientError { online: i32, }, - /// OAuth error + /// OAuth error (e.g. the access token could not be obtained) #[error("oauth error: {0}")] - OAuth(String), + OAuth(#[from] longbridge_oauth::OAuthError), /// Server-sent events stream error #[error("sse stream error: {0}")] diff --git a/rust/crates/httpclient/src/request.rs b/rust/crates/httpclient/src/request.rs index ae9db9289e..dc71aadbdb 100644 --- a/rust/crates/httpclient/src/request.rs +++ b/rust/crates/httpclient/src/request.rs @@ -314,10 +314,7 @@ where DcRegion::from_credentials(&[app_key, access_token, app_secret]), ), AuthConfig::OAuth(oauth) => { - let token = oauth - .access_token() - .await - .map_err(|e| HttpClientError::OAuth(e.to_string()))?; + let token = oauth.access_token().await.map_err(HttpClientError::OAuth)?; // Derive DC region from the token prefix (us_→US, others→AP). // The token is sent as-is (including any prefix); the gateway // accepts the full token and routes via the x-dc-region header. diff --git a/rust/crates/oauth/Cargo.toml b/rust/crates/oauth/Cargo.toml index 288dc19f68..33cbd9ae03 100644 --- a/rust/crates/oauth/Cargo.toml +++ b/rust/crates/oauth/Cargo.toml @@ -21,4 +21,4 @@ tokio = { workspace = true, features = ["rt", "time", "sync", "net"] } tracing.workspace = true [dev-dependencies] -tokio = { workspace = true, features = ["rt-multi-thread", "macros"] } +tokio = { workspace = true, features = ["rt-multi-thread", "macros", "io-util", "test-util"] } diff --git a/rust/crates/oauth/src/client.rs b/rust/crates/oauth/src/client.rs index cb70f43639..19ed99032a 100644 --- a/rust/crates/oauth/src/client.rs +++ b/rust/crates/oauth/src/client.rs @@ -1,11 +1,11 @@ -use std::{fmt, sync::Arc}; +use std::{fmt, sync::Arc, time::Duration}; use longbridge_geo::is_cn; use oauth2::{ - AccessToken, AuthUrl, AuthorizationCode, ClientId, CsrfToken, PkceCodeChallenge, - PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, RevocationErrorResponseType, - RevocationUrl, Scope, StandardErrorResponse, StandardRevocableToken, TokenUrl, - basic::BasicClient, reqwest::async_http_client, + AccessToken, AsyncHttpClient, AuthUrl, AuthorizationCode, ClientId, CsrfToken, EndpointNotSet, + EndpointSet, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, + RevocationErrorResponseType, RevocationUrl, Scope, StandardErrorResponse, + StandardRevocableToken, TokenUrl, basic::BasicClient, reqwest, }; use poem::listener::{Acceptor, Listener, TcpListener}; @@ -20,19 +20,46 @@ const OAUTH_BASE_URL: &str = "https://openapi.longbridge.com/oauth2"; const OAUTH_BASE_URL_CN: &str = "https://openapi.longbridge.cn/oauth2"; const OAUTH_BASE_URL_TEST: &str = "https://openapi-global.longbridge.xyz/oauth2"; -async fn oauth_base_url() -> &'static str { +#[cfg(test)] +tokio::task_local! { + pub(crate) static TEST_OAUTH_BASE_URL: String; +} + +async fn oauth_base_url() -> String { + // Keep protocol tests on their own loopback server without process-wide + // state. + #[cfg(test)] + { + if let Ok(base) = TEST_OAUTH_BASE_URL.try_with(Clone::clone) { + return base; + } + // Fail closed: a test outside the scope (e.g. a request moved into a + // spawned task, which does not inherit task-locals) must not silently + // reach the real endpoints. Only the endpoint-selection child process + // resolves them on purpose, with the region fixed by environment. + assert!( + std::env::var("TEST_OAUTH_EXPECTED_AUTHORIZATION_URL").is_ok(), + "test reached the real OAuth endpoint; run it inside TEST_OAUTH_BASE_URL.scope" + ); + } + if std::env::var("LONGBRIDGE_ENV") .map(|v| v == "staging") .unwrap_or(false) { - OAUTH_BASE_URL_TEST + OAUTH_BASE_URL_TEST.to_string() } else if is_cn().await { - OAUTH_BASE_URL_CN + OAUTH_BASE_URL_CN.to_string() } else { - OAUTH_BASE_URL + OAUTH_BASE_URL.to_string() } } +/// The oauth2 client with the authorization, token and revocation endpoints +/// set. +type LongbridgeClient = + BasicClient; + /// Default port for the local OAuth callback server. pub(crate) const DEFAULT_CALLBACK_PORT: u16 = 60355; @@ -127,6 +154,13 @@ impl OAuth { /// The refresh token is revoked when present, otherwise the access token. /// This only calls the server: the in-memory and persisted tokens are left /// untouched. Call [`clear`](OAuth::clear) afterwards to drop them locally. + /// + /// The token lock is held for the whole request, so a concurrent + /// [`access_token`](OAuth::access_token) refresh cannot rotate the refresh + /// token between reading and revoking it (which would revoke a stale token + /// while the new one stays valid). While it runs, `access_token` (and every + /// request that needs a token) waits; the request is therefore bounded by a + /// 30-second timeout, as is a token refresh it may have to wait for. pub async fn revoke(&self) -> OAuthResult<()> { // `from_token` clients have no client ID: the token was issued // elsewhere, so the server cannot revoke it on our behalf. @@ -138,41 +172,59 @@ impl OAuth { }); } - let token = self - .0 - .token - .lock() - .await - .clone() - .ok_or(OAuthError::NoTokenAvailable)?; + // Built before taking the lock: the first call may probe the region. + let client = create_oauth_client( + &self.0.client_id, + &format!("http://localhost:{}/callback", self.0.callback_port), + ) + .await; + let http_client = create_http_client().map_err(|e| OAuthError::RevokeFailed { + message: e.to_string(), + })?; + self.revoke_with(&client, &http_client).await + } + + /// Revoke through `http_client` (injectable for tests), holding the token + /// lock for the whole request. + async fn revoke_with(&self, client: &LongbridgeClient, http_client: &C) -> OAuthResult<()> + where + C: for<'c> AsyncHttpClient<'c>, + { + let guard = self.0.token.lock().await; + let token = guard.as_ref().ok_or(OAuthError::NoTokenAvailable)?; - let revocable = match token.refresh_token { + let revocable = match &token.refresh_token { Some(refresh_token) => { - StandardRevocableToken::RefreshToken(RefreshToken::new(refresh_token)) + StandardRevocableToken::RefreshToken(RefreshToken::new(refresh_token.clone())) + } + None => { + StandardRevocableToken::AccessToken(AccessToken::new(token.access_token.clone())) } - None => StandardRevocableToken::AccessToken(AccessToken::new(token.access_token)), }; tracing::debug!(client_id = %self.0.client_id, "revoking OAuth token"); - let client = create_oauth_client( - &self.0.client_id, - &format!("http://localhost:{}/callback", self.0.callback_port), - ) - .await; - client + let request = client .revoke_token(revocable) .map_err(|e| OAuthError::RevokeFailed { message: e.to_string(), })? - .request_async(async_http_client) + .request_async(http_client); + tokio::time::timeout(TOKEN_REQUEST_TIMEOUT, request) .await + .map_err(|_| { + tracing::error!(client_id = %self.0.client_id, "token revocation timed out"); + OAuthError::RevokeFailed { + message: format!("timed out after {}s", TOKEN_REQUEST_TIMEOUT.as_secs()), + } + })? .map_err(|e| { let message = revoke_error_message(&e); tracing::error!(client_id = %self.0.client_id, error = %message, "failed to revoke token"); OAuthError::RevokeFailed { message } })?; + drop(guard); tracing::info!(client_id = %self.0.client_id, "token revoked"); Ok(()) } @@ -238,15 +290,18 @@ impl OAuth { } tracing::debug!(client_id = %self.0.client_id, "exchanging authorization code for token"); + let http_client = create_http_client().map_err(|e| OAuthError::ExchangeCodeFailed { + message: e.to_string(), + })?; let token_response = client .exchange_code(AuthorizationCode::new(code)) .set_pkce_verifier(pkce_verifier) - .request_async(async_http_client) + .request_async(&http_client) .await .map_err(|e| { tracing::error!(client_id = %self.0.client_id, error = %e, "failed to exchange authorization code for token"); OAuthError::ExchangeCodeFailed { - message: e.to_string(), + message: error_chain(&e), } })?; @@ -269,14 +324,27 @@ impl OAuth { &format!("http://localhost:{}/callback", self.0.callback_port), ) .await; - let token_response = client - .exchange_refresh_token(&RefreshToken::new(refresh_token_str.to_string())) - .request_async(async_http_client) + // Refreshes run under the token lock (see `access_token`), so a hung + // request must not hold it forever. + let http_client = create_http_client().map_err(|e| OAuthError::RefreshTokenFailed { + message: e.to_string(), + })?; + let refresh_token = RefreshToken::new(refresh_token_str.to_string()); + let request = client + .exchange_refresh_token(&refresh_token) + .request_async(&http_client); + let token_response = tokio::time::timeout(TOKEN_REQUEST_TIMEOUT, request) .await + .map_err(|_| { + tracing::error!(client_id = %self.0.client_id, "token refresh timed out"); + OAuthError::RefreshTokenFailed { + message: format!("timed out after {}s", TOKEN_REQUEST_TIMEOUT.as_secs()), + } + })? .map_err(|e| { tracing::error!(client_id = %self.0.client_id, error = %e, "failed to refresh token"); OAuthError::RefreshTokenFailed { - message: e.to_string(), + message: error_chain(&e), } })?; @@ -293,6 +361,30 @@ impl OAuth { } } +/// Upper bound for a refresh or revocation request; both hold the token lock. +const TOKEN_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); + +/// Total timeout of the HTTP client, which also bounds the authorization code +/// exchange. It must exceed `TOKEN_REQUEST_TIMEOUT`, so that for refresh and +/// revoke the tokio timeout fires first and reports "timed out after 30s" +/// instead of the client's generic transport error. +const HTTP_REQUEST_TIMEOUT: Duration = Duration::from_secs(35); +const _: () = assert!(HTTP_REQUEST_TIMEOUT.as_secs() > TOKEN_REQUEST_TIMEOUT.as_secs()); + +/// `e` followed by its `source()` chain. oauth2 5 reports every transport +/// failure as a bare "Request failed"; the cause (connection refused, timed +/// out, ...) is only in the sources. +fn error_chain(e: &dyn std::error::Error) -> String { + let mut message = e.to_string(); + let mut source = e.source(); + while let Some(cause) = source { + message.push_str(": "); + message.push_str(&cause.to_string()); + source = cause.source(); + } + message +} + /// Describe a revocation failure, including the server's `error` and /// `error_description` (the `Display` of `ServerResponse` omits them). fn revoke_error_message( @@ -306,7 +398,7 @@ where Some(desc) => format!("{}: {desc}", resp.error()), None => resp.error().to_string(), }, - other => other.to_string(), + other => error_chain(other), } } @@ -315,7 +407,9 @@ where /// PKCE (RFC 7636, `S256`) is required by the server for public clients and /// ignored for clients that don't need it. The returned verifier must be sent /// with the token exchange. -fn build_authorize_url(client: &BasicClient) -> (oauth2::url::Url, CsrfToken, PkceCodeVerifier) { +fn build_authorize_url( + client: &LongbridgeClient, +) -> (oauth2::url::Url, CsrfToken, PkceCodeVerifier) { let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); let (auth_url, csrf_token) = client .authorize_url(CsrfToken::new_random) @@ -326,25 +420,37 @@ fn build_authorize_url(client: &BasicClient) -> (oauth2::url::Url, CsrfToken, Pk } /// Build the oauth2 BasicClient for Longbridge endpoints. -async fn create_oauth_client(client_id: &str, redirect_uri: &str) -> BasicClient { - build_oauth_client(oauth_base_url().await, client_id, redirect_uri) +async fn create_oauth_client(client_id: &str, redirect_uri: &str) -> LongbridgeClient { + build_oauth_client(&oauth_base_url().await, client_id, redirect_uri) } -fn build_oauth_client(base: &str, client_id: &str, redirect_uri: &str) -> BasicClient { - BasicClient::new( - ClientId::new(client_id.to_string()), - None, - AuthUrl::new(format!("{base}/authorize")).unwrap(), - Some(TokenUrl::new(format!("{base}/token")).unwrap()), - ) - .set_redirect_uri(RedirectUrl::new(redirect_uri.to_string()).unwrap()) - .set_revocation_uri(RevocationUrl::new(format!("{base}/revoke")).unwrap()) +fn build_oauth_client(base: &str, client_id: &str, redirect_uri: &str) -> LongbridgeClient { + BasicClient::new(ClientId::new(client_id.to_string())) + .set_auth_uri(AuthUrl::new(format!("{base}/authorize")).unwrap()) + .set_token_uri(TokenUrl::new(format!("{base}/token")).unwrap()) + .set_redirect_uri(RedirectUrl::new(redirect_uri.to_string()).unwrap()) + .set_revocation_url(RevocationUrl::new(format!("{base}/revoke")).unwrap()) +} + +/// HTTP client for token requests. +fn create_http_client() -> Result { + reqwest::Client::builder() + // Preserve the previous OAuth transport's protection against redirects. + .redirect(reqwest::redirect::Policy::none()) + // Bounds every token request, including the authorization code + // exchange. Refresh and revoke are wrapped in TOKEN_REQUEST_TIMEOUT, + // which must fire first so they report "timed out after 30s". + .connect_timeout(Duration::from_secs(10)) + .timeout(HTTP_REQUEST_TIMEOUT) + .build() } #[cfg(test)] mod tests { use std::collections::HashMap; + use oauth2::{HttpRequest, HttpResponse}; + use super::*; use crate::storage::StoredToken; @@ -462,6 +568,158 @@ mod tests { })) } + #[tokio::test] + async fn revoke_holds_token_lock_for_the_whole_request() { + let oauth = OAuth(Arc::new(OAuthInner { + client_id: "my-client".to_string(), + callback_port: DEFAULT_CALLBACK_PORT, + storage: Arc::new(LegacyStorage), + token: tokio::sync::Mutex::new(Some(OAuthToken { + client_id: "my-client".to_string(), + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at: u64::MAX, + })), + })); + let client = build_oauth_client( + OAUTH_BASE_URL, + "my-client", + "http://localhost:60355/callback", + ); + + // oauth2 5 requires an `Fn` client; it is called exactly once here. + let (started_tx, started_rx) = tokio::sync::oneshot::channel::(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel::<()>(); + let channels = std::sync::Mutex::new(Some((started_tx, release_rx))); + let http_client = move |req: HttpRequest| { + let (started_tx, release_rx) = channels.lock().unwrap().take().unwrap(); + async move { + started_tx.send(req).unwrap(); + release_rx.await.unwrap(); + Ok::<_, std::io::Error>(HttpResponse::new(Vec::new())) + } + }; + + let task = { + let oauth = oauth.clone(); + tokio::spawn(async move { oauth.revoke_with(&client, &http_client).await }) + }; + + let req = started_rx.await.unwrap(); + let body = String::from_utf8(req.into_body()).unwrap(); + assert!(body.contains("token=refresh"), "{body}"); + assert!(body.contains("token_type_hint=refresh_token"), "{body}"); + // A refresh (access_token) cannot run while the request is in flight. + assert!(oauth.0.token.try_lock().is_err()); + + release_tx.send(()).unwrap(); + task.await.unwrap().unwrap(); + assert!(oauth.0.token.try_lock().is_ok()); + } + + #[test] + fn error_chain_includes_the_transport_cause() { + let e: RequestTokenError< + std::io::Error, + StandardErrorResponse, + > = RequestTokenError::Request(std::io::Error::other("connection refused")); + assert_eq!(error_chain(&e), "Request failed: connection refused"); + assert_eq!( + revoke_error_message(&e), + "Request failed: connection refused" + ); + } + + #[tokio::test(start_paused = true)] + async fn revoke_times_out_and_releases_the_token_lock() { + let oauth = oauth_with_storage(Arc::new(LegacyStorage)); + let client = build_oauth_client( + OAUTH_BASE_URL, + "my-client", + "http://localhost:60355/callback", + ); + // The request never completes; the paused clock auto-advances to the + // timeout. + let http_client = + |_req: HttpRequest| std::future::pending::>(); + + match oauth.revoke_with(&client, &http_client).await { + Err(OAuthError::RevokeFailed { message }) => { + assert_eq!(message, "timed out after 30s") + } + other => panic!("unexpected result: {other:?}"), + } + assert_eq!(oauth.access_token().await.unwrap(), "access"); + } + + #[test] + fn oauth_error_codes_are_stable() { + let io = || std::io::Error::other("x"); + let json = || serde_json::from_str::<()>("x").unwrap_err(); + let path = std::path::PathBuf::new; + let msg = String::new; + let cases = [ + (OAuthError::Other(msg()), 0), + (OAuthError::NoTokenAvailable, 1), + (OAuthError::NoRefreshToken, 2), + (OAuthError::RefreshTokenFailed { message: msg() }, 3), + (OAuthError::RevokeFailed { message: msg() }, 4), + (OAuthError::AuthorizationFailed { message: msg() }, 5), + (OAuthError::AuthorizationTimeout, 6), + (OAuthError::ExchangeCodeFailed { message: msg() }, 7), + (OAuthError::CsrfTokenMismatch, 8), + ( + OAuthError::BindCallbackFailed { + port: 0, + message: msg(), + }, + 9, + ), + (OAuthError::LocalAddressFailed, 10), + (OAuthError::CallbackChannelClosed, 11), + (OAuthError::NoHomeDir, 12), + ( + OAuthError::TokenFileRead { + path: path(), + source: io(), + }, + 13, + ), + ( + OAuthError::TokenFileParse { + path: path(), + source: json(), + }, + 14, + ), + ( + OAuthError::TokenFileWrite { + path: path(), + source: io(), + }, + 15, + ), + ( + OAuthError::TokenFileDelete { + path: path(), + source: io(), + }, + 16, + ), + ( + OAuthError::CreateDirFailed { + path: path(), + source: io(), + }, + 17, + ), + (OAuthError::SerializeToken { source: json() }, 18), + ]; + for (err, code) in cases { + assert_eq!(err.code(), code, "{err:?}"); + } + } + #[tokio::test] async fn clear_keeps_memory_token_when_storage_delete_fails() { let oauth = oauth_with_storage(Arc::new(FailingDelete)); diff --git a/rust/crates/oauth/src/error.rs b/rust/crates/oauth/src/error.rs index 22fcddb5ec..e5567f5656 100644 --- a/rust/crates/oauth/src/error.rs +++ b/rust/crates/oauth/src/error.rs @@ -134,5 +134,58 @@ pub enum OAuthError { Other(String), } +impl OAuthError { + /// Stable numeric code identifying the kind of error, so that callers of + /// the language bindings (where the error crosses the FFI boundary as a + /// message plus a code) can tell failures apart without parsing messages. + /// + /// | Code | Variant | + /// |------|---------| + /// | 1 | [`NoTokenAvailable`](Self::NoTokenAvailable) | + /// | 2 | [`NoRefreshToken`](Self::NoRefreshToken) | + /// | 3 | [`RefreshTokenFailed`](Self::RefreshTokenFailed) | + /// | 4 | [`RevokeFailed`](Self::RevokeFailed) | + /// | 5 | [`AuthorizationFailed`](Self::AuthorizationFailed) | + /// | 6 | [`AuthorizationTimeout`](Self::AuthorizationTimeout) | + /// | 7 | [`ExchangeCodeFailed`](Self::ExchangeCodeFailed) | + /// | 8 | [`CsrfTokenMismatch`](Self::CsrfTokenMismatch) | + /// | 9 | [`BindCallbackFailed`](Self::BindCallbackFailed) | + /// | 10 | [`LocalAddressFailed`](Self::LocalAddressFailed) | + /// | 11 | [`CallbackChannelClosed`](Self::CallbackChannelClosed) | + /// | 12 | [`NoHomeDir`](Self::NoHomeDir) | + /// | 13 | [`TokenFileRead`](Self::TokenFileRead) | + /// | 14 | [`TokenFileParse`](Self::TokenFileParse) | + /// | 15 | [`TokenFileWrite`](Self::TokenFileWrite) | + /// | 16 | [`TokenFileDelete`](Self::TokenFileDelete) | + /// | 17 | [`CreateDirFailed`](Self::CreateDirFailed) | + /// | 18 | [`SerializeToken`](Self::SerializeToken) | + /// | 0 | [`Other`](Self::Other) | + /// + /// Codes are never reused; a new variant gets a new code. + pub fn code(&self) -> i64 { + match self { + OAuthError::Other(_) => 0, + OAuthError::NoTokenAvailable => 1, + OAuthError::NoRefreshToken => 2, + OAuthError::RefreshTokenFailed { .. } => 3, + OAuthError::RevokeFailed { .. } => 4, + OAuthError::AuthorizationFailed { .. } => 5, + OAuthError::AuthorizationTimeout => 6, + OAuthError::ExchangeCodeFailed { .. } => 7, + OAuthError::CsrfTokenMismatch => 8, + OAuthError::BindCallbackFailed { .. } => 9, + OAuthError::LocalAddressFailed => 10, + OAuthError::CallbackChannelClosed => 11, + OAuthError::NoHomeDir => 12, + OAuthError::TokenFileRead { .. } => 13, + OAuthError::TokenFileParse { .. } => 14, + OAuthError::TokenFileWrite { .. } => 15, + OAuthError::TokenFileDelete { .. } => 16, + OAuthError::CreateDirFailed { .. } => 17, + OAuthError::SerializeToken { .. } => 18, + } + } +} + /// Result type for OAuth operations pub type OAuthResult = std::result::Result; diff --git a/rust/crates/oauth/src/lib.rs b/rust/crates/oauth/src/lib.rs index 02cc74f0b2..affe3fad00 100644 --- a/rust/crates/oauth/src/lib.rs +++ b/rust/crates/oauth/src/lib.rs @@ -42,6 +42,8 @@ pub use storage::{FileTokenStorage, StoredToken, TokenStorage}; #[cfg(test)] mod tests { + mod flows; + use std::{ sync::{Arc, Mutex}, time::{SystemTime, UNIX_EPOCH}, @@ -475,10 +477,11 @@ mod tests { storage: default_storage(), token: tokio::sync::Mutex::new(None), })); - assert!(matches!( - oauth.revoke().await, - Err(crate::OAuthError::NoTokenAvailable) - )); + // Keep `revoke` off the real endpoints and the region probe. + let result = crate::client::TEST_OAUTH_BASE_URL + .scope("http://127.0.0.1:9/oauth2".to_string(), oauth.revoke()) + .await; + assert!(matches!(result, Err(crate::OAuthError::NoTokenAvailable))); } #[test] diff --git a/rust/crates/oauth/src/tests/flows.rs b/rust/crates/oauth/src/tests/flows.rs new file mode 100644 index 0000000000..10e1f5b6c3 --- /dev/null +++ b/rust/crates/oauth/src/tests/flows.rs @@ -0,0 +1,1031 @@ +//! Regression tests for the public OAuth token lifecycle and endpoint +//! selection. + +use std::{ + collections::{BTreeMap, VecDeque}, + process::Command, + sync::{Arc, Mutex}, + time::Duration, +}; + +use oauth2::url::{Url, form_urlencoded}; +use poem::{ + EndpointExt, Request, Response, Route, Server, handler, + http::{Method, StatusCode}, + listener::{Acceptor, Listener, TcpListener}, + web::Data, +}; +use tokio::{io::AsyncWriteExt, net::TcpStream, task::JoinHandle, time::timeout}; + +use super::{MemoryStorage, now_secs}; + +/// Upper bound for a whole loopback flow. It only guards against hangs, so it +/// is generous for slow CI runners. +const HANG_GUARD: Duration = Duration::from_secs(30); +use crate::{ + OAuth, OAuthBuilder, OAuthError, OAuthResult, StoredToken, TokenStorage, + client::TEST_OAUTH_BASE_URL, +}; + +impl TokenStorage for Arc { + fn load(&self, client_id: &str) -> Option { + self.as_ref().load(client_id) + } + + fn save(&self, token: &StoredToken) -> OAuthResult<()> { + self.as_ref().save(token) + } + + fn delete(&self, client_id: &str) -> OAuthResult<()> { + self.as_ref().delete(client_id) + } +} + +struct FailSecondSaveStorage { + inner: Arc, + save_attempts: Mutex, +} + +impl TokenStorage for FailSecondSaveStorage { + fn load(&self, client_id: &str) -> Option { + self.inner.load(client_id) + } + + fn save(&self, token: &StoredToken) -> OAuthResult<()> { + let mut attempts = self.save_attempts.lock().unwrap(); + *attempts += 1; + if *attempts == 2 { + return Err(OAuthError::Other("test storage save failed".to_string())); + } + self.inner.save(token) + } +} + +#[derive(Clone)] +struct TokenRequest { + method: Method, + path: String, + content_type: Option, + authorization: Option, + body: String, +} + +struct TokenServerState { + responses: VecDeque<(StatusCode, String)>, + requests: Vec, + redirect_location: Option, +} + +struct TokenServer { + base_url: String, + state: Arc>, + task: JoinHandle<()>, +} + +impl TokenServer { + async fn start(responses: impl IntoIterator) -> Self { + #[handler] + async fn token_handler( + request: &Request, + body: String, + state: Data<&Arc>>, + ) -> Response { + let mut state = state.lock().unwrap(); + state.requests.push(TokenRequest { + method: request.method().clone(), + path: request.uri().path().to_string(), + content_type: request + .headers() + .get("content-type") + .map(|value| value.to_str().unwrap().to_string()), + authorization: request + .headers() + .get("authorization") + .map(|value| value.to_str().unwrap().to_string()), + body, + }); + let (status, body) = state + .responses + .pop_front() + .expect("unexpected token request"); + let mut response = Response::builder() + .status(status) + .content_type("application/json"); + if status.is_redirection() + && let Some(location) = state.redirect_location.as_deref() + { + response = response.header("location", location); + } + response.body(body) + } + + let acceptor = TcpListener::bind("127.0.0.1:0") + .into_acceptor() + .await + .unwrap(); + let port = acceptor + .local_addr() + .into_iter() + .find_map(|address| address.as_socket_addr().map(|address| address.port())) + .unwrap(); + let state = Arc::new(Mutex::new(TokenServerState { + responses: responses.into_iter().collect(), + requests: Vec::new(), + redirect_location: None, + })); + let app = Route::new() + .at("/oauth2/token", poem::post(token_handler)) + .at("/oauth2/revoke", poem::post(token_handler)) + .data(Arc::clone(&state)); + let task = tokio::spawn(async move { + Server::new_with_acceptor(acceptor) + .run(app) + .await + .expect("token server failed"); + }); + + Self { + base_url: format!("http://127.0.0.1:{port}/oauth2"), + state, + task, + } + } + + fn requests(&self) -> Vec { + self.state.lock().unwrap().requests.clone() + } + + fn redirect_to(&self, location: String) { + self.state.lock().unwrap().redirect_location = Some(location); + } +} + +impl Drop for TokenServer { + fn drop(&mut self) { + self.task.abort(); + } +} + +fn successful_token_response( + access_token: &str, + refresh_token: &str, + expires_in: u64, +) -> (StatusCode, String) { + ( + StatusCode::OK, + serde_json::json!({ + "access_token": access_token, + "refresh_token": refresh_token, + "token_type": "Bearer", + "expires_in": expires_in + }) + .to_string(), + ) +} + +fn assert_refresh_requests(server: &TokenServer, expected_refresh_tokens: &[&str]) { + let requests = server.requests(); + assert_eq!(requests.len(), expected_refresh_tokens.len() + 1); + for (request, refresh_token) in requests[1..].iter().zip(expected_refresh_tokens) { + assert_eq!(request.method, Method::POST); + assert_eq!(request.path, "/oauth2/token"); + assert!(request.authorization.is_none()); + let form: BTreeMap<_, _> = form_urlencoded::parse(request.body.as_bytes()) + .into_owned() + .collect(); + assert_eq!( + form, + BTreeMap::from([ + ("client_id".to_string(), "test-client".to_string()), + ("grant_type".to_string(), "refresh_token".to_string()), + ("refresh_token".to_string(), (*refresh_token).to_string()), + ]) + ); + } +} + +fn callback_url(redirect_uri: &str, code: &str, state: &str) -> Url { + let mut url = Url::parse(redirect_uri).unwrap(); + assert_eq!(url.path(), "/callback"); + url.query_pairs_mut() + .append_pair("code", code) + .append_pair("state", state); + url +} + +async fn send_callback(url: Url) { + assert_eq!(url.scheme(), "http"); + assert_eq!(url.host_str(), Some("localhost")); + let port = url.port().expect("callback must use an explicit port"); + let mut stream = TcpStream::connect(("127.0.0.1", port)).await.unwrap(); + let request = format!( + "GET {}?{} HTTP/1.1\r\nHost: localhost:{port}\r\nConnection: close\r\n\r\n", + url.path(), + url.query().unwrap() + ); + stream.write_all(request.as_bytes()).await.unwrap(); + stream.shutdown().await.unwrap(); +} + +async fn build_with_callback(storage: impl TokenStorage) -> OAuthResult { + let callback = Mutex::new(None); + let result = OAuthBuilder::new("test-client") + .callback_port(0) + .token_storage(storage) + .build(|url| { + let url = Url::parse(url).unwrap(); + let params: BTreeMap<_, _> = url.query_pairs().into_owned().collect(); + let redirect = callback_url( + params.get("redirect_uri").unwrap(), + "authorization-code", + params.get("state").unwrap(), + ); + *callback.lock().unwrap() = Some(tokio::spawn(send_callback(redirect))); + }) + .await; + let callback = callback.lock().unwrap().take(); + if let Some(callback) = callback { + callback.await.unwrap(); + } + result +} + +#[tokio::test] +async fn build_authorizes_and_persists_token() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([( + StatusCode::OK, + serde_json::json!({ + "access_token": "authorized-access-token", + "refresh_token": "authorized-refresh-token", + "token_type": "Bearer", + "expires_in": 3600 + }) + .to_string(), + )]) + .await; + let storage = Arc::new(MemoryStorage::default()); + let authorization_url = Mutex::new(None); + let callback = Mutex::new(None); + let started_at = now_secs(); + + let oauth = TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = OAuthBuilder::new("test-client") + .callback_port(0) + .token_storage(Arc::clone(&storage)) + .build(|url| { + let url = Url::parse(url).unwrap(); + let params: BTreeMap<_, _> = url.query_pairs().into_owned().collect(); + assert_eq!(url.path(), "/oauth2/authorize"); + assert_eq!(params.get("client_id").unwrap(), "test-client"); + assert_eq!(params.get("response_type").unwrap(), "code"); + assert!(!params.get("state").unwrap().is_empty()); + let redirect = callback_url( + params.get("redirect_uri").unwrap(), + "authorization-code+ /value", + params.get("state").unwrap(), + ); + *authorization_url.lock().unwrap() = Some(url); + *callback.lock().unwrap() = Some(tokio::spawn(send_callback(redirect))); + }) + .await + .unwrap(); + assert_eq!( + oauth.access_token().await.unwrap(), + "authorized-access-token" + ); + oauth + }) + .await; + + let callback = callback.lock().unwrap().take().unwrap(); + callback.await.unwrap(); + assert_eq!(oauth.client_id(), "test-client"); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.client_id, "test-client"); + assert_eq!(stored.access_token, "authorized-access-token"); + assert_eq!( + stored.refresh_token.as_deref(), + Some("authorized-refresh-token") + ); + assert!(stored.expires_at >= started_at + 3600); + assert!(stored.expires_at <= now_secs() + 3600); + assert_eq!(storage.save_count(), 1); + + let authorization_url = authorization_url.lock().unwrap().take().unwrap(); + let authorization_params: BTreeMap<_, _> = + authorization_url.query_pairs().into_owned().collect(); + let requests = server.requests(); + assert_eq!(requests.len(), 1); + let request = &requests[0]; + assert_eq!(request.method, Method::POST); + assert_eq!(request.path, "/oauth2/token"); + assert_eq!( + request.content_type.as_deref(), + Some("application/x-www-form-urlencoded") + ); + assert!(request.authorization.is_none()); + let form: BTreeMap<_, _> = form_urlencoded::parse(request.body.as_bytes()) + .into_owned() + .collect(); + assert_eq!(authorization_params["code_challenge_method"], "S256"); + let verifier = oauth2::PkceCodeVerifier::new(form["code_verifier"].clone()); + assert_eq!( + authorization_params["code_challenge"], + oauth2::PkceCodeChallenge::from_code_verifier_sha256(&verifier).as_str() + ); + let expected = BTreeMap::from([ + ("client_id".to_string(), "test-client".to_string()), + ("grant_type".to_string(), "authorization_code".to_string()), + ("code".to_string(), "authorization-code+ /value".to_string()), + ("code_verifier".to_string(), form["code_verifier"].clone()), + ( + "redirect_uri".to_string(), + authorization_params.get("redirect_uri").unwrap().clone(), + ), + ]); + assert_eq!(form, expected); + assert!(!form.contains_key("client_secret")); + }) + .await + .expect("OAuth authorization flow timed out"); +} + +#[tokio::test] +async fn revoke_rejects_insecure_endpoint_without_changing_local_tokens() { + timeout(HANG_GUARD, async { + for refresh_token in [Some("cached-refresh-token"), None] { + let server = TokenServer::start([]).await; + let storage = Arc::new(MemoryStorage::with_token(StoredToken { + client_id: "test-client".to_string(), + access_token: "cached-access-token".to_string(), + refresh_token: refresh_token.map(str::to_string), + expires_at: now_secs() + 7200, + })); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = OAuthBuilder::new("test-client") + .token_storage(Arc::clone(&storage)) + .build(|_| panic!("cached token must not trigger authorization")) + .await + .unwrap(); + assert!(matches!( + oauth.revoke().await, + Err(OAuthError::RevokeFailed { message }) + if message == "Scheme for revocation endpoint URL must be HTTPS" + )); + assert_eq!(oauth.access_token().await.unwrap(), "cached-access-token"); + }) + .await; + + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "cached-access-token"); + assert_eq!(stored.refresh_token.as_deref(), refresh_token); + assert_eq!(storage.save_count(), 0); + assert!(server.requests().is_empty()); + } + }) + .await + .expect("OAuth insecure revocation rejection timed out"); +} + +#[tokio::test] +async fn build_rejects_mismatched_csrf_state_without_exchanging_token() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([]).await; + let storage = Arc::new(MemoryStorage::default()); + let callback = Mutex::new(None); + + let result = TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + OAuthBuilder::new("test-client") + .callback_port(0) + .token_storage(Arc::clone(&storage)) + .build(|url| { + let url = Url::parse(url).unwrap(); + let params: BTreeMap<_, _> = url.query_pairs().into_owned().collect(); + let mismatched_state = + format!("{}-mismatched", params.get("state").unwrap()); + let redirect = callback_url( + params.get("redirect_uri").unwrap(), + "authorization-code", + &mismatched_state, + ); + *callback.lock().unwrap() = Some(tokio::spawn(send_callback(redirect))); + }) + .await + }) + .await; + + let callback = callback.lock().unwrap().take().unwrap(); + callback.await.unwrap(); + assert!( + matches!(&result, Err(OAuthError::CsrfTokenMismatch)), + "unexpected build result: {result:?}" + ); + assert!(server.requests().is_empty()); + assert_eq!(storage.save_count(), 0); + assert!(storage.load("test-client").is_none()); + }) + .await + .expect("OAuth CSRF rejection flow timed out"); +} + +#[tokio::test] +async fn build_propagates_access_denied_callback_without_exchanging_token() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([]).await; + let storage = Arc::new(MemoryStorage::default()); + let callback = Mutex::new(None); + + let result = TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + OAuthBuilder::new("test-client") + .callback_port(0) + .token_storage(Arc::clone(&storage)) + .build(|url| { + let url = Url::parse(url).unwrap(); + let params: BTreeMap<_, _> = url.query_pairs().into_owned().collect(); + let mut redirect = Url::parse(params.get("redirect_uri").unwrap()).unwrap(); + assert_eq!(redirect.path(), "/callback"); + redirect + .query_pairs_mut() + .append_pair("error", "access_denied"); + *callback.lock().unwrap() = Some(tokio::spawn(send_callback(redirect))); + }) + .await + }) + .await; + + let callback = callback.lock().unwrap().take().unwrap(); + callback.await.unwrap(); + match result { + Err(OAuthError::AuthorizationFailed { message }) => { + assert_eq!(message, "access_denied"); + } + other => panic!("unexpected access_denied callback result: {other:?}"), + } + assert!(server.requests().is_empty()); + assert_eq!(storage.save_count(), 0); + assert!(storage.load("test-client").is_none()); + }) + .await + .expect("OAuth access_denied callback flow timed out"); +} + +async fn assert_refresh_persists_and_caches(response: String, expected_refresh_token: &str) { + timeout(HANG_GUARD, async { + let server = TokenServer::start([ + ( + StatusCode::OK, + serde_json::json!({ + "access_token": "expired-access-token", + "refresh_token": "old-refresh-token", + "token_type": "Bearer", + "expires_in": 0 + }) + .to_string(), + ), + (StatusCode::OK, response), + ]) + .await; + let storage = Arc::new(MemoryStorage::default()); + let started_at = now_secs(); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(Arc::clone(&storage)).await.unwrap(); + assert_eq!(storage.save_count(), 1); + assert_eq!( + storage.load("test-client").unwrap().access_token, + "expired-access-token" + ); + assert_eq!(server.requests().len(), 1); + + assert_eq!( + oauth.access_token().await.unwrap(), + "refreshed-access-token" + ); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.client_id, "test-client"); + assert_eq!(stored.access_token, "refreshed-access-token"); + assert_eq!( + stored.refresh_token.as_deref(), + Some(expected_refresh_token) + ); + assert!(stored.expires_at >= started_at + 3600); + assert!(stored.expires_at <= now_secs() + 3600); + assert_eq!(storage.save_count(), 2); + + let requests = server.requests(); + assert_eq!(requests.len(), 2); + let request = &requests[1]; + assert_eq!(request.method, Method::POST); + assert_eq!(request.path, "/oauth2/token"); + assert!(request.authorization.is_none()); + let form: BTreeMap<_, _> = form_urlencoded::parse(request.body.as_bytes()) + .into_owned() + .collect(); + assert_eq!( + form, + BTreeMap::from([ + ("client_id".to_string(), "test-client".to_string()), + ("grant_type".to_string(), "refresh_token".to_string()), + ("refresh_token".to_string(), "old-refresh-token".to_string()), + ]) + ); + + assert_eq!( + oauth.access_token().await.unwrap(), + "refreshed-access-token" + ); + assert_eq!(server.requests().len(), 2); + assert_eq!(storage.save_count(), 2); + }) + .await; + }) + .await + .expect("OAuth refresh flow timed out"); +} + +#[tokio::test] +async fn access_token_rotates_refresh_token_and_caches_result() { + assert_refresh_persists_and_caches( + serde_json::json!({ + "access_token": "refreshed-access-token", + "refresh_token": "new-refresh-token", + "token_type": "Bearer", + "expires_in": 3600 + }) + .to_string(), + "new-refresh-token", + ) + .await; +} + +#[tokio::test] +async fn access_token_preserves_refresh_token_when_response_omits_it() { + assert_refresh_persists_and_caches( + serde_json::json!({ + "access_token": "refreshed-access-token", + "token_type": "Bearer", + "expires_in": 3600 + }) + .to_string(), + "old-refresh-token", + ) + .await; +} + +#[tokio::test] +async fn access_token_retries_failed_refresh_with_original_token() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([ + successful_token_response("expired-access-token", "old-refresh-token", 0), + ( + StatusCode::BAD_REQUEST, + serde_json::json!({ + "error": "invalid_grant", + "error_description": "refresh token rejected" + }) + .to_string(), + ), + successful_token_response("retry-access-token", "retry-refresh-token", 3600), + ]) + .await; + let storage = Arc::new(MemoryStorage::default()); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(Arc::clone(&storage)).await.unwrap(); + let error = oauth.access_token().await.unwrap_err(); + assert!( + matches!(&error, OAuthError::RefreshTokenFailed { message } if !message.is_empty()), + "unexpected refresh error: {error:?}" + ); + assert_eq!(storage.save_count(), 1); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "expired-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("old-refresh-token")); + assert_refresh_requests(&server, &["old-refresh-token"]); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "retry-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("retry-refresh-token")); + assert_eq!(storage.save_count(), 2); + assert_refresh_requests(&server, &["old-refresh-token", "old-refresh-token"]); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + assert_eq!(server.requests().len(), 3); + assert_eq!(storage.save_count(), 2); + }) + .await; + }) + .await + .expect("OAuth failed-refresh retry flow timed out"); +} + +#[tokio::test] +async fn access_token_preserves_cached_token_until_refreshed_token_is_saved() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([ + successful_token_response("expired-access-token", "old-refresh-token", 0), + successful_token_response("unsaved-access-token", "unsaved-refresh-token", 3600), + successful_token_response("retry-access-token", "retry-refresh-token", 3600), + ]) + .await; + let storage = Arc::new(MemoryStorage::default()); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(FailSecondSaveStorage { + inner: Arc::clone(&storage), + save_attempts: Mutex::new(0), + }) + .await + .unwrap(); + let error = oauth.access_token().await.unwrap_err(); + assert!( + matches!(&error, OAuthError::Other(message) if message == "test storage save failed"), + "unexpected storage error: {error:?}" + ); + assert_eq!(storage.save_count(), 1); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "expired-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("old-refresh-token")); + assert_refresh_requests(&server, &["old-refresh-token"]); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "retry-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("retry-refresh-token")); + assert_eq!(storage.save_count(), 2); + assert_refresh_requests(&server, &["old-refresh-token", "old-refresh-token"]); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + assert_eq!(server.requests().len(), 3); + assert_eq!(storage.save_count(), 2); + }) + .await; + }) + .await + .expect("OAuth failed-save retry flow timed out"); +} + +#[tokio::test] +async fn build_propagates_authorization_exchange_error_without_saving_token() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([( + StatusCode::BAD_REQUEST, + serde_json::json!({ + "error": "invalid_grant", + "error_description": "authorization code rejected" + }) + .to_string(), + )]) + .await; + let storage = Arc::new(MemoryStorage::default()); + + let error = TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + build_with_callback(Arc::clone(&storage)).await + }) + .await + .unwrap_err(); + + assert!( + matches!(&error, OAuthError::ExchangeCodeFailed { message } if !message.is_empty()), + "unexpected authorization exchange error: {error:?}" + ); + assert_eq!(storage.save_count(), 0); + assert!(storage.load("test-client").is_none()); + let requests = server.requests(); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].method, Method::POST); + assert_eq!(requests[0].path, "/oauth2/token"); + let form: BTreeMap<_, _> = form_urlencoded::parse(requests[0].body.as_bytes()) + .into_owned() + .collect(); + assert_eq!(form.get("client_id").unwrap(), "test-client"); + assert_eq!(form.get("grant_type").unwrap(), "authorization_code"); + assert_eq!(form.get("code").unwrap(), "authorization-code"); + }) + .await + .expect("OAuth authorization exchange error flow timed out"); +} + +#[tokio::test] +async fn build_reauthorizes_when_cached_token_refresh_fails() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([ + ( + StatusCode::BAD_REQUEST, + serde_json::json!({ + "error": "invalid_grant", + "error_description": "cached refresh token rejected" + }) + .to_string(), + ), + successful_token_response("authorized-access-token", "authorized-refresh-token", 3600), + ]) + .await; + let storage = Arc::new(MemoryStorage::with_token(StoredToken { + client_id: "test-client".to_string(), + access_token: "expired-access-token".to_string(), + refresh_token: Some("old-refresh-token".to_string()), + expires_at: 0, + })); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(Arc::clone(&storage)).await.unwrap(); + assert_eq!( + oauth.access_token().await.unwrap(), + "authorized-access-token" + ); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.client_id, "test-client"); + assert_eq!(stored.access_token, "authorized-access-token"); + assert_eq!( + stored.refresh_token.as_deref(), + Some("authorized-refresh-token") + ); + assert_eq!(storage.save_count(), 1); + + let requests = server.requests(); + assert_eq!(requests.len(), 2); + let refresh_form: BTreeMap<_, _> = + form_urlencoded::parse(requests[0].body.as_bytes()) + .into_owned() + .collect(); + assert_eq!( + refresh_form, + BTreeMap::from([ + ("client_id".to_string(), "test-client".to_string()), + ("grant_type".to_string(), "refresh_token".to_string()), + ("refresh_token".to_string(), "old-refresh-token".to_string()), + ]) + ); + let authorization_form: BTreeMap<_, _> = + form_urlencoded::parse(requests[1].body.as_bytes()) + .into_owned() + .collect(); + assert_eq!(authorization_form.get("client_id").unwrap(), "test-client"); + assert_eq!( + authorization_form.get("grant_type").unwrap(), + "authorization_code" + ); + assert_eq!( + authorization_form.get("code").unwrap(), + "authorization-code" + ); + }) + .await; + }) + .await + .expect("OAuth refresh fallback authorization flow timed out"); +} + +#[tokio::test] +async fn build_does_not_follow_authorization_token_redirect() { + timeout(HANG_GUARD, async { + let probe = TokenServer::start([successful_token_response( + "redirected-access-token", + "redirected-refresh-token", + 3600, + )]) + .await; + let server = TokenServer::start([(StatusCode::TEMPORARY_REDIRECT, "{}".to_string())]).await; + server.redirect_to(format!("{}/token", probe.base_url)); + let storage = Arc::new(MemoryStorage::default()); + + let error = TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + build_with_callback(Arc::clone(&storage)).await + }) + .await + .unwrap_err(); + + assert!( + matches!(&error, OAuthError::ExchangeCodeFailed { message } if !message.is_empty()), + "unexpected authorization redirect error: {error:?}" + ); + assert_eq!(storage.save_count(), 0); + assert!(storage.load("test-client").is_none()); + let requests = server.requests(); + assert_eq!(requests.len(), 1); + let form: BTreeMap<_, _> = form_urlencoded::parse(requests[0].body.as_bytes()) + .into_owned() + .collect(); + assert_eq!(form.get("grant_type").unwrap(), "authorization_code"); + assert!(probe.requests().is_empty()); + }) + .await + .expect("OAuth authorization redirect rejection flow timed out"); +} + +#[tokio::test] +async fn access_token_does_not_follow_refresh_redirect_and_can_retry() { + timeout(HANG_GUARD, async { + let probe = TokenServer::start([successful_token_response( + "redirected-access-token", + "redirected-refresh-token", + 3600, + )]) + .await; + let server = TokenServer::start([ + successful_token_response("expired-access-token", "old-refresh-token", 0), + (StatusCode::TEMPORARY_REDIRECT, "{}".to_string()), + successful_token_response("retry-access-token", "retry-refresh-token", 3600), + ]) + .await; + server.redirect_to(format!("{}/token", probe.base_url)); + let storage = Arc::new(MemoryStorage::default()); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(Arc::clone(&storage)).await.unwrap(); + let error = oauth.access_token().await.unwrap_err(); + assert!( + matches!(&error, OAuthError::RefreshTokenFailed { message } if !message.is_empty()), + "unexpected refresh redirect error: {error:?}" + ); + assert_eq!(storage.save_count(), 1); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "expired-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("old-refresh-token")); + assert_refresh_requests(&server, &["old-refresh-token"]); + assert!(probe.requests().is_empty()); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + let stored = storage.load("test-client").unwrap(); + assert_eq!(stored.access_token, "retry-access-token"); + assert_eq!(stored.refresh_token.as_deref(), Some("retry-refresh-token")); + assert_eq!(storage.save_count(), 2); + assert_refresh_requests(&server, &["old-refresh-token", "old-refresh-token"]); + + assert_eq!(oauth.access_token().await.unwrap(), "retry-access-token"); + assert_eq!(server.requests().len(), 3); + assert_eq!(storage.save_count(), 2); + assert!(probe.requests().is_empty()); + }) + .await; + }) + .await + .expect("OAuth refresh redirect rejection flow timed out"); +} + +#[tokio::test] +async fn build_defaults_missing_expiry_to_one_hour() { + timeout(HANG_GUARD, async { + let server = TokenServer::start([( + StatusCode::OK, + serde_json::json!({ + "access_token": "default-expiry-access-token", + "refresh_token": "default-expiry-refresh-token", + "token_type": "Bearer" + }) + .to_string(), + )]) + .await; + let storage = Arc::new(MemoryStorage::default()); + let started_at = now_secs(); + + TEST_OAUTH_BASE_URL + .scope(server.base_url.clone(), async { + let oauth = build_with_callback(Arc::clone(&storage)).await.unwrap(); + let stored = storage.load("test-client").unwrap(); + assert!(stored.expires_at >= started_at + 3600); + assert!(stored.expires_at <= now_secs() + 3600); + assert_eq!( + oauth.access_token().await.unwrap(), + "default-expiry-access-token" + ); + assert_eq!(storage.save_count(), 1); + assert_eq!(server.requests().len(), 1); + }) + .await; + }) + .await + .expect("OAuth default expiry flow timed out"); +} + +#[cfg(feature = "blocking")] +#[test] +fn build_blocking_loads_valid_cached_token_without_authorizing() { + let storage = Arc::new(MemoryStorage::with_token(StoredToken { + client_id: "test-client".to_string(), + access_token: "cached-access-token".to_string(), + refresh_token: Some("cached-refresh-token".to_string()), + expires_at: now_secs() + 7200, + })); + let oauth = OAuthBuilder::new("test-client") + .token_storage(Arc::clone(&storage)) + .build_blocking(|_| panic!("valid cached token must not trigger authorization")) + .unwrap(); + assert_eq!(oauth.client_id(), "test-client"); + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let token = runtime.block_on(async { + timeout(HANG_GUARD, oauth.access_token()) + .await + .expect("cached access token lookup timed out") + .unwrap() + }); + assert_eq!(token, "cached-access-token"); + assert_eq!(storage.save_count(), 0); +} + +#[test] +fn authorization_endpoint_selection_uses_isolated_environments() { + for (environment, region, expected_url) in [ + ( + Some("staging"), + "CN", + "https://openapi-global.longbridge.xyz/oauth2/authorize", + ), + (None, "CN", "https://openapi.longbridge.cn/oauth2/authorize"), + ( + None, + "US", + "https://openapi.longbridge.com/oauth2/authorize", + ), + ] { + let mut command = Command::new(std::env::current_exe().unwrap()); + command + .args([ + "--ignored", + "--exact", + "tests::flows::authorization_endpoint_selection_child", + "--nocapture", + ]) + .env("LONGBRIDGE_REGION", region) + .env_remove("LONGPORT_REGION") + .env("TEST_OAUTH_EXPECTED_AUTHORIZATION_URL", expected_url); + if let Some(environment) = environment { + command.env("LONGBRIDGE_ENV", environment); + } else { + command.env_remove("LONGBRIDGE_ENV"); + } + let output = command + .output() + .expect("failed to launch endpoint selection child"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + output.status.success() + && stdout.contains(&format!("verified authorization endpoint: {expected_url}")), + "endpoint selection failed for environment={environment:?}, region={region}:\n{}\n{}", + stdout, + String::from_utf8_lossy(&output.stderr) + ); + } +} + +#[tokio::test] +#[ignore = "launched by authorization_endpoint_selection_uses_isolated_environments"] +async fn authorization_endpoint_selection_child() { + // Only meaningful when launched by the parent test (which checks this + // test's output); skip under a plain `--include-ignored` run. + let Ok(expected_url) = std::env::var("TEST_OAUTH_EXPECTED_AUTHORIZATION_URL") else { + return; + }; + timeout(HANG_GUARD, async { + let storage = Arc::new(MemoryStorage::default()); + let callback = Mutex::new(None); + let result = OAuthBuilder::new("test-client") + .callback_port(0) + .token_storage(Arc::clone(&storage)) + .build(|url| { + let url = Url::parse(url).unwrap(); + assert_eq!( + format!("{}{}", url.origin().ascii_serialization(), url.path()), + expected_url + ); + let params: BTreeMap<_, _> = url.query_pairs().into_owned().collect(); + let mismatched_state = format!("{}-mismatched", params.get("state").unwrap()); + let redirect = callback_url( + params.get("redirect_uri").unwrap(), + "authorization-code", + &mismatched_state, + ); + *callback.lock().unwrap() = Some(tokio::spawn(send_callback(redirect))); + }) + .await; + let callback = callback.lock().unwrap().take().unwrap(); + callback.await.unwrap(); + assert!( + matches!(&result, Err(OAuthError::CsrfTokenMismatch)), + "unexpected endpoint selection result: {result:?}" + ); + assert_eq!(storage.save_count(), 0); + assert!(storage.load("test-client").is_none()); + println!("verified authorization endpoint: {expected_url}"); + }) + .await + .expect("OAuth endpoint selection callback flow timed out"); +} diff --git a/rust/crates/oauth/src/token.rs b/rust/crates/oauth/src/token.rs index 2adc68562e..7461b10348 100644 --- a/rust/crates/oauth/src/token.rs +++ b/rust/crates/oauth/src/token.rs @@ -37,11 +37,10 @@ pub(crate) struct OAuthToken { impl OAuthToken { /// Build from oauth2 crate token response. - pub(crate) fn from_oauth2_response(client_id: String, token_response: &T) -> Self - where - TT: oauth2::TokenType, - T: TokenResponse, - { + pub(crate) fn from_oauth2_response( + client_id: String, + token_response: &T, + ) -> Self { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() diff --git a/rust/src/error.rs b/rust/src/error.rs index 84edabfd84..ced88eeba0 100644 --- a/rust/src/error.rs +++ b/rust/src/error.rs @@ -70,7 +70,7 @@ pub enum Error { /// OAuth error #[error("oauth error: {0}")] - OAuth(String), + OAuth(#[from] longbridge_oauth::OAuthError), /// A conversation event stream ended before a final result was observed #[error("conversation stream ended before a final result was observed")] @@ -118,6 +118,12 @@ impl Error { SimpleError::Other(err.to_string()) } } + Error::HttpClient(HttpClientError::OAuth(err)) | Error::OAuth(err) => { + SimpleError::OAuth { + code: err.code(), + message: err.to_string(), + } + } Error::WsClient(WsClientError::ResponseError { detail: Some(detail), .. @@ -138,7 +144,6 @@ impl Error { | Error::ConversationStreamEnded => SimpleError::Other(self.to_string()), #[cfg(feature = "blocking")] Error::Blocking(_) => SimpleError::Other(self.to_string()), - Error::OAuth(msg) => SimpleError::OAuth(msg), } } } @@ -169,8 +174,14 @@ pub enum SimpleError { #[error("other error: {0}")] Other(String), /// OAuth error - #[error("oauth error: {0}")] - OAuth(String), + #[error("oauth error: {message}")] + OAuth { + /// Stable error code, see + /// [`OAuthError::code`](longbridge_oauth::OAuthError::code) + code: i64, + /// Error message + message: String, + }, } impl From for SimpleError { @@ -200,7 +211,7 @@ impl SimpleError { SimpleError::Http { .. } => SimpleErrorKind::Http, SimpleError::OpenApi { .. } => SimpleErrorKind::OpenApi, SimpleError::Other(_) => SimpleErrorKind::Other, - SimpleError::OAuth(_) => SimpleErrorKind::OAuth, + SimpleError::OAuth { .. } => SimpleErrorKind::OAuth, } } @@ -210,7 +221,7 @@ impl SimpleError { SimpleError::Http { status_code } => Some(*status_code as i64), SimpleError::OpenApi { code, .. } => Some(*code), SimpleError::Other(_) => None, - SimpleError::OAuth(_) => None, + SimpleError::OAuth { code, .. } => Some(*code), } } @@ -220,7 +231,7 @@ impl SimpleError { SimpleError::Http { .. } => None, SimpleError::OpenApi { trace_id, .. } => Some(trace_id), SimpleError::Other(_) => None, - SimpleError::OAuth(_) => None, + SimpleError::OAuth { .. } => None, } } @@ -230,7 +241,30 @@ impl SimpleError { SimpleError::Http { .. } => "bad status code", SimpleError::OpenApi { message, .. } => message.as_str(), SimpleError::Other(message) => message.as_str(), - SimpleError::OAuth(message) => message.as_str(), + SimpleError::OAuth { message, .. } => message.as_str(), } } } + +#[cfg(test)] +mod tests { + use longbridge_oauth::OAuthError; + + use super::*; + + #[test] + fn oauth_errors_keep_their_code() { + let err = Error::OAuth(OAuthError::NoTokenAvailable).into_simple_error(); + assert_eq!(err.kind(), SimpleErrorKind::OAuth); + assert_eq!(err.code(), Some(1)); + assert_eq!(err.message(), "no token available"); + + // A request that could not obtain a token reports the same code. + let err = Error::HttpClient(HttpClientError::OAuth(OAuthError::RevokeFailed { + message: "x".to_string(), + })) + .into_simple_error(); + assert_eq!(err.kind(), SimpleErrorKind::OAuth); + assert_eq!(err.code(), Some(4)); + } +}