Merge pull request #877 from portier/fix/metrics

Fix bridge request counters
portier · May 3, 2024 · 26b682a · 26b682a
2 parents 94f5ea9 + 492e5d4
commit 26b682a
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 16 deletions.
diff --git a/src/bridges/email.rs b/src/bridges/email.rs
@@ -27,10 +27,6 @@ pub struct EmailBridgeData {
 /// A form is rendered as an alternative way to confirm, without following the link. Submitting the
 /// form results in the same callback as the email link.
 pub async fn auth(ctx: &mut Context, email_addr: EmailAddress) -> HandlerResult {
-    if !ctx.app.uncounted_emails.contains(&email_addr) {
-        metrics::AUTH_EMAIL_REQUESTS.inc();
-    }
-
     // Generate a 12-character one-time pad.
     let code = random_zbase32(12, &ctx.app.rng).await;
     // For display, we split it in two groups of 6.
@@ -81,6 +77,11 @@ pub async fn auth(ctx: &mut Context, email_addr: EmailAddress) -> HandlerResult
         ));
     }
 
+    // Increment the counter only after the session was claimed.
+    if !ctx.app.uncounted_emails.contains(&email_addr) {
+        metrics::AUTH_EMAIL_REQUESTS.inc();
+    }
+
     // Send the mail.
     let ok = ctx
         .app

diff --git a/src/bridges/oidc.rs b/src/bridges/oidc.rs
@@ -98,11 +98,10 @@ pub async fn auth(
     let provider_origin = validation::parse_oidc_href(&link.href).ok_or_else(|| {
         BrokerError::Provider(format!("invalid href (validation failed): {}", link.href))
     })?;
-    let mut bridge_data = match link.rel {
+    let metric;
+    let mut bridge_data;
+    match link.rel {
         Relation::Portier => {
-            if is_counted {
-                metrics::AUTH_OIDC_REQUESTS_PORTIER.inc();
-            }
             #[cfg(not(feature = "insecure"))]
             {
                 if link.href.scheme() != "https" {
@@ -112,19 +111,17 @@ pub async fn auth(
                     )));
                 }
             }
-            OidcBridgeData {
+            metric = &metrics::AUTH_OIDC_REQUESTS_PORTIER;
+            bridge_data = OidcBridgeData {
                 link: link.clone(),
                 origin: provider_origin,
                 client_id: ctx.app.public_url.clone(),
                 nonce: provider_nonce,
                 signing_alg: SigningAlgorithm::Rs256,
-            }
+            };
         }
         // Delegate to the OpenID Connect bridge for Google, if configured.
         Relation::Google => {
-            if is_counted {
-                metrics::AUTH_OIDC_REQUESTS_GOOGLE.inc();
-            }
             let client_id = ctx
                 .app
                 .google_client_id
@@ -135,15 +132,16 @@ pub async fn auth(
                     "invalid href: Google provider only supports {GOOGLE_IDP_ORIGIN}"
                 )));
             }
-            OidcBridgeData {
+            metric = &metrics::AUTH_OIDC_REQUESTS_GOOGLE;
+            bridge_data = OidcBridgeData {
                 link: link.clone(),
                 origin: provider_origin,
                 client_id: client_id.clone(),
                 nonce: provider_nonce,
                 signing_alg: SigningAlgorithm::Rs256,
-            }
+            };
         }
-    };
+    }
 
     // Retrieve the provider's configuration.
     let (
@@ -216,6 +214,11 @@ pub async fn auth(
         return Err(BrokerError::ProviderCancelled);
     }
 
+    // Increment the counter only after the session was claimed.
+    if is_counted {
+        metric.inc();
+    }
+
     if ctx.want_json {
         Ok(json_response(&json!({
             "result": "redirect_to_provider",