fix: logout (#45)

Co-authored-by: Pierre Le Guen <26087574+PierreLeGuen@users.noreply.github.com>

Author: hanakannzashi

crates/api/src/error.rs

@@ -111,6 +111,12 @@ impl ApiError {
             .with_details("Session token must start with 'sess_' and be 37 characters long")
     }
 
+    /// Session id not found
+    pub fn session_id_not_found() -> Self {
+        Self::not_found("Session id not found")
+            .with_details("The provided session id does not match any active session.")
+    }
+
     /// Session token not found
     pub fn session_not_found() -> Self {
         Self::unauthorized("Session not found").with_details(

crates/api/src/openapi.rs

@@ -38,6 +38,8 @@ use utoipa::OpenApi;
         crate::models::UserProfileResponse,
         crate::models::AuthResponse,
         crate::error::ApiErrorResponse,
+        // Auth request models
+        crate::routes::oauth::LogoutRequest,
         // User settings models
         crate::models::UserSettingsResponse,
         crate::models::UpdateUserSettingsPartiallyRequest,

crates/api/src/routes/mod.rs

@@ -55,6 +55,14 @@ pub fn create_router_with_cors(app_state: AppState, allowed_origins: Vec<String>
     // OAuth routes (public, no auth required)
     let auth_routes = oauth::create_oauth_router();
 
+    // Logout route (requires authentication)
+    let logout_route = Router::new()
+        .route("/logout", axum::routing::post(oauth::logout))
+        .layer(from_fn_with_state(
+            auth_state.clone(),
+            crate::middleware::auth_middleware,
+        ));
+
     // Attestation routes (public, no auth required)
     let attestation_routes = attestation::create_attestation_router();
 
@@ -80,6 +88,7 @@ pub fn create_router_with_cors(app_state: AppState, allowed_origins: Vec<String>
     let router = Router::new()
         .route("/health", get(health_check))
         .nest("/v1/auth", auth_routes)
+        .nest("/v1/auth", logout_route) // Logout route with auth middleware
         .nest("/v1/users", user_routes)
         .nest("/v1/admin", admin_routes)
         .merge(api_routes) // Merge instead of nest since api routes already have /v1 prefix

crates/api/src/routes/oauth.rs

@@ -1,13 +1,15 @@
-use crate::{error::ApiError, state::AppState};
+use crate::{error::ApiError, middleware::AuthenticatedUser, state::AppState};
+use axum::extract::Query;
 use axum::{
-    extract::{Query, State},
+    extract::{Extension, State},
     http::StatusCode,
     response::Redirect,
     routing::get,
-    Router,
+    Json, Router,
 };
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use services::SessionId;
+use utoipa::ToSchema;
 
 /// Query parameters for OAuth callback
 #[derive(Debug, Deserialize)]
@@ -23,6 +25,13 @@ pub struct OAuthInitQuery {
     pub frontend_callback: Option<String>,
 }
 
+/// Request body for logout
+#[derive(Debug, Deserialize, Serialize, ToSchema)]
+pub struct LogoutRequest {
+    /// Session ID to revoke
+    pub session_id: SessionId,
+}
+
 /// Request body for mock login (test only)
 #[cfg(feature = "test")]
 #[derive(Debug, Deserialize)]
@@ -148,9 +157,10 @@ pub async fn oauth_callback(
     tracing::info!("Redirecting to frontend: {}", frontend_url);
 
     let mut callback_url = format!(
-        "{}/auth/callback?token={}&expires_at={}",
+        "{}/auth/callback?token={}&session_id={}&expires_at={}",
         frontend_url,
         urlencoding::encode(&token),
+        urlencoding::encode(&session.session_id.to_string()),
         urlencoding::encode(&session.expires_at.to_rfc3339())
     );
     if is_new_user {
@@ -214,14 +224,15 @@ pub async fn github_login(
 
 /// Handler for logout
 #[utoipa::path(
-    get,
+    post,
     path = "/v1/auth/logout",
     tag = "Auth",
-    params(
-        ("session_id" = uuid::Uuid, Query, description = "Session ID to revoke")
-    ),
+    request_body = LogoutRequest,
     responses(
         (status = 204, description = "Successfully logged out"),
+        (status = 401, description = "Unauthorized", body = crate::error::ApiErrorResponse),
+        (status = 403, description = "Forbidden - session does not belong to authenticated user", body = crate::error::ApiErrorResponse),
+        (status = 404, description = "Session not found", body = crate::error::ApiErrorResponse),
         (status = 500, description = "Internal server error", body = crate::error::ApiErrorResponse)
     ),
     security(
@@ -230,9 +241,41 @@ pub async fn github_login(
 )]
 pub async fn logout(
     State(app_state): State<AppState>,
-    Query(session_id): Query<SessionId>,
+    Extension(authenticated_user): Extension<AuthenticatedUser>,
+    Json(request): Json<LogoutRequest>,
 ) -> Result<StatusCode, ApiError> {
-    tracing::info!("Logout requested for session_id: {}", session_id);
+    let session_id = request.session_id;
+    tracing::info!(
+        "Logout requested for session_id: {} by user_id: {}",
+        session_id,
+        authenticated_user.user_id
+    );
+
+    // Verify that the session belongs to the authenticated user
+    let session = app_state
+        .session_repository
+        .get_session_by_id(session_id)
+        .await
+        .map_err(|e| {
+            tracing::error!("Failed to get session {}: {}", session_id, e);
+            ApiError::logout_failed()
+        })?;
+
+    let session = session.ok_or_else(|| {
+        tracing::warn!("Session {} not found", session_id);
+        ApiError::session_id_not_found()
+    })?;
+
+    // Verify that the session belongs to the authenticated user
+    if session.user_id != authenticated_user.user_id {
+        tracing::warn!(
+            "User {} attempted to logout session {} which belongs to user {}",
+            authenticated_user.user_id,
+            session_id,
+            session.user_id
+        );
+        return Err(ApiError::forbidden("You can only logout your own sessions"));
+    }
 
     app_state
         .oauth_service
@@ -243,7 +286,11 @@ pub async fn logout(
             ApiError::logout_failed()
         })?;
 
-    tracing::info!("Session {} successfully revoked", session_id);
+    tracing::info!(
+        "Session {} successfully revoked by user_id: {}",
+        session_id,
+        authenticated_user.user_id
+    );
 
     Ok(StatusCode::NO_CONTENT)
 }
@@ -317,16 +364,14 @@ pub async fn mock_login(
     }))
 }
 
-/// Create OAuth router with all routes
+/// Create OAuth router with all routes (excluding logout, which requires auth)
 pub fn create_oauth_router() -> Router<AppState> {
     let router = Router::new()
         // OAuth initiation routes
         .route("/google", get(google_login))
         .route("/github", get(github_login))
         // Unified callback route for all providers
-        .route("/callback", get(oauth_callback))
-        // Logout route
-        .route("/logout", get(logout));
+        .route("/callback", get(oauth_callback));
 
     // Add mock login route only in test builds
     #[cfg(feature = "test")]

crates/database/src/repositories/session_repository.rs

@@ -117,6 +117,44 @@ impl SessionRepository for PostgresSessionRepository {
         Ok(result)
     }
 
+    async fn get_session_by_id(
+        &self,
+        session_id: SessionId,
+    ) -> anyhow::Result<Option<UserSession>> {
+        tracing::debug!("Looking up session by session_id: {}", session_id);
+
+        let client = self.pool.get().await?;
+
+        let row = client
+            .query_opt(
+                "SELECT id, user_id, created_at, expires_at 
+                 FROM sessions 
+                 WHERE id = $1",
+                &[&session_id],
+            )
+            .await?;
+
+        let result = row.map(|r| UserSession {
+            session_id: r.get(0),
+            user_id: r.get(1),
+            created_at: r.get(2),
+            expires_at: r.get(3),
+            token: None, // Never return the token on retrieval
+        });
+
+        if let Some(ref session) = result {
+            tracing::debug!(
+                "Session found: session_id={}, user_id={}",
+                session.session_id,
+                session.user_id
+            );
+        } else {
+            tracing::debug!("No session found for session_id: {}", session_id);
+        }
+
+        Ok(result)
+    }
+
     async fn delete_session(&self, session_id: SessionId) -> anyhow::Result<()> {
         tracing::info!("Deleting session: session_id={}", session_id);
 

crates/services/src/auth/ports.rs

@@ -57,6 +57,10 @@ pub trait SessionRepository: Send + Sync {
         token_hash: String,
     ) -> anyhow::Result<Option<UserSession>>;
 
+    /// Retrieve a session by session ID
+    async fn get_session_by_id(&self, session_id: SessionId)
+        -> anyhow::Result<Option<UserSession>>;
+
     /// Delete a session
     async fn delete_session(&self, session_id: SessionId) -> anyhow::Result<()>;
 }