fix(identity): address auth review findings

crates/zagrosi-identity/src/error.rs

@@ -73,6 +73,17 @@ pub enum IdentityError {
     #[error("database error: {0}")]
     Database(#[source] Box<sqlx::Error>),
 
+    /// HTTP response construction failed because a generated header
+    /// value contained bytes that cannot be represented in an HTTP
+    /// header. This is an internal invariant breach, never a caller
+    /// validation failure.
+    #[error("response header malformed: {reason}")]
+    ResponseHeaderMalformed {
+        /// Human-readable description of the malformed generated
+        /// header.
+        reason: String,
+    },
+
     /// Raw token string failed prefix / body validation. Domain-layer
     /// `domain::token_format::parse_raw` is the single chokepoint; this
     /// variant is returned rather than the gateway-facing
@@ -677,6 +688,7 @@ impl axum::response::IntoResponse for IdentityError {
             | Self::MalformedEnvelope(_)
             | Self::UnknownKeyId(_)
             | Self::Database(_)
+            | Self::ResponseHeaderMalformed { .. }
             | Self::Argon2ProfileTooSlow { .. }
             | Self::Argon2Internal(_)
             | Self::MalformedRateLimit { .. }

crates/zagrosi-identity/src/http/auth.rs

@@ -3,18 +3,22 @@
 #![allow(clippy::doc_markdown, clippy::too_long_first_doc_paragraph)]
 //! Sign-up / sign-in / sign-out HTTP handlers.
 
-use std::net::IpAddr;
-
-use axum::Json;
 use axum::extract::{ConnectInfo, State};
-use axum::http::StatusCode;
-use axum::response::IntoResponse;
+use axum::http::{HeaderMap, HeaderValue, StatusCode, header};
+use axum::response::{IntoResponse, Response};
+use axum::{Extension, Json};
+use chrono::{DateTime, Utc};
 use uuid::Uuid;
+use zagrosi_core::AuthContext;
 
-use crate::error::Result;
+use crate::error::{IdentityError, Result};
 use crate::http::IdentityState;
 use crate::service::signin::SignInRequest;
 use crate::service::signup::{SignUpRequest, SignUpResponse};
+use crate::session::{
+    IssuedSession, SessionAttachment, build_clear_csrf_cookie, build_clear_session_cookie,
+    generate_csrf_value,
+};
 
 /// JSON body for `POST /v1/auth/sign-up`.
 #[derive(Debug, serde::Deserialize)]
@@ -63,14 +67,46 @@ pub struct SignInResponse {
     /// Issued session id (for client-side telemetry; cookie is what
     /// authorises subsequent requests).
     pub session_id: Uuid,
+    /// Issued session metadata.
+    pub session: SignInSession,
+    /// Raw `sid_*` credential for non-browser clients that use bearer
+    /// authentication instead of the emitted session cookie.
+    pub session_token: String,
+    /// CSRF double-submit value emitted both in the response body and
+    /// the readable CSRF cookie.
+    pub csrf_token: String,
+}
+
+/// Session metadata returned after password sign-in.
+#[derive(Debug, serde::Serialize)]
+pub struct SignInSession {
+    /// Session row identifier.
+    pub id: Uuid,
+    /// Owning user.
+    pub user_id: Uuid,
+    /// Active organisation selected at issue time, when known.
+    pub org_id: Option<Uuid>,
+    /// Hard expiry timestamp for the issued session.
+    pub expires_at: DateTime<Utc>,
+}
+
+impl From<&IssuedSession> for SignInSession {
+    fn from(session: &IssuedSession) -> Self {
+        Self {
+            id: session.id,
+            user_id: session.user_id,
+            org_id: session.org_id,
+            expires_at: session.expires_at,
+        }
+    }
 }
 
 /// `POST /v1/auth/sign-in` handler.
 pub async fn sign_in(
     State(state): State<IdentityState>,
     ConnectInfo(addr): ConnectInfo<std::net::SocketAddr>,
     Json(body): Json<SignInBody>,
-) -> Result<Json<SignInResponse>> {
+) -> Result<Response> {
     let session = state
         .service
         .sign_in(SignInRequest {
@@ -80,32 +116,96 @@ pub async fn sign_in(
             correlation_id: Uuid::now_v7(),
         })
         .await?;
-    Ok(Json(SignInResponse {
-        status: "ok",
-        session_id: session.id,
-    }))
-}
-
-/// JSON body for `POST /v1/auth/sign-out`.
-#[derive(Debug, serde::Deserialize)]
-pub struct SignOutBody {
-    /// Session id to revoke.
-    pub session_id: Uuid,
+    let attachment = SessionAttachment::new(session.raw_token.clone(), generate_csrf_value());
+    let mut headers = HeaderMap::new();
+    append_set_cookie(&mut headers, &attachment.session_set_cookie())?;
+    append_set_cookie(&mut headers, &attachment.csrf_set_cookie())?;
+    Ok((
+        StatusCode::OK,
+        headers,
+        Json(SignInResponse {
+            status: "ok",
+            session_id: session.id,
+            session: SignInSession::from(&session),
+            session_token: attachment.raw_session_token,
+            csrf_token: attachment.csrf_value,
+        }),
+    )
+        .into_response())
 }
 
 /// `POST /v1/auth/sign-out` handler.
 pub async fn sign_out(
     State(state): State<IdentityState>,
+    ctx: Option<Extension<AuthContext>>,
     ConnectInfo(addr): ConnectInfo<std::net::SocketAddr>,
-    Json(body): Json<SignOutBody>,
-) -> Result<StatusCode> {
-    let _ip: IpAddr = addr.ip();
+) -> Result<Response> {
+    let Some(Extension(ctx)) = ctx else {
+        return Err(IdentityError::InvalidCredentials);
+    };
     state
         .service
-        .sign_out(body.session_id, None, Some(addr.ip()), Uuid::now_v7())
+        .sign_out(
+            ctx.session_id(),
+            Some(ctx.subject_id()),
+            Some(addr.ip()),
+            Uuid::now_v7(),
+        )
         .await?;
-    Ok(StatusCode::NO_CONTENT)
+    let mut headers = HeaderMap::new();
+    append_set_cookie(&mut headers, &build_clear_session_cookie())?;
+    append_set_cookie(&mut headers, &build_clear_csrf_cookie())?;
+    Ok((StatusCode::NO_CONTENT, headers).into_response())
+}
+
+fn append_set_cookie(headers: &mut HeaderMap, value: &str) -> Result<()> {
+    let header_value =
+        HeaderValue::from_str(value).map_err(|_| IdentityError::ResponseHeaderMalformed {
+            reason: "set-cookie header value contains illegal byte".into(),
+        })?;
+    headers.append(header::SET_COOKIE, header_value);
+    Ok(())
 }
 
-#[allow(dead_code)]
-fn dummy_into_response_use<T: IntoResponse>(_: T) {}
+#[cfg(test)]
+mod tests {
+    #![allow(clippy::expect_used)]
+
+    use super::*;
+
+    #[test]
+    fn sign_in_response_surfaces_session_credential_material() {
+        let issued = IssuedSession {
+            id: Uuid::now_v7(),
+            user_id: Uuid::now_v7(),
+            org_id: Some(Uuid::now_v7()),
+            expires_at: Utc::now(),
+            raw_token: "sid_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
+        };
+        let attachment = SessionAttachment::new(issued.raw_token.clone(), "c".repeat(43));
+        let response = SignInResponse {
+            status: "ok",
+            session_id: issued.id,
+            session: SignInSession::from(&issued),
+            session_token: attachment.raw_session_token,
+            csrf_token: attachment.csrf_value,
+        };
+
+        assert_eq!(response.session_id, issued.id);
+        assert_eq!(
+            response.session_token,
+            "sid_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+        );
+        assert_eq!(response.csrf_token, "c".repeat(43));
+        assert_eq!(response.session.user_id, issued.user_id);
+    }
+
+    #[test]
+    fn appends_multiple_set_cookie_headers() {
+        let mut headers = HeaderMap::new();
+        append_set_cookie(&mut headers, "a=b").expect("first cookie is valid");
+        append_set_cookie(&mut headers, "c=d").expect("second cookie is valid");
+
+        assert_eq!(headers.get_all(header::SET_COOKIE).iter().count(), 2);
+    }
+}

crates/zagrosi-identity/src/http/mod.rs

@@ -15,7 +15,7 @@
 //! - [`router`] returns the public surface only:
 //!   - `POST /v1/auth/sign-up`
 //!   - `POST /v1/auth/sign-in`
-//!   - `POST /v1/auth/sign-out`
+//!   - `POST /v1/auth/sign-out` (requires an attached `AuthContext`)
 //!   - `POST /v1/auth/password-reset/request`
 //!   - `POST /v1/auth/password-reset/confirm`
 //!   - `GET  /v1/auth/password-reset/landing`

crates/zagrosi-identity/src/service/signup.rs

@@ -144,15 +144,18 @@ impl IdentityService {
         let mut tx = self.pool.begin().await?;
         let user = self
             .user_repo
-            .create(NewUser {
-                id: user_id,
-                email: &req.email,
-                display_name: &req.display_name,
-                password_hash: Some(&phc),
-                password_updated_at: Some(now),
-                password_hash_version: 1,
-                external_id: None,
-            })
+            .create_in_tx(
+                &mut tx,
+                NewUser {
+                    id: user_id,
+                    email: &req.email,
+                    display_name: &req.display_name,
+                    password_hash: Some(&phc),
+                    password_updated_at: Some(now),
+                    password_hash_version: 1,
+                    external_id: None,
+                },
+            )
             .await?;
         self.email_verification_repo
             .insert(

crates/zagrosi-identity/src/session/cookie.rs

@@ -35,6 +35,18 @@ pub const CSRF_COOKIE_NAME: &str = "__Host-zagrosi_csrf";
 /// [`crate::http::csrf::csrf_middleware`].
 pub const CSRF_HEADER_NAME: &str = "x-zagrosi-csrf";
 
+/// Render a `Set-Cookie` value that clears the browser session cookie.
+#[must_use]
+pub fn build_clear_session_cookie() -> String {
+    format!("{SESSION_COOKIE_NAME}=; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age=0")
+}
+
+/// Render a `Set-Cookie` value that clears the CSRF cookie.
+#[must_use]
+pub fn build_clear_csrf_cookie() -> String {
+    format!("{CSRF_COOKIE_NAME}=; Path=/; Secure; SameSite=Lax; Max-Age=0")
+}
+
 /// Issued cookie pair the auth handler attaches to its response.
 ///
 /// `session` carries the raw `sid_*` token; `csrf` carries the
@@ -146,4 +158,26 @@ mod tests {
         assert_eq!(attach.raw_session_token, raw);
         assert_eq!(attach.csrf_value, "c".repeat(43));
     }
+
+    #[test]
+    fn clear_session_cookie_expires_host_cookie() {
+        let rendered = build_clear_session_cookie();
+        assert!(rendered.contains("__Host-zagrosi_sid="));
+        assert!(rendered.contains("Path=/"));
+        assert!(rendered.contains("Secure"));
+        assert!(rendered.contains("HttpOnly"));
+        assert!(rendered.contains("SameSite=Lax"));
+        assert!(rendered.contains("Max-Age=0"));
+    }
+
+    #[test]
+    fn clear_csrf_cookie_expires_host_cookie_without_httponly() {
+        let rendered = build_clear_csrf_cookie();
+        assert!(rendered.contains("__Host-zagrosi_csrf="));
+        assert!(rendered.contains("Path=/"));
+        assert!(rendered.contains("Secure"));
+        assert!(rendered.contains("SameSite=Lax"));
+        assert!(rendered.contains("Max-Age=0"));
+        assert!(!rendered.contains("HttpOnly"));
+    }
 }

crates/zagrosi-identity/src/session/mod.rs

@@ -44,7 +44,10 @@ pub mod write_behind;
 
 pub use cache::{CachedSession, SessionCache};
 pub use continuation::SessionView;
-pub use cookie::{CSRF_COOKIE_NAME, CSRF_HEADER_NAME, SESSION_COOKIE_NAME, SessionAttachment};
+pub use cookie::{
+    CSRF_COOKIE_NAME, CSRF_HEADER_NAME, SESSION_COOKIE_NAME, SessionAttachment,
+    build_clear_csrf_cookie, build_clear_session_cookie,
+};
 pub use events::{BusError, SessionEventBus};
 pub use introspector::IdentitySessionIntrospector;
 pub use issuer::{IdentitySessionIssuer, generate_csrf_value};

documentation/api/identity.openapi.yaml

@@ -61,7 +61,7 @@ paths:
     post:
       tags: [Auth]
       operationId: signIn
-      summary: Verify email and password and issue browser session cookies.
+      summary: Verify email and password and issue browser session cookies plus bearer fallback tokens.
       requestBody:
         required: true
         content:
@@ -91,13 +91,6 @@ paths:
       summary: Revoke the current browser session.
       security:
         - SessionCookie: []
-      requestBody:
-        required: false
-        content:
-          application/json:
-            schema:
-              type: object
-              additionalProperties: false
       responses:
         "204":
           description: Signed out.
@@ -1155,10 +1148,37 @@ components:
           type: string
     SignInResponse:
       type: object
-      required: [session]
+      required: [status, session_id, session, session_token, csrf_token]
       properties:
+        status:
+          type: string
+          enum: [ok]
+        session_id:
+          type: string
+          format: uuid
         session:
-          $ref: "#/components/schemas/Session"
+          type: object
+          required: [id, user_id, expires_at]
+          properties:
+            id:
+              type: string
+              format: uuid
+            user_id:
+              type: string
+              format: uuid
+            org_id:
+              type: string
+              format: uuid
+              nullable: true
+            expires_at:
+              type: string
+              format: date-time
+        session_token:
+          $ref: "#/components/schemas/PrefixedToken"
+        csrf_token:
+          type: string
+          minLength: 43
+          maxLength: 43
     EmailRequest:
       type: object
       required: [email]