Skip to content

Commit 24191c5

Browse files
committed
fix missing page body on authentication error
remove duplicated code add test for http basic auth
1 parent a789e74 commit 24191c5

File tree

7 files changed

+140
-60
lines changed

7 files changed

+140
-60
lines changed

src/app_config.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,18 +124,32 @@ fn cannonicalize_if_possible(path: &std::path::Path) -> PathBuf {
124124
/// This should be called only once at the start of the program.
125125
pub fn load() -> anyhow::Result<AppConfig> {
126126
let configuration_directory = &configuration_directory();
127-
log::debug!(
128-
"Loading configuration from {:?}",
129-
cannonicalize_if_possible(configuration_directory)
130-
);
131-
let config_file = configuration_directory.join("sqlpage");
132-
Config::builder()
127+
load_from_directory(configuration_directory)
128+
}
129+
130+
/// Parses and loads the configuration from the given directory.
131+
pub fn load_from_directory(directory: &Path) -> anyhow::Result<AppConfig> {
132+
let cannonical = cannonicalize_if_possible(directory);
133+
log::debug!("Loading configuration from {:?}", cannonical);
134+
let config_file = directory.join("sqlpage");
135+
let mut app_config = load_from_file(&config_file)?;
136+
app_config.configuration_directory = directory.into();
137+
Ok(app_config)
138+
}
139+
140+
/// Parses and loads the configuration from the given file.
141+
pub fn load_from_file(config_file: &Path) -> anyhow::Result<AppConfig> {
142+
let config = Config::builder()
133143
.add_source(config::File::from(config_file).required(false))
134144
.add_source(env_config())
135145
.add_source(env_config().prefix("SQLPAGE"))
136-
.build()?
146+
.build()?;
147+
log::trace!("Configuration sources: {}", config.cache);
148+
let app_config = config
137149
.try_deserialize::<AppConfig>()
138-
.with_context(|| "Unable to load configuration")
150+
.with_context(|| "Unable to load configuration")?;
151+
log::debug!("Loaded configuration: {:?}", app_config);
152+
Ok(app_config)
139153
}
140154

141155
fn env_config() -> config::Environment {

src/filesystem.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ impl FileSystem {
7676
priviledged: bool,
7777
) -> anyhow::Result<Vec<u8>> {
7878
let local_path = self.safe_local_path(app_state, path, priviledged)?;
79+
log::debug!("Reading file {path:?} from {local_path:?}");
7980
let local_result = tokio::fs::read(&local_path).await;
8081
match (local_result, &self.db_fs_queries) {
8182
(Ok(f), _) => Ok(f),

src/main.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ async fn main() {
1616

1717
async fn start() -> anyhow::Result<()> {
1818
let app_config = app_config::load()?;
19-
log::debug!("Starting with the following configuration: {app_config:#?}");
2019
let state = AppState::init(&app_config).await?;
2120
webserver::database::migrations::apply(&app_config, &state.db).await?;
2221
log::debug!("Starting server...");

src/render.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use crate::templates::SplitTemplate;
22
use crate::webserver::http::LayoutContext;
3+
use crate::webserver::ErrorWithStatus;
34
use crate::AppState;
45
use actix_web::cookie::time::format_description::well_known::Rfc3339;
56
use actix_web::cookie::time::OffsetDateTime;
67
use actix_web::http::{header, StatusCode};
7-
use actix_web::{HttpResponse, HttpResponseBuilder};
8+
use actix_web::{HttpResponse, HttpResponseBuilder, ResponseError};
89
use anyhow::{bail, format_err, Context as AnyhowContext};
910
use async_recursion::async_recursion;
1011
use handlebars::{BlockContext, Context, JsonValue, RenderError, Renderable};
@@ -202,18 +203,21 @@ impl<'a, W: std::io::Write> HeaderContext<'a, W> {
202203
}
203204
log::debug!("Authentication failed");
204205
// The authentication failed
205-
if let Some(link) = get_object_str(&data, "link") {
206-
self.response.status(StatusCode::FOUND);
207-
self.response.insert_header((header::LOCATION, link));
208-
self.has_status = true;
209-
} else {
210-
self.response.status(StatusCode::UNAUTHORIZED);
206+
let http_response: HttpResponse = if let Some(link) = get_object_str(&data, "link") {
211207
self.response
212-
.insert_header((header::WWW_AUTHENTICATE, "Basic realm=\"Auth required\""));
213-
self.has_status = true;
214-
}
215-
// Set an empty response body
216-
let http_response = self.response.body(());
208+
.status(StatusCode::FOUND)
209+
.insert_header((header::LOCATION, link))
210+
.body(
211+
"Sorry, but you are not authorized to access this page. \
212+
Redirecting to the login page...",
213+
)
214+
} else {
215+
ErrorWithStatus {
216+
status: StatusCode::UNAUTHORIZED,
217+
}
218+
.error_response()
219+
};
220+
self.has_status = true;
217221
Ok(PageContext::Close(http_response))
218222
}
219223

src/webserver/error_with_status.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
use actix_web::{
2-
http::{header::ContentType, StatusCode},
2+
http::{
3+
header::{self, ContentType},
4+
StatusCode,
5+
},
36
ResponseError,
47
};
58

@@ -19,8 +22,18 @@ impl ResponseError for ErrorWithStatus {
1922
self.status
2023
}
2124
fn error_response(&self) -> actix_web::HttpResponse {
22-
actix_web::HttpResponse::build(self.status)
23-
.content_type(ContentType::plaintext())
24-
.body(self.status.to_string())
25+
let mut resp_builder = actix_web::HttpResponse::build(self.status);
26+
resp_builder.content_type(ContentType::plaintext());
27+
if self.status == StatusCode::UNAUTHORIZED {
28+
resp_builder.insert_header((
29+
header::WWW_AUTHENTICATE,
30+
header::HeaderValue::from_static(
31+
"Basic realm=\"Authentication required\", charset=\"UTF-8\"",
32+
),
33+
));
34+
resp_builder.body("Sorry, but you are not authorized to access this page.")
35+
} else {
36+
resp_builder.body(self.status.to_string())
37+
}
2538
}
2639
}

src/webserver/http.rs

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ use actix_web::{
1212
dev::ServiceResponse, middleware, middleware::Logger, web, web::Bytes, App, HttpResponse,
1313
HttpServer,
1414
};
15+
use actix_web::{HttpResponseBuilder, ResponseError};
1516

1617
use super::https::make_auto_rustls_config;
1718
use super::static_content;
18-
use actix_web::body::{BoxBody, MessageBody};
19+
use actix_web::body::MessageBody;
1920
use anyhow::{bail, Context};
2021
use chrono::{DateTime, Utc};
2122
use futures_util::stream::Stream;
@@ -268,42 +269,30 @@ fn send_anyhow_error(
268269
env: app_config::DevOrProd,
269270
) {
270271
log::error!("An error occurred before starting to send the response body: {e:#}");
271-
let mut resp = HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR);
272+
let mut resp = HttpResponseBuilder::new(StatusCode::INTERNAL_SERVER_ERROR);
272273
let mut body = "Sorry, but we were not able to process your request. \n\n".to_owned();
273274
if env.is_prod() {
274275
body.push_str("Contact the administrator for more information. A detailed error message has been logged.");
275276
} else {
276277
use std::fmt::Write;
277278
write!(body, "{e:?}").unwrap();
278279
}
279-
resp = resp.set_body(BoxBody::new(body));
280-
resp.headers_mut().insert(
280+
resp.insert_header((
281281
header::CONTENT_TYPE,
282282
header::HeaderValue::from_static("text/plain"),
283-
);
284-
if let Some(&ErrorWithStatus { status }) = e.downcast_ref() {
285-
*resp.status_mut() = status;
286-
if status == StatusCode::UNAUTHORIZED {
287-
resp.headers_mut().insert(
288-
header::WWW_AUTHENTICATE,
289-
header::HeaderValue::from_static(
290-
"Basic realm=\"Authentication required\", charset=\"UTF-8\"",
291-
),
292-
);
293-
resp = resp.set_body(BoxBody::new(
294-
"Sorry, but you are not authorized to access this page.",
295-
));
296-
}
297-
};
298-
if let Some(sqlx::Error::PoolTimedOut) = e.downcast_ref() {
283+
));
284+
let resp = if let Some(e @ &ErrorWithStatus { .. }) = e.downcast_ref() {
285+
e.error_response()
286+
} else if let Some(sqlx::Error::PoolTimedOut) = e.downcast_ref() {
299287
// People are HTTP connections faster than we can open SQL connections. Ask them to slow down politely.
300288
use rand::Rng;
301-
*resp.status_mut() = StatusCode::SERVICE_UNAVAILABLE;
302-
resp.headers_mut().insert(
289+
resp.status(StatusCode::SERVICE_UNAVAILABLE).insert_header((
303290
header::RETRY_AFTER,
304291
header::HeaderValue::from(rand::thread_rng().gen_range(1..=15)),
305-
);
306-
}
292+
)).body("The database is currently too busy to handle your request. Please try again later.\n\n".to_owned() + &body)
293+
} else {
294+
resp.body(body)
295+
};
307296
resp_send
308297
.send(resp)
309298
.unwrap_or_else(|_| log::error!("could not send headers"));

tests/index.rs

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::HashMap;
1+
use std::{collections::HashMap, path::PathBuf};
22

33
use actix_web::{
44
body::MessageBody,
@@ -7,7 +7,11 @@ use actix_web::{
77
test::{self, TestRequest},
88
HttpResponse,
99
};
10-
use sqlpage::{app_config::AppConfig, webserver::http::main_handler, AppState};
10+
use sqlpage::{
11+
app_config::AppConfig,
12+
webserver::{self, http::main_handler},
13+
AppState,
14+
};
1115

1216
#[actix_web::test]
1317
async fn test_index_ok() {
@@ -408,20 +412,66 @@ async fn test_with_site_prefix() {
408412
);
409413
}
410414

415+
async fn make_app_data_for_official_website() -> actix_web::web::Data<AppState> {
416+
init_log();
417+
let config_path = std::path::Path::new("examples/official-site/sqlpage");
418+
let mut app_config = sqlpage::app_config::load_from_directory(config_path).unwrap();
419+
app_config.web_root = PathBuf::from("examples/official-site");
420+
let app_state = make_app_data_from_config(app_config.clone()).await;
421+
webserver::database::migrations::apply(&app_config, &app_state.db)
422+
.await
423+
.unwrap();
424+
app_state
425+
}
426+
427+
#[actix_web::test]
428+
async fn test_official_website_documentation() {
429+
let app_data = make_app_data_for_official_website().await;
430+
let resp = req_path_with_app_data("/documentation.sql?component=button", app_data)
431+
.await
432+
.unwrap();
433+
assert_eq!(resp.status(), http::StatusCode::OK);
434+
let body = test::read_body(resp).await;
435+
let body_str = String::from_utf8(body.to_vec()).unwrap();
436+
assert!(
437+
body_str.contains(r#"<button type="submit" form="poem" formaction="?action"#),
438+
"{body_str}\nexpected to contain a button with formaction"
439+
);
440+
}
441+
442+
#[actix_web::test]
443+
async fn test_official_website_basic_auth_example() {
444+
let resp = req_path_with_app_data(
445+
"/examples/authentication/basic_auth.sql",
446+
make_app_data_for_official_website().await,
447+
)
448+
.await
449+
.unwrap();
450+
assert_eq!(resp.status(), http::StatusCode::UNAUTHORIZED);
451+
let body = test::read_body(resp).await;
452+
let body_str = String::from_utf8(body.to_vec()).unwrap();
453+
assert!(
454+
body_str.contains("not authorized"),
455+
"{body_str}\nexpected to contain Unauthorized"
456+
);
457+
}
458+
411459
async fn get_request_to(path: &str) -> actix_web::Result<TestRequest> {
412460
let data = make_app_data().await;
413461
Ok(test::TestRequest::get()
414462
.uri(path)
415463
.insert_header(ContentType::plaintext())
416464
.app_data(data))
417465
}
466+
async fn make_app_data_from_config(config: AppConfig) -> actix_web::web::Data<AppState> {
467+
let state = AppState::init(&config).await.unwrap();
468+
actix_web::web::Data::new(state)
469+
}
418470

419471
async fn make_app_data() -> actix_web::web::Data<AppState> {
420472
init_log();
421473
let config = test_config();
422-
let state = AppState::init(&config).await.unwrap();
423-
424-
actix_web::web::Data::new(state)
474+
make_app_data_from_config(config).await
425475
}
426476

427477
async fn req_path(
@@ -431,16 +481,23 @@ async fn req_path(
431481
main_handler(req).await
432482
}
433483

434-
async fn req_path_with_app_data(
484+
async fn srv_req_path_with_app_data(
435485
path: impl AsRef<str>,
436486
app_data: actix_web::web::Data<AppState>,
437-
) -> Result<actix_web::dev::ServiceResponse, actix_web::Error> {
438-
let req = test::TestRequest::get()
487+
) -> actix_web::dev::ServiceRequest {
488+
test::TestRequest::get()
439489
.uri(path.as_ref())
490+
.app_data(app_data)
440491
.insert_header(("cookie", "test_cook=123"))
441492
.insert_header(("authorization", "Basic dGVzdDp0ZXN0")) // test:test
442-
.app_data(app_data)
443-
.to_srv_request();
493+
.to_srv_request()
494+
}
495+
496+
async fn req_path_with_app_data(
497+
path: impl AsRef<str>,
498+
app_data: actix_web::web::Data<AppState>,
499+
) -> Result<actix_web::dev::ServiceResponse, actix_web::Error> {
500+
let req = srv_req_path_with_app_data(path, app_data).await;
444501
main_handler(req).await
445502
}
446503

@@ -461,5 +518,8 @@ pub fn test_config() -> AppConfig {
461518
}
462519

463520
fn init_log() {
464-
let _ = env_logger::builder().is_test(true).try_init();
521+
let _ = env_logger::builder()
522+
.is_test(true)
523+
.filter(Some("sqlpage"), log::LevelFilter::Trace)
524+
.try_init();
465525
}

0 commit comments

Comments
 (0)