Skip to content

Commit 24e8069

Browse files
committed
Decode configured OIDC prefixes too, not just the request path
A site_prefix containing a percent-encoded character (e.g. a space -> %20) is stored encoded in protected_paths/public_paths. Decoding only the request path made the decoded path fail to match the still-encoded prefix, so protected pages (including the default ["/"]) became public. Decode both sides. Adds regression tests.
1 parent efc2c96 commit 24e8069

1 file changed

Lines changed: 58 additions & 8 deletions

File tree

src/webserver/oidc.rs

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,28 @@ impl TryFrom<&AppConfig> for OidcConfig {
140140
impl OidcConfig {
141141
#[must_use]
142142
pub fn is_public_path(&self, path: &str) -> bool {
143-
// Percent-decode the request path before matching, so that an
144-
// encoded protected prefix (e.g. "/%70rotected" == "/protected")
145-
// is classified the same way the router resolves it once decoded.
146-
// Otherwise an unauthenticated request could bypass the protected
147-
// path rules while still executing the protected SQL file.
148-
let decoded = percent_encoding::percent_decode_str(path).decode_utf8_lossy();
143+
// Percent-decode before matching, so an encoded protected prefix
144+
// (e.g. "/%70rotected" == "/protected") is classified the same way the
145+
// router resolves it once decoded. Both the request path AND the
146+
// configured prefixes are decoded: `site_prefix` may itself contain
147+
// percent-encoded characters (e.g. a space -> "%20"), so the stored
148+
// prefixes can be encoded too. Comparing a decoded path against an
149+
// encoded prefix would never match and would wrongly make every
150+
// protected page public. Decode both sides so the comparison matches
151+
// what routing actually serves.
152+
fn decode(s: &str) -> std::borrow::Cow<'_, str> {
153+
percent_encoding::percent_decode_str(s).decode_utf8_lossy()
154+
}
155+
let decoded = decode(path);
149156
let path = decoded.as_ref();
150-
!self.protected_paths.iter().any(|p| path.starts_with(p))
151-
|| self.public_paths.iter().any(|p| path.starts_with(p))
157+
!self
158+
.protected_paths
159+
.iter()
160+
.any(|p| path.starts_with(decode(p).as_ref()))
161+
|| self
162+
.public_paths
163+
.iter()
164+
.any(|p| path.starts_with(decode(p).as_ref()))
152165
}
153166

154167
/// Creates a custom ID token verifier that supports multiple issuers
@@ -1308,6 +1321,43 @@ mod tests {
13081321
);
13091322
}
13101323

1324+
#[test]
1325+
fn encoded_site_prefix_keeps_protected_paths_protected() {
1326+
// When site_prefix contains a percent-encoded character (e.g. a space),
1327+
// the configured prefixes are stored encoded, e.g. "/my%20app/protected".
1328+
// The request path is percent-decoded before matching, so the prefixes
1329+
// must be decoded the same way; otherwise nothing matches and protected
1330+
// pages become public.
1331+
let config = test_oidc_config_with_paths(vec!["/my%20app/protected".to_string()], vec![]);
1332+
1333+
assert!(
1334+
!config.is_public_path("/my%20app/protected/page.sql"),
1335+
"protected page under an encoded site_prefix must not be public"
1336+
);
1337+
// The encoded-byte bypass must still be blocked here too.
1338+
assert!(
1339+
!config.is_public_path("/my%20app/%70rotected/"),
1340+
"/my%20app/%70rotected/ decodes to the protected path"
1341+
);
1342+
// A non-protected page under the same site_prefix stays public.
1343+
assert!(
1344+
config.is_public_path("/my%20app/login.sql"),
1345+
"non-protected page should remain public"
1346+
);
1347+
}
1348+
1349+
#[test]
1350+
fn encoded_site_prefix_default_protected_root_protects_everything() {
1351+
// Default oidc_protected_paths ["/"] under an encoded site_prefix becomes
1352+
// ["/my%20app/"]. Every page under the prefix must stay protected.
1353+
let config = test_oidc_config_with_paths(vec!["/my%20app/".to_string()], vec![]);
1354+
1355+
assert!(
1356+
!config.is_public_path("/my%20app/anything.sql"),
1357+
"with a protected root, every page under the site_prefix must be protected"
1358+
);
1359+
}
1360+
13111361
#[test]
13121362
fn evicts_excess_tmp_login_flow_state_cookies() {
13131363
let request = (0..MAX_OIDC_PARALLEL_LOGIN_FLOWS)

0 commit comments

Comments
 (0)