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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions packages/at_client/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## 3.0.61
- fix: Sync/Monitor bug while running onboarding_cli with at_chops using pkam from secure element
## 3.0.60
- feat: Add `useRemoteAtServer` to PutRequestOptions. When set, the update
request will be sent directly to the remote atServer
Expand Down
5 changes: 4 additions & 1 deletion packages/at_client/lib/src/client/remote_secondary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ class RemoteSecondary implements Secondary {
AtClientManager.getInstance().secondaryAddressFinder,
secureSocketConfig: secureSocketConfig,
clientConfig: _getClientConfig());

logger.finer(
'signingAlgoType: ${preference.signingAlgoType} hashingAlgoType: ${preference.hashingAlgoType}');
atLookUp.signingAlgoType = preference.signingAlgoType;
atLookUp.hashingAlgoType = preference.hashingAlgoType;
atLookUp.atChops = atChops;
}

Expand Down
14 changes: 9 additions & 5 deletions packages/at_client/lib/src/manager/monitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,15 @@ class Monitor {
'Authenticating the monitor connection: from result:$fromResponse');
if (_preference.useAtChops) {
_logger.finer('Using AtChops to do the PKAM signing');
var signingResult =
// ignore: deprecated_member_use
atChops!.signString(fromResponse, SigningKeyType.pkamSha256);
_logger.finer('Sending command pkam:${signingResult.result}');
await _monitorConnection!.write('pkam:${signingResult.result}\n');
final atSigningInput = AtSigningInput(fromResponse)
..signingAlgoType = _preference.signingAlgoType
..hashingAlgoType = _preference.hashingAlgoType
..signingMode = AtSigningMode.pkam;
var signingResult = atChops!.sign(atSigningInput);
var pkamCommand =
'pkam:signingAlgo:${_preference.signingAlgoType.name}:hashingAlgo:${_preference.hashingAlgoType.name}:${signingResult.result}\n';
_logger.finer('Sending command $pkamCommand');
await _monitorConnection!.write(pkamCommand);
} else {
var key = RSAPrivateKey.fromString(_preference.privateKey!);
var sha256signature =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:at_chops/at_chops.dart';
import 'package:at_client/at_client.dart';
import 'package:at_client/src/preference/at_client_particulars.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -104,6 +105,12 @@ class AtClientPreference {

@experimental
AtClientParticulars atClientParticulars = AtClientParticulars();

//signing algorithm to use for pkam authentication
SigningAlgoType signingAlgoType = SigningAlgoType.rsa2048;

//hashing algorithm to use for pkam authentication
HashingAlgoType hashingAlgoType = HashingAlgoType.sha256;
}

@Deprecated("Use SyncService")
Expand Down
2 changes: 1 addition & 1 deletion packages/at_client/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: The at_client library is the non-platform specific Client SDK which
##
##
## NB: When incrementing the version, please also increment the version in AtClientConfig file
version: 3.0.60
version: 3.0.61
## NB: When incrementing the version, please also increment the version in AtClientConfig file
##

Expand Down
36 changes: 35 additions & 1 deletion packages/at_client/test/monitor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:at_chops/at_chops.dart';
import 'package:at_client/at_client.dart';
import 'package:at_client/src/manager/monitor.dart';
import 'package:at_client/src/preference/monitor_preference.dart';
Expand Down Expand Up @@ -35,6 +36,10 @@ class MockOutboundConnection extends Mock implements OutboundConnection {}
class MockMonitorOutboundConnectionFactory extends Mock
implements MonitorOutboundConnectionFactory {}

class MockAtChops extends Mock implements AtChops {}

class FakeAtSigningInput extends Fake implements AtSigningInput {}

/// Note: The test code here prioritizes brevity over isolation
/// So while, right now, the tests are all passing despite sharing their mock objects, at some point
/// we will add a test where that assumption doesn't hold any more, and the tests will start failing
Expand All @@ -47,6 +52,7 @@ void main() {
OutboundConnection mockOutboundConnection = MockOutboundConnection();
SecureSocket mockSocket = MockSecureSocket();
MonitorPreference monitorPreference = MonitorPreference()..keepAlive = true;
AtChops mockAtChops = MockAtChops();
late Function(dynamic data) socketOnDataFn;
late Function() socketOnDoneFn;
late Function(Exception e) socketOnErrorFn;
Expand All @@ -69,8 +75,9 @@ void main() {
reset(mockSocket);
reset(mockOutboundConnection);
reset(mockMonitorOutboundConnectionFactory);
reset(mockAtChops);
mockMonitorConnectivityChecker.shouldFail = false;

atClientPreference.useAtChops = false;
when(() => mockRemoteSecondary.isAvailable()).thenAnswer((_) async => true);
when(() => mockRemoteSecondary.findSecondaryUrl())
.thenAnswer((_) async => fakeSecondaryUrl);
Expand Down Expand Up @@ -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?

test('test monitor start when useAtChops is true', () async {
AtSigningResult mockSigningResult = AtSigningResult()
..result = 'mock_signing_result';
atClientPreference.useAtChops = true;
registerFallbackValue(FakeAtSigningInput());
when(() => mockAtChops.sign(any()))
.thenAnswer((_) => mockSigningResult);
Monitor monitor = Monitor(
(String received) => print('onResponse: $json'),
(e) => print('onError: $e'),
atSign,
atClientPreference,
monitorPreference,
() => print('onRetry called'),
monitorConnectivityChecker: mockMonitorConnectivityChecker,
remoteSecondary: mockRemoteSecondary,
monitorOutboundConnectionFactory:
mockMonitorOutboundConnectionFactory,
atChops: mockAtChops);

Future<void> monitorStartFuture =
monitor.start(lastNotificationTime: null);
await monitorStartFuture;
expect(monitor.status, MonitorStatus.started);
});
});
});
}