feat(middleware): add smart public route matching with extension detection
This commit is contained in:
@@ -0,0 +1,76 @@
|
|||||||
|
# Plan: Mejora de Public Routes en JWT Middleware
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
|
||||||
|
Modificar la lógica de verificación de rutas públicas en el JWT middleware para que:
|
||||||
|
- Si el path tiene extensión de archivo → hacer match exacto del archivo
|
||||||
|
- Si el path NO tiene extensión → hacer match por prefijo (permitir todo bajo esa ruta)
|
||||||
|
|
||||||
|
## Implementation Plan
|
||||||
|
|
||||||
|
### Análisis del código actual
|
||||||
|
|
||||||
|
El código en `src/middleware/jwt.rs:56` hace:
|
||||||
|
```rust
|
||||||
|
let is_public_path = self.public_routes.contains(&req.uri().path().to_string());
|
||||||
|
```
|
||||||
|
|
||||||
|
Esto hace un match exacto, lo cual es limitante.
|
||||||
|
|
||||||
|
### Modificaciones requeridas
|
||||||
|
### Modificaciones requeridas:
|
||||||
|
|
||||||
|
- [x] Modificar la función `is_public_path` para detectar si la ruta tiene extensión de archivo
|
||||||
|
- [x] Si tiene extensión → usar match exacto (comportamiento actual)
|
||||||
|
- [x] Si NO tiene extensión → usar match por prefijo (permitir `/static/*` automáticamente)
|
||||||
|
- [x] Agregar helper function para detectar extensiones de archivo comunes
|
||||||
|
### Lógica de verificación propuesta
|
||||||
|
|
||||||
|
```
|
||||||
|
Para cada public_route en public_routes:
|
||||||
|
1. Obtener el path de la request
|
||||||
|
2. Si public_route tiene extensión de archivo:
|
||||||
|
- Comparar exactamente (path == public_route)
|
||||||
|
3. Si public_route NO tiene extensión:
|
||||||
|
- Comparar si path EMPIEZA con public_route + "/"
|
||||||
|
```
|
||||||
|
|
||||||
|
### Ejemplos de comportamiento
|
||||||
|
|
||||||
|
| public_route | request_path | resultado |
|
||||||
|
|--------------|--------------|-----------|
|
||||||
|
| `/static/logo.png` | `/static/logo.png` | ✓ público |
|
||||||
|
| `/static/logo.png` | `/static/other.png` | ✗ requiere auth |
|
||||||
|
| `/static` | `/static/file.js` | ✓ público |
|
||||||
|
| `/static` | `/static/css/style.css` | ✓ público |
|
||||||
|
| `/static` | `/static` | ✓ público |
|
||||||
|
| `/api` | `/api/users` | ✓ público |
|
||||||
|
|
||||||
|
### Extensiones válidas a considerar
|
||||||
|
|
||||||
|
Extensions comunes: `.html`, `.js`, `.css`, `.json`, `.png`, `.jpg`, `.jpeg`, `.gif`, `.svg`, `.ico`, `.woff`, `.woff2`, `.ttf`, `.eot`, `.txt`, `.xml`, `.csv`, `.webp`
|
||||||
|
|
||||||
|
## Verification Criteria
|
||||||
|
|
||||||
|
- [x] Requests a archivos exactos (con extensión) requieren match completo
|
||||||
|
- [x] Requests a directorios/rutas (sin extensión) permiten todos los subpaths
|
||||||
|
- [x] El código mantiene backward compatibility con configs existentes
|
||||||
|
- [x] La lógica es eficiente (no itera innecesariamente)
|
||||||
|
- [x] Tests unitarios verifican todos los casos de uso
|
||||||
|
|
||||||
|
## Potential Risks
|
||||||
|
|
||||||
|
1. **Breaking change**: Si alguien configuró `public_routes: ["/static/file.js"]` esperando que también permita otros archivos, ahora solo permitirá ese archivo específico
|
||||||
|
- Mitigation: Documentar el cambio y notificar a los usuarios
|
||||||
|
|
||||||
|
## Alternative Approaches
|
||||||
|
|
||||||
|
1. **Usar glob patterns**: Aceptar patrones como `/static/**` explícitamente
|
||||||
|
- Más flexible pero más complejo de implementar
|
||||||
|
- Requiere cambiar el formato de configuración
|
||||||
|
|
||||||
|
2. **Usar regex**: Permitir expresiones regulares en las rutas públicas
|
||||||
|
- Muy flexible pero potencial security risk si no se sanitiza bien
|
||||||
|
|
||||||
|
3. **Mantener ambos modos**: Agregar un flag para elegir entre modo exacto o prefijo
|
||||||
|
- Más complejo pero backwards compatible
|
||||||
+178
-1
@@ -7,6 +7,17 @@ use hyper::body::Incoming;
|
|||||||
use jsonwebtoken::{Algorithm, DecodingKey, Validation, decode};
|
use jsonwebtoken::{Algorithm, DecodingKey, Validation, decode};
|
||||||
use log::error;
|
use log::error;
|
||||||
|
|
||||||
|
/// Common file extensions that indicate a file path
|
||||||
|
const FILE_EXTENSIONS: &[&str] = &[
|
||||||
|
".html", ".htm", ".js", ".mjs", ".css", ".scss", ".sass", ".less",
|
||||||
|
".json", ".xml", ".yaml", ".yml", ".toml", ".env",
|
||||||
|
".png", ".jpg", ".jpeg", ".gif", ".svg", ".ico", ".webp", ".avif", ".bmp",
|
||||||
|
".woff", ".woff2", ".ttf", ".eot", ".otf", ".css",
|
||||||
|
".pdf", ".txt", ".md", ".csv", ".xlsx", ".docx", ".zip", ".tar", ".gz",
|
||||||
|
".mp4", ".webm", ".mp3", ".wav", ".ogg", ".flac",
|
||||||
|
".wasm", ".br",
|
||||||
|
];
|
||||||
|
|
||||||
pub struct JwtMiddleware {
|
pub struct JwtMiddleware {
|
||||||
decoding_key: DecodingKey,
|
decoding_key: DecodingKey,
|
||||||
public_routes: Vec<String>,
|
public_routes: Vec<String>,
|
||||||
@@ -25,6 +36,46 @@ impl JwtMiddleware {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Determines if the given path has a file extension.
|
||||||
|
/// Returns true if the last segment of the path contains a dot followed by a known extension.
|
||||||
|
pub fn has_file_extension(path: &str) -> bool {
|
||||||
|
// Get the last segment of the path (after the last '/')
|
||||||
|
if let Some(segment) = path.rsplit('/').next() {
|
||||||
|
// Check if it contains a dot and has a known extension
|
||||||
|
if segment.contains('.') {
|
||||||
|
let lower = segment.to_lowercase();
|
||||||
|
return FILE_EXTENSIONS.iter().any(|ext| lower.ends_with(ext));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Checks if a request path is a public route.
|
||||||
|
/// - For routes WITH a file extension: exact match required
|
||||||
|
/// - For routes WITHOUT a file extension: prefix match (allows all subpaths)
|
||||||
|
/// - Special case: "/" as public route allows everything
|
||||||
|
pub fn is_public_route(public_routes: &[String], request_path: &str) -> bool {
|
||||||
|
// Special case: "/" allows everything
|
||||||
|
if public_routes.iter().any(|r| r == "/") {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
public_routes.iter().any(|route| {
|
||||||
|
// Skip empty routes
|
||||||
|
if route.is_empty() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if Self::has_file_extension(route) {
|
||||||
|
// Exact match for file paths
|
||||||
|
request_path == route
|
||||||
|
} else {
|
||||||
|
// Prefix match for directory paths (allows /route and /route/*)
|
||||||
|
request_path == route || request_path.starts_with(&format!("{}/", route))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
fn validate_request(
|
fn validate_request(
|
||||||
&self,
|
&self,
|
||||||
req: &Request<Incoming>,
|
req: &Request<Incoming>,
|
||||||
@@ -53,7 +104,8 @@ impl JwtMiddleware {
|
|||||||
|
|
||||||
impl Middleware for JwtMiddleware {
|
impl Middleware for JwtMiddleware {
|
||||||
fn run(&self, mut req: Request<Incoming>) -> MiddlewareFuture<'_> {
|
fn run(&self, mut req: Request<Incoming>) -> MiddlewareFuture<'_> {
|
||||||
let is_public_path = self.public_routes.contains(&req.uri().path().to_string());
|
let request_path = req.uri().path().to_string();
|
||||||
|
let is_public_path = Self::is_public_route(&self.public_routes, &request_path);
|
||||||
|
|
||||||
Box::pin(async move {
|
Box::pin(async move {
|
||||||
match self.validate_request(&req) {
|
match self.validate_request(&req) {
|
||||||
@@ -74,3 +126,128 @@ impl Middleware for JwtMiddleware {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_has_file_extension_with_valid_extensions() {
|
||||||
|
let paths = vec![
|
||||||
|
"/static/logo.png",
|
||||||
|
"/images/banner.jpg",
|
||||||
|
"/assets/app.js",
|
||||||
|
"/styles/main.css",
|
||||||
|
"/data/config.json",
|
||||||
|
"/docs/README.MD",
|
||||||
|
];
|
||||||
|
|
||||||
|
for path in paths {
|
||||||
|
assert!(
|
||||||
|
JwtMiddleware::has_file_extension(path),
|
||||||
|
"Expected {} to have a file extension",
|
||||||
|
path
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_has_file_extension_without_extensions() {
|
||||||
|
let paths = vec![
|
||||||
|
"/api",
|
||||||
|
"/api/users",
|
||||||
|
"/static",
|
||||||
|
"/static/css",
|
||||||
|
"/admin/settings",
|
||||||
|
"/v1",
|
||||||
|
];
|
||||||
|
|
||||||
|
for path in paths {
|
||||||
|
assert!(
|
||||||
|
!JwtMiddleware::has_file_extension(path),
|
||||||
|
"Expected {} to NOT have a file extension",
|
||||||
|
path
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_has_file_extension_unusual_extensions() {
|
||||||
|
// Paths with dots but not known extensions should return false
|
||||||
|
assert!(!JwtMiddleware::has_file_extension("/api/v1.0/users"));
|
||||||
|
assert!(!JwtMiddleware::has_file_extension("/files/.hidden/test"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_public_route_exact_match_for_files() {
|
||||||
|
let public_routes = vec!["/static/logo.png".to_string()];
|
||||||
|
|
||||||
|
// Exact match should work
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/logo.png"));
|
||||||
|
|
||||||
|
// Different file in same directory should NOT be public
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&public_routes, "/static/other.png"));
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&public_routes, "/static/image.jpg"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_public_route_prefix_match_for_directories() {
|
||||||
|
let public_routes = vec!["/static".to_string()];
|
||||||
|
|
||||||
|
// Directory itself should be public
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static"));
|
||||||
|
|
||||||
|
// Any file under the directory should be public
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/app.js"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/css/main.css"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/images/logo.png"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/deep/nested/path/file.txt"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_public_route_multiple_routes() {
|
||||||
|
let public_routes = vec![
|
||||||
|
"/static".to_string(),
|
||||||
|
"/public/file.css".to_string(),
|
||||||
|
"/api/health".to_string(),
|
||||||
|
];
|
||||||
|
|
||||||
|
// Exact file match
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/public/file.css"));
|
||||||
|
|
||||||
|
// Directory prefix match
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/static/app.js"));
|
||||||
|
|
||||||
|
// API endpoint
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/api/health"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/api/health/detailed"));
|
||||||
|
|
||||||
|
// Non-public paths
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&public_routes, "/api/users"));
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&public_routes, "/admin"));
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&public_routes, "/private/data"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_public_route_case_insensitive_extensions() {
|
||||||
|
let public_routes = vec!["/assets/LOGO.PNG".to_string()];
|
||||||
|
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/assets/LOGO.PNG"));
|
||||||
|
// Note: exact match is case-sensitive for the path, only extension check is case-insensitive
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_public_route_edge_cases() {
|
||||||
|
// Root path "/" as public route allows everything (including all subpaths)
|
||||||
|
let public_routes = vec!["/".to_string()];
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/any/path"));
|
||||||
|
assert!(JwtMiddleware::is_public_route(&public_routes, "/deep/nested/route"));
|
||||||
|
|
||||||
|
// Empty route should not match anything
|
||||||
|
let empty_routes = vec!["".to_string()];
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&empty_routes, "/any/path"));
|
||||||
|
assert!(!JwtMiddleware::is_public_route(&empty_routes, "/"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user