oxidecomputer · augustuswm · Mar 20, 2026 · Mar 20, 2026 · Mar 20, 2026 · Mar 20, 2026
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -34,6 +34,7 @@ newtype-uuid = { version = "1.3.2", features = ["schemars08", "serde", "v4"] }
 oauth2 = { version = "5.0.0", default-features = false, features = ["rustls-tls"] }
 oauth2-reqwest = "0.1.0-alpha.3"
 partial-struct = { git = "https://github.com/oxidecomputer/partial-struct" }
+percent-encoding = "2.3.2"
 proc-macro2 = "1"
 quote = "1"
 rand = "0.8.5"

diff --git a/v-api/Cargo.toml b/v-api/Cargo.toml
@@ -26,6 +26,7 @@ oauth2 = { workspace = true }
 oauth2-reqwest = { workspace = true }
 newtype-uuid = { workspace = true }
 partial-struct = { workspace = true }
+percent-encoding = { workspace = true }
 rand = { workspace = true, features = ["std"] }
 rand_core = { workspace = true, features = ["std"] }
 reqwest = { workspace = true }

diff --git a/v-api/src/endpoints/handlers.rs b/v-api/src/endpoints/handlers.rs
@@ -283,7 +283,7 @@ mod macros {
                 rqctx: RequestContext<$context_type>,
                 path: Path<OAuthProviderNameParam>,
                 query: Query<OAuthAuthzCodeReturnQuery>,
-            ) -> Result<HttpResponseTemporaryRedirect, HttpError> {
+            ) -> Result<Response<Body>, HttpError> {
                 authz_code_callback_op(&rqctx, path, query).await
             }
 

diff --git a/v-api/src/endpoints/login/oauth/code.rs b/v-api/src/endpoints/login/oauth/code.rs
@@ -4,10 +4,10 @@
 
 use base64::{prelude::BASE64_URL_SAFE_NO_PAD, Engine};
 use chrono::{TimeDelta, Utc};
+use cookie::{Cookie, SameSite};
 use dropshot::{
-    http_response_temporary_redirect, Body, ClientErrorStatusCode, HttpError, HttpResponseOk,
-    HttpResponseTemporaryRedirect, Path, Query, RequestContext, RequestInfo, SharedExtractor,
-    TypedBody,
+    Body, ClientErrorStatusCode, HttpError, HttpResponseOk, Path, Query, RequestContext,
+    RequestInfo, SharedExtractor, TypedBody,
 };
 use dropshot_authorization_header::basic::BasicAuth;
 use http::{
@@ -19,6 +19,7 @@ use newtype_uuid::TypedUuid;
 use oauth2::{
     AuthorizationCode, CsrfToken, PkceCodeChallenge, PkceCodeVerifier, Scope, TokenResponse,
 };
+use percent_encoding::{percent_encode, NON_ALPHANUMERIC};
 use schemars::JsonSchema;
 use secrecy::SecretString;
 use serde::{Deserialize, Serialize};
@@ -205,8 +206,10 @@ where
     // Assign any scope errors that arose
     attempt.error = scope_error;
 
-    // Add in the user defined state and redirect uri
-    attempt.state = Some(query.state);
+    // Add in the user defined state and redirect uri. State is an arbitrary value and may be
+    // malicious. It must be url-encoded before being presented back to the client. Therefore we
+    // process once before storing so all downstream consumers see the encoded value.
+    attempt.state = Some(percent_encode(query.state.as_bytes(), NON_ALPHANUMERIC).to_string());
 
     // If the remote provider supports pkce, set up a challenge
     let pkce_challenge = if provider.supports_pkce() {
@@ -247,8 +250,13 @@ fn oauth_redirect_response(
 
     // Create an attempt cookie header for storing the login attempt. This also acts as our csrf
     // check
-    let login_cookie = HeaderValue::from_str(&format!("{}={}", LOGIN_ATTEMPT_COOKIE, attempt.id))
-        .map_err(to_internal_error)?;
+    let mut cookie = Cookie::new(LOGIN_ATTEMPT_COOKIE, attempt.id.to_string());
+    cookie.set_http_only(true);
+    cookie.set_same_site(SameSite::Lax);
+    cookie.set_secure(public_url.starts_with("https"));
+    cookie.set_max_age(cookie::time::Duration::seconds(600));
+
+    let login_cookie = HeaderValue::from_str(&cookie.to_string()).map_err(to_internal_error)?;
 
     // Generate the url to the remote provider that the user will be redirected to
     let mut authz_url = client
@@ -339,7 +347,7 @@ pub async fn authz_code_callback_op<T>(
     rqctx: &RequestContext<impl ApiContext<AppPermissions = T>>,
     path: Path<OAuthProviderNameParam>,
     query: Query<OAuthAuthzCodeReturnQuery>,
-) -> Result<HttpResponseTemporaryRedirect, HttpError>
+) -> Result<Response<Body>, HttpError>
 where
     T: VAppPermission + PermissionStorage,
 {
@@ -356,9 +364,25 @@ where
     // Verify and extract the attempt id before performing any work
     let attempt_id = verify_csrf(&rqctx.request, &query)?;
 
-    http_response_temporary_redirect(
-        authz_code_callback_op_inner(ctx, &attempt_id, query.code, query.error).await?,
-    )
+    // Clear the login attempt cookie
+    let mut cookie = Cookie::new(LOGIN_ATTEMPT_COOKIE, "");
+    cookie.set_http_only(true);
+    cookie.set_same_site(SameSite::Lax);
+    cookie.set_secure(ctx.public_url().starts_with("https"));
+    cookie.set_max_age(cookie::time::Duration::seconds(0));
+    let login_cookie = HeaderValue::from_str(&cookie.to_string()).map_err(to_internal_error)?;
+
+    Ok(Response::builder()
+        .status(StatusCode::TEMPORARY_REDIRECT)
+        .header(SET_COOKIE, login_cookie)
+        .header(
+            LOCATION,
+            HeaderValue::from_str(
+                &authz_code_callback_op_inner(ctx, &attempt_id, query.code, query.error).await?,
+            )
+            .map_err(to_internal_error)?,
+        )
+        .body(Body::empty())?)
 }
 
 pub async fn authz_code_callback_op_inner<T>(
@@ -927,7 +951,11 @@ mod tests {
             .unwrap()
         );
         assert_eq!(
-            attempt.id.to_string().as_str(),
+            format!(
+                "{}; HttpOnly; SameSite=Lax; Secure; Max-Age=600",
+                attempt.id
+            )
+            .as_str(),
             String::from_utf8(
                 response
                     .headers()