Skip to content

Commit 9f22532

Browse files
committed
validate oidc cookies
- Updated `get_sqlpage_auth_cookie` to return a result for better error handling and validation of the SQLPage auth cookie. - Improved logging throughout the OIDC service for better traceability of requests and responses. - Adjusted the handling of OIDC callback parameters to include context in error messages.
1 parent e305b89 commit 9f22532

File tree

1 file changed

+43
-14
lines changed

1 file changed

+43
-14
lines changed

src/webserver/oidc.rs

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ use actix_web::{
1010
use anyhow::{anyhow, Context};
1111
use awc::Client;
1212
use openidconnect::{
13-
core::{CoreAuthDisplay, CoreAuthenticationFlow},
13+
core::{CoreAuthDisplay, CoreAuthenticationFlow, CoreGenderClaim, CoreIdToken},
1414
AsyncHttpClient, CsrfToken, EmptyAdditionalClaims, EndpointMaybeSet, EndpointNotSet,
15-
EndpointSet, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope, TokenResponse,
15+
EndpointSet, IdToken, IdTokenClaims, IssuerUrl, Nonce, OAuth2TokenResponse, RedirectUrl, Scope,
16+
TokenResponse,
1617
};
1718
use serde::Deserialize;
1819

@@ -182,12 +183,13 @@ impl<S> OidcService<S> {
182183
&self,
183184
request: ServiceRequest,
184185
) -> LocalBoxFuture<Result<ServiceResponse<BoxBody>, Error>> {
185-
// Check if this is the OIDC callback URL
186+
log::debug!("Handling unauthenticated request to {}", request.path());
186187
if request.path() == SQLPAGE_REDIRECT_URI {
188+
log::debug!("The request is the OIDC callback");
187189
return self.handle_oidc_callback(request);
188190
}
189191

190-
// If not the callback URL, redirect to auth as before
192+
log::debug!("Redirecting to OIDC provider");
191193
let auth_url = self.build_auth_url(&request);
192194
Box::pin(async move { Ok(request.into_response(build_redirect_response(auth_url))) })
193195
}
@@ -228,14 +230,19 @@ where
228230

229231
fn call(&self, request: ServiceRequest) -> Self::Future {
230232
log::debug!("Started OIDC middleware with config: {:?}", self.config);
231-
match get_sqlpage_auth_cookie(&request) {
232-
Some(cookie) => {
233+
let oidc_client = Arc::clone(&self.oidc_client);
234+
match get_sqlpage_auth_cookie(&oidc_client, &request) {
235+
Ok(Some(cookie)) => {
233236
log::trace!("Found SQLPage auth cookie: {cookie}");
234237
}
235-
None => {
238+
Ok(None) => {
236239
log::trace!("No SQLPage auth cookie found");
237240
return self.handle_unauthenticated_request(request);
238241
}
242+
Err(e) => {
243+
log::error!("Found an invalid SQLPage auth cookie: {e}");
244+
return self.handle_unauthenticated_request(request);
245+
}
239246
}
240247
let future = self.service.call(request);
241248
Box::pin(async move {
@@ -250,8 +257,13 @@ async fn process_oidc_callback(
250257
http_client: &Arc<AwcHttpClient>,
251258
query_string: &str,
252259
) -> anyhow::Result<HttpResponse> {
253-
let params = Query::<OidcCallbackParams>::from_query(query_string)?.into_inner();
260+
let params = Query::<OidcCallbackParams>::from_query(query_string)
261+
.with_context(|| format!("{SQLPAGE_REDIRECT_URI}: failed to parse OIDC callback parameters from {query_string}"))?
262+
.into_inner();
263+
log::debug!("Processing OIDC callback with params: {params:?}. Requesting token...");
254264
let token_response = exchange_code_for_token(oidc_client, http_client, params).await?;
265+
log::debug!("Received token response: {token_response:?}");
266+
// TODO: redirect to the original URL instead of /
255267
let mut response = build_redirect_response(format!("/"));
256268
set_auth_cookie(&mut response, &token_response)?;
257269
Ok(response)
@@ -277,12 +289,14 @@ fn set_auth_cookie(
277289
token_response: &openidconnect::core::CoreTokenResponse,
278290
) -> anyhow::Result<()> {
279291
let access_token = token_response.access_token();
280-
log::debug!("Received access token: {}", access_token.secret());
292+
log::trace!("Received access token: {}", access_token.secret());
281293
let id_token = token_response
282294
.id_token()
283295
.context("No ID token found in the token response. You may have specified an oauth2 provider that does not support OIDC.")?;
284296

285-
let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token.to_string())
297+
let id_token_str = id_token.to_string();
298+
log::trace!("Setting auth cookie: {SQLPAGE_AUTH_COOKIE_NAME}=\"{id_token_str}\"");
299+
let cookie = actix_web::cookie::Cookie::build(SQLPAGE_AUTH_COOKIE_NAME, id_token_str)
286300
.secure(true)
287301
.http_only(true)
288302
.same_site(actix_web::cookie::SameSite::Lax)
@@ -299,10 +313,25 @@ fn build_redirect_response(target_url: String) -> HttpResponse {
299313
.body("Redirecting...")
300314
}
301315

302-
fn get_sqlpage_auth_cookie(request: &ServiceRequest) -> Option<String> {
303-
let cookie = request.cookie(SQLPAGE_AUTH_COOKIE_NAME)?;
304-
log::error!("TODO: actually check the validity of the cookie");
305-
Some(cookie.value().to_string())
316+
fn get_sqlpage_auth_cookie(
317+
oidc_client: &OidcClient,
318+
request: &ServiceRequest,
319+
) -> anyhow::Result<Option<String>> {
320+
let Some(cookie) = request.cookie(SQLPAGE_AUTH_COOKIE_NAME) else {
321+
return Ok(None);
322+
};
323+
let cookie_value = cookie.value().to_string();
324+
325+
let verifier = oidc_client.id_token_verifier();
326+
let id_token = CoreIdToken::from_str(&cookie_value)
327+
.with_context(|| anyhow!("Invalid SQLPage auth cookie"))?;
328+
329+
let nonce_verifier = |_: Option<&Nonce>| Ok(());
330+
let claims: &IdTokenClaims<EmptyAdditionalClaims, CoreGenderClaim> = id_token
331+
.claims(&verifier, nonce_verifier)
332+
.with_context(|| anyhow!("Invalid SQLPage auth cookie"))?;
333+
log::debug!("The current user is: {claims:?}");
334+
Ok(Some(cookie_value))
306335
}
307336

308337
pub struct AwcHttpClient {

0 commit comments

Comments
 (0)