Skip to content

Centralize canonical route authorization for OIDC #1479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lovasoa merged 2 commits into from
Sep 24, 2026
+421 −142
Merged

Centralize canonical route authorization for OIDC #1479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35fe27e
refactor(oidc) :: centralize route authorization
lovasoa Sep 24, 2026
68a1edd
fix(oidc) :: keep top-level builtin assets public
lovasoa Sep 24, 2026
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
31 changes: 15 additions & 16 deletions src/webserver/http.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::filesystem::FileAccess;
use crate::webserver::routing::RoutingAction::{
CustomNotFound, Execute, NotFound, Redirect, Serve,
};
use crate::webserver::routing::{AppFileStore, calculate_route};
use crate::webserver::routing::{AppFileStore, CanonicalRequestPath, ResolvedRoute, resolve_route};
use actix_web::body::MessageBody;
use anyhow::{Context, bail};
use chrono::{DateTime, Utc};
Expand Down Expand Up @@ -489,21 +489,20 @@ pub async fn main_handler(
.uri()
.path_and_query()
.ok_or_else(|| ErrorBadRequest("expected valid path with query from request"))?;
let authorized_route = service_request
.extensions_mut()
.remove::<super::routing::RoutingAction>();
let routing_action = match authorized_route {
Some(action) => Ok(action),
None => calculate_route(path_and_query, &store, &app_state.config).await,
};
let routing_action = match routing_action {
Ok(action) => action,
Err(e) => {
let e = e.context(format!(
"The server was unable to fulfill your request. \n\
The following page is not accessible: {path_and_query:?}"
));
return Err(anyhow_err_to_actix(e, app_state));
let authorized_route = { service_request.extensions_mut().remove::<ResolvedRoute>() };
let routing_action = if let Some(route) = authorized_route {
route.into_action()
} else {
let path = CanonicalRequestPath::parse(path_and_query, &app_state.config.site_prefix);
match resolve_route(&path, path_and_query, &store, &app_state.config).await {
Ok(route) => route.into_action(),
Err(e) => {
let e = e.context(format!(
"The server was unable to fulfill your request. \n\
The following page is not accessible: {path_and_query:?}"
));
return Err(anyhow_err_to_actix(e, app_state));
}
}
};
match routing_action {
Expand Down
185 changes: 96 additions & 89 deletions src/webserver/oidc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use std::{future::Future, pin::Pin, str::FromStr, sync::Arc};
use tokio::time::Instant;

use crate::webserver::http_client::get_http_client_from_appdata;
use crate::webserver::routing::{AppFileStore, RoutingAction, calculate_route};
use crate::webserver::routing::{
AppFileStore, CanonicalRequestPath, RoutingAction, canonical_url_path, resolve_route,
};
use crate::{AppState, app_config::AppConfig};
use actix_web::http::header;
use actix_web::{
Expand Down Expand Up @@ -140,47 +142,8 @@ impl TryFrom<&AppConfig> for OidcConfig {
impl OidcConfig {
#[must_use]
pub fn is_public_path(&self, path: &str) -> bool {
let Some(path) = normalize_oidc_path(path) else {
return false;
};
self.is_public_normalized_path(&path)
}

fn is_public_normalized_path(&self, path: &str) -> bool {
// Filesystems can resolve differently cased paths to the same file.
// Protect all ASCII case aliases; a public override must still match
// the configured spelling exactly.
!self.protected_paths.iter().any(|p| {
normalize_oidc_path(p).is_none_or(|prefix| {
path.to_ascii_lowercase()
.starts_with(&prefix.to_ascii_lowercase())
})
}) || self
.public_paths
.iter()
.any(|p| normalize_oidc_path(p).is_some_and(|prefix| path.starts_with(&prefix)))
}

fn is_public_route(&self, action: &RoutingAction) -> bool {
let (RoutingAction::Execute(path) | RoutingAction::CustomNotFound(path)) = action else {
return true;
};
let Some(site_prefix) = normalize_oidc_path(&self.site_prefix) else {
return false;
};
let mut route = site_prefix.trim_end_matches('/').to_string();
for component in path.components() {
if let std::path::Component::Normal(name) = component {
let Some(name) = name.to_str() else {
return false;
};
route.push('/');
route.push_str(name);
} else {
return false;
}
}
self.is_public_normalized_path(&route)
let path = canonical_url_path(path);
OidcPathPolicy::new(self).is_public(path.as_deref())
}

/// Creates a custom ID token verifier that supports multiple issuers
Expand Down Expand Up @@ -223,35 +186,53 @@ impl OidcConfig {
}
}

/// Match the router's decoded filesystem path components before applying URL
/// prefix rules. Preserve a final slash: `/public` and `/public/` are different
/// routes. Invalid or ambiguous paths must never be classified as public.
fn normalize_oidc_path(path: &str) -> Option<String> {
use std::path::{Component, Path};
/// Prefixes are decoded once when the OIDC state starts. The protected
/// spelling is case-folded conservatively for case-insensitive file stores;
/// public exceptions still require their configured spelling.
struct OidcPathPolicy {
protected: Option<Vec<String>>,
public: Vec<String>,
}

let decoded = percent_encoding::percent_decode_str(path)
.decode_utf8()
.ok()?;
if !decoded.starts_with('/') || decoded.contains('\\') {
return None;
impl OidcPathPolicy {
fn new(config: &OidcConfig) -> Self {
Self {
protected: config
.protected_paths
.iter()
.map(|path| canonical_url_path(path).map(|path| path.to_ascii_lowercase()))
.collect(),
public: config
.public_paths
.iter()
.filter_map(|path| canonical_url_path(path))
.collect(),
}
}
let mut normalized = String::from("/");
for component in Path::new(decoded.as_ref()).components() {
match component {
Component::RootDir | Component::CurDir => {}
Component::Normal(name) => {
if normalized.len() > 1 {
normalized.push('/');
}
normalized.push_str(name.to_str()?);

fn is_public(&self, path: Option<&str>) -> bool {
let (Some(path), Some(protected)) = (path, self.protected.as_ref()) else {
return false;
};
let folded = path.to_ascii_lowercase();
!protected.iter().any(|prefix| folded.starts_with(prefix))
|| self.public.iter().any(|prefix| path.starts_with(prefix))
}

fn is_public_route(&self, action: &RoutingAction, site_prefix: &str) -> bool {
match action {
RoutingAction::Execute(_)
| RoutingAction::CustomNotFound(_)
| RoutingAction::Serve(_) => {
let resource = crate::webserver::routing::canonical_resource_url_for_action(
site_prefix,
action,
);
self.is_public(resource.as_deref())
}
Component::ParentDir | Component::Prefix(_) => return None,
RoutingAction::Redirect(_) | RoutingAction::NotFound => true,
}
}
if decoded.ends_with('/') && normalized != "/" {
normalized.push('/');
}
Some(normalized)
}

fn get_app_host(config: &AppConfig) -> String {
Expand Down Expand Up @@ -288,6 +269,7 @@ struct OidcSnapshot {

pub struct OidcState {
pub config: OidcConfig,
path_policy: OidcPathPolicy,
/// Current snapshot. The lock is only held for the instant
/// needed to clone/swap the Arc — never across await points.
snapshot: std::sync::RwLock<Arc<OidcSnapshot>>,
Expand All @@ -301,6 +283,7 @@ impl OidcState {
let (client, end_session_endpoint) = build_oidc_client(&oidc_cfg, &http_client).await?;

Ok(Self {
path_policy: OidcPathPolicy::new(&oidc_cfg),
config: oidc_cfg,
snapshot: std::sync::RwLock::new(Arc::new(OidcSnapshot {
client,
Expand Down Expand Up @@ -409,13 +392,6 @@ impl OidcState {
}
}

fn request_is_builtin_static_path(request: &ServiceRequest, config: &OidcConfig) -> bool {
request
.path()
.strip_prefix(&config.site_prefix)
.is_some_and(|path| path.starts_with("sqlpage/"))
}

pub async fn initialize_oidc_state(
app_config: &AppConfig,
) -> anyhow::Result<Option<Arc<OidcState>>> {
Expand Down Expand Up @@ -566,26 +542,42 @@ async fn handle_unauthenticated_request(
) -> MiddlewareResponse {
log::debug!("Handling unauthenticated request to {}", request.path());

if oidc_state.config.is_public_path(request.path()) {
let path_and_query = request.uri().path_and_query();
let canonical = path_and_query
.map(|path| CanonicalRequestPath::parse(path, &oidc_state.config.site_prefix));
// Built-in assets have fixed content and are served by their own Actix
// services, so they stay public even when their top-level URLs match a
// configured protected-path prefix.
if canonical
.as_ref()
.is_some_and(CanonicalRequestPath::is_builtin_static)
{
return MiddlewareResponse::Forward(request);
}
if canonical
.as_ref()
.is_some_and(|path| oidc_state.path_policy.is_public(path.normalized_url()))
{
let canonical = canonical.expect("checked above");
let route = {
let app_state = request.app_data::<web::Data<AppState>>();
let path_and_query = request.uri().path_and_query();
match (app_state, path_and_query) {
(Some(state), Some(path_and_query)) => {
let store = AppFileStore::new(&state.sql_file_cache, &state.file_system, state);
Some(calculate_route(path_and_query, &store, &state.config).await)
Some(resolve_route(&canonical, path_and_query, &store, &state.config).await)
}
_ => None,
}
};
match route {
Some(Ok(action)) if oidc_state.config.is_public_route(&action) => {
Some(Ok(route))
if oidc_state
.path_policy
.is_public_route(route.action(), &oidc_state.config.site_prefix) =>
{
// Reuse the authorized route in the main handler so a file-store
// change between this check and dispatch cannot change its identity.
request.extensions_mut().insert(action);
return MiddlewareResponse::Forward(request);
}
Some(Err(_)) if request_is_builtin_static_path(&request, &oidc_state.config) => {
request.extensions_mut().insert(route);
return MiddlewareResponse::Forward(request);
}
_ => {}
Expand Down Expand Up @@ -1691,10 +1683,22 @@ mod tests {
assert!(config.is_public_path("/users/profile.sql"));
}

#[test]
fn compiled_policy_checks_the_resolved_static_target_and_fails_closed() {
let config = test_oidc_config_with_paths(vec!["/private/secret.txt".to_string()], vec![]);
let policy = OidcPathPolicy::new(&config);
assert!(policy.is_public(Some("/private/alias.txt")));
let action = RoutingAction::Serve("private/secret.txt".into());
assert!(!policy.is_public_route(&action, &config.site_prefix));

let invalid = test_oidc_config_with_paths(vec!["/%ff".to_string()], vec![]);
assert!(!OidcPathPolicy::new(&invalid).is_public(Some("/public/page.sql")));
}

#[tokio::test]
async fn resolved_sql_file_aliases_do_not_bypass_oidc_protection() {
use crate::filesystem::FileAccess;
use crate::webserver::routing::FileStore;
use crate::webserver::routing::{FileStore, calculate_route};
use awc::http::uri::PathAndQuery;

struct OneFile(&'static str);
Expand All @@ -1713,6 +1717,7 @@ mod tests {
("/private/missing", "private/404.sql"),
] {
let oidc = test_oidc_config_with_paths(vec![format!("/{file}")], vec![]);
let policy = OidcPathPolicy::new(&oidc);
assert!(oidc.is_public_path(url), "{url} is a distinct URL spelling");
let route = calculate_route(
&PathAndQuery::try_from(url).unwrap(),
Expand All @@ -1728,25 +1733,27 @@ mod tests {
);
}
assert!(
!oidc.is_public_route(&route),
!policy.is_public_route(&route, &oidc.site_prefix),
"{url} resolves to a protected SQL file"
);
}

let mut prefixed_oidc =
test_oidc_config_with_paths(vec!["/my%20app/private/secret.sql".to_string()], vec![]);
prefixed_oidc.site_prefix = "/my%20app/".to_string();
assert!(
!prefixed_oidc.is_public_route(&RoutingAction::Execute("private/secret.sql".into()))
);
assert!(!OidcPathPolicy::new(&prefixed_oidc).is_public_route(
&RoutingAction::Execute("private/secret.sql".into()),
&prefixed_oidc.site_prefix
));

let public_oidc = test_oidc_config_with_paths(
vec!["/private".to_string()],
vec!["/private/public/".to_string()],
);
assert!(
public_oidc.is_public_route(&RoutingAction::Execute("private/public/index.sql".into()))
);
assert!(OidcPathPolicy::new(&public_oidc).is_public_route(
&RoutingAction::Execute("private/public/index.sql".into()),
&public_oidc.site_prefix
));
}

#[test]
Expand Down
Loading
Loading