From 9128d65fe54d191583a0903392560cfd3416e180 Mon Sep 17 00:00:00 2001 From: Trezy Date: Sun, 22 Mar 2026 13:54:27 -0500 Subject: [PATCH] fix: dont skip HappyView in the external auth callbackloop --- src/external_auth/routes.rs | 55 +++++++++++++++++++------------------ src/plugin/executor.rs | 28 +++++++++++-------- tests/plugin_executor.rs | 6 +++- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/external_auth/routes.rs b/src/external_auth/routes.rs index 9a24482..833c79f 100644 --- a/src/external_auth/routes.rs +++ b/src/external_auth/routes.rs @@ -87,6 +87,7 @@ async fn authorize( let state_param = uuid::Uuid::new_v4().to_string(); // Store state -> user mapping for callback validation + // Store the frontend's redirect_uri so we can redirect back after callback state::store_state( &app_state.db, app_state.db_backend, @@ -125,8 +126,16 @@ async fn authorize( .await .map_err(|e| AppError::Internal(e.to_string()))?; + // Build the backend callback URL for OpenID/OAuth return_to + // This ensures the auth provider redirects back to the backend, not the frontend + let callback_url = format!( + "{}/external-auth/{}/callback", + app_state.config.public_url.trim_end_matches('/'), + plugin_id + ); + let authorize_url = instance - .call_get_authorize_url(&state_param, &query.redirect_uri, &config) + .call_get_authorize_url(&state_param, &callback_url, &config) .await .map_err(|e| AppError::Internal(e.to_string()))?; @@ -136,13 +145,6 @@ async fn authorize( }))) } -#[derive(Deserialize)] -struct CallbackQuery { - code: Option, - state: Option, - error: Option, -} - fn redirect_with_params(base_uri: &str, params: &[(&str, &str)]) -> Redirect { let separator = if base_uri.contains('?') { "&" } else { "?" }; let query: String = params @@ -156,18 +158,22 @@ fn redirect_with_params(base_uri: &str, params: &[(&str, &str)]) -> Redirect { async fn callback( State(app_state): State, Path(plugin_id): Path, - Query(query): Query, + Query(query_params): Query>, ) -> Result { - // Phase 1: Extract params — errors here have no redirect target, so return HTTP errors - let code = query.code.ok_or_else(|| { - AppError::BadRequest(query.error.unwrap_or_else(|| "Missing code".into())) - })?; - let state_param = query - .state + // Phase 1: Extract state param — required for both OAuth and OpenID + // For OAuth: "state" param + // For OpenID 2.0: also "state" (we pass it via return_to query string) + let state_param = query_params + .get("state") .ok_or_else(|| AppError::BadRequest("Missing state".into()))?; + // Check for error param (OAuth error response) + if let Some(error) = query_params.get("error") { + return Err(AppError::BadRequest(error.clone())); + } + // Phase 2: Consume state — after this we have a redirect_uri - let stored_state = state::consume_state(&app_state.db, app_state.db_backend, &state_param) + let stored_state = state::consume_state(&app_state.db, app_state.db_backend, state_param) .await .map_err(|_| AppError::BadRequest("Invalid or expired state".into()))?; @@ -180,7 +186,7 @@ async fn callback( } // Phase 4: Everything after this redirects on error (never returns HTTP error) - match callback_inner(&app_state, &stored_state, &code, &state_param).await { + match callback_inner(&app_state, &stored_state, &query_params).await { Ok(()) => Ok(redirect_with_params( &stored_state.redirect_uri, &[("auth", "success")], @@ -202,8 +208,7 @@ async fn callback( async fn callback_inner( app_state: &AppState, stored_state: &state::StoredState, - code: &str, - state_param: &str, + params: &HashMap, ) -> Result<(), Box> { let config = serde_json::Value::Null; let secrets = load_plugin_secrets( @@ -232,9 +237,7 @@ async fn callback_inner( ) .await?; - let token_set = instance - .call_handle_callback(code, state_param, &config) - .await?; + let token_set = instance.call_handle_callback(params, &config).await?; let profile = instance .call_get_profile(&token_set.access_token, &config) @@ -306,16 +309,16 @@ async fn connect_with_config( ); // For API key auth, we pass the user's config to handle_callback - // The "code" is empty since there's no OAuth flow + // The params are empty since there's no OAuth flow let mut instance = executor .instantiate(&plugin_id, user_did, secrets, body.config.clone()) .await .map_err(|e| AppError::Internal(e.to_string()))?; - // Call handle_callback with the config as the callback params - // The plugin will extract the api_key from the config + // Call handle_callback with empty params (for API key auth, the config contains the key) + let empty_params = HashMap::new(); let token_set = instance - .call_handle_callback("", "", &body.config) + .call_handle_callback(&empty_params, &body.config) .await .map_err(|e| AppError::Internal(e.to_string()))?; diff --git a/src/plugin/executor.rs b/src/plugin/executor.rs index 5a4d46a..1d3ada6 100644 --- a/src/plugin/executor.rs +++ b/src/plugin/executor.rs @@ -116,19 +116,24 @@ impl PluginInstance { self.call_plugin_function("get_authorize_url", &input).await } - /// Call handle_callback(code, state, config) + /// Call handle_callback with all callback parameters + /// + /// For OAuth2: params contains "code" and "state" + /// For OpenID 2.0: params contains "openid.claimed_id", "openid.identity", etc. pub async fn call_handle_callback( &mut self, - code: &str, - state: &str, + params: &HashMap, config: &serde_json::Value, ) -> Result { - let input = serde_json::json!({ - "code": code, - "state": state, - "config": config - }); - self.call_plugin_function("handle_callback", &input).await + // Build input with all params flattened at the top level + let mut input = serde_json::Map::new(); + for (k, v) in params { + input.insert(k.clone(), serde_json::Value::String(v.clone())); + } + input.insert("config".to_string(), config.clone()); + + self.call_plugin_function("handle_callback", &serde_json::Value::Object(input)) + .await } /// Call refresh_tokens(refresh_token, config) @@ -445,12 +450,11 @@ mod tests { fn _check_call_handle_callback<'a>( inst: &'a mut PluginInstance, - code: &'a str, - state: &'a str, + params: &'a HashMap, config: &'a serde_json::Value, ) -> impl std::future::Future> + 'a { - inst.call_handle_callback(code, state, config) + inst.call_handle_callback(params, config) } fn _check_call_refresh_tokens<'a>( diff --git a/tests/plugin_executor.rs b/tests/plugin_executor.rs index 258a1a2..b830f18 100644 --- a/tests/plugin_executor.rs +++ b/tests/plugin_executor.rs @@ -127,8 +127,12 @@ async fn test_handle_callback() { .await .expect("Failed to instantiate"); + let mut params = HashMap::new(); + params.insert("code".to_string(), "code123".to_string()); + params.insert("state".to_string(), "state123".to_string()); + let tokens = instance - .call_handle_callback("code123", "state123", &serde_json::Value::Null) + .call_handle_callback(¶ms, &serde_json::Value::Null) .await .expect("Failed to handle callback"); -- 2.51.2