Fix MFA codes (#881)

DefGuard · Dec 24, 2024 · ac793c7 · ac793c7
1 parent 5a62e1c
commit ac793c7
Show file tree

Hide file tree

Showing 17 changed files with 222 additions and 214 deletions.
diff --git a/.github/workflows/build-docker.yml b/.github/workflows/build-docker.yml
@@ -23,18 +23,17 @@ jobs:
       - ${{ matrix.runner }}
     strategy:
       matrix:
-        # cpu: [arm64, amd64, arm/v7]
-        cpu: [amd64]
+        cpu: [arm64, amd64, arm/v7]
         include:
-          # - cpu: arm64
-          #   runner: ARM64
-          #   tag: arm64
+          - cpu: arm64
+            runner: ARM64
+            tag: arm64
           - cpu: amd64
             runner: X64
             tag: amd64
-          # - cpu: arm/v7
-          #   runner: ARM
-          #   tag: armv7
+          - cpu: arm/v7
+            runner: ARM
+            tag: armv7
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -63,30 +62,30 @@ jobs:
           cache-from: type=gha
           cache-to: type=gha,mode=max
 
-  # docker-manifest:
-  #   runs-on: [self-hosted, Linux]
-  #   needs: [build-docker]
-  #   steps:
-  #     - name: Docker meta
-  #       id: meta
-  #       uses: docker/metadata-action@v5
-  #       with:
-  #         images: |
-  #           ${{ env.GHCR_REPO }}
-  #         flavor: ${{ inputs.flavor }}
-  #         tags: ${{ inputs.tags }}
-  #     - name: Login to GitHub container registry
-  #       uses: docker/login-action@v3
-  #       with:
-  #         registry: ghcr.io
-  #         username: ${{ github.actor }}
-  #         password: ${{ secrets.GITHUB_TOKEN }}
-  #     - name: Create and push manifests
-  #       run: |
-  #         tags='${{ env.GHCR_REPO }}:${{ github.sha }} ${{ steps.meta.outputs.tags }}'
-  #         for tag in ${tags}
-  #         do
-  #           docker manifest rm ${tag} || true
-  #           docker manifest create ${tag} ${{ env.GHCR_REPO }}:${{ github.sha }}-amd64 ${{ env.GHCR_REPO }}:${{ github.sha }}-arm64 ${{ env.GHCR_REPO }}:${{ github.sha }}-armv7
-  #           docker manifest push ${tag}
-  #         done
+  docker-manifest:
+    runs-on: [self-hosted, Linux]
+    needs: [build-docker]
+    steps:
+      - name: Docker meta
+        id: meta
+        uses: docker/metadata-action@v5
+        with:
+          images: |
+            ${{ env.GHCR_REPO }}
+          flavor: ${{ inputs.flavor }}
+          tags: ${{ inputs.tags }}
+      - name: Login to GitHub container registry
+        uses: docker/login-action@v3
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+      - name: Create and push manifests
+        run: |
+          tags='${{ env.GHCR_REPO }}:${{ github.sha }} ${{ steps.meta.outputs.tags }}'
+          for tag in ${tags}
+          do
+            docker manifest rm ${tag} || true
+            docker manifest create ${tag} ${{ env.GHCR_REPO }}:${{ github.sha }}-amd64 ${{ env.GHCR_REPO }}:${{ github.sha }}-arm64 ${{ env.GHCR_REPO }}:${{ github.sha }}-armv7
+            docker manifest push ${tag}
+          done
diff --git a/.github/workflows/current.yml b/.github/workflows/current.yml
@@ -20,7 +20,7 @@ jobs:
         type=ref,event=branch
         type=sha
 
-  # trigger-e2e:
-  #   needs: build-current
-  #   uses: ./.github/workflows/e2e.yml
-  #   secrets: inherit
+  trigger-e2e:
+    needs: build-current
+    uses: ./.github/workflows/e2e.yml
+    secrets: inherit
diff --git a/Cargo.toml b/Cargo.toml
@@ -135,5 +135,7 @@ worker = []
 wireguard = []
 
 [profile.release]
+codegen-units = 1
+panic = "abort"
 lto = "thin"
 strip = "symbols"
diff --git a/proto b/proto
diff --git a/src/db/models/user.rs b/src/db/models/user.rs
@@ -137,15 +137,12 @@ impl<I> User<I> {
     }
 
     pub(crate) fn verify_password(&self, password: &str) -> Result<(), HashError> {
-        match &self.password_hash {
-            Some(hash) => {
-                let parsed_hash = PasswordHash::new(hash)?;
-                Argon2::default().verify_password(password.as_bytes(), &parsed_hash)
-            }
-            None => {
-                error!("Password not set for user {}", self.username);
-                Err(HashError::Password)
-            }
+        if let Some(hash) = &self.password_hash {
+            let parsed_hash = PasswordHash::new(hash)?;
+            Argon2::default().verify_password(password.as_bytes(), &parsed_hash)
+        } else {
+            error!("Password not set for user {}", self.username);
+            Err(HashError::Password)
         }
     }
 
@@ -570,15 +567,36 @@ impl User<Id> {
                 if code == expected_code {
                     return true;
                 }
+                debug!(
+                    "Email MFA verification TOTP code for user {} doesn't fit current time
+                    frame, checking the previous one.
+                    Expected: {expected_code}, got: {code}",
+                    self.username
+                );
 
                 let previous_code = totp_custom::<Sha1>(
                     timeout,
                     EMAIL_CODE_DIGITS,
                     email_mfa_secret,
                     timestamp.as_secs() - timeout,
                 );
-                return code == previous_code;
+
+                if code == previous_code {
+                    return true;
+                }
+                debug!(
+                    "Email MFA verification TOTP code for user {} doesn't fit previous time frame,
+                    expected: {previous_code}, got: {code}",
+                    self.username
+                );
+                return false;
             }
+            debug!(
+                "Couldn't calculate current timestamp when verifying email MFA code for user {}",
+                self.username
+            );
+        } else {
+            debug!("Email MFA secret not configured for user {}", self.username);
         }
         false
     }

diff --git a/src/enterprise/directory_sync/google.rs b/src/enterprise/directory_sync/google.rs
@@ -3,7 +3,7 @@ use std::{str::FromStr, time::Duration};
 use super::{DirectoryGroup, DirectorySync, DirectorySyncError, DirectoryUser};
 use chrono::Utc;
 use jsonwebtoken::{encode, Algorithm, EncodingKey, Header};
-use reqwest::Url;
+use reqwest::{header::AUTHORIZATION, Url};
 
 const SCOPES: &str = "openid email profile https://www.googleapis.com/auth/admin.directory.customer.readonly https://www.googleapis.com/auth/admin.directory.group.readonly https://www.googleapis.com/auth/admin.directory.user.readonly";
 const ACCESS_TOKEN_URL: &str = "https://oauth2.googleapis.com/token";
@@ -24,6 +24,7 @@ struct Claims {
 }
 
 impl Claims {
+    #[must_use]
     fn new(iss: &str, sub: &str) -> Self {
         let now = chrono::Utc::now();
         let now_timestamp = now.timestamp();
@@ -45,7 +46,6 @@ pub struct ServiceAccountConfig {
     client_email: String,
 }
 
-#[derive(Debug)]
 pub struct GoogleDirectorySync {
     service_account_config: ServiceAccountConfig,
     access_token: Option<String>,
@@ -122,6 +122,7 @@ where
 }
 
 impl GoogleDirectorySync {
+    #[must_use]
     pub fn new(private_key: &str, client_email: &str, admin_email: &str) -> Self {
         Self {
             service_account_config: ServiceAccountConfig {
@@ -144,10 +145,8 @@ impl GoogleDirectorySync {
 
     pub fn is_token_expired(&self) -> bool {
         debug!("Checking if Google directory sync token is expired");
-        self.token_expiry
-            .map(|expiry| expiry < Utc::now())
-            // No token = expired token
-            .unwrap_or(true)
+        // No token = expired token
+        self.token_expiry.map_or(true, |expiry| expiry < Utc::now())
     }
 
     #[cfg(not(test))]
@@ -169,10 +168,7 @@ impl GoogleDirectorySync {
         let client = reqwest::Client::new();
         let response = client
             .get(url)
-            .header(
-                reqwest::header::AUTHORIZATION,
-                format!("Bearer {}", access_token),
-            )
+            .header(AUTHORIZATION, format!("Bearer {access_token}"))
             .timeout(REQUEST_TIMEOUT)
             .send()
             .await?;
@@ -198,7 +194,7 @@ impl GoogleDirectorySync {
         let client = reqwest::Client::builder().build()?;
         let response = client
             .get(url)
-            .header("Authorization", format!("Bearer {}", access_token))
+            .header(AUTHORIZATION, format!("Bearer {access_token}"))
             .timeout(REQUEST_TIMEOUT)
             .send()
             .await?;
@@ -229,7 +225,7 @@ impl GoogleDirectorySync {
         let client = reqwest::Client::builder().build()?;
         let response = client
             .get(url)
-            .header("Authorization", format!("Bearer {}", access_token))
+            .header(AUTHORIZATION, format!("Bearer {access_token}"))
             .timeout(REQUEST_TIMEOUT)
             .send()
             .await?;
@@ -276,7 +272,7 @@ impl GoogleDirectorySync {
         let client = reqwest::Client::builder().build()?;
         let response = client
             .get(url)
-            .header("Authorization", format!("Bearer {}", access_token))
+            .header(AUTHORIZATION, format!("Bearer {access_token}"))
             .timeout(REQUEST_TIMEOUT)
             .send()
             .await?;
@@ -296,7 +292,7 @@ impl GoogleDirectorySync {
         let client = reqwest::Client::builder().build()?;
         let result = client
             .get(url)
-            .header("Authorization", format!("Bearer {}", access_token))
+            .header(AUTHORIZATION, format!("Bearer {access_token}"))
             .timeout(REQUEST_TIMEOUT)
             .send()
             .await?;
@@ -359,7 +355,7 @@ impl DirectorySync for GoogleDirectorySync {
         debug!("Getting all users");
         let response = self.query_all_users().await?;
         debug!("Got all users response");
-        Ok(response.users.into_iter().map(|u| u.into()).collect())
+        Ok(response.users.into_iter().map(Into::into).collect())
     }
 
     async fn test_connection(&self) -> Result<(), DirectorySyncError> {