diff --git a/.gitignore b/.gitignore index 89f041d5..e9e86ec1 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,6 @@ tui-pages-cli/ tui-canvas-validation-core/ WEBSITE.md mutants*/ -AGENT_TESTING_NOTES.md -AGENT_TESTING_NOTES_CLIENT.md -AGENT_TESTING_NOTES_WEB.md +testing/ run_bins/ result diff --git a/web/src/pages/add_logic/loader.rs b/web/src/pages/add_logic/loader.rs index 8e21a151..91e00f86 100644 --- a/web/src/pages/add_logic/loader.rs +++ b/web/src/pages/add_logic/loader.rs @@ -50,7 +50,7 @@ pub(crate) async fn load_page( }) .collect(); Ok(AddLogicPageState { - nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "admin", &authorization), tables, form, error, diff --git a/web/src/pages/add_table/loader.rs b/web/src/pages/add_table/loader.rs index a8b25889..641b4134 100644 --- a/web/src/pages/add_table/loader.rs +++ b/web/src/pages/add_table/loader.rs @@ -124,7 +124,7 @@ pub(crate) async fn load_page( draft.set_available_relation_table_options(table_options); Ok(AddTablePageState { - nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "admin", &authorization), profiles: tree .profiles .into_iter() diff --git a/web/src/pages/add_validation/loader.rs b/web/src/pages/add_validation/loader.rs index 1e75aabd..7e3ffaa5 100644 --- a/web/src/pages/add_validation/loader.rs +++ b/web/src/pages/add_validation/loader.rs @@ -43,7 +43,7 @@ pub(crate) async fn load_page( .flat_map(|profile| profile.tables.iter().map(|table| table.name.clone())) .collect(); Ok(ValidationPageState { - nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "admin", &authorization), profiles, tables, form, diff --git a/web/src/pages/admin/admin/loader.rs b/web/src/pages/admin/admin/loader.rs index 74cd35c7..a2245157 100644 --- a/web/src/pages/admin/admin/loader.rs +++ b/web/src/pages/admin/admin/loader.rs @@ -166,7 +166,7 @@ pub(crate) async fn load_admin_page( }; Ok(AdminPageState { - nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "admin", &authorization), profiles, selected_profile, tables, diff --git a/web/src/pages/admin/ecb/loader.rs b/web/src/pages/admin/ecb/loader.rs index 74e27b17..43ee1be9 100644 --- a/web/src/pages/admin/ecb/loader.rs +++ b/web/src/pages/admin/ecb/loader.rs @@ -61,7 +61,7 @@ pub(crate) async fn load_ecb_page( .into_inner(); Ok(EcbPageState { - nav: crate::ui::Nav::new(headers, "ecb").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "ecb", &authorization), verified_through_date: status.verified_through_date, latest_verifiable_date: status.latest_verifiable_date, healthy: status.healthy, diff --git a/web/src/pages/admin/table_definition/loader.rs b/web/src/pages/admin/table_definition/loader.rs index 1797b88c..542cabb3 100644 --- a/web/src/pages/admin/table_definition/loader.rs +++ b/web/src/pages/admin/table_definition/loader.rs @@ -274,7 +274,7 @@ pub(crate) async fn load_page( inputs.columns.table_name = inputs.selection.table.clone(); Ok(TableDefinitionPageState { - nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), + nav: crate::ui::Nav::from_authorization(headers, "admin", &authorization), tables, link_targets, detail, diff --git a/web/src/pages/analytics/loader.rs b/web/src/pages/analytics/loader.rs index 8b6cbde9..695d7b74 100644 --- a/web/src/pages/analytics/loader.rs +++ b/web/src/pages/analytics/loader.rs @@ -9,7 +9,6 @@ use crate::{ AnalyticsTable, ExecuteAnalyticsQueryRequest, GetAnalyticsCatalogRequest, GetAnalyticsCatalogResponse, analytics_value, }, - auth::GetAuthorizationRequest, definitions::common::Empty, services::authenticated_request, }; @@ -48,21 +47,6 @@ pub(crate) async fn load_profiles( .collect()) } -pub(crate) async fn load_navigation( - state: AppState, - headers: &HeaderMap, -) -> crate::ui::Nav { - let Ok(request) = authenticated_request(headers, GetAuthorizationRequest {}) else { - return crate::ui::Nav::new(headers, "analytics"); - }; - let mut auth = state.auth; - match auth.get_authorization(request).await { - Ok(response) => crate::ui::Nav::new(headers, "analytics") - .with_authorization(response.get_ref()), - Err(_) => crate::ui::Nav::new(headers, "analytics"), - } -} - pub(crate) async fn load_catalog( state: AppState, headers: &HeaderMap, diff --git a/web/src/pages/analytics/logic.rs b/web/src/pages/analytics/logic.rs index 1d31dd29..24984508 100644 --- a/web/src/pages/analytics/logic.rs +++ b/web/src/pages/analytics/logic.rs @@ -22,7 +22,9 @@ pub(crate) async fn analytics_page( State(state): State, headers: HeaderMap, ) -> Html { - Html(ui::render_page(loader::load_navigation(state, &headers).await)) + Html(ui::render_page( + crate::ui::Nav::for_request(state, &headers, "analytics").await, + )) } pub(crate) async fn load_profiles( @@ -31,15 +33,15 @@ pub(crate) async fn load_profiles( ) -> Html { match loader::load_profiles(state, &headers).await { Ok(profiles) => Html(ui::render_profile_options( - crate::ui::Nav::new(&headers, ""), + crate::ui::Nav::without_authorization(&headers, ""), &profiles, )), Err(LoadError::Unauthenticated) => Html(ui::render_profile_options_error( - crate::ui::Nav::new(&headers, ""), + crate::ui::Nav::without_authorization(&headers, ""), &tr!(Locale::from_headers(&headers), "analytics-sign-in"), )), Err(LoadError::Backend(message)) => Html(ui::render_profile_options_error( - crate::ui::Nav::new(&headers, ""), + crate::ui::Nav::without_authorization(&headers, ""), &message, )), } @@ -64,7 +66,7 @@ pub(crate) async fn load_catalog( } match loader::load_catalog(state, &headers, input.profile_name).await { Ok(catalog) => Html(ui::render_catalog( - crate::ui::Nav::new(&headers, ""), + crate::ui::Nav::without_authorization(&headers, ""), &catalog, )) .into_response(), diff --git a/web/src/pages/import_export/export/loader.rs b/web/src/pages/import_export/export/loader.rs index b237fed3..1852840d 100644 --- a/web/src/pages/import_export/export/loader.rs +++ b/web/src/pages/import_export/export/loader.rs @@ -10,7 +10,7 @@ pub(crate) async fn load_page( ) -> Result { let catalog = load_catalog(state, headers, "read").await?; Ok(ExportPageState { - nav: crate::ui::Nav::new(headers, "").with_authorization(&catalog.authorization), + nav: crate::ui::Nav::from_authorization(headers, "", &catalog.authorization), catalog, }) } diff --git a/web/src/pages/import_export/import/loader.rs b/web/src/pages/import_export/import/loader.rs index 1d8fef4d..cc17184b 100644 --- a/web/src/pages/import_export/import/loader.rs +++ b/web/src/pages/import_export/import/loader.rs @@ -15,7 +15,7 @@ pub(crate) async fn load_page( ) -> Result { let catalog = load_catalog(state, headers, "insert").await?; Ok(ImportPageState { - nav: crate::ui::Nav::new(headers, "").with_authorization(&catalog.authorization), + nav: crate::ui::Nav::from_authorization(headers, "", &catalog.authorization), catalog, form, error, diff --git a/web/src/pages/login/logic.rs b/web/src/pages/login/logic.rs index 93124abe..622c3b6d 100644 --- a/web/src/pages/login/logic.rs +++ b/web/src/pages/login/logic.rs @@ -19,14 +19,20 @@ use super::{state::{ChangePasswordInput, LoginInput}, ui}; pub(crate) async fn login_page( headers: HeaderMap, ) -> Html { - Html(ui::render_page(Nav::new(&headers, "login"))) + Html(ui::render_page(Nav::without_authorization(&headers, "login"))) } -pub(crate) async fn password_page(headers: HeaderMap) -> Response { +pub(crate) async fn password_page( + State(state): State, + headers: HeaderMap, +) -> Response { if authenticated_request(&headers, ()).is_err() { return axum::response::Redirect::to("/login").into_response(); } - Html(ui::render_password_page(Nav::new(&headers, ""))).into_response() + Html(ui::render_password_page( + Nav::for_request(state, &headers, "").await, + )) + .into_response() } pub(crate) async fn change_password( diff --git a/web/src/pages/login/ui.rs b/web/src/pages/login/ui.rs index a0867373..c9ff3361 100644 --- a/web/src/pages/login/ui.rs +++ b/web/src/pages/login/ui.rs @@ -48,3 +48,33 @@ pub(crate) fn render_password_success(locale: Locale) -> String { &tr!(locale, "password-changed-message"), )) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::{AuthorizationSnapshot, Permission}; + + /// The password page is not itself a nav entry, which is exactly why it + /// once rendered with an unresolved `Nav` and lost every capability link + /// in the navbar. It carries the same navbar as any other page. + #[test] + fn the_password_page_carries_the_navbar_the_signed_in_user_has_earned() { + let authorization = AuthorizationSnapshot { + role: "admin".to_string(), + permissions: vec![Permission { + object: crate::authz::STRUCT_TABLE.to_string(), + action: crate::authz::MANAGE.to_string(), + }], + }; + let nav = Nav::from_authorization( + &axum::http::HeaderMap::new(), + "", + &authorization, + ); + + let rendered = render_password_page(nav); + + assert!(rendered.contains("href=\"/admin\""), "{rendered}"); + assert!(!rendered.contains("href=\"/login\""), "{rendered}"); + } +} diff --git a/web/src/pages/permissions/common/loader.rs b/web/src/pages/permissions/common/loader.rs index 6e398b7f..e4b08d69 100644 --- a/web/src/pages/permissions/common/loader.rs +++ b/web/src/pages/permissions/common/loader.rs @@ -52,7 +52,7 @@ pub(crate) async fn access( /// The navbar every permissions page carries. `"permissions"` is a nav entry of /// its own, so the section is reachable without going through the admin panel. pub(crate) fn nav(headers: &HeaderMap, access: &Access) -> crate::ui::Nav { - crate::ui::Nav::new(headers, "permissions").with_authorization(&access.authorization) + crate::ui::Nav::from_authorization(headers, "permissions", &access.authorization) } pub(crate) async fn roles( diff --git a/web/src/pages/register/logic.rs b/web/src/pages/register/logic.rs index ecb8a35a..59b8fc52 100644 --- a/web/src/pages/register/logic.rs +++ b/web/src/pages/register/logic.rs @@ -13,7 +13,7 @@ use crate::{ use super::{state::RegisterInput, ui}; pub(crate) async fn register_page(headers: HeaderMap) -> Html { - Html(ui::render_page(Nav::new(&headers, "register"))) + Html(ui::render_page(Nav::without_authorization(&headers, "register"))) } pub(crate) async fn register( diff --git a/web/src/ui/mod.rs b/web/src/ui/mod.rs index 20773817..e66c5ee1 100644 --- a/web/src/ui/mod.rs +++ b/web/src/ui/mod.rs @@ -25,10 +25,68 @@ pub(crate) struct Nav { } impl Nav { + /// The navbar for a page, resolved in full: this fetches the caller's + /// authorization itself, so a handler cannot forget to and end up + /// rendering a navbar with every capability link missing. + /// /// `active` is the nav link to highlight: `"admin"`, `"permissions"`, /// `"analytics"`, `"login"`, or `""` for pages that are not themselves nav /// entries. - pub(crate) fn new(headers: &HeaderMap, active: &'static str) -> Self { + pub(crate) async fn for_request( + state: crate::AppState, + headers: &HeaderMap, + active: &'static str, + ) -> Self { + let Ok(request) = + crate::services::authenticated_request(headers, crate::auth::GetAuthorizationRequest {}) + else { + return Self::without_authorization(headers, active); + }; + let mut auth = state.auth; + match auth.get_authorization(request).await { + Ok(response) => Self::from_authorization(headers, active, response.get_ref()), + Err(_) => Self::without_authorization(headers, active), + } + } + + /// [`Self::for_request`] for a handler that already holds the snapshot, + /// because gating the page needed it — the same navbar without a second + /// round-trip. + pub(crate) fn from_authorization( + headers: &HeaderMap, + active: &'static str, + authorization: &crate::auth::AuthorizationSnapshot, + ) -> Self { + let mut nav = Self::without_authorization(headers, active); + // The backend answered for this caller, so they are signed in — a + // stronger fact than the cookie's mere presence, which is all + // `without_authorization` has to go on. + nav.authenticated = true; + nav.role = authorization.role.clone(); + nav.can_admin = crate::authz::can_open_admin(authorization); + // Permissions is its own nav section, so it is not gated on the admin + // panel: managing either roles or users is enough to open it. + nav.can_permissions = crate::authz::can_manage(authorization, crate::authz::STRUCT_ROLE) + || crate::authz::can_manage(authorization, crate::authz::STRUCT_USER); + nav.can_import = authorization.permissions.iter().any(|permission| { + permission.action == "insert" && permission.object.starts_with("data:") + }); + nav.can_export = authorization.permissions.iter().any(|permission| { + permission.action == "read" && permission.object.starts_with("data:") + }); + nav.can_ecb = crate::authz::can_read_ecb(authorization); + nav + } + + /// A navbar with no capabilities at all — every `can_*` link hidden. + /// + /// This is only correct for markup that has no navbar to get wrong: the + /// login and register pages, the HTMX fragments that carry a `Nav` purely + /// to translate their labels, and the error pages rendered when the + /// authorization could not be loaded in the first place. A page rendering + /// `ui/navbar.html` for a signed-in user wants [`Self::for_request`]; + /// reaching for this one there is what leaves the navbar half empty. + pub(crate) fn without_authorization(headers: &HeaderMap, active: &'static str) -> Self { Self { locale: crate::i18n::Locale::from_headers(headers), authenticated: crate::cookie_value(headers, SESSION_COOKIE).is_some(), @@ -91,25 +149,6 @@ impl Nav { self.locale.lookup_args(key, &map) } - pub(crate) fn with_authorization( - mut self, - authorization: &crate::auth::AuthorizationSnapshot, - ) -> Self { - self.role = authorization.role.clone(); - self.can_admin = crate::authz::can_open_admin(authorization); - // Permissions is its own nav section, so it is not gated on the admin - // panel: managing either roles or users is enough to open it. - self.can_permissions = crate::authz::can_manage(authorization, crate::authz::STRUCT_ROLE) - || crate::authz::can_manage(authorization, crate::authz::STRUCT_USER); - self.can_import = authorization.permissions.iter().any(|permission| { - permission.action == "insert" && permission.object.starts_with("data:") - }); - self.can_export = authorization.permissions.iter().any(|permission| { - permission.action == "read" && permission.object.starts_with("data:") - }); - self.can_ecb = crate::authz::can_read_ecb(authorization); - self - } } /// The escaping Askama's `escape` filter would have applied, for the values