Skip to content

Commit 1c3b2c7

Browse files
committed
Fix OIDC SSO redirect loop after provider key rotation
This commit adds automatic OIDC provider metadata refresh when token validation fails. Previously, tokens would become invalid when providers rotated their signing keys, requiring a manual SQLPage restart to refresh the metadata.
1 parent 93812ea commit 1c3b2c7

File tree

2 files changed

+8
-9
lines changed

2 files changed

+8
-9
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
- The file-based routing system was improved. Now, requests to `/xxx` redirect to `/xxx/` only if `/xxx/index.sql` exists.
5151
- fix: When single sign on is enabled, and an anonymous user visits a page with URL parameters, the user is correctly redirected to the page with the parameters after login.
5252
- Added support for reading custom claims in JWT tokens generated by OIDC providers. This means that you can configure your Single-Sign-On provider to store custom pieces of information about your users, such as roles or permissions, and use them in your SQL queries in SQLPage.
53+
- Implement OIDC provider metadata refresh. This fixes a bug where leaving a SQLPage instance running with SSO enabled would cause infinite redirect loops on login after some time. Since most providers rotate their signing keys regularly and sqlpage only fetched the metadata once at startup, the only way to fix the issue was to restart SQLPage manually.
5354

5455
## v0.35.2
5556
- Fix a bug with zero values being displayed with a non-zero height in stacked bar charts.

src/webserver/oidc.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ impl OidcState {
178178
last_update: Instant::now(),
179179
}
180180
}
181-
Err(e) => log::error!("Failed to refresh OIDC client: {e}"),
181+
Err(e) => log::error!("Failed to refresh OIDC client: {e:#}"),
182182
}
183183
}
184184

@@ -375,12 +375,13 @@ async fn handle_unauthenticated_request(
375375
async fn handle_oidc_callback(
376376
oidc_state: &OidcState,
377377
request: ServiceRequest,
378-
) -> Result<ServiceResponse<BoxBody>, Error> {
378+
) -> actix_web::Result<ServiceResponse> {
379379
let query_string = request.query_string();
380380
match process_oidc_callback(oidc_state, query_string, &request).await {
381381
Ok(response) => Ok(request.into_response(response)),
382382
Err(e) => {
383-
log::error!("Failed to process OIDC callback with params {query_string}: {e}");
383+
log::error!("Failed to process OIDC callback with params {query_string}: {e:#}");
384+
oidc_state.refresh_on_error(&request).await;
384385
let resp = build_auth_provider_redirect_response(oidc_state, &request).await;
385386
Ok(request.into_response(resp))
386387
}
@@ -483,11 +484,8 @@ async fn set_auth_cookie(
483484
.id_token()
484485
.context("No ID token found in the token response. You may have specified an oauth2 provider that does not support OIDC.")?;
485486

486-
let claims = oidc_state
487-
.get_token_claims(id_token.clone(), None)
488-
.await
489-
.context("Invalid token returned by OIDC provider")?;
490-
let expiration = claims.expiration();
487+
let claims_res = oidc_state.get_token_claims(id_token.clone(), None).await;
488+
let expiration = claims_res.context("Parsing ID token claims")?.expiration();
491489
let max_age_seconds = expiration.signed_duration_since(Utc::now()).num_seconds();
492490

493491
let id_token_str = id_token.to_string();
@@ -847,7 +845,7 @@ impl AudienceVerifier {
847845

848846
/// Validate that a redirect URL is safe to use (prevents open redirect attacks)
849847
fn validate_redirect_url(url: String) -> String {
850-
if url.starts_with('/') && !url.starts_with("//") {
848+
if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(SQLPAGE_REDIRECT_URI) {
851849
return url;
852850
}
853851
log::warn!("Refusing to redirect to {url}");

0 commit comments

Comments
 (0)