Skip to content
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.

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

fix: Monitor/sync bug while using at_chops with pkam from secure element #1050

Merged
merged 14 commits into from
May 26, 2023

Conversation

murali-shris
Copy link
Member

@murali-shris murali-shris commented May 26, 2023

- What I did

  • fixed sync and monitor bug when at_chops for pkam secure element is injected via at_onboarding_cli
    - How I did it
  • added two attributes signingAlgoType and hashingAlgoType to AtClientPreference with default value.
  • Replaced deprecated atChops!.signString(..) in Monitor with atChops.sign(..) method which will use the signingAlgoType and hashingAlgoType set in AtClientPreference
  • in RemoteSecondary set the signingAlgoType and hashingAlgoType to atLookup object
    - How to verify it
  • run the unit test monitor_test.dart
  • run at_chops/example/zariot/onboard_secure_element.dart (branch monitor_bug_secure_element) on a pi device with pkam private key on sim.

closes atsign-foundation/at_libraries#351

@murali-shris murali-shris marked this pull request as ready for review May 26, 2023 09:55
@murali-shris murali-shris changed the title fix: Monitor bug while using at_chops with pkam from secure element fix: Monitor/sync bug while using at_chops with pkam from secure element May 26, 2023
@gkc
Copy link
Contributor

gkc commented May 26, 2023

@murali-shris Is there a bug with the sync's connection auth also, or is it just the monitor?

@gkc
Copy link
Contributor

gkc commented May 26, 2023

Oh ignore previous question I see you have changed RemoteSecondary

@@ -676,5 +683,32 @@ void main() {
() async => await monitor.getQueueResponse(maxWaitTimeInMillis: 10),
throwsA(predicate((dynamic e) => e is AtClientException)));
});
group('A group of tests for monitor when useAtChops is true', () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also have an additional auth test for RemoteSecondary for when useAtChops is true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authenticate and authenticateCram methods in RemoteSecondary are no longer used.
executeVerb and executeCommand methods in RemoteSecondary, delegates the call to atLookup which does the auth.
Can I explore writing tests for AtLookup auth?

@gkc
Copy link
Contributor

gkc commented May 26, 2023

Oh I see. No need for any more then. Will approve and merge.

@gkc gkc merged commit 0745a9d into trunk May 26, 2023
@gkc gkc deleted the monitor_bug_secure_element branch May 26, 2023 15:26
@gkc
Copy link
Contributor

gkc commented May 26, 2023

Please don't publish yet, we've not published 3.0.60 so we should probably change version back to 3.0.60 to prevent any confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

authentication with sync enabled failing if pkam is in secure element
2 participants