Resolve comments

autonomys · Apr 30, 2024 · 76c3a5f · 76c3a5f
1 parent f2d0318
commit 76c3a5f
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 24 deletions.
diff --git a/domains/pallets/auto-id/src/lib.rs b/domains/pallets/auto-id/src/lib.rs
@@ -108,7 +108,7 @@ impl Certificate {
         }
     }
 
-    fn serial(&self) -> U256 {
+    fn serial(&self) -> Serial {
         match self {
             Certificate::X509(cert) => cert.serial,
         }
@@ -217,11 +217,11 @@ pub struct CertificateAction {
 
 #[frame_support::pallet]
 mod pallet {
+    use super::*;
     use crate::{AutoId, Identifier, RegisterAutoId, Serial, Signature};
     use frame_support::pallet_prelude::*;
     use frame_support::traits::Time;
     use frame_system::pallet_prelude::*;
-    use scale_info::prelude::collections::BTreeSet;
 
     #[pallet::config]
     pub trait Config: frame_system::Config {
@@ -391,6 +391,16 @@ impl<T: Config> Pallet<T> {
                         Error::<T>::ExpiredCertificate
                     );
 
+                    ensure!(
+                        !CertificateRevocationList::<T>::get(issuer_id).map_or(false, |serials| {
+                            serials.iter().any(|s| {
+                                *s == issuer_auto_id.certificate.serial()
+                                    || *s == tbs_certificate.serial
+                            })
+                        }),
+                        Error::<T>::CertificateRevoked
+                    );
+
                     issuer_auto_id
                         .certificate
                         .issue_certificate_serial::<T>(tbs_certificate.serial)?;
@@ -451,21 +461,21 @@ impl<T: Config> Pallet<T> {
     ) -> DispatchResult {
         let auto_id = AutoIds::<T>::get(auto_id_identifier).ok_or(Error::<T>::UnknownAutoId)?;
 
-        let issuer_id = match auto_id.certificate.issuer_id() {
-            Some(issuer_id) => issuer_id,
+        let (issuer_id, mut issuer_auto_id) = match auto_id.certificate.issuer_id() {
+            Some(issuer_id) => (
+                issuer_id,
+                AutoIds::<T>::get(issuer_id).ok_or(Error::<T>::UnknownIssuer)?,
+            ),
             // self revoke
-            None => auto_id_identifier,
+            None => (auto_id_identifier, auto_id.clone()),
         };
 
-        let mut issuer_auto_id = AutoIds::<T>::get(issuer_id).ok_or(Error::<T>::UnknownIssuer)?;
-
         ensure!(
-            !CertificateRevocationList::<T>::get(issuer_id).map_or(false, |serials| serials
-                .iter()
-                .filter(|serial| *serial == &auto_id.certificate.serial()
-                    || *serial == &issuer_auto_id.certificate.serial())
-                .count()
-                > 0),
+            !CertificateRevocationList::<T>::get(issuer_id).map_or(false, |serials| {
+                serials.iter().any(|s| {
+                    *s == auto_id.certificate.serial() || *s == issuer_auto_id.certificate.serial()
+                })
+            }),
             Error::<T>::CertificateAlreadyRevoked
         );
 

diff --git a/domains/pallets/auto-id/src/tests.rs b/domains/pallets/auto-id/src/tests.rs
@@ -308,19 +308,22 @@ fn test_self_revoke_certificate() {
 
         assert_eq!(auto_id.certificate.nonce(), U256::one());
 
-        // try revoking leaf certificate when issuer is revoked
-        let leaf_id = register_leaf_auto_id(auto_id_identifier);
-        let leaf_auto_id = AutoIds::<Test>::get(leaf_id).unwrap();
-        let signing_data = CertificateAction {
-            id: leaf_id,
-            nonce: leaf_auto_id.certificate.nonce(),
-            action_type: CertificateActionType::RevokeCertificate,
-        };
-        let signature = sign_preimage(signing_data.encode(), false);
+        // try issuing leaf certificate when issuer is revoked
+        let cert = include_bytes!("../res/leaf.cert.der").to_vec();
+        let (_, cert) = x509_parser::certificate::X509Certificate::from_der(&cert).unwrap();
+        let _ = identifier_from_x509_cert(Some(auto_id_identifier), &cert);
 
         assert_noop!(
-            Pallet::<Test>::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature),
-            Error::<Test>::CertificateAlreadyRevoked
+            Pallet::<Test>::register_auto_id(
+                RawOrigin::Signed(1).into(),
+                RegisterAutoId::X509(RegisterAutoIdX509::Leaf {
+                    issuer_id: auto_id_identifier,
+                    certificate: cert.tbs_certificate.as_ref().to_vec().into(),
+                    signature_algorithm: algorithm_to_der(cert.signature_algorithm.clone()),
+                    signature: cert.signature_value.as_ref().to_vec(),
+                }),
+            ),
+            Error::<Test>::CertificateRevoked,
         );
     })
 }