Imrpove code structure

Author: marlonbaeten

auth-service/src/handlers/acs.rs

@@ -1,10 +1,17 @@
+// Helpers return Result<T, Response> so the orchestrator can fan out early-return
+// HTTP responses; the Err size is dominated by axum::Response (~128 bytes) which
+// is fine for non-hot-path handler control flow.
+#![allow(clippy::result_large_err)]
+
 use crate::{
     bindings::soap::{send_soap_request, wrap_soap},
+    config::AuthConfig,
     saml::{
-        messages::create_artifact_resolve,
+        idp_metadata::IdpMetadata,
+        messages::{CreatedMessage, create_artifact_resolve},
         validation::{
-            MINIMUM_LOA, ValidateAssertionOpts, validate_artifact_response, validate_assertion,
-            validate_response,
+            Claims, MINIMUM_LOA, ValidateAssertionOpts, validate_artifact_response,
+            validate_assertion, validate_response,
         },
     },
     state::{AuthServiceState, AuthState},
@@ -64,77 +71,141 @@ where
     );
 
     // 1. Create signed ArtifactResolve (eID §7.5)
+    let resolve = match build_artifact_resolve(artifact, cfg, rd, &dv_keys.signing[0]) {
+        Ok(r) => r,
+        Err(resp) => return resp,
+    };
+
+    // 2. Wrap in SOAP and send to ARS over mTLS (eID §9.4)
+    let soap_response = match send_artifact_resolve(&resolve.xml, rd, cfg).await {
+        Ok(r) => r,
+        Err(resp) => return resp,
+    };
+
+    // 3. Validate ArtifactResponse (eID §7.6.1) using RD signing certs from metadata
+    let response_xml = match resolve_artifact_response(&soap_response, rd, &resolve.id) {
+        Ok(xml) => xml,
+        Err(resp) => return resp,
+    };
+
+    // 4. Validate Response (eID §7.6.2) — handle cancellation / IdP errors
+    let assertion_xml = match resolve_response(&response_xml) {
+        Ok(xml) => xml,
+        Err(resp) => return resp,
+    };
+
+    // 5. Validate Assertion (eID §7.6.3, §7.6.3.5).
+    let claims = match resolve_assertion(&assertion_xml, &auth_state, cfg, rd) {
+        Ok(c) => c,
+        Err(resp) => return resp,
+    };
+
+    // SECURITY: never log decrypted SubjectID values — they are PII (BSN /
+    // pseudonym per eID §7.6.3.4). Log only non-PII metadata for tracing.
+    info!(
+        "[DV] Authentication successful. acting_subject_present={}, \
+         legal_subject_present={}, loa={:?}, authenticating_authority={:?}, \
+         service_uuid_present={}",
+        claims.acting_subject_id.is_some(),
+        claims.legal_subject_id.is_some(),
+        claims.authn_context_class_ref.as_deref(),
+        claims.authenticating_authority.as_deref(),
+        claims.service_uuid.is_some(),
+    );
+
+    // eID §9.7: consume the matched pending request ID to prevent replay.
+    if let Some(ref irt) = claims.in_response_to {
+        debug!("[DV][ACS] Consuming pending AuthnRequest id={irt}");
+        auth_state.consume_pending_request(irt.as_str());
+    }
+
+    // 6. Hand off to the embedding application to create its session.
+    debug!("[DV][ACS] Step 6: handing off to AuthState::on_authenticated");
+    state.on_authenticated(claims, jar, &headers).await
+}
+
+fn build_artifact_resolve(
+    artifact: &str,
+    cfg: &AuthConfig,
+    rd: &IdpMetadata,
+    signing_key: &crate::keys::KeyPair,
+) -> Result<CreatedMessage, Response> {
     debug!("[DV][ACS] Step 1: building signed ArtifactResolve");
-    let resolve = match create_artifact_resolve(
-        artifact,
-        &cfg.dv.entity_id,
-        &rd.ars_url,
-        &dv_keys.signing[0],
-    ) {
+    match create_artifact_resolve(artifact, &cfg.dv.entity_id, &rd.ars_url, signing_key) {
         Ok(m) => {
             debug!(
                 "[DV][ACS] ArtifactResolve built: id={}, xml_len={}",
                 m.id,
                 m.xml.len()
             );
-            m
+            Ok(m)
         }
         Err(e) => {
             error!("[DV] Failed to create ArtifactResolve: {e}");
-            return (
+            Err((
                 StatusCode::INTERNAL_SERVER_ERROR,
                 "ArtifactResolve build failed",
             )
-                .into_response();
+                .into_response())
         }
-    };
+    }
+}
 
-    // 2. Wrap in SOAP and send to ARS over mTLS (eID §9.4)
+async fn send_artifact_resolve(
+    resolve_xml: &str,
+    rd: &IdpMetadata,
+    cfg: &AuthConfig,
+) -> Result<String, Response> {
     debug!("[DV][ACS] Step 2: sending ArtifactResolve over mTLS SOAP back-channel");
-    let soap_xml = wrap_soap(&resolve.xml);
-    let soap_response = match send_soap_request(&rd.ars_url, &soap_xml, &cfg.tls).await {
+    let soap_xml = wrap_soap(resolve_xml);
+    match send_soap_request(&rd.ars_url, &soap_xml, &cfg.tls).await {
         Ok(r) => {
             debug!(
                 "[DV][ACS] SOAP back-channel returned response (len={})",
                 r.len()
             );
-            r
+            Ok(r)
         }
         Err(e) => {
             error!("[DV] SOAP back-channel failed: {e}");
-            return (StatusCode::BAD_GATEWAY, "ArtifactResolve failed").into_response();
+            Err((StatusCode::BAD_GATEWAY, "ArtifactResolve failed").into_response())
         }
-    };
+    }
+}
 
-    // 3. Validate ArtifactResponse (eID §7.6.1) using RD signing certs from metadata
+fn resolve_artifact_response(
+    soap_response: &str,
+    rd: &IdpMetadata,
+    expected_id: &str,
+) -> Result<String, Response> {
     debug!(
         "[DV][ACS] Step 3: validating ArtifactResponse against {} RD signing key(s), \
          expected InResponseTo={}",
         rd.signing_keys.len(),
-        resolve.id
+        expected_id
     );
-    let rd_signing = &rd.signing_keys;
-    let art_result = validate_artifact_response(&soap_response, rd_signing, &resolve.id);
+    let art_result = validate_artifact_response(soap_response, &rd.signing_keys, expected_id);
     if !art_result.valid {
         error!(
             "[DV] ArtifactResponse validation failed: {:?}",
             art_result.errors
         );
-        return (
+        return Err((
             StatusCode::BAD_REQUEST,
             format!(
                 "ArtifactResponse validation failed: {}",
                 art_result.errors.join("; ")
             ),
         )
-            .into_response();
+            .into_response());
     }
     debug!("[DV][ACS] Step 3: ArtifactResponse OK");
+    Ok(art_result.response_xml.unwrap())
+}
 
-    // 4. Validate Response (eID §7.6.2) — handle cancellation / IdP errors
+fn resolve_response(response_xml: &str) -> Result<String, Response> {
     debug!("[DV][ACS] Step 4: validating inner Response status");
-    let response_xml = art_result.response_xml.unwrap();
-    let resp_result = validate_response(&response_xml);
+    let resp_result = validate_response(response_xml);
 
     if !resp_result.valid {
         let errors_str = resp_result.errors.join("; ");
@@ -144,21 +215,27 @@ where
         // TVS "Checklist Testen" v2.1 T3: user cancelled — take them home.
         if is_authn_failed || is_cancelled {
             warn!("[DV] Authentication cancelled by user");
-            return Redirect::to("/").into_response();
+            return Err(Redirect::to("/").into_response());
         }
 
         // TVS "Checklist Testen" v2.1 L10: DigiD error — terminate the flow.
         warn!("[DV] Authentication failed: {errors_str}");
-        return (StatusCode::BAD_REQUEST, "Authentication failed").into_response();
+        return Err((StatusCode::BAD_REQUEST, "Authentication failed").into_response());
     }
     debug!("[DV][ACS] Step 4: Response status Success");
 
-    let Some(assertion_xml) = resp_result.assertion_xml else {
+    resp_result.assertion_xml.ok_or_else(|| {
         warn!("[DV][ACS] No Assertion element extracted from successful Response");
-        return (StatusCode::BAD_REQUEST, "No assertion in response").into_response();
-    };
+        (StatusCode::BAD_REQUEST, "No assertion in response").into_response()
+    })
+}
 
-    // 5. Validate Assertion (eID §7.6.3, §7.6.3.5).
+fn resolve_assertion(
+    assertion_xml: &str,
+    auth_state: &AuthServiceState,
+    cfg: &AuthConfig,
+    rd: &IdpMetadata,
+) -> Result<Claims, Response> {
     // Rule 4: snapshot of pending request IDs for InResponseTo verification.
     let pending_ids = auth_state.pending_request_ids();
     let pending_refs: Vec<&str> = pending_ids.iter().map(|s| s.as_str()).collect();
@@ -169,6 +246,7 @@ where
         pending_refs.len()
     );
 
+    let dv_keys = auth_state.dv_keys();
     let priv_keys: Vec<(&str, &str, &str)> = dv_keys
         .encryption
         .iter()
@@ -182,11 +260,11 @@ where
         .collect();
 
     let assert_result = validate_assertion(ValidateAssertionOpts {
-        assertion_xml: &assertion_xml,
+        assertion_xml,
         dv_entity_id: &cfg.dv.entity_id,
         expected_recipient: &cfg.dv.acs_url,
         pending_request_ids: &pending_refs,
-        trusted_keys: rd_signing,
+        trusted_keys: &rd.signing_keys,
         private_keys: &priv_keys,
         minimum_loa: Some(MINIMUM_LOA),
     });
@@ -196,37 +274,15 @@ where
             "[DV] Assertion validation failed: {:?}",
             assert_result.errors
         );
-        return (
+        return Err((
             StatusCode::BAD_REQUEST,
             format!(
                 "Assertion validation failed: {}",
                 assert_result.errors.join("; ")
             ),
         )
-            .into_response();
+            .into_response());
     }
 
-    let claims = assert_result.claims.unwrap();
-    // SECURITY: never log decrypted SubjectID values — they are PII (BSN /
-    // pseudonym per eID §7.6.3.4). Log only non-PII metadata for tracing.
-    info!(
-        "[DV] Authentication successful. acting_subject_present={}, \
-         legal_subject_present={}, loa={:?}, authenticating_authority={:?}, \
-         service_uuid_present={}",
-        claims.acting_subject_id.is_some(),
-        claims.legal_subject_id.is_some(),
-        claims.authn_context_class_ref.as_deref(),
-        claims.authenticating_authority.as_deref(),
-        claims.service_uuid.is_some(),
-    );
-
-    // eID §9.7: consume the matched pending request ID to prevent replay.
-    if let Some(ref irt) = claims.in_response_to {
-        debug!("[DV][ACS] Consuming pending AuthnRequest id={irt}");
-        auth_state.consume_pending_request(irt.as_str());
-    }
-
-    // 6. Hand off to the embedding application to create its session.
-    debug!("[DV][ACS] Step 6: handing off to AuthState::on_authenticated");
-    state.on_authenticated(claims, jar, &headers).await
+    Ok(assert_result.claims.unwrap())
 }