Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/api/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Comment on lines +114 to +117
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent naming: 'Session id' should be 'Session ID' (uppercase ID) to match the naming convention used in other error messages and documentation in the codebase.

Suggested change
/// 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 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.")

Copilot uses AI. Check for mistakes.
}

/// Session token not found
pub fn session_not_found() -> Self {
Self::unauthorized("Session not found").with_details(
Expand Down
2 changes: 2 additions & 0 deletions crates/api/src/openapi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions crates/api/src/routes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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
Expand Down
77 changes: 61 additions & 16 deletions crates/api/src/routes/oauth.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 253 is duplicated on line 268. Remove the duplicate comment on line 268 since the verification logic (lines 269-277) is adequately explained by the first comment.

Suggested change
// Verify that the session belongs to the authenticated user

Copilot uses AI. Check for mistakes.
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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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")]
Expand Down
38 changes: 38 additions & 0 deletions crates/database/src/repositories/session_repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions crates/services/src/auth/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()>;
}
Expand Down