Repository navigation
feat(tsa): CORS configurable pour l'API HTTP, activé pour demo.open-eidas.eu - #7
Merged
Merged
Conversation
…idas.eu Ajoute OPENEIDAS_CORS_ALLOWED_ORIGIN (oe-config, oe-httpapi) : quand définie, une couche CORS restreint Access-Control-Allow-Origin à cette seule origine sur le routeur HTTP de tsa-server. Vide/absent par défaut, sans effet sur les déploiements existants. Expose tsa.corsAllowedOrigin dans le chart Helm et l'active dans values-staging.yaml pour https://demo.open-eidas.eu (la démo web à venir, hébergée sur GitHub Pages), seule origine autorisée à appeler l'API de staging depuis un navigateur.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Validate configured origins and add negative-origin and POST preflight coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds configurable single-origin CORS support to the TSA HTTP API, enabled for the staging demo deployment.
Changes:
- Adds
OPENEIDAS_CORS_ALLOWED_ORIGINconfiguration. - Applies CORS middleware to the HTTP router.
- Updates Helm values and adds integration coverage.
File summaries
| File | Summary |
|---|---|
deploy/helm/open-eidas/values.yaml |
Defines the default CORS setting. |
deploy/helm/open-eidas/values-staging.yaml |
Enables the staging demo origin. |
deploy/helm/open-eidas/templates/tsa/deployment.yaml |
Injects the CORS environment variable. |
crates/oe-httpapi/tests/end_to_end.rs |
Tests configured CORS behavior. |
crates/oe-httpapi/src/lib.rs |
Configures the CORS middleware. |
crates/oe-httpapi/Cargo.toml |
Enables the CORS feature. |
crates/oe-crosstsa/tests/against_local_server.rs |
Updates test router options. |
crates/oe-config/src/lib.rs |
Loads and tests the new setting. |
bin/tsa-server/src/main.rs |
Passes configuration to the router. |
Review details
Suppressed comments (3)
crates/oe-config/src/lib.rs:179
- This new user-facing environment variable is missing from the TSA configuration table in
docs/API.md(which currently documents the otherOPENEIDAS_*settings and their defaults). Please document its empty/absent default and the exact-origin behavior so non-Helm operators can discover and configure it.
cors_allowed_origin: env::var("OPENEIDAS_CORS_ALLOWED_ORIGIN")
.ok()
.filter(|s| !s.is_empty()),
crates/oe-httpapi/src/lib.rs:71
- The web demo's JSON
POST /api/v1/timestampwill trigger a CORS preflight, but the new test only exercises a simpleGET. Add anOPTIONSrequest withAccess-Control-Request-Method: POSTandAccess-Control-Request-Headers: content-typeso regressions in theseallow_methods/allow_headerssettings cannot leave the browser integration broken while the test still passes.
.allow_methods([axum::http::Method::GET, axum::http::Method::POST])
.allow_headers([axum::http::header::CONTENT_TYPE]),
crates/oe-httpapi/tests/end_to_end.rs:209
- The integration test only performs GETs from the allowed origin. It does not verify that a different origin is rejected or exercise the browser preflight required for POST requests with a non-safelisted
Content-Type; a regression in the single-origin check orallow_methods/allow_headerscould therefore pass while the demo cannot call the API. Add negative-origin and OPTIONS preflight assertions.
let resp = reqwest::Client::new()
.get(format!("http://{addr}/api/v1/policy"))
.header("Origin", "https://demo.open-eidas.eu")
.send()
.await
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+65
to
+69
| if let Some(origin) = opts.cors_allowed_origin.as_deref() { | ||
| if let Ok(origin) = axum::http::HeaderValue::from_str(origin) { | ||
| router = router.layer( | ||
| tower_http::cors::CorsLayer::new() | ||
| .allow_origin(origin) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Résumé
OPENEIDAS_CORS_ALLOWED_ORIGIN(oe-config,oe-httpapi) : restreintAccess-Control-Allow-Originà une origine unique sur le routeur HTTP detsa-server. Absent/vide par défaut (pas de CORS), sans impact sur les déploiements existants.tsa.corsAllowedOrigindans le chart Helm, activé dansvalues-staging.yamlpourhttps://demo.open-eidas.eu(démo web à venir, GitHub Pages, dans le repo séparéopen-eidas/demo).Plan de test
cargo test -p oe-config -p oe-httpapi(nouveaux tests : lecture de la variable, en-tête CORS présent seulement si configuré et reflétant exactement l'origine)cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --check