Refactor/v0.1.3 cleanup

.env.example

@@ -32,4 +32,8 @@ SESSION_SECRET=devsecret-change-me-in-production
 
 # Misc
 # RUST_LOG=debug
+# App network ports:
+# - APP_PORT: host port exposed by Docker
+# - SERVER_PORT: internal app listen port
 # APP_PORT=8789
+# SERVER_PORT=8789

.github/workflows/ci.yml

@@ -19,24 +19,14 @@ jobs:
         with:
           components: clippy, rustfmt
 
-      - name: Cache cargo
-        uses: actions/cache@v4
-        with:
-          path: |
-            ~/.cargo/registry
-            ~/.cargo/git
-            target
-          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
-          restore-keys: ${{ runner.os }}-cargo-
+      - name: Cache Rust build artifacts
+        uses: Swatinem/rust-cache@v2
 
       - name: Check formatting
         run: cargo fmt --check
 
       - name: Clippy
-        run: cargo clippy -- -D warnings
-
-      - name: Build
-        run: cargo build --release
+        run: cargo clippy --all-targets -- -D warnings
 
       - name: Test
         run: cargo test

compose.yaml

@@ -12,14 +12,15 @@ services:
     container_name: netweave
     environment:
       DATABASE_URL: "postgres://${DATABASE_USER}:${DATABASE_PASSWORD}@db:5432/${DATABASE_NAME}"
+      SERVER_PORT: ${SERVER_PORT:-8789}
       DEFAULT_ADMIN_USER: ${DEFAULT_ADMIN_USER:-admin}
       DEFAULT_ADMIN_PASSWORD: ${DEFAULT_ADMIN_PASSWORD:-adminpassword}
       ENCRYPTION_KEY: ${ENCRYPTION_KEY}
       SESSION_SECRET: ${SESSION_SECRET}
       RUST_LOG: ${RUST_LOG:-info}
       SESSION_SECURE_COOKIE: ${SESSION_SECURE_COOKIE:-false}
     ports:
-      - "${APP_PORT:-8789}:8789"
+      - "${APP_PORT:-8789}:${SERVER_PORT:-8789}"
     depends_on:
       db:
         condition: service_healthy

src/auth/mod.rs

@@ -1,3 +1,8 @@
+//! # Authentication module
+//!
+//! Manages authentication using server-side sessions (tower-sessions) and,
+//! optionally, OIDC for login.
+
 use crate::db::Db;
 use crate::handlers::common::{AppError, AppResult};
 use crate::AppState;
@@ -18,6 +23,7 @@ pub mod oidc;
 
 pub const AUTH_SESSION_KEY: &str = "auth_user";
 
+/// User role enum
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub enum Role {
     #[serde(rename = "ADMIN")]
@@ -56,12 +62,20 @@ impl FromStr for Role {
     }
 }
 
+/// Ensures that default admin user exsit if one was configured using ENV variables.
+/// Should be used only on startup. It won't override existing user data.
 pub async fn ensure_default_users(db: &Db) {
     if let (Ok(username), Ok(password)) = (
         std::env::var("DEFAULT_ADMIN_USER"),
         std::env::var("DEFAULT_ADMIN_PASSWORD"),
     ) {
-        let hashed = hash(password, DEFAULT_COST).expect("Failed to hash password");
+        let hashed = match hash(password, DEFAULT_COST) {
+            Ok(hashed) => hashed,
+            Err(e) => {
+                tracing::error!("Failed to hash default admin password: {}", e);
+                return;
+            }
+        };
         match db
             .create_user(&username, "admin@homelab.local", Some(&hashed), "ADMIN")
             .await
@@ -72,46 +86,54 @@ pub async fn ensure_default_users(db: &Db) {
     }
 }
 
+/// Authenticated user info
 #[derive(Serialize, Deserialize, Clone, Debug)]
 pub struct AuthUser {
     pub id: Uuid,
     pub username: String,
     pub role: Role,
 }
 
+/// Login DTO
 #[derive(Deserialize)]
 pub struct LoginPayload {
     pub username: String,
     pub password: String,
 }
 
+/// Auth response
 #[derive(Serialize)]
 pub struct AuthStatusResponse {
     pub status: &'static str,
     pub message: &'static str,
 }
 
+/// Logout response DTO
 #[derive(Serialize)]
 pub struct LogoutResponse {
     pub status: &'static str,
 }
 
+/// OIDC status response DTO
 #[derive(Serialize)]
 pub struct OidcStatusResponse {
     pub oidc_enabled: bool,
 }
 
+/// Change password DTO
 #[derive(Deserialize)]
 pub struct ChangePasswordPayload {
     pub current_password: Option<String>,
     pub new_password: String,
 }
 
+/// Main login handler
 pub async fn login_username_password(
     State(state): State<AppState>,
     session: Session,
     axum::Json(payload): axum::Json<LoginPayload>,
 ) -> AppResult<Json<AuthStatusResponse>> {
+    // see [../utils/rate_limiter]
     if !state.login_rate_limiter.check(&payload.username).await {
         return Err(AppError::TooManyRequests(
             "Too many login attempts. Please try again later.".into(),
@@ -163,6 +185,7 @@ pub async fn login_username_password(
     ))
 }
 
+/// Returns self user data
 pub async fn me_handler(session: Session) -> AppResult<Json<AuthUser>> {
     let user = session
         .get::<AuthUser>(AUTH_SESSION_KEY)
@@ -178,6 +201,7 @@ pub async fn me_handler(session: Session) -> AppResult<Json<AuthUser>> {
     }
 }
 
+/// Handles user self password change
 pub async fn change_password_handler(
     State(state): State<AppState>,
     session: Session,
@@ -187,16 +211,13 @@ pub async fn change_password_handler(
         return Err(AppError::BadRequest("New password cannot be empty".into()));
     }
 
-    let Some(auth_user) = session
+    // todo maybe clean this app using native error mapping
+    let auth_user = session
         .get::<AuthUser>(AUTH_SESSION_KEY)
         .await
-        .unwrap_or_else(|e| {
-            tracing::warn!("Failed to read auth session: {}", e);
-            None
-        })
-    else {
-        return Err(AppError::Unauthorized("Not authenticated".into()));
-    };
+        .ok()
+        .flatten()
+        .ok_or(AppError::Unauthorized("Not authenticated".into()))?;
 
     let user = state
         .db
@@ -209,6 +230,7 @@ pub async fn change_password_handler(
     }
 
     // Require current password only when an existing local password is set.
+    // OIDC user can change password without having set one.
     if let Some(existing_hash) = user.password_hash.filter(|h| !h.is_empty()) {
         let Some(current_password) = payload.current_password.as_deref() else {
             return Err(AppError::BadRequest("Current password is required".into()));
@@ -237,31 +259,41 @@ pub async fn change_password_handler(
     }))
 }
 
+// SECTION: OIDC endpoints
+//
+// Check [oidc guide](https://openid.net/developers/how-connect-works/)
+
+/// Checks if oidc is enabled
 pub async fn check_oidc_handler(
     State(state): State<AppState>,
 ) -> AppResult<Json<OidcStatusResponse>> {
-    Ok(Json(OidcStatusResponse {
-        oidc_enabled: state.oidc.is_some(),
-    }))
+    let oidc_enabled = state.oidc.read().await.is_some();
+    Ok(Json(OidcStatusResponse { oidc_enabled }))
 }
 
 const OIDC_NONCE_KEY: &str = "oidc_nonce";
 const OIDC_CSRF_KEY: &str = "oidc_csrf";
 
+/// Oidc login handler
 pub async fn oidc_login(State(state): State<AppState>, session: Session) -> AppResult<Redirect> {
-    if let Some(oidc) = &state.oidc {
+    let oidc = state.oidc.read().await.clone();
+
+    if let Some(oidc) = oidc {
         let (auth_url, csrf_token, nonce) = oidc.authorization_url();
+
         if let Err(e) = session.insert(OIDC_NONCE_KEY, nonce.secret().clone()).await {
             tracing::error!("Failed to store OIDC nonce in session: {}", e);
             return Err(AppError::Internal(e.into()));
         }
+
         if let Err(e) = session
             .insert(OIDC_CSRF_KEY, csrf_token.secret().clone())
             .await
         {
             tracing::error!("Failed to store OIDC CSRF in session: {}", e);
             return Err(AppError::Internal(e.into()));
         }
+
         Ok(Redirect::to(auth_url.as_str()))
     } else {
         Err(AppError::ServiceUnavailable(
@@ -270,18 +302,20 @@ pub async fn oidc_login(State(state): State<AppState>, session: Session) -> AppR
     }
 }
 
+/// OIDC auth callback parameters
 #[derive(Deserialize)]
 pub struct AuthCallbackParams {
     code: String,
     state: String,
 }
 
+/// OIDC callback endpoint
 pub async fn oidc_callback(
     State(state): State<AppState>,
     session: Session,
     Query(params): Query<AuthCallbackParams>,
 ) -> impl IntoResponse {
-    let oidc = match &state.oidc {
+    let oidc = match state.oidc.read().await.clone() {
         Some(s) => s,
         None => {
             return (

src/auth/oidc.rs

@@ -14,23 +14,14 @@ pub struct OidcService {
 
 impl OidcService {
     pub async fn from_config(config: &OidcConfig) -> Result<Self> {
-        let mut issuer_str = config.discovery_url.clone();
-        if issuer_str.ends_with(".well-known/openid-configuration") {
-            issuer_str = issuer_str
-                .strip_suffix(".well-known/openid-configuration")
-                .unwrap()
-                .to_string();
-        }
-        if issuer_str.ends_with(".well-known/openid-configuration/") {
-            issuer_str = issuer_str
-                .strip_suffix(".well-known/openid-configuration/")
-                .unwrap()
-                .to_string();
-        }
+        // config.rs normalizes discovery_url to end with /.well-known/openid-configuration.
+        // The openidconnect crate needs the issuer base URL (without the well-known path).
+        let issuer_str = config
+            .discovery_url
+            .trim_end_matches(".well-known/openid-configuration")
+            .trim_end_matches('/');
         // openidconnect-rs often requires a trailing slash for authentik issuers.
-        if !issuer_str.ends_with('/') {
-            issuer_str.push('/');
-        }
+        let issuer_str = format!("{issuer_str}/");
 
         let issuer_url = IssuerUrl::new(issuer_str)?;
         let client_id = ClientId::new(config.client_id.clone());