feat: Persist OTPs in keystore and prevent reuse of OTPs

packages/at_secondary_server/CHANGELOG.md

@@ -2,6 +2,8 @@
 - fix: Implement notify ephemeral changes - Send notification with value without caching the key on receiver's secondary server
 - feat: Implement AtRateLimiter to limit the enrollment requests on a particular connection
 - fix: Upgraded at_commons to 3.0.56
+- fix: Enable client to set OTP expiry via OTP verb
+- fix: Prevent reuse of OTP
 - fix: Modify sync_progressive_verb_handler to filter responses on enrolled namespaces if authenticated via APKAM 
 ## 3.0.35
 - chore: Upgraded at_persistence_secondary_server to 3.0.57 for memory optimization in commit log

packages/at_secondary_server/lib/src/verb/handler/abstract_verb_handler.dart

@@ -152,4 +152,25 @@ abstract class AbstractVerbHandler implements VerbHandler {
       return false;
     }
   }
+
+
+  /// This function checks the validity of a provided OTP.
+  /// It returns true if the OTP is valid; otherwise, it returns false.
+  /// If the OTP is not found in the keystore, it also returns false.
+  ///
+  /// Additionally, this function removes the OTP from the keystore to prevent its reuse.
+  Future<bool> isOTPValid(String? otp) async {
+    if (otp == null) {
+      return false;
+    }
+    String otpKey =
+        'private:${otp.toLowerCase()}${AtSecondaryServerImpl.getInstance().currentAtSign}';
+    AtData otpAtData;
+    try {
+      otpAtData = await keyStore.get(otpKey);
+    } on KeyNotFoundException {
+      return false;
+    }
+    return SecondaryUtil.isActiveKey(otpAtData);
+  }
 }

packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart

@@ -8,7 +8,6 @@ import 'package:at_secondary/src/server/at_secondary_config.dart';
 import 'package:at_secondary/src/constants/enroll_constants.dart';
 import 'package:at_secondary/src/enroll/enroll_datastore_value.dart';
 import 'package:at_secondary/src/utils/notification_util.dart';
-import 'package:at_secondary/src/verb/handler/otp_verb_handler.dart';
 import 'package:at_server_spec/at_server_spec.dart';
 import 'package:at_server_spec/at_verb_spec.dart';
 import 'package:meta/meta.dart';
@@ -50,15 +49,15 @@ class EnrollVerbHandler extends AbstractVerbHandler {
     try {
       // Ensure that enrollParams are present for all enroll operation
       // Exclude operation 'list' which does not have enrollParams
-      if (verbParams[enrollParams] == null) {
+      if (verbParams[AtConstants.enrollParams] == null) {
         if (operation != 'list') {
           logger.severe(
-              'Enroll params is empty | EnrollParams: ${verbParams[enrollParams]}');
+              'Enroll params is empty | EnrollParams: ${verbParams[AtConstants.enrollParams]}');
           throw IllegalArgumentException('Enroll parameters not provided');
         }
       } else {
-        enrollVerbParams =
-            EnrollParams.fromJson(jsonDecode(verbParams[enrollParams]!));
+        enrollVerbParams = EnrollParams.fromJson(
+            jsonDecode(verbParams[AtConstants.enrollParams]!));
       }
       switch (operation) {
         case 'request':
@@ -116,14 +115,17 @@ class EnrollVerbHandler extends AbstractVerbHandler {
       throw AtThrottleLimitExceeded(
           'Enrollment requests have exceeded the limit within the specified time frame');
     }
-    if (!atConnection.getMetaData().isAuthenticated) {
-      var otp = enrollParams.otp;
-      if (otp == null ||
-          (await OtpVerbHandler.cache.get(otp.toString()) == null)) {
+
+    // OTP is sent only in enrollment request which is submitted on
+    // unauthenticated connection.
+    if (atConnection.getMetaData().isAuthenticated == false) {
+      var isValid = await isOTPValid(enrollParams.otp);
+      if (!isValid) {
         throw AtEnrollmentException(
             'invalid otp. Cannot process enroll request');
       }
     }
+
     var enrollNamespaces = enrollParams.namespaces ?? {};
     var newEnrollmentId = Uuid().v4();
     var key =
@@ -154,8 +156,8 @@ class EnrollVerbHandler extends AbstractVerbHandler {
       await _storeEncryptionKeys(newEnrollmentId, enrollParams, currentAtSign);
       // store this apkam as default pkam public key for old clients
       // The keys with AT_PKAM_PUBLIC_KEY does not sync to client.
-      await keyStore.put(
-          AT_PKAM_PUBLIC_KEY, AtData()..data = enrollParams.apkamPublicKey!);
+      await keyStore.put(AtConstants.atPkamPublicKey,
+          AtData()..data = enrollParams.apkamPublicKey!);
       enrollData = AtData()..data = jsonEncode(enrollmentValue.toJson());
     } else {
       enrollmentValue.approval = EnrollApproval(EnrollStatus.pending.name);
@@ -171,6 +173,9 @@ class EnrollVerbHandler extends AbstractVerbHandler {
     }
     logger.finer('enrollData: $enrollData');
     await keyStore.put('$key$currentAtSign', enrollData, skipCommit: true);
+    // Remove the OTP from keystore to prevent reuse.
+    await keyStore.remove(
+        'private:${enrollParams.otp?.toLowerCase()}${AtSecondaryServerImpl.getInstance().currentAtSign}');
   }
 
   /// Handles enrollment approve, deny and revoke requests.
@@ -224,10 +229,6 @@ class EnrollVerbHandler extends AbstractVerbHandler {
     // If an enrollment is approved, we need the enrollment to be active
     // to subsequently revoke the enrollment. Hence reset TTL and
     // expiredAt on metadata.
-    /* TODO: Currently TTL is reset on all the enrollments.
-        However, if the enrollment state is denied or revoked,
-        unless we wanted to display denied or revoked enrollments in the UI,
-        we can let the TTL be, so that the enrollment will be deleted subsequently.*/
     await _updateEnrollmentValueAndResetTTL(
         '$enrollmentKey$currentAtSign', enrollDataStoreValue);
     // when enrollment is approved store the apkamPublicKey of the enrollment
@@ -252,13 +253,13 @@ class EnrollVerbHandler extends AbstractVerbHandler {
     var privKeyJson = {};
     privKeyJson['value'] = enrollParams.encryptedDefaultEncryptedPrivateKey;
     await keyStore.put(
-        '$newEnrollmentId.$defaultEncryptionPrivateKey.$enrollManageNamespace$atSign',
+        '$newEnrollmentId.${AtConstants.defaultEncryptionPrivateKey}.$enrollManageNamespace$atSign',
         AtData()..data = jsonEncode(privKeyJson),
         skipCommit: true);
     var selfKeyJson = {};
     selfKeyJson['value'] = enrollParams.encryptedDefaultSelfEncryptionKey;
     await keyStore.put(
-        '$newEnrollmentId.$defaultSelfEncryptionKey.$enrollManageNamespace$atSign',
+        '$newEnrollmentId.${AtConstants.defaultSelfEncryptionKey}.$enrollManageNamespace$atSign',
         AtData()..data = jsonEncode(selfKeyJson),
         skipCommit: true);
   }
@@ -338,7 +339,7 @@ class EnrollVerbHandler extends AbstractVerbHandler {
       String key, EnrollParams enrollParams, String atSign) async {
     try {
       var notificationValue = {};
-      notificationValue[apkamEncryptedSymmetricKey] =
+      notificationValue[AtConstants.apkamEncryptedSymmetricKey] =
           enrollParams.encryptedAPKAMSymmetricKey;
       logger.finer('notificationValue:$notificationValue');
       final atNotification = (AtNotificationBuilder()
@@ -355,10 +356,10 @@ class EnrollVerbHandler extends AbstractVerbHandler {
       logger.finer('notification generated: $notificationId');
     } on Exception catch (e, trace) {
       logger.severe(
-          'Exception while storing notification key $enrollmentId. Exception $e. Trace $trace');
+          'Exception while storing notification key ${AtConstants.enrollmentId}. Exception $e. Trace $trace');
     } on Error catch (e, trace) {
       logger.severe(
-          'Error while storing notification key $enrollmentId. Error $e. Trace $trace');
+          'Error while storing notification key ${AtConstants.enrollmentId}. Error $e. Trace $trace');
     }
   }
 

packages/at_secondary_server/lib/src/verb/handler/otp_verb_handler.dart

@@ -1,24 +1,24 @@
 import 'dart:collection';
 import 'dart:math';
 import 'package:at_commons/at_commons.dart';
+import 'package:at_secondary/src/server/at_secondary_impl.dart';
 import 'package:at_server_spec/at_server_spec.dart';
+import 'package:meta/meta.dart';
 import 'package:uuid/uuid.dart';
 import 'abstract_verb_handler.dart';
 import 'package:at_server_spec/at_verb_spec.dart';
 import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart';
-import 'package:expire_cache/expire_cache.dart';
 
 class OtpVerbHandler extends AbstractVerbHandler {
   static Otp otpVerb = Otp();
-  static final expireDuration = Duration(seconds: 90);
-  static ExpireCache<String, String> cache =
-      ExpireCache<String, String>(expireDuration: expireDuration);
+
+  @visibleForTesting
+  int otpExpiryInMills = Duration(minutes: 5).inMilliseconds;
 
   OtpVerbHandler(SecondaryKeyStore keyStore) : super(keyStore);
 
   @override
-  bool accept(String command) =>
-      command == 'otp:get' || command.startsWith('otp:validate');
+  bool accept(String command) => command == 'otp:get';
 
   @override
   Verb getVerb() => otpVerb;
@@ -29,6 +29,10 @@ class OtpVerbHandler extends AbstractVerbHandler {
       HashMap<String, String?> verbParams,
       InboundConnection atConnection) async {
     final operation = verbParams['operation'];
+    if (verbParams[AtConstants.ttl] != null &&
+        verbParams[AtConstants.ttl]!.isNotEmpty) {
+      otpExpiryInMills = int.parse(verbParams[AtConstants.ttl]!);
+    }
     switch (operation) {
       case 'get':
         if (!atConnection.getMetaData().isAuthenticated) {
@@ -40,16 +44,15 @@ class OtpVerbHandler extends AbstractVerbHandler {
         }
         // If OTP generated do not have digits, generate again.
         while (RegExp(r'\d').hasMatch(response.data!) == false);
-        await cache.set(response.data!, response.data!);
-        break;
-      case 'validate':
-        String? otp = verbParams['otp'];
-        if (otp != null && (await cache.get(otp)) == otp) {
-          response.data = 'valid';
-        } else {
-          response.data = 'invalid';
-        }
+        await keyStore.put(
+            'private:${response.data}${AtSecondaryServerImpl.getInstance().currentAtSign}',
+            AtData()
+              ..data =
+                  '${DateTime.now().toUtc().add(Duration(milliseconds: otpExpiryInMills)).millisecondsSinceEpoch}'
+              ..metaData = (AtMetaData()..ttl = otpExpiryInMills));
         break;
+      default:
+        throw InvalidSyntaxException('$operation is not a valid operation');
     }
   }
 

packages/at_secondary_server/pubspec.yaml

@@ -19,19 +19,18 @@ dependencies:
   basic_utils: 5.6.1
   ecdsa: 0.0.4
   encrypt: 5.0.3
-  at_commons: 3.0.56
+  at_commons: 3.0.57
   at_utils: 3.0.15
   at_chops: 1.0.4
   at_lookup: 3.0.40
   at_server_spec: 3.0.15
   at_persistence_spec: 2.0.14
   at_persistence_secondary_server: 3.0.58
-  expire_cache: ^2.0.1
   intl: ^0.18.1
   json_annotation: ^4.8.0
   version: 3.0.2
   meta: 1.10.0
-  mutex: 3.0.1
+  mutex: 3.1.0
   yaml: 3.1.2
   logging: 1.2.0
 

packages/at_secondary_server/test/enroll_verb_test.dart

@@ -41,7 +41,6 @@ void main() {
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(
           response, otpVerbParams, inboundConnection);
-      print('OTP: ${response.data}');
       // Enroll request 2
       enrollmentRequest =
           'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"buzz":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}';
@@ -81,6 +80,31 @@ void main() {
       expect(enrollmentValue.namespaces.containsKey('__manage'), true);
       expect(enrollmentValue.namespaces.containsKey('*'), true);
     });
+
+    test('A test to verify OTP is deleted once it is used to submit an enrollment',() async {
+      Response response = Response();
+      // OTP Verb
+      inboundConnection.getMetaData().isAuthenticated = true;
+      inboundConnection.getMetaData().sessionID = 'dummy_session';
+      HashMap<String, String?> otpVerbParams =
+      getVerbParam(VerbSyntax.otp, 'otp:get');
+      OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
+      await otpVerbHandler.processVerb(
+          response, otpVerbParams, inboundConnection);
+      String otp = response.data!;
+
+      String enrollmentRequest =
+      'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"buzz":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}';
+      HashMap<String, String?> enrollmentRequestVerbParams =
+          getVerbParam(VerbSyntax.enroll, enrollmentRequest);
+      inboundConnection.getMetaData().isAuthenticated = false;
+      EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore);
+      await enrollVerbHandler.processVerb(
+          response, enrollmentRequestVerbParams, inboundConnection);
+      String enrollmentId = jsonDecode(response.data!)['enrollmentId'];
+      expect(enrollmentId, isNotNull);
+      expect(await enrollVerbHandler.isOTPValid(otp), false);
+    });
     tearDown(() async => await verbTestsTearDown());
   });
   group('A group of tests to verify enroll list operation', () {
@@ -316,7 +340,7 @@ void main() {
     test('A test to verify enrollment request without otp throws exception',
         () async {
       String enrollmentRequest =
-          'enroll:request:{"appname":"wavi","devicename":"mydevice","namespaces":{"wavi":"r"},"apkampublickey":"dummy_apkam_public_key"}';
+          'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"apkamPublicKey":"dummy_apkam_public_key"}';
       HashMap<String, String?> verbParams =
           getVerbParam(VerbSyntax.enroll, enrollmentRequest);
       inboundConnection.getMetaData().isAuthenticated = false;