From 502d708be946d53032a0016caf493cd7e5989273 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Mon, 7 Oct 2024 15:01:58 +0530 Subject: [PATCH 1/4] fix: Add retry logic in "_waitForPkamAuthSuccess" to retry when secondary is temporarily not reachable --- .../onboard/at_onboarding_service_impl.dart | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart index 0d0c37cb..439ac805 100644 --- a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart +++ b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart @@ -31,6 +31,7 @@ class AtOnboardingServiceImpl implements AtOnboardingService { AtSignLogger logger = AtSignLogger('OnboardingCli'); AtOnboardingPreference atOnboardingPreference; AtLookUp? _atLookUp; + final _maxActivationRetries = 5; /// The object which controls what types of AtClients, NotificationServices /// and SyncServices get created when we call [AtClientManager.setCurrentAtSign]. @@ -321,15 +322,28 @@ class AtOnboardingServiceImpl implements AtOnboardingService { Duration retryInterval, { bool logProgress = true, }) async { + int retryAttempt = 1; while (true) { logger.info('Attempting pkam auth'); if (logProgress) { stderr.write('Checking ... '); } - bool pkamAuthSucceeded = await _attemptPkamAuth( - atLookUp, - enrollmentIdFromServer, - ); + bool pkamAuthSucceeded = false; + try { + pkamAuthSucceeded = + await _attemptPkamAuth(atLookUp, enrollmentIdFromServer); + } catch (e) { + String message = + 'Exception occurred when authenticating the atSign: $_atSign caused by ${e.toString()}'; + if (retryAttempt > _maxActivationRetries) { + message += ' Activation failed after $_maxActivationRetries attempts'; + logger.severe(message); + rethrow; + } + logger + .severe('$message. Attempting to retry for $retryAttempt attempt'); + retryAttempt++; + } if (pkamAuthSucceeded) { if (logProgress) { stderr.writeln(' approved.'); @@ -366,9 +380,6 @@ class AtOnboardingServiceImpl implements AtOnboardingService { } else if (e.message.contains('error:AT0025')) { throw AtEnrollmentException('enrollment denied'); } - } catch (e) { - logger.shout('Unexpected exception: $e'); - rethrow; } finally { logger.finer('_attemptPkamAuth: complete'); } From f5e2b3f90d142dd11b2b668806799f2d946c2280 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 8 Oct 2024 09:00:23 +0530 Subject: [PATCH 2/4] fix: Add dart docs and modify the code --- .../onboard/at_onboarding_service_impl.dart | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart index 439ac805..fc29284b 100644 --- a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart +++ b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart @@ -330,8 +330,24 @@ class AtOnboardingServiceImpl implements AtOnboardingService { } bool pkamAuthSucceeded = false; try { + // _attemptPkamAuth returns boolean value true when authentication is successful. + // Returns UnAuthenticatedException when authentication fails. pkamAuthSucceeded = await _attemptPkamAuth(atLookUp, enrollmentIdFromServer); + } on UnAuthenticatedException catch (e) { + // Error codes AT0401 and AT0026 indicate authentication failure due to unapproved enrollment. Retry until the enrollment is approved. + // The variable _pkamAuthSucceeded is false, allowing for PKAM authentication retries. + // Avoid checking "retryAttempt > _maxActivationRetries" here, as we want to continue retrying until enrollment is approved. + // The check for "retryAttempt > _maxActivationRetries" should only occur when the secondary server is unreachable due to network issues. + if (e.message.contains('error:AT0401') || + e.message.contains('error:AT0026')) { + logger.info('Pkam auth failed: ${e.message}'); + } + // Error code AT0025 represents Enrollment denied. Therefore, no need to retry; throw exception. + else if (e.message.contains('error:AT0025')) { + throw AtEnrollmentException( + 'The enrollment: $enrollmentIdFromServer is denied'); + } } catch (e) { String message = 'Exception occurred when authenticating the atSign: $_atSign caused by ${e.toString()}'; @@ -361,29 +377,21 @@ class AtOnboardingServiceImpl implements AtOnboardingService { } } - /// Try a single PKAM auth + /// Executes PKAM authentication on the secondary server. + /// + /// Returns `true` if the authentication is successful. + /// + /// Returns UnAuthenticated Exception when authentication fails. Future _attemptPkamAuth(AtLookUp atLookUp, String enrollmentId) async { - try { - logger.finer('_attemptPkamAuth: Calling atLookUp.pkamAuthenticate'); - var pkamResult = - await atLookUp.pkamAuthenticate(enrollmentId: enrollmentId); - logger.finer( - '_attemptPkamAuth: atLookUp.pkamAuthenticate returned $pkamResult'); - if (pkamResult) { - return true; - } - } on UnAuthenticatedException catch (e) { - if (e.message.contains('error:AT0401') || - e.message.contains('error:AT0026')) { - logger.info('Pkam auth failed: ${e.message}'); - return false; - } else if (e.message.contains('error:AT0025')) { - throw AtEnrollmentException('enrollment denied'); - } - } finally { - logger.finer('_attemptPkamAuth: complete'); - } - return false; + logger.finer('_attemptPkamAuth: Calling atLookUp.pkamAuthenticate'); + // atLookUp.pkamAuthenticate returns true when authentication is successful. + // when authentication fails, returns UnAuthenticatedException. + var pkamResult = + await atLookUp.pkamAuthenticate(enrollmentId: enrollmentId); + logger.finer( + '_attemptPkamAuth: atLookUp.pkamAuthenticate returned $pkamResult'); + + return pkamResult; } ///write newly created encryption keypairs into atKeys file From a612f70d6e31bef54e7a39a9eed0c3895039f46e Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 8 Oct 2024 09:45:01 +0530 Subject: [PATCH 3/4] fix: Modify error message to fix failing functional test --- .../test/enrollment_test.dart | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/at_onboarding_cli_functional_tests/test/enrollment_test.dart b/tests/at_onboarding_cli_functional_tests/test/enrollment_test.dart index 8d6e9c7a..d458c8eb 100644 --- a/tests/at_onboarding_cli_functional_tests/test/enrollment_test.dart +++ b/tests/at_onboarding_cli_functional_tests/test/enrollment_test.dart @@ -237,10 +237,14 @@ void main() { var completer = Completer(); // Create a Completer //4. Subscribe to enrollment notifications; we will deny it when it arrives + String enrollmentId = ''; onboardingService_1.atClient!.notificationService .subscribe(regex: '.__manage') .listen(expectAsync1((notification) async { logger.finer('got enroll notification'); + final notificationKey = notification.key; + enrollmentId = notificationKey.substring( + 0, notificationKey.indexOf('.new.enrollments')); expect(notification.value, isNotNull); var notificationValueJson = jsonDecode(notification.value!); expect(notificationValueJson['encryptedApkamSymmetricKey'], @@ -258,7 +262,7 @@ void main() { AtOnboardingServiceImpl? onboardingService_2 = AtOnboardingServiceImpl(atSign, preference_1); - Future expectLaterFuture = expectLater( + expectLater( onboardingService_2.enroll( 'buzz', 'iphone', @@ -267,8 +271,8 @@ void main() { retryInterval: Duration(seconds: 5), ), throwsA(predicate((dynamic e) => - e is AtEnrollmentException && e.message == 'enrollment denied'))); - print(await expectLaterFuture); + e is AtEnrollmentException && + e.message == 'The enrollment: $enrollmentId is denied'))); await completer.future; await onboardingService_1.close(); From cf24f66df02bd97f98ec1bbbb561e6ad8f7737f8 Mon Sep 17 00:00:00 2001 From: Sitaram Kalluri Date: Tue, 8 Oct 2024 17:26:57 +0530 Subject: [PATCH 4/4] fix: Remove _attemptPkamAuth method and invoke pkamAuthenticate in _waitForPkamAuthSuccess --- .../onboard/at_onboarding_service_impl.dart | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart index fc29284b..3db31f3a 100644 --- a/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart +++ b/packages/at_onboarding_cli/lib/src/onboard/at_onboarding_service_impl.dart @@ -332,8 +332,8 @@ class AtOnboardingServiceImpl implements AtOnboardingService { try { // _attemptPkamAuth returns boolean value true when authentication is successful. // Returns UnAuthenticatedException when authentication fails. - pkamAuthSucceeded = - await _attemptPkamAuth(atLookUp, enrollmentIdFromServer); + pkamAuthSucceeded = await atLookUp.pkamAuthenticate( + enrollmentId: enrollmentIdFromServer); } on UnAuthenticatedException catch (e) { // Error codes AT0401 and AT0026 indicate authentication failure due to unapproved enrollment. Retry until the enrollment is approved. // The variable _pkamAuthSucceeded is false, allowing for PKAM authentication retries. @@ -377,23 +377,6 @@ class AtOnboardingServiceImpl implements AtOnboardingService { } } - /// Executes PKAM authentication on the secondary server. - /// - /// Returns `true` if the authentication is successful. - /// - /// Returns UnAuthenticated Exception when authentication fails. - Future _attemptPkamAuth(AtLookUp atLookUp, String enrollmentId) async { - logger.finer('_attemptPkamAuth: Calling atLookUp.pkamAuthenticate'); - // atLookUp.pkamAuthenticate returns true when authentication is successful. - // when authentication fails, returns UnAuthenticatedException. - var pkamResult = - await atLookUp.pkamAuthenticate(enrollmentId: enrollmentId); - logger.finer( - '_attemptPkamAuth: atLookUp.pkamAuthenticate returned $pkamResult'); - - return pkamResult; - } - ///write newly created encryption keypairs into atKeys file Future _generateAtKeysFile( at_auth.AtAuthKeys atAuthKeys, {