fix: Synchronize all AUTH tokens

Previously, AUTH tokens were only synchronized if an INVITE token was
generated at the same time. This led to several user-visible bugs.

Now, we always send a sync message after generating a new AUTH token.

Since the INVITE token may have stayed the same, I added a UNIQUE bound
to the `tokens` table, in order to prevent saving the same INVITE token
many times.

src/securejoin.rs

@@ -127,9 +127,6 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
         None => None,
     };
     let grpid = chat.as_ref().map(|c| c.grpid.as_str());
-    let sync_token = token::lookup(context, Namespace::InviteNumber, grpid)
-        .await?
-        .is_none();
     // Invite number is used to request the inviter key.
     let invitenumber = token::lookup_or_new(context, Namespace::InviteNumber, grpid).await?;
 
@@ -156,12 +153,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
         .unwrap_or_default();
 
     let qr = if let Some(chat) = chat {
-        if sync_token {
-            context
-                .sync_qr_code_tokens(Some(chat.grpid.as_str()))
-                .await?;
-            context.scheduler.interrupt_smtp().await;
-        }
+        context
+            .sync_qr_code_tokens(Some(chat.grpid.as_str()))
+            .await?;
+        context.scheduler.interrupt_smtp().await;
 
         let chat_name = chat.get_name();
         let chat_name_shortened = shorten_name(chat_name, 25);
@@ -190,10 +185,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
         let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS)
             .to_string()
             .replace("%20", "+");
-        if sync_token {
-            context.sync_qr_code_tokens(None).await?;
-            context.scheduler.interrupt_smtp().await;
-        }
+
+        context.sync_qr_code_tokens(None).await?;
+        context.scheduler.interrupt_smtp().await;
+
         format!(
             "https://i.delta.chat/#{fingerprint}&v=3&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}",
         )

src/securejoin/securejoin_tests.rs

@@ -1388,6 +1388,11 @@ async fn test_auth_token_is_synchronized() -> Result<()> {
     // This creates another auth token; both of them need to be synchronized
     let qr2 = get_securejoin_qr(alice1, None).await?;
     sync(alice1, alice2).await;
+
+    // Note that Bob will throw away the AUTH token after sending `vc-request-with-auth`.
+    // Therefore, he will fail to decrypt the answer from alice,
+    // which leads to a "decryption failed: missing key" message in the logs.
+    // This is fine.
     tcm.exec_securejoin_qr_multi_device(bob, &[alice1, alice2], &qr2)
         .await;
 
@@ -1400,11 +1405,33 @@ async fn test_auth_token_is_synchronized() -> Result<()> {
     assert_eq!(chatlist.len(), 1);
 
     for qr in [qr1, qr2] {
-        let qr = check_qr(alice2, &qr).await?;
-        let qr = QrInvite::try_from(qr)?;
+        let qr = check_qr(bob, &qr).await?;
+        let qr = QrInvite::try_from(dbg!(qr))?;
         assert!(token::exists(alice2, Namespace::InviteNumber, qr.invitenumber()).await?);
         assert!(token::exists(alice2, Namespace::Auth, qr.authcode()).await?);
     }
 
+    // Check that alice2 only saves the invite number twice:
+    let invite_count: u32 = alice2
+        .sql
+        .query_get_value(
+            "SELECT COUNT(*) FROM tokens WHERE namespc=?;",
+            (Namespace::InviteNumber,),
+        )
+        .await?
+        .unwrap();
+    assert_eq!(invite_count, 1);
+
+    // ...but knows two AUTH tokens:
+    let auth_count: u32 = alice2
+        .sql
+        .query_get_value(
+            "SELECT COUNT(*) FROM tokens WHERE namespc=?;",
+            (Namespace::Auth,),
+        )
+        .await?
+        .unwrap();
+    assert_eq!(auth_count, 2);
+
     Ok(())
 }

src/sql/migrations.rs

@@ -1525,6 +1525,27 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT;
         .await?;
     }
 
+    // Add UNIQUE bound to token, in order to avoid saving the same token multiple times
+    inc_and_check(&mut migration_version, 146)?;
+    if dbversion < migration_version {
+        sql.execute_migration(
+            "CREATE TABLE tokens_new (
+                id INTEGER PRIMARY KEY,
+                namespc INTEGER DEFAULT 0,
+                foreign_key TEXT DEFAULT '',
+                token TEXT DEFAULT '' UNIQUE,
+                timestamp INTEGER DEFAULT 0,
+                foreign_id INTEGER NOT NULL DEFAULT 0
+            ) STRICT;
+            INSERT OR IGNORE INTO tokens_new
+                SELECT id, namespc, foreign_key, token, timestamp, foreign_id FROM tokens;
+            DROP TABLE tokens;
+            ALTER TABLE tokens_new RENAME TO tokens;",
+            migration_version,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?

src/token.rs

@@ -33,7 +33,7 @@ pub async fn save(
     context
         .sql
         .execute(
-            "INSERT INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)",
+            "INSERT OR IGNORE INTO tokens (namespc, foreign_key, token, timestamp) VALUES (?, ?, ?, ?)",
             (namespace, foreign_key.unwrap_or(""), token, timestamp),
         )
         .await?;
@@ -56,7 +56,7 @@ pub async fn lookup(
     context
         .sql
         .query_get_value(
-            "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY timestamp DESC LIMIT 1",
+            "SELECT token FROM tokens WHERE namespc=? AND foreign_key=? ORDER BY id DESC LIMIT 1",
             (namespace, foreign_key.unwrap_or("")),
         )
         .await
@@ -66,7 +66,7 @@ pub async fn lookup_all(context: &Context, namespace: Namespace) -> Result<Vec<S
     context
         .sql
         .query_map_vec(
-            "SELECT token FROM tokens WHERE namespc=? ORDER BY timestamp DESC",
+            "SELECT token FROM tokens WHERE namespc=? ORDER BY id DESC",
             (namespace,),
             |row| Ok(row.get(0)?),
         )