Stop accepting LegacyEncryptor-issued tokens, except admin keys (#41761)

These have not been issued in a long time.
Keep accepting old admin keys, because those are long-lived, but log when we encounter them.

GitOrigin-RevId: d5ea33f36b5bab715839a8e66f07b719119cbb84

Author: goffrie

crates/keybroker/src/broker.rs

@@ -105,6 +105,7 @@ use crate::{
     legacy_encryptor::LegacyEncryptor,
     metrics::{
         log_actions_token_expired,
+        log_legacy_admin_key,
         log_store_file_auth_expired,
     },
     secret::InstanceSecret,
@@ -886,11 +887,12 @@ impl KeyBroker {
     pub fn is_encrypted_admin_key(&self, key: &str) -> bool {
         let encrypted_part = split_admin_key(key).map(|(_, key)| key).unwrap_or(key);
         let admin_key: Result<AdminKeyProto, _> = self
-            .encryptor
-            .decode_proto(ADMIN_KEY_VERSION, encrypted_part)
+            .admin_key_encryptor
+            .decrypt_proto(ADMIN_KEY_VERSION, encrypted_part)
             .or_else(|_| {
-                self.admin_key_encryptor
-                    .decrypt_proto(ADMIN_KEY_VERSION, encrypted_part)
+                self.encryptor
+                    .decode_proto(ADMIN_KEY_VERSION, encrypted_part)
+                    .inspect(|_| log_legacy_admin_key())
             });
         admin_key.is_ok()
     }
@@ -905,11 +907,12 @@ impl KeyBroker {
             identity,
             is_read_only,
         } = self
-            .encryptor
-            .decode_proto(ADMIN_KEY_VERSION, encrypted_part)
+            .admin_key_encryptor
+            .decrypt_proto(ADMIN_KEY_VERSION, encrypted_part)
             .or_else(|_| {
-                self.admin_key_encryptor
-                    .decrypt_proto(ADMIN_KEY_VERSION, encrypted_part)
+                self.encryptor
+                    .decode_proto(ADMIN_KEY_VERSION, encrypted_part)
+                    .inspect(|_| log_legacy_admin_key())
             })
             .with_context(|| format!("Couldn't decode the AdminKeyProto {key}"))?;
         let instance_name = instance_name
@@ -947,12 +950,8 @@ impl KeyBroker {
             authorization_type,
             component_id,
         } = self
-            .encryptor
-            .decode_proto(STORE_FILE_AUTHZ_VERSION, store_file_authorization)
-            .or_else(|_| {
-                self.store_file_encryptor
-                    .decrypt_proto(STORE_FILE_AUTHZ_VERSION, store_file_authorization)
-            })
+            .store_file_encryptor
+            .decrypt_proto(STORE_FILE_AUTHZ_VERSION, store_file_authorization)
             .context(ErrorMetadata::unauthenticated(
                 "StorageTokenInvalid",
                 "Couldn't decode the StoreFileAuthorization token",
@@ -1047,9 +1046,8 @@ impl KeyBroker {
     ) -> anyhow::Result<Cursor> {
         let cursor_version = persistence_version.index_key_version(CURSOR_VERSION);
         let proto: InstanceCursorProto = self
-            .encryptor
-            .decode_proto(cursor_version, &cursor)
-            .or_else(|_| self.cursor_encryptor.decrypt_proto(cursor_version, &cursor))
+            .cursor_encryptor
+            .decrypt_proto(cursor_version, &cursor)
             .with_context(cursor_parse_error)?;
         self.proto_to_cursor(proto)
     }
@@ -1081,12 +1079,8 @@ impl KeyBroker {
             None => Ok(QueryJournal::new()),
             Some(journal) => {
                 let proto: InstanceQueryJournalProto = self
-                    .encryptor
-                    .decode_proto(query_journal_version, &journal)
-                    .or_else(|_| {
-                        self.journal_encryptor
-                            .decrypt_proto(query_journal_version, &journal)
-                    })
+                    .journal_encryptor
+                    .decrypt_proto(query_journal_version, &journal)
                     .with_context(cursor_parse_error)?;
                 let end_cursor = match proto.end_cursor {
                     Some(cursor) => Some(self.proto_to_cursor(cursor)?),
@@ -1122,12 +1116,8 @@ impl KeyBroker {
             issued_s,
             component_id,
         } = self
-            .encryptor
-            .decode_proto(ACTION_KEY_VERSION, token)
-            .or_else(|_| {
-                self.action_callback_encryptor
-                    .decrypt_proto(ACTION_KEY_VERSION, token)
-            })
+            .action_callback_encryptor
+            .decrypt_proto(ACTION_KEY_VERSION, token)
             .with_context(|| format!("Couldn't decode ActionCallbackTokenProto {token}"))?;
 
         anyhow::ensure!(issued_s != 0, "ActionCallbackTokenProto missing issued_s");
@@ -1395,5 +1385,6 @@ mod tests {
             let roundtripped = AdminIdentity::from_proto_unchecked(proto).unwrap();
             assert_eq!(admin_identity, roundtripped);
         }
+
     }
 }

crates/keybroker/src/metrics.rs

@@ -18,3 +18,11 @@ register_convex_counter!(
 pub fn log_actions_token_expired() {
     log_counter(&KEYBROKER_ACTIONS_TOKEN_EXPIRED_TOTAL, 1);
 }
+
+register_convex_counter!(
+    KEYBROKER_LEGACY_ADMIN_KEY_TOTAL,
+    "Number of times that a LegacyEncryptor-issued admin key is encountered"
+);
+pub fn log_legacy_admin_key() {
+    log_counter(&KEYBROKER_LEGACY_ADMIN_KEY_TOTAL, 1);
+}

npm-packages/js-integration-tests/fileStorage.test.ts

@@ -345,7 +345,7 @@ describe("File storage with HTTPClient", () => {
   test("expired upload url", async () => {
     // This test uses a token that was created with the dev secret. The target will fail to decode the token if the secret is different.
     if (!process.env.SKIP_INSTANCE_SECRET_TESTS) {
-      const expiredPostUrl = `${deploymentUrl}/api/storage/upload?token=012df53dad7f240d44729afc7d018378f47916060026f5870066118050545b6f45f52467338e8bd2826bf358c80b7d7846a080719fa0fc0a2d69e7`;
+      const expiredPostUrl = `${deploymentUrl}/api/storage/upload?token=01fadba622425353075d310561a03992352aa952b2f0113f3cd713b6bfa023c765be18fb122abb7ffd5505b81e8362`;
       const postResult = await fetch(expiredPostUrl, {
         method: "POST",
         body: "helloworld",