Removed unprotected server function exposing bookings, remaining ones require login. Fixes #19

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.claude/rules/backend.md

@@ -28,7 +28,10 @@ paths:
 ## Authorization
 
 - Every server fn that reads or changes non-public data starts with `perm_check!(is_logged_in)` or `perm_check!(is_admin)`; use `user_check!(login)` when a non-admin may only touch their own record. The macros return early with 403 and `ApiResponse::Error`, so they only work in functions returning `Result<ApiResponse<_>, _>`.
-- The auth middleware protects only `/admin*` page requests. `/api/*` is not covered by it — a server fn without `perm_check!` is public. Only the public booking form endpoints should be.
+- The auth middleware protects only `/admin*` page requests. `/api/*` is not covered by it — a server fn without `perm_check!` is public, whether or not any page calls it. Only the endpoints of the public booking form and of the customer's booking page should be.
+- A server fn that does not return `ApiResponse` can't use the macros; it starts with `require_login().await?` (`src/backend/user.rs`).
+- The test `every_server_fn_checks_permissions_or_is_listed_as_public` in `src/backend/mod.rs` scans the sources and fails for a server fn that neither checks the user nor is on its `PUBLIC_SERVER_FNS` list. Put a function on the list only when it returns nothing about other people — in particular never a booking `uuid`, which is the secret of the customer's booking page.
+- Server-side code must not call a permission-checked server fn to get data for a request of a customer (e.g. mail templates when a booking is made); use the helper underneath (`message_for_type`).
 - Plain Actix handlers must check the session themselves.
 - Files are uploaded through a multipart server function (`upload_image` in `src/backend/appearance.rs`): `#[server(input = MultipartFormData)]`, called in the browser through `Action::new_local` with a `web_sys::FormData`. The server stores a file under a name it makes itself, checks size and content, and never trusts the name sent by the browser. The multipart codec panics on a request without a boundary; `is_broken_upload` in `src/main.rs` refuses those first — extend it when adding another multipart function.
 - HTML that comes from a user and is rendered with `inner_html` (the text under the banner) must go through `sanitize_text` in `src/backend/rich_text.rs` before it is stored. Extend its allow-list together with the editor toolbar, never the other way round.

src/backend/mail.rs

@@ -253,8 +253,13 @@ cfg_if! { if #[cfg(feature = "ssr")] {
     }
 }}
 
+/// A mail template for the mail settings. Code that sends e-mails reads templates with
+/// `message_for_type`, it runs for requests without login too.
 #[server]
 pub async fn get_message(msg_type: MessageType) -> Result<Message, ServerFnError> {
+    use crate::backend::user::require_login;
+
+    require_login().await?;
     let pool = get_pool().await?;
 
     Ok(message_for_type(&msg_type, &pool).await?)

src/backend/mod.rs

@@ -135,4 +135,85 @@ cfg_if!{
             Ok(data.mailer().clone())
         }
     }
-}
+}
+
+#[cfg(test)]
+mod tests {
+    use std::fs;
+    use regex::Regex;
+
+    /// Server functions that anybody may call. Everything under `/api` is reachable without
+    /// login unless the function checks the user itself, so each of these is here on purpose:
+    /// the public booking form, the customer's page of a booking, and logging in.
+    /// Before adding a name, make sure the function returns nothing about other people.
+    const PUBLIC_SERVER_FNS: [&str; 18] = [
+        // Look of the public page
+        "get_appearance",
+        // The customer's own data from their cookie
+        "get_remembered",
+        // Data of the booking form
+        "get_hours", "get_hours_for_day", "get_closing_time", "get_closing_times",
+        "get_properties", "get_active_properties", "get_public_form_data", "get_pow",
+        "create_reservation",
+        // The customer's page of a booking; the uuid from the e-mail is the secret
+        "get_booking", "cancel_booking",
+        // Session of the caller
+        "login", "logout", "auth_check", "admin_check", "get_user",
+    ];
+
+    /// Names of the server functions in a source file with the information whether they check the user
+    fn server_fns(source: &str) -> Vec<(String, bool)> {
+        let header = Regex::new(r"#\[server[^\]]*\]\s*pub async fn (\w+)").unwrap();
+        let headers = header.captures_iter(source)
+            .map(|c| (c.get(0).unwrap().start(), c[1].to_string()))
+            .collect::<Vec<_>>();
+
+        headers.iter().enumerate().map(|(index, (start, name))| {
+            // The body ends where the next server function starts; close enough for a check
+            let end = headers.get(index + 1).map(|next| next.0).unwrap_or(source.len());
+            let body = &source[*start..end];
+            let checked = body.contains("perm_check!(") || body.contains("user_check!(") || body.contains("require_login().await?");
+
+            (name.clone(), checked)
+        }).collect()
+    }
+
+    #[test]
+    fn server_fns_are_recognized() {
+        let source = "#[server]\npub async fn open() -> X {\n    1\n}\n\n\
+            #[server(name = A, prefix = \"/api\", endpoint = \"a\")]\npub async fn closed() -> X {\n    perm_check!(is_admin);\n}\n\n\
+            #[server]\npub async fn other() -> X {\n    require_login().await?;\n}\n";
+
+        assert_eq!(server_fns(source), vec![
+            ("open".to_string(), false), ("closed".to_string(), true), ("other".to_string(), true)]);
+    }
+
+    /// Fails when a server function neither checks the user nor is listed as public
+    #[test]
+    fn every_server_fn_checks_permissions_or_is_listed_as_public() {
+        let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/src/backend");
+        let mut unchecked = vec![];
+        let mut found = 0;
+
+        for entry in fs::read_dir(dir).unwrap() {
+            let path = entry.unwrap().path();
+            if path.extension().is_some_and(|e| e == "rs") {
+                for (name, checked) in server_fns(&fs::read_to_string(&path).unwrap()) {
+                    found += 1;
+                    if !checked {
+                        unchecked.push(name);
+                    }
+                }
+            }
+        }
+        unchecked.sort();
+
+        let mut public = PUBLIC_SERVER_FNS.to_vec();
+        public.sort();
+
+        assert!(found > 40, "only {} server functions found, the scan is broken", found);
+        assert_eq!(unchecked, public,
+            "server functions without a permission check differ from the list of public ones; \
+            add perm_check!/user_check!/require_login to the function, or put it on the list if it is meant to be public");
+    }
+}

src/backend/reservation.rs

@@ -23,7 +23,7 @@ cfg_if! { if #[cfg(feature = "ssr")] {
     use crate::backend::data::{PublicBookingItem};
     use chrono::NaiveDateTime;
     use crate::backend::mail::MailMessage;
-    use crate::backend::mail::get_message;
+    use crate::backend::mail::message_for_type;
     use crate::error::AppError;
     use sqlx::PgPool;
     use crate::backend::user::admin_email;
@@ -229,9 +229,15 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         Ok(())
     }
 
+    /// Mail template of the given type. Not through the `get_message` server function: that one
+    /// is for logged-in users, and e-mails are sent for requests of customers too.
+    async fn template(msg_type: MessageType) -> Result<Message, AppError> {
+        Ok(message_for_type(&msg_type, &get_pool().await?).await?)
+    }
+
     async fn notify_new_all(admin_mail: String, reservation: &ResSumWithItems) -> Result<(), AppError> {
         let mailing = get_mailing().await?;
-        let msg = get_message(MessageType::NewReservation).await?;
+        let msg = template(MessageType::NewReservation).await?;
 
         let link = link_for(&reservation.summary.uuid).await;
 
@@ -244,7 +250,7 @@ cfg_if! { if #[cfg(feature = "ssr")] {
 
     async fn notify_new(uuid: Uuid) -> Result<(), AppError> {
         let mailing = get_mailing().await?;
-        let msg = get_message(MessageType::NewReservationCust).await?;
+        let msg = template(MessageType::NewReservationCust).await?;
         let reservation = reservation_by_uuid(uuid).await?;
         let admin_mail = admin_email().await;
 
@@ -277,12 +283,12 @@ cfg_if! { if #[cfg(feature = "ssr")] {
     }
 
     async fn notify_approve(uuid: Uuid) -> Result<(), AppError> {
-        send_notify(uuid, get_message(MessageType::ReservationApp).await?, true).await
+        send_notify(uuid, template(MessageType::ReservationApp).await?, true).await
     }
 
     /// There is nothing left to cancel, so no link is appended
     async fn notify_cancel(uuid: Uuid) -> Result<(), AppError> {
-        send_notify(uuid, get_message(MessageType::ReservationCanceled).await?, false).await
+        send_notify(uuid, template(MessageType::ReservationCanceled).await?, false).await
     }
 
     /// The customer cancelled: a confirmation for them and a message for the staff
@@ -290,7 +296,7 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         notify_cancel(uuid).await?;
 
         let mailing = get_mailing().await?;
-        let msg = get_message(MessageType::CustomerCanceled).await?;
+        let msg = template(MessageType::CustomerCanceled).await?;
         let reservation = reservation_by_uuid(uuid).await?;
         let Some(admin_mail) = admin_email().await else {
             return Err(AppError::MailSendError("No admin mail".to_string()))
@@ -497,15 +503,6 @@ fn num_days(month: u32, year: i32) -> i64 {
         .num_days()
 }
 
-#[server]
-pub async fn get_reservations_for_month(month: u32, year: i32) -> Result<ApiResponse<Vec<ResSumWithItems>>, ServerFnError> {
-    let data = reservations_in_range(&NaiveDate::from_ymd_opt(year, month, 1).unwrap(),
-        &NaiveDate::from_ymd_opt(year, month, num_days(month, year).to_u32().unwrap_or_default()).unwrap(),
-        None, Page::default()).await?;
-
-    Ok(ApiResponse::Data(data.0))
-}
-
 #[server]
 pub async fn approve(uuid: String) -> Result<ApiResponse<()>, ServerFnError> {
     use crate::perm_check;
@@ -667,6 +664,9 @@ pub async fn year_chart(dark: bool) -> Result<ApiResponse<String>, ServerFnError
 
 #[server]
 pub async fn years() -> Result<Vec<i32>, ServerFnError> {
+    use crate::backend::user::require_login;
+
+    require_login().await?;
     let pool = get_pool().await?;
 
     let data: Result<Vec<(f64,)>, Error> = query_as("SELECT DISTINCT date_part('year', date) as year FROM reservation_sum ORDER BY year DESC")
@@ -678,6 +678,9 @@ pub async fn years() -> Result<Vec<i32>, ServerFnError> {
 
 #[server]
 pub async fn months(year: i32) -> Result<Vec<u32>, ServerFnError> {
+    use crate::backend::user::require_login;
+
+    require_login().await?;
     let pool = get_pool().await?;
 
     let data: Result<Vec<(f64,)>, Error> = query_as("SELECT DISTINCT date_part('month', date) as month \

src/backend/user.rs

@@ -72,6 +72,21 @@ cfg_if! { if #[cfg(feature = "ssr")] {
         logged_in_user().await.is_some()
     }
 
+    /// Permission check for server functions that don't return `ApiResponse`, where the
+    /// `perm_check!` macro can't be used: answers 403 with an error unless a user is logged in.
+    pub async fn require_login() -> Result<(), ServerFnError> {
+        if is_logged_in().await {
+            return Ok(())
+        }
+
+        if let Some(response) = use_context::<leptos_actix::ResponseOptions>() {
+            response.set_status(actix_web::http::StatusCode::FORBIDDEN);
+        }
+        warn!("Permission denied for a request without login");
+
+        Err(ServerFnError::new("Forbidden"))
+    }
+
     pub async fn is_admin() -> bool {
         if let Some(user) = logged_in_user().await {
             user.admin