From 30403540cadbb63f5c29507a8baf4fcb1ac882ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AD=A6=E5=AE=81?= Date: Tue, 3 Nov 2026 14:29:35 +0800 Subject: [PATCH] feat(oauth): add OAuth revoke/clear and use PKCE in the authorization flow - Add `OAuth.revoke()` (RFC 7009) and `OAuth.clear()` to every SDK layer (Rust, C, C++, Java, Node.js, Python), plus `TokenStorage::delete` and `OAuthError::{RevokeFailed, TokenFileDelete}`. - `clear()` deletes the persisted token first and clears memory only on success; `revoke()` rejects `from_token` clients with a clear error. - `TokenStorage::delete` defaults to an error so `clear()` never reports success for a backend it cannot wipe. - C: NULL handles passed to `lb_oauth_revoke` / `lb_oauth_clear` are reported through the callback instead of crashing. - Python: blocking `revoke()` / `clear()` release the GIL and reuse the shared Tokio runtime. - Use PKCE (S256) in the browser authorization flow; the server requires it for public clients. - Mark `OAuthError` as `#[non_exhaustive]`. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_017BJcMMB4GdF4MgepE5mW2K --- CHANGELOG.md | 17 ++ c/csrc/include/longbridge.h | 26 ++ c/src/oauth.rs | 62 ++++ cpp/include/oauth.hpp | 18 ++ cpp/src/oauth.cpp | 24 ++ .../src/main/java/com/longbridge/OAuth.java | 36 +++ .../main/java/com/longbridge/SdkNative.java | 4 + java/src/oauth.rs | 40 +++ nodejs/index.d.ts | 27 ++ nodejs/src/oauth.rs | 37 +++ python/pysrc/longbridge/openapi.pyi | 22 ++ python/src/oauth.rs | 49 ++++ rust/crates/oauth/src/client.rs | 265 +++++++++++++++++- rust/crates/oauth/src/error.rs | 18 ++ rust/crates/oauth/src/lib.rs | 79 ++++++ rust/crates/oauth/src/storage.rs | 19 +- rust/crates/oauth/src/token.rs | 20 ++ 17 files changed, 755 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e6c0d090a..7928c97e43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### 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.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:** `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. + +### Breaking changes + +- **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:** 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 ### Added diff --git a/c/csrc/include/longbridge.h b/c/csrc/include/longbridge.h index d81cba568f..8b35cf19b7 100644 --- a/c/csrc/include/longbridge.h +++ b/c/csrc/include/longbridge.h @@ -15652,6 +15652,32 @@ struct lb_oauth_t *lb_oauth_clone(const struct lb_oauth_t *oauth); */ void lb_oauth_free(struct lb_oauth_t *oauth); +/** + * Asynchronously revoke the current token on the server (RFC 7009) + * + * The refresh token is revoked when present, otherwise the access token. + * Only the server is contacted; the cached token on disk is left untouched + * (use `lb_oauth_clear` to remove it). + * + * @param oauth OAuth 2.0 client; if NULL, `callback` receives an error + * @param callback Async completion callback; `error` is set on failure + * @param userdata Opaque pointer forwarded to `callback` + */ +void lb_oauth_revoke(const struct lb_oauth_t *oauth, lb_async_callback_t callback, void *userdata); + +/** + * Asynchronously clear the token locally + * + * Drops the in-memory token and deletes the cached token file + * (`~/.longbridge/openapi/tokens/`). The server is not contacted + * (use `lb_oauth_revoke` for that). + * + * @param oauth OAuth 2.0 client; if NULL, `callback` receives an error + * @param callback Async completion callback; `error` is set on failure + * @param userdata Opaque pointer forwarded to `callback` + */ +void lb_oauth_clear(const struct lb_oauth_t *oauth, lb_async_callback_t callback, void *userdata); + const struct lb_portfolio_context_t *lb_portfolio_context_new(const struct lb_config_t *config); void lb_portfolio_context_retain(const struct lb_portfolio_context_t *ctx); diff --git a/c/src/oauth.rs b/c/src/oauth.rs index 5f3598c324..d27d650a7c 100644 --- a/c/src/oauth.rs +++ b/c/src/oauth.rs @@ -78,3 +78,65 @@ pub unsafe extern "C" fn lb_oauth_free(oauth: *mut COAuth) { drop(Box::from_raw(oauth)); } } + +/// Clone the inner client; `None` if the handle is NULL (e.g. a moved-from C++ +/// wrapper). +unsafe fn clone_inner(oauth: *const COAuth) -> Option { + oauth.as_ref().map(|oauth| oauth.inner.clone()) +} + +fn null_oauth_error() -> longbridge::Error { + longbridge::Error::OAuth("OAuth client is NULL".to_string()) +} + +/// Asynchronously revoke the current token on the server (RFC 7009) +/// +/// The refresh token is revoked when present, otherwise the access token. +/// Only the server is contacted; the cached token on disk is left untouched +/// (use `lb_oauth_clear` to remove it). +/// +/// @param oauth OAuth 2.0 client; if NULL, `callback` receives an error +/// @param callback Async completion callback; `error` is set on failure +/// @param userdata Opaque pointer forwarded to `callback` +#[unsafe(no_mangle)] +pub unsafe extern "C" fn lb_oauth_revoke( + oauth: *const COAuth, + callback: CAsyncCallback, + userdata: *mut c_void, +) { + let oauth = clone_inner(oauth); + execute_async::(callback, std::ptr::null(), userdata, async move { + oauth + .ok_or_else(null_oauth_error)? + .revoke() + .await + .map_err(|e| longbridge::Error::OAuth(e.to_string()))?; + Ok(()) + }); +} + +/// Asynchronously clear the token locally +/// +/// Drops the in-memory token and deletes the cached token file +/// (`~/.longbridge/openapi/tokens/`). The server is not contacted +/// (use `lb_oauth_revoke` for that). +/// +/// @param oauth OAuth 2.0 client; if NULL, `callback` receives an error +/// @param callback Async completion callback; `error` is set on failure +/// @param userdata Opaque pointer forwarded to `callback` +#[unsafe(no_mangle)] +pub unsafe extern "C" fn lb_oauth_clear( + oauth: *const COAuth, + callback: CAsyncCallback, + userdata: *mut c_void, +) { + let oauth = clone_inner(oauth); + execute_async::(callback, std::ptr::null(), userdata, async move { + oauth + .ok_or_else(null_oauth_error)? + .clear() + .await + .map_err(|e| longbridge::Error::OAuth(e.to_string()))?; + Ok(()) + }); +} diff --git a/cpp/include/oauth.hpp b/cpp/include/oauth.hpp index 7beff5b9f6..363d76ac18 100644 --- a/cpp/include/oauth.hpp +++ b/cpp/include/oauth.hpp @@ -29,6 +29,24 @@ class OAuth ~OAuth(); operator const lb_oauth_t*() const { return oauth_; } + + /// Asynchronously revoke the current token on the server (RFC 7009) + /// + /// The refresh token is revoked when present, otherwise the access token. + /// Only the server is contacted; the cached token on disk is left untouched + /// (see `clear`). + /// + /// @param callback Invoked on completion; check `status()` for errors + void revoke(AsyncCallback callback) const; + + /// Asynchronously clear the token locally + /// + /// Drops the in-memory token and deletes the cached token file + /// (`~/.longbridge/openapi/tokens/`). The server is not contacted + /// (see `revoke`). + /// + /// @param callback Invoked on completion; check `status()` for errors + void clear(AsyncCallback callback) const; }; /// Builder for constructing an OAuth 2.0 client diff --git a/cpp/src/oauth.cpp b/cpp/src/oauth.cpp index 942dd0fe48..4d13b13682 100644 --- a/cpp/src/oauth.cpp +++ b/cpp/src/oauth.cpp @@ -51,6 +51,30 @@ OAuth::~OAuth() lb_oauth_free(oauth_); } +void +OAuth::revoke(AsyncCallback callback) const +{ + lb_oauth_revoke( + oauth_, + [](const lb_async_result_t* res) { + auto callback_ptr = callback::get_async_callback(res->userdata); + (*callback_ptr)(AsyncResult(NoContext{}, Status(res->error), nullptr)); + }, + new AsyncCallback(callback)); +} + +void +OAuth::clear(AsyncCallback callback) const +{ + lb_oauth_clear( + oauth_, + [](const lb_async_result_t* res) { + auto callback_ptr = callback::get_async_callback(res->userdata); + (*callback_ptr)(AsyncResult(NoContext{}, Status(res->error), nullptr)); + }, + new AsyncCallback(callback)); +} + // ── OAuthBuilder ───────────────────────────────────────────────────────────── OAuthBuilder::OAuthBuilder(const std::string& client_id, uint16_t callback_port) diff --git a/java/javasrc/src/main/java/com/longbridge/OAuth.java b/java/javasrc/src/main/java/com/longbridge/OAuth.java index 9400957769..787637bc5c 100644 --- a/java/javasrc/src/main/java/com/longbridge/OAuth.java +++ b/java/javasrc/src/main/java/com/longbridge/OAuth.java @@ -1,5 +1,7 @@ package com.longbridge; +import java.util.concurrent.CompletableFuture; + /** * OAuth 2.0 client handle for Longbridge OpenAPI * @@ -46,6 +48,40 @@ long getRaw() { return r; } + /** + * Revoke the current token on the server (RFC 7009). + * + *

+ * The refresh token is revoked when present, otherwise the access token. + * Only the server is contacted; the cached token on disk is left untouched + * (see {@link #clear()}). + * + * @return A Future that completes when the token has been revoked + */ + public CompletableFuture revoke() { + long r = getRaw(); + return AsyncCallback.executeTask((callback) -> { + SdkNative.oauthRevoke(r, callback); + }); + } + + /** + * Clear the token locally. + * + *

+ * Drops the in-memory token and deletes the cached token file + * ({@code ~/.longbridge/openapi/tokens/}). The server is not + * contacted (see {@link #revoke()}). + * + * @return A Future that completes when the local token has been cleared + */ + public CompletableFuture clear() { + long r = getRaw(); + return AsyncCallback.executeTask((callback) -> { + SdkNative.oauthClear(r, callback); + }); + } + @Override public synchronized void close() { long h = this.raw; diff --git a/java/javasrc/src/main/java/com/longbridge/SdkNative.java b/java/javasrc/src/main/java/com/longbridge/SdkNative.java index 5e296f3866..9d52f266e9 100644 --- a/java/javasrc/src/main/java/com/longbridge/SdkNative.java +++ b/java/javasrc/src/main/java/com/longbridge/SdkNative.java @@ -66,6 +66,10 @@ public static native void configRefreshAccessToken(long config, OffsetDateTime e public static native void oauthBuild(String clientId, int callbackPort, Consumer openUrlCallback, AsyncCallback callback); + public static native void oauthRevoke(long oauth, AsyncCallback callback); + + public static native void oauthClear(long oauth, AsyncCallback callback); + public static native void freeOAuth(long oauth); public static native long newAssetContext(long config); diff --git a/java/src/oauth.rs b/java/src/oauth.rs index 43bb88f555..66d75e5188 100644 --- a/java/src/oauth.rs +++ b/java/src/oauth.rs @@ -66,3 +66,43 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_freeOAuth( ) { drop(Box::from_raw(oauth as *mut OAuth)); } + +/// Asynchronously revoke the current token on the server (RFC 7009). +#[unsafe(no_mangle)] +pub unsafe extern "system" fn Java_com_longbridge_SdkNative_oauthRevoke( + mut env: JNIEnv, + _class: JClass, + oauth: jlong, + callback: JObject, +) { + jni_result(&mut env, (), |env| { + let oauth = (*(oauth as *const OAuth)).clone(); + async_util::execute(env, callback, async move { + oauth + .revoke() + .await + .map_err(|e| crate::error::JniError::Other(e.to_string())) + })?; + Ok(()) + }) +} + +/// Asynchronously clear the token locally (memory and persisted file). +#[unsafe(no_mangle)] +pub unsafe extern "system" fn Java_com_longbridge_SdkNative_oauthClear( + mut env: JNIEnv, + _class: JClass, + oauth: jlong, + callback: JObject, +) { + jni_result(&mut env, (), |env| { + let oauth = (*(oauth as *const OAuth)).clone(); + async_util::execute(env, callback, async move { + oauth + .clear() + .await + .map_err(|e| crate::error::JniError::Other(e.to_string())) + })?; + Ok(()) + }) +} diff --git a/nodejs/index.d.ts b/nodejs/index.d.ts index ad1a992a39..6a76b4cf96 100644 --- a/nodejs/index.d.ts +++ b/nodejs/index.d.ts @@ -2551,6 +2551,33 @@ export declare class OAuth { * `HttpClient.fromOAuth` */ static build(clientId: string, onOpenUrl: ((err: Error | null, arg: string) => void), callbackPort?: number | undefined | null): Promise + /** + * Revoke the current token on the server (RFC 7009). + * + * The refresh token is revoked when present, otherwise the access token. + * Only the server is contacted; the cached token on disk is left + * untouched (see `clear`). + * + * @example + * ```javascript + * const oauth = await OAuth.build('your-client-id', (_, url) => console.log('Open:', url)); + * await oauth.revoke(); + * ``` + */ + revoke(): Promise + /** + * Clear the token locally. + * + * Drops the in-memory token and deletes the cached token file + * (`~/.longbridge/openapi/tokens/`). The server is not + * contacted (see `revoke`). + * + * @example + * ```javascript + * await oauth.clear(); + * ``` + */ + clear(): Promise } /** diff --git a/nodejs/src/oauth.rs b/nodejs/src/oauth.rs index ca8e22b1d0..3e67959de4 100644 --- a/nodejs/src/oauth.rs +++ b/nodejs/src/oauth.rs @@ -56,4 +56,41 @@ impl OAuth { .map_err(|e| napi::Error::from_reason(e.to_string()))?; Ok(OAuth(oauth)) } + + /// Revoke the current token on the server (RFC 7009). + /// + /// The refresh token is revoked when present, otherwise the access token. + /// Only the server is contacted; the cached token on disk is left + /// untouched (see `clear`). + /// + /// @example + /// ```javascript + /// const oauth = await OAuth.build('your-client-id', (_, url) => console.log('Open:', url)); + /// await oauth.revoke(); + /// ``` + #[napi] + pub async fn revoke(&self) -> Result<()> { + self.0 + .revoke() + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) + } + + /// Clear the token locally. + /// + /// Drops the in-memory token and deletes the cached token file + /// (`~/.longbridge/openapi/tokens/`). The server is not + /// contacted (see `revoke`). + /// + /// @example + /// ```javascript + /// await oauth.clear(); + /// ``` + #[napi] + pub async fn clear(&self) -> Result<()> { + self.0 + .clear() + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) + } } diff --git a/python/pysrc/longbridge/openapi.pyi b/python/pysrc/longbridge/openapi.pyi index 0c898bbba0..11e6258a30 100644 --- a/python/pysrc/longbridge/openapi.pyi +++ b/python/pysrc/longbridge/openapi.pyi @@ -224,6 +224,28 @@ class OAuth: :meth:`Config.from_oauth` or :meth:`HttpClient.from_oauth`. """ + def revoke(self) -> None: + """ + Revoke the current token on the server (RFC 7009), blocking. + + The refresh token is revoked when present, otherwise the access token. + Only the server is contacted; the cached token on disk + (``~/.longbridge/openapi/tokens/``) is left untouched. + Call :meth:`clear` afterwards to drop it locally. + """ + async def revoke_async(self) -> None: + """Async version of :meth:`revoke`.""" + def clear(self) -> None: + """ + Clear the token locally, blocking. + + Drops the in-memory token and deletes the cached token file + (``~/.longbridge/openapi/tokens/``). The server is not + contacted; use :meth:`revoke` for that. + """ + async def clear_async(self) -> None: + """Async version of :meth:`clear`.""" + class OAuthBuilder: """ Builder for the OAuth 2.0 authorization flow. diff --git a/python/src/oauth.rs b/python/src/oauth.rs index e00b4c7120..f29f7ea0a6 100644 --- a/python/src/oauth.rs +++ b/python/src/oauth.rs @@ -15,6 +15,55 @@ fn oauth_err(e: OAuthError) -> PyErr { #[pyclass(name = "OAuth")] pub(crate) struct OAuth(pub(crate) longbridge::oauth::OAuth); +#[pymethods] +impl OAuth { + /// Revoke the current token on the server (RFC 7009), blocking. + /// + /// The refresh token is revoked when present, otherwise the access token. + /// Only the server is contacted; the cached token on disk is left + /// untouched (see :meth:`clear`). + fn revoke(&self, py: Python<'_>) -> PyResult<()> { + let oauth = self.0.clone(); + // Release the GIL while waiting on the network so other Python threads + // keep running. + py.detach(|| longbridge::runtime_handle().block_on(oauth.revoke())) + .map_err(oauth_err) + } + + /// Revoke the current token on the server (RFC 7009), async. + /// + /// Returns: + /// Awaitable that resolves to ``None`` + fn revoke_async<'py>(&self, py: Python<'py>) -> PyResult> { + let oauth = self.0.clone(); + pyo3_async_runtimes::tokio::future_into_py(py, async move { + oauth.revoke().await.map_err(oauth_err) + }) + } + + /// Clear the token locally, blocking. + /// + /// Drops the in-memory token and deletes the cached token file + /// (``~/.longbridge/openapi/tokens/``). The server is not + /// contacted (see :meth:`revoke`). + fn clear(&self, py: Python<'_>) -> PyResult<()> { + let oauth = self.0.clone(); + py.detach(|| longbridge::runtime_handle().block_on(oauth.clear())) + .map_err(oauth_err) + } + + /// Clear the token locally, async. + /// + /// Returns: + /// Awaitable that resolves to ``None`` + fn clear_async<'py>(&self, py: Python<'py>) -> PyResult> { + let oauth = self.0.clone(); + pyo3_async_runtimes::tokio::future_into_py(py, async move { + oauth.clear().await.map_err(oauth_err) + }) + } +} + /// Builder for the OAuth 2.0 authorization flow /// /// Args: diff --git a/rust/crates/oauth/src/client.rs b/rust/crates/oauth/src/client.rs index 1872ecf6a8..cb70f43639 100644 --- a/rust/crates/oauth/src/client.rs +++ b/rust/crates/oauth/src/client.rs @@ -2,8 +2,10 @@ use std::{fmt, sync::Arc}; use longbridge_geo::is_cn; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, CsrfToken, RedirectUrl, RefreshToken, RevocationUrl, - Scope, TokenUrl, basic::BasicClient, reqwest::async_http_client, + AccessToken, AuthUrl, AuthorizationCode, ClientId, CsrfToken, PkceCodeChallenge, + PkceCodeVerifier, RedirectUrl, RefreshToken, RequestTokenError, RevocationErrorResponseType, + RevocationUrl, Scope, StandardErrorResponse, StandardRevocableToken, TokenUrl, + basic::BasicClient, reqwest::async_http_client, }; use poem::listener::{Acceptor, Listener, TcpListener}; @@ -120,6 +122,84 @@ impl OAuth { }) } + /// Revoke the current token on the server (RFC 7009). + /// + /// 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. + 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. + if self.0.client_id.is_empty() { + return Err(OAuthError::RevokeFailed { + message: "token was not issued through an OAuth client (created with \ + `from_token`), nothing to revoke" + .to_string(), + }); + } + + let token = self + .0 + .token + .lock() + .await + .clone() + .ok_or(OAuthError::NoTokenAvailable)?; + + let revocable = match token.refresh_token { + Some(refresh_token) => { + StandardRevocableToken::RefreshToken(RefreshToken::new(refresh_token)) + } + 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 + .revoke_token(revocable) + .map_err(|e| OAuthError::RevokeFailed { + message: e.to_string(), + })? + .request_async(async_http_client) + .await + .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 } + })?; + + tracing::info!(client_id = %self.0.client_id, "token revoked"); + Ok(()) + } + + /// Drop the token locally: clear the in-memory copy and delete the + /// persisted one from the configured storage backend (by default + /// `~/.longbridge/openapi/tokens/`). + /// + /// This does not contact the server; use [`revoke`](OAuth::revoke) for + /// that. After this call [`access_token`](OAuth::access_token) fails with + /// [`OAuthError::NoTokenAvailable`] until a new client is built. + /// + /// The persisted token is deleted first: if that fails the error is + /// returned and the in-memory token is kept, so memory and storage never + /// disagree about whether the client is logged in. + pub async fn clear(&self) -> OAuthResult<()> { + // Hold the lock across the delete so a concurrent refresh cannot + // re-save the token between the delete and the in-memory clear. + let mut guard = self.0.token.lock().await; + // `from_token` clients have no client ID and nothing persisted. + if !self.0.client_id.is_empty() { + self.0.storage.delete(&self.0.client_id)?; + } + *guard = None; + Ok(()) + } + /// Run the browser authorization flow and return a new token. pub(crate) async fn authorize_inner(&self, open_url: impl Fn(&str)) -> OAuthResult { let acceptor = TcpListener::bind(format!("127.0.0.1:{}", self.0.callback_port)) @@ -144,10 +224,7 @@ impl OAuth { ) .await; - let (auth_url, csrf_token) = client - .authorize_url(CsrfToken::new_random) - .add_scope(Scope::new(String::new())) - .url(); + let (auth_url, csrf_token, pkce_verifier) = build_authorize_url(&client); tracing::info!("starting OAuth authorization flow"); open_url(auth_url.as_str()); @@ -163,6 +240,7 @@ impl OAuth { tracing::debug!(client_id = %self.0.client_id, "exchanging authorization code for token"); let token_response = client .exchange_code(AuthorizationCode::new(code)) + .set_pkce_verifier(pkce_verifier) .request_async(async_http_client) .await .map_err(|e| { @@ -215,9 +293,44 @@ impl OAuth { } } +/// Describe a revocation failure, including the server's `error` and +/// `error_description` (the `Display` of `ServerResponse` omits them). +fn revoke_error_message( + e: &RequestTokenError>, +) -> String +where + RE: std::error::Error + 'static, +{ + match e { + RequestTokenError::ServerResponse(resp) => match resp.error_description() { + Some(desc) => format!("{}: {desc}", resp.error()), + None => resp.error().to_string(), + }, + other => other.to_string(), + } +} + +/// Build the authorization URL with a fresh CSRF token and PKCE challenge. +/// +/// 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) { + let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); + let (auth_url, csrf_token) = client + .authorize_url(CsrfToken::new_random) + .add_scope(Scope::new(String::new())) + .set_pkce_challenge(pkce_challenge) + .url(); + (auth_url, csrf_token, pkce_verifier) +} + /// Build the oauth2 BasicClient for Longbridge endpoints. async fn create_oauth_client(client_id: &str, redirect_uri: &str) -> BasicClient { - let base = oauth_base_url().await; + 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, @@ -227,3 +340,141 @@ async fn create_oauth_client(client_id: &str, redirect_uri: &str) -> BasicClient .set_redirect_uri(RedirectUrl::new(redirect_uri.to_string()).unwrap()) .set_revocation_uri(RevocationUrl::new(format!("{base}/revoke")).unwrap()) } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + use crate::storage::StoredToken; + + #[test] + fn authorize_url_carries_pkce_challenge_matching_verifier() { + let client = build_oauth_client( + OAUTH_BASE_URL, + "my-client", + "http://localhost:60355/callback", + ); + let (url, csrf, verifier) = build_authorize_url(&client); + + let query: HashMap = url.query_pairs().into_owned().collect(); + assert_eq!(query["code_challenge_method"], "S256"); + assert_eq!( + query["code_challenge"], + PkceCodeChallenge::from_code_verifier_sha256(&verifier).as_str() + ); + assert_eq!(query["state"], *csrf.secret()); + assert_eq!(query["client_id"], "my-client"); + } + + #[test] + fn authorize_url_uses_a_fresh_verifier_each_time() { + let client = build_oauth_client(OAUTH_BASE_URL, "c", "http://localhost:60355/callback"); + let (_, _, v1) = build_authorize_url(&client); + let (_, _, v2) = build_authorize_url(&client); + assert_ne!(v1.secret(), v2.secret()); + } + + #[test] + fn revoke_error_message_includes_server_error_and_description() { + let with_desc: RequestTokenError = + RequestTokenError::ServerResponse(StandardErrorResponse::new( + RevocationErrorResponseType::UnsupportedTokenType, + Some("token type not supported".to_string()), + None, + )); + assert_eq!( + revoke_error_message(&with_desc), + "unsupported_token_type: token type not supported" + ); + + let without_desc: RequestTokenError = + RequestTokenError::ServerResponse(StandardErrorResponse::new( + RevocationErrorResponseType::UnsupportedTokenType, + None, + None, + )); + assert_eq!( + revoke_error_message(&without_desc), + "unsupported_token_type" + ); + } + + #[test] + fn revoke_error_message_falls_back_to_display_for_other_errors() { + let e: RequestTokenError< + std::io::Error, + StandardErrorResponse, + > = RequestTokenError::Other("boom".to_string()); + assert_eq!(revoke_error_message(&e), e.to_string()); + } + + #[tokio::test] + async fn revoke_rejects_from_token_clients() { + let oauth = OAuth::from_token("external-token"); + assert!(matches!( + oauth.revoke().await, + Err(OAuthError::RevokeFailed { .. }) + )); + } + + /// Storage whose `delete` always fails. + struct FailingDelete; + + impl TokenStorage for FailingDelete { + fn load(&self, _client_id: &str) -> Option { + None + } + + fn save(&self, _token: &StoredToken) -> OAuthResult<()> { + Ok(()) + } + + fn delete(&self, _client_id: &str) -> OAuthResult<()> { + Err(OAuthError::Other("delete failed".to_string())) + } + } + + /// Storage that only implements the required methods. + struct LegacyStorage; + + impl TokenStorage for LegacyStorage { + fn load(&self, _client_id: &str) -> Option { + None + } + + fn save(&self, _token: &StoredToken) -> OAuthResult<()> { + Ok(()) + } + } + + fn oauth_with_storage(storage: Arc) -> OAuth { + OAuth(Arc::new(OAuthInner { + client_id: "my-client".to_string(), + callback_port: DEFAULT_CALLBACK_PORT, + storage, + token: tokio::sync::Mutex::new(Some(OAuthToken { + client_id: "my-client".to_string(), + access_token: "access".to_string(), + refresh_token: None, + expires_at: u64::MAX, + })), + })) + } + + #[tokio::test] + async fn clear_keeps_memory_token_when_storage_delete_fails() { + let oauth = oauth_with_storage(Arc::new(FailingDelete)); + + assert!(oauth.clear().await.is_err()); + assert_eq!(oauth.access_token().await.unwrap(), "access"); + } + + #[tokio::test] + async fn clear_fails_for_storage_without_delete_support() { + let oauth = oauth_with_storage(Arc::new(LegacyStorage)); + + assert!(matches!(oauth.clear().await, Err(OAuthError::Other(_)))); + assert_eq!(oauth.access_token().await.unwrap(), "access"); + } +} diff --git a/rust/crates/oauth/src/error.rs b/rust/crates/oauth/src/error.rs index b45a7e016d..22fcddb5ec 100644 --- a/rust/crates/oauth/src/error.rs +++ b/rust/crates/oauth/src/error.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; /// Error type for OAuth operations #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum OAuthError { /// Cannot determine home directory for token storage #[error("cannot determine home directory")] @@ -57,6 +58,16 @@ pub enum OAuthError { source: std::io::Error, }, + /// Failed to delete token file + #[error("failed to delete token file {path}: {source}")] + TokenFileDelete { + /// Path to the token file + path: PathBuf, + /// I/O error from deleting the file + #[source] + source: std::io::Error, + }, + /// No token available (not loaded and authorization not yet completed) #[error("no token available")] NoTokenAvailable, @@ -96,6 +107,13 @@ pub enum OAuthError { message: String, }, + /// Failed to revoke token on the server + #[error("failed to revoke token: {message}")] + RevokeFailed { + /// Server or protocol error message + message: String, + }, + /// OAuth authorization failed (user denied or server error in callback) #[error("OAuth authorization failed: {message}")] AuthorizationFailed { diff --git a/rust/crates/oauth/src/lib.rs b/rust/crates/oauth/src/lib.rs index 8d27d1a266..02cc74f0b2 100644 --- a/rust/crates/oauth/src/lib.rs +++ b/rust/crates/oauth/src/lib.rs @@ -71,6 +71,11 @@ mod tests { *self.save_count.lock().unwrap() += 1; Ok(()) } + + fn delete(&self, _client_id: &str) -> OAuthResult<()> { + *self.token.lock().unwrap() = None; + Ok(()) + } } impl MemoryStorage { @@ -402,6 +407,80 @@ mod tests { let _ = fs::remove_file(&path); } + #[test] + fn test_file_token_storage_delete() { + use crate::storage::FileTokenStorage; + + let client_id = format!("__test_delete__{}", std::process::id()); + let storage = FileTokenStorage; + storage + .save(&StoredToken { + client_id: client_id.clone(), + access_token: "a".to_string(), + refresh_token: None, + expires_at: 9999999999, + }) + .unwrap(); + assert!(storage.load(&client_id).is_some()); + + storage.delete(&client_id).unwrap(); + assert!(storage.load(&client_id).is_none()); + // Deleting again (missing file) is not an error. + storage.delete(&client_id).unwrap(); + } + + #[tokio::test] + async fn test_oauth_clear_drops_memory_and_storage() { + let storage = Arc::new(MemoryStorage::with_token(StoredToken { + client_id: "my-client".to_string(), + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at: now_secs() + 7200, + })); + + struct Shared(Arc); + impl TokenStorage for Shared { + fn load(&self, client_id: &str) -> Option { + self.0.load(client_id) + } + fn save(&self, token: &StoredToken) -> OAuthResult<()> { + self.0.save(token) + } + fn delete(&self, client_id: &str) -> OAuthResult<()> { + self.0.delete(client_id) + } + } + + let oauth = OAuthBuilder::new("my-client") + .token_storage(Shared(Arc::clone(&storage))) + .build(|_url| panic!("browser flow must not be triggered")) + .await + .unwrap(); + assert!(storage.load("my-client").is_some()); + + oauth.clear().await.unwrap(); + + assert!(storage.load("my-client").is_none()); + assert!(matches!( + oauth.access_token().await, + Err(crate::OAuthError::NoTokenAvailable) + )); + } + + #[tokio::test] + async fn test_oauth_revoke_without_token_fails() { + let oauth = OAuth(Arc::new(OAuthInner { + client_id: "c".to_string(), + callback_port: DEFAULT_CALLBACK_PORT, + storage: default_storage(), + token: tokio::sync::Mutex::new(None), + })); + assert!(matches!( + oauth.revoke().await, + Err(crate::OAuthError::NoTokenAvailable) + )); + } + #[test] fn test_file_token_storage_missing_returns_none() { use crate::storage::FileTokenStorage; diff --git a/rust/crates/oauth/src/storage.rs b/rust/crates/oauth/src/storage.rs index e96038a812..ea3accbec3 100644 --- a/rust/crates/oauth/src/storage.rs +++ b/rust/crates/oauth/src/storage.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use crate::{ - error::OAuthResult, + error::{OAuthError, OAuthResult}, token::{OAuthToken, token_path_for_client_id}, }; @@ -51,6 +51,19 @@ pub trait TokenStorage: Send + Sync + 'static { /// Persist `token`. Called after every successful authorization or refresh. fn save(&self, token: &StoredToken) -> OAuthResult<()>; + + /// Remove the persisted token for `client_id`. + /// + /// Must succeed (return `Ok`) when no token is stored. The default + /// implementation returns an error, so existing implementations keep + /// compiling but [`OAuth::clear`](crate::OAuth::clear) fails loudly + /// instead of pretending to wipe a backend it cannot touch; override it to + /// support `clear`. + fn delete(&self, _client_id: &str) -> OAuthResult<()> { + Err(OAuthError::Other( + "this TokenStorage does not implement `delete`".to_string(), + )) + } } /// Default file-based token storage. @@ -68,6 +81,10 @@ impl TokenStorage for FileTokenStorage { let path = token_path_for_client_id(&token.client_id)?; OAuthToken::from(token.clone()).save_to_path(path) } + + fn delete(&self, client_id: &str) -> OAuthResult<()> { + OAuthToken::delete_from_path(token_path_for_client_id(client_id)?) + } } pub(crate) fn default_storage() -> Arc { diff --git a/rust/crates/oauth/src/token.rs b/rust/crates/oauth/src/token.rs index 2c17ca6fad..2adc68562e 100644 --- a/rust/crates/oauth/src/token.rs +++ b/rust/crates/oauth/src/token.rs @@ -87,6 +87,26 @@ impl OAuthToken { Ok(token) } + /// Delete the token file at the given path. A missing file is not an + /// error. + pub(crate) fn delete_from_path(path: impl AsRef) -> OAuthResult<()> { + let path = path.as_ref(); + match std::fs::remove_file(path) { + Ok(()) => { + tracing::debug!(path = %path.display(), "token file deleted"); + Ok(()) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => { + tracing::error!(path = %path.display(), error = %e, "failed to delete token file"); + Err(OAuthError::TokenFileDelete { + path: path.to_path_buf(), + source: e, + }) + } + } + } + /// Save token to a JSON file at the given path. pub(crate) fn save_to_path(&self, path: impl AsRef) -> OAuthResult<()> { let path = path.as_ref();