Added the IssuerPermissions back to solve an bug (#127)

* Added the IssuerPermissions back to solve an bug
Added some unit tests back

* initialise_issuer also grant the issuer that access

* Added a unit tests that tests the revocation of the topic permissions
Added a unit tests that tests force-remove and its revocation of
topic permissions

Author: roy-powerfinance

prml/consortium-permission/src/lib.rs

@@ -72,6 +72,18 @@ pub type Topic = Vec<u8>;
 /// Type used for values of corresponding topics.
 pub type Value = Vec<u8>;
 
+/// Allows runtime implmentation of issuer configuration.
+pub trait IssuerPermissions {
+    type AccountId;
+    type Topic;
+
+    /// Grants the permission to issue claims for a new topic.
+    fn grant_issuer_permissions(issuer: &Self::AccountId, topic: &Topic);
+
+    /// Revokes the permission to issue claims for a topic.
+    fn revoke_issuer_permissions(issuer: &Self::AccountId, topic: &Topic);
+}
+
 /// The module's config trait.
 pub trait Trait: frame_system::Trait {
     /// The overarching event type.
@@ -80,6 +92,8 @@ pub trait Trait: frame_system::Trait {
     type MaximumTopicSize: Get<usize>;
     /// The maximum number of bytes allowed for a value.
     type MaximumValueSize: Get<usize>;
+    /// Provides an interface for setting issuer permissions
+    type IssuerPermissions: IssuerPermissions<AccountId = <Self as frame_system::Trait>::AccountId, Topic = Topic>;
 }
 
 decl_storage! {
@@ -171,6 +185,8 @@ decl_module! {
             current_topics.push(topic.clone());
             Issuers::<T>::insert(who.clone(), current_topics);
 
+            T::IssuerPermissions::grant_issuer_permissions(&who, &topic);
+
             Self::deposit_event(RawEvent::IssuerWithTopicAdded(who, topic));
         }
 
@@ -195,6 +211,8 @@ decl_module! {
                 Issuers::<T>::insert(who.clone(), current_topics);
             }
 
+            T::IssuerPermissions::revoke_issuer_permissions(&who, &topic);
+
             Self::deposit_event(RawEvent::IssuerWithTopicRemoved(who, topic));
         }
 
@@ -203,6 +221,14 @@ decl_module! {
         /// Requires Root.
         pub fn force_remove_issuer(origin, who: T::AccountId) {
             ensure_root(origin)?;
+
+            // Notify the revocation of all current permissions.
+            let current_topics = Self::issuers(&who);
+            for topic in current_topics {
+                T::IssuerPermissions::revoke_issuer_permissions(&who, &topic);
+            }
+
+            // Remove topics for this issuer.
             Issuers::<T>::remove(&who);
 
             Self::deposit_event(RawEvent::IssuerForceRemoved(who));
@@ -270,6 +296,9 @@ impl<T: Trait> Module<T> {
     fn initialise_issuers(issuers: &Vec<(T::AccountId, Vec<Topic>)>) {
         for (issuer, topics) in issuers {
             Issuers::<T>::insert(issuer, topics);
+            for topic in topics {
+                T::IssuerPermissions::grant_issuer_permissions(&issuer, &topic);
+            }
         }
     }
 

prml/consortium-permission/src/mock.rs

@@ -36,6 +36,35 @@ pub const ACCESS_TOPIC: &[u8; 6] = b"access";
 /// Reserved value for access to submit an extrinsic.
 pub const ACCESS_VALUE: u8 = 1;
 
+pub struct IssuerPermissionsMock;
+
+impl IssuerPermissions for IssuerPermissionsMock {
+    type AccountId = AccountId;
+    type Topic = Topic;
+    /// When an issuer is authorized to make claims on the "access" topic, also grant themself the
+    /// "access" permission.
+    fn grant_issuer_permissions(issuer: &Self::AccountId, topic: &Topic) {
+        if *topic == ACCESS_TOPIC {
+            ConsortiumPermission::do_make_claim(
+                issuer,
+                issuer,
+                ACCESS_TOPIC.to_vec().as_ref(),
+                vec![ACCESS_VALUE].as_ref(),
+            );
+        }
+    }
+    /// When an issuer's authority on the "access" topic is revoked, also revoke their self-claimed
+    /// "access" permission.
+    fn revoke_issuer_permissions(issuer: &Self::AccountId, topic: &Topic) {
+        if *topic == ACCESS_TOPIC {
+            let (claim_issuer, _) = ConsortiumPermission::claim((issuer, ACCESS_TOPIC.to_vec()));
+            if claim_issuer == *issuer {
+                ConsortiumPermission::do_revoke_claim(*issuer, ACCESS_TOPIC.to_vec());
+            }
+        }
+    }
+}
+
 impl_outer_origin! {
     pub enum Origin for Test  where system = frame_system {}
 }
@@ -88,6 +117,7 @@ impl Trait for Test {
     type Event = TestEvent;
     type MaximumTopicSize = MaximumTopicSize;
     type MaximumValueSize = MaximumValueSize;
+    type IssuerPermissions = IssuerPermissionsMock;
 }
 
 #[derive(Default)]

prml/consortium-permission/src/tests.rs

@@ -61,6 +61,20 @@ fn add_issuer_with_topic_requires_root() {
         });
 }
 
+#[test]
+fn added_issuer_has_access_true() {
+    ExtBuilder::default()
+        .topic(ACCESS_TOPIC, true)
+        .build()
+        .execute_with(|| {
+            assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+            assert_eq!(
+                ConsortiumPermission::claim((BOB, ACCESS_TOPIC.to_vec())),
+                (BOB, vec![ACCESS_VALUE])
+            );
+        });
+}
+
 #[test]
 fn add_issuer_with_topic_rejects_invalid_topics() {
     ExtBuilder::default()
@@ -305,6 +319,28 @@ fn remove_issuer_with_topic_emits_events() {
         });
 }
 
+#[test]
+fn removed_issuer_loses_only_self_assigned_access() {
+    ExtBuilder::default().genesis_topic(ACCESS_TOPIC).build().execute_with(|| {
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, ALICE, ACCESS_TOPIC.to_vec()));
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+
+        // Revoking the "access" authority should also revoke the self-claimed "access" permission.
+        assert_ok!(ConsortiumPermission::remove_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+        assert_eq!(ConsortiumPermission::holder_claims(BOB), Vec::<Topic>::default());
+
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+        assert_ok!(ConsortiumPermission::make_claim(Origin::signed(ALICE), BOB, ACCESS_TOPIC.to_vec(), vec![ACCESS_VALUE]));
+
+        // Since the "access" claim is now made by alice, BOB should keep the access permission even if its
+        // authority on the "access" topic has been revoked.
+        assert_ok!(ConsortiumPermission::remove_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+        assert_eq!(ConsortiumPermission::claim((BOB, ACCESS_TOPIC.to_vec())), (ALICE, vec![ACCESS_VALUE]) );
+
+    });
+}
+
+
 #[test]
 fn force_remove_issuer_works() {
     ExtBuilder::default()
@@ -342,6 +378,28 @@ fn force_remove_issuer_works() {
         });
 }
 
+#[test]
+fn force_remove_issuer_loses_only_self_assigned_access() {
+    ExtBuilder::default().genesis_topic(ACCESS_TOPIC).build().execute_with(|| {
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, ALICE, ACCESS_TOPIC.to_vec()));
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+
+        // Force-removing Bob should also revoke his self-claimed "access" permission.
+        assert_ok!(ConsortiumPermission::force_remove_issuer(Origin::ROOT, BOB));
+        assert_eq!(ConsortiumPermission::holder_claims(BOB), Vec::<Topic>::default());
+
+        assert_ok!(ConsortiumPermission::add_issuer_with_topic(Origin::ROOT, BOB, ACCESS_TOPIC.to_vec()));
+        assert_ok!(ConsortiumPermission::make_claim(Origin::signed(ALICE), BOB, ACCESS_TOPIC.to_vec(), vec![ACCESS_VALUE]));
+
+        // Since the "access" claim is now made by alice, BOB should keep the access permission
+        // even if he is force_removed
+        assert_ok!(ConsortiumPermission::force_remove_issuer(Origin::ROOT, BOB));
+        assert_eq!(ConsortiumPermission::claim((BOB, ACCESS_TOPIC.to_vec())), (ALICE, vec![ACCESS_VALUE]) );
+
+    });
+}
+
+
 // Claims
 #[test]
 fn claim_extrinsics_must_be_signed() {