Skip to content

fix(fc-traits-authn): apply an ExtrinsicContext parameter on Challenger #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pandres95 wants to merge 1 commit into from
Closed

fix(fc-traits-authn): apply an ExtrinsicContext parameter on Challenger #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
345 changes: 156 additions & 189 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

53 changes: 29 additions & 24 deletions pallets/pass/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
type BenchmarkHelper: BenchmarkHelper<Self, I>;
}

#[pallet::pallet]

Check warning on line 93 in pallets/pass/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

using `map_err` over `inspect_err` 

warning: using `map_err` over `inspect_err`
  --> pallets/pass/src/lib.rs:93:15
   |
93 |     #[pallet::pallet]
   |               ^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
   = note: `#[warn(clippy::manual_inspect)]` on by default
help: try
   |
93 -     #[pallet::pallet]
93 +     #[pallet::&inspect_err]
   |
pub struct Pallet<T, I = ()>(_);

// Storage
Expand Down Expand Up @@ -163,8 +163,11 @@
}

#[pallet::feeless_if(
|_: &OriginFor<T>, device_id: &DeviceId, credential: &CredentialOf<T, I>, _: &Option<BlockNumberFor<T>>| -> bool {
Pallet::<T, I>::try_authenticate(device_id, credential).is_ok()
|origin: &OriginFor<T>, device_id: &DeviceId, credential: &CredentialOf<T, I>, _: &Option<BlockNumberFor<T>>| -> bool {
let Ok(who) = ensure_signed(origin.clone()) else {
return false;
};
Pallet::<T, I>::try_authenticate(&who, device_id, credential).is_ok()
}
)]
#[pallet::call_index(3)]
Expand All @@ -175,7 +178,7 @@
duration: Option<BlockNumberFor<T>>,
) -> DispatchResult {
let who = ensure_signed(origin)?;
let account_id = Self::try_authenticate(&device_id, &credential)?;
let account_id = Self::try_authenticate(&who, &device_id, &credential)?;
Self::try_add_session(&who, &account_id, duration)?;
Ok(())
}
Expand All @@ -200,7 +203,7 @@
) -> DispatchResult {
let account_id = if let Some((device_id, credential)) = maybe_credential {
let account_id = Self::account_id_for(credential.user_id())?;
Self::do_authenticate(credential, device_id)?;
Self::try_authenticate_for_dispatch(credential, device_id, &call)?;
account_id
} else {
Self::ensure_signer_is_valid_session(origin)?
Expand Down Expand Up @@ -269,6 +272,7 @@
}

pub(crate) fn try_authenticate(
session_key: &T::AccountId,
device_id: &DeviceId,
credential: &CredentialOf<T, I>,
) -> Result<T::AccountId, DispatchError> {
Expand All @@ -280,18 +284,36 @@
let device =
Devices::<T, I>::get(&account_id, device_id).ok_or(Error::<T, I>::DeviceNotFound)?;
device
.verify_user(credential)
.verify_user(credential, &session_key.encode())
.ok_or(Error::<T, I>::CredentialInvalid)?;

Ok(account_id)
}

pub(crate) fn try_authenticate_for_dispatch(
credential: CredentialOf<T, I>,
device_id: DeviceId,
call: &RuntimeCallFor<T>,
) -> Result<T::AccountId, DispatchError> {
let account_id = Self::account_id_for(credential.user_id())?;
ensure!(
Self::account_exists(&account_id),
Error::<T, I>::AccountNotFound
);
let device =
Devices::<T, I>::get(&account_id, device_id).ok_or(Error::<T, I>::DeviceNotFound)?;
device
.verify_user(&credential, &call.encode())
.ok_or(Error::<T, I>::CredentialInvalid)?;
Ok(account_id)
}

pub(crate) fn do_add_device(
who: &T::AccountId,
attestation: DeviceAttestationOf<T, I>,
) -> DispatchResult {
let device_id = attestation.device_id();
let device = T::Authenticator::verify_device(attestation.clone())
let device = T::Authenticator::verify_device(attestation.clone(), &[])
.ok_or(Error::<T, I>::DeviceAttestationInvalid)?;

Devices::<T, I>::insert(who, device_id, device);
Expand Down Expand Up @@ -330,23 +352,6 @@
}
}

pub(crate) fn do_authenticate(
credential: CredentialOf<T, I>,
device_id: DeviceId,
) -> Result<T::AccountId, DispatchError> {
let account_id = Self::account_id_for(credential.user_id())?;
ensure!(
Self::account_exists(&account_id),
Error::<T, I>::AccountNotFound
);
let device =
Devices::<T, I>::get(&account_id, device_id).ok_or(Error::<T, I>::DeviceNotFound)?;
device
.verify_user(&credential)
.ok_or(Error::<T, I>::CredentialInvalid)?;
Ok(account_id)
}

fn do_remove_session(session_key: &T::AccountId) {
Self::cancel_scheduled_session_key_removal(session_key);
// Decrements the provider reference of this `Session` key account once it's expired.
Expand Down Expand Up @@ -380,7 +385,7 @@
.min(T::MaxSessionDuration::get());
let until = block_number + session_duration;

Sessions::<T, I>::insert(session_key.clone(), (account_id.clone(), until));
Sessions::<T, I>::insert(session_key.clone(), (account_id.clone(), until.clone()));

Check warning on line 388 in pallets/pass/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

using `clone` on type `<<<T as Config>::Block as Block>::Header as Header>::Number` which implements the `Copy` trait 

warning: using `clone` on type `<<<T as Config>::Block as Block>::Header as Header>::Number` which implements the `Copy` trait
   --> pallets/pass/src/lib.rs:388:76
    |
388 |         Sessions::<T, I>::insert(session_key.clone(), (account_id.clone(), until.clone()));
    |                                                                            ^^^^^^^^^^^^^ help: try removing the `clone` call: `until`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
    = note: `#[warn(clippy::clone_on_copy)]` on by default
Self::schedule_next_removal(session_key, duration)?;

Self::deposit_event(Event::<T, I>::SessionCreated {
Expand Down
8 changes: 4 additions & 4 deletions pallets/pass/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,19 @@ impl pallet_pass::BenchmarkHelper<Test> for BenchmarkHelper {
fn device_attestation(device_id: DeviceId) -> DeviceAttestationOf<Test, ()> {
PassDeviceAttestation::AuthenticatorAAuthenticator(authenticator_a::DeviceAttestation {
device_id,
challenge: authenticator_a::Authenticator::generate(&()),
challenge: authenticator_a::Authenticator::generate(&(), &[]),
})
}

fn credential(user_id: HashedUserId) -> CredentialOf<Test, ()> {
fn credential(user_id: HashedUserId, xtc: &impl ExtrinsicContext) -> CredentialOf<Test, ()> {
PassCredential::AuthenticatorAAuthenticator(authenticator_a::Credential {
user_id,
challenge: authenticator_a::Authenticator::generate(&()),
challenge: authenticator_a::Authenticator::generate(&(), xtc),
})
}
}

pub fn new_test_ext() -> sp_io::TestExternalities {
pub fn new_test_ext() -> TestExternalities {
let mut ext = TestExternalities::new(Default::default());
ext.execute_with(|| {
System::set_block_number(1);
Expand Down
8 changes: 4 additions & 4 deletions pallets/pass/src/mock/authenticators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ pub mod authenticator_a {
impl Challenger for Authenticator {
type Context = ();

fn generate(_: &Self::Context) -> Challenge {
let (hash, _) = RandomnessFromBlockNumber::random_seed();
fn generate<Xtc: ExtrinsicContext>(_: &Self::Context, xtc: &Xtc) -> Challenge {
let (hash, _) = RandomnessFromBlockNumber::random(xtc.as_ref());
hash.0
}
}
Expand Down Expand Up @@ -178,8 +178,8 @@ pub mod authenticator_b {
impl Challenger for AuthenticatorB {
type Context = DeviceId;

fn generate(context: &Self::Context) -> Challenge {
let (hash, _) = RandomnessFromBlockNumber::random(context);
fn generate<Xtc: ExtrinsicContext>(context: &Self::Context, xtc: &Xtc) -> Challenge {
let (hash, _) = RandomnessFromBlockNumber::random(&[context, xtc.as_ref()].concat());
hash.0
}
}
Expand Down
Loading
Loading