Merge pull request #1968 from atsign-foundation/feat/set-ttl-for-spp

packages/at_secondary_server/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 3.0.47
 - feat: Introduced a dedicated namespace for storing OTPs
+- feat: allow a ttl to be set for a semi-permanent passcode (spp)
 
 ## 3.0.46
 - fix: Default OTP expiry value remains unchanged for the subsequent "otp:" requests

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

@@ -270,43 +270,47 @@ abstract class AbstractVerbHandler implements VerbHandler {
   ///
   /// Additionally, this function removes the OTP from the keystore to prevent
   /// its reuse.
-  Future<bool> isOTPValid(String? otp) async {
-    if (otp == null) {
+  Future<bool> isPasscodeValid(String? passcode) async {
+    if (passcode == null) {
       return false;
     }
-    // Check if user has configured SPP(Semi-Permanent Pass-code).
+    // 1. Check if user has configured an SPP(Semi-Permanent Pass-code).
     // If SPP key is available, check if the otp sent is a valid pass code.
     // If yes, return true, else check it is a valid OTP.
-    String sppKey =
-        'private:spp.${OtpVerbHandler.otpNamespace}${AtSecondaryServerImpl.getInstance().currentAtSign}';
-    if (!keyStore.isKeyExists(sppKey)) {
+    String passcodeKey = OtpVerbHandler.passcodeKey(passcode, isSpp: true);
+    if (!keyStore.isKeyExists(passcodeKey)) {
       // if new SPPKey does not exist in keystore, check for SPP data against legacy SPP key
       // New SPP key has __otp namespace, legacy key does NOT have any namespace
-      sppKey =
+      passcodeKey =
           'private:spp${AtSecondaryServerImpl.getInstance().currentAtSign}';
     }
     try {
-      AtData? sppAtData = await keyStore.get(sppKey);
+      AtData? sppAtData = await keyStore.get(passcodeKey);
       // SPP has a special key so we have to check the value that was stored
       // (which is the actual SPP)
       // By comparison, OTPs are stored with the key being ${OTP}.__otp@alice
       // i.e. the OTP is part of the key, and the stored data is irrelevant
-      if (sppAtData?.data?.toLowerCase() == otp.toLowerCase()) {
-        return true;
+      if (sppAtData?.data?.toLowerCase() == passcode.toLowerCase()) {
+        if (SecondaryUtil.isActiveKey(sppAtData)) {
+          return true;
+        } else {
+          logger.finest(
+              'SPP found in KeyStore but has expired. Validating as OTP');
+        }
       }
     } on KeyNotFoundException {
       logger.finest('No SPP found in KeyStore. Validating as OTP');
     }
 
-    // If not a valid SPP, then check against OTP keys
-    String otpKey =
-        'private:${otp.toLowerCase()}.${OtpVerbHandler.otpNamespace}${AtSecondaryServerImpl.getInstance().currentAtSign}';
+    // 2. If not a valid SPP, then check against OTP keys
+    String otpKey = OtpVerbHandler.passcodeKey(passcode, isSpp: false);
     if (!keyStore.isKeyExists(otpKey)) {
       // if new OTPKey does not exist in keystore, check for OTP data against legacy OTPKey
       // New OTP key has __otp namespace, legacy key does not have namespace
       otpKey =
-          'private:${otp.toLowerCase()}${AtSecondaryServerImpl.getInstance().currentAtSign}';
+          'private:${passcode.toLowerCase()}${AtSecondaryServerImpl.getInstance().currentAtSign}';
     }
+
     AtData? otpAtData;
     try {
       otpAtData ??= await keyStore.get(otpKey);

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

@@ -191,7 +191,7 @@ class EnrollVerbHandler extends AbstractVerbHandler {
     // OTP is sent only in enrollment request which is submitted on
     // unauthenticated connection.
     if (atConnection.metaData.isAuthenticated == false) {
-      var isValid = await isOTPValid(enrollParams.otp);
+      var isValid = await isPasscodeValid(enrollParams.otp);
       if (!isValid) {
         _lastInvalidOtpReceivedInMills =
             DateTime.now().toUtc().millisecondsSinceEpoch;

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

@@ -36,17 +36,15 @@ class OtpVerbHandler extends AbstractVerbHandler {
       InboundConnection atConnection) async {
     final operation = verbParams['operation'];
     if (!atConnection.metaData.isAuthenticated) {
-      throw UnAuthenticatedException(
-          'otp:get requires authenticated connection');
+      throw UnAuthenticatedException('otp: requires authenticated connection');
     }
     switch (operation) {
       case 'get':
         String otp = generateOTP();
         // Extract the ttl from the verb parameters if supplied, or use the default value.
-        int otpExpiryInMillis =
-            int.tryParse(verbParams[AtConstants.ttl] ?? '') ??
-                defaultOtpExpiry.inMilliseconds;
-        await saveOTP(otp, otpExpiryInMillis);
+        int otpTtl = int.tryParse(verbParams[AtConstants.ttl] ?? '') ??
+            defaultOtpExpiry.inMilliseconds;
+        await savePasscode(otp, ttl: otpTtl, isSpp: false);
         response.data = otp;
         break;
       case 'put':
@@ -57,10 +55,12 @@ class OtpVerbHandler extends AbstractVerbHandler {
           throw InvalidRequestException(
               'Client not allowed to not store semi permanent pass code');
         }
-        String? otp = verbParams['otp'];
-        await keyStore.put(
-            'private:spp.$otpNamespace${AtSecondaryServerImpl.getInstance().currentAtSign}',
-            AtData()..data = otp);
+        int sppTtl = int.tryParse(verbParams[AtConstants.ttl] ?? '') ?? -1;
+        String? spp = verbParams['otp'];
+        if (spp == null) {
+          throw InvalidRequestException('otp:put requires a passcode');
+        }
+        await savePasscode(spp, ttl: sppTtl, isSpp: true);
         response.data = 'ok';
         break;
       default:
@@ -100,12 +100,21 @@ class OtpVerbHandler extends AbstractVerbHandler {
     return otp;
   }
 
-  Future<void> saveOTP(String otp, int otpExpiryInMillis) async {
+  static String passcodeKey(String passcode, {required bool isSpp}) {
+    return isSpp
+        ? 'private:spp.$otpNamespace'
+            '${AtSecondaryServerImpl.getInstance().currentAtSign}'
+        : 'private:${passcode.toLowerCase()}.$otpNamespace'
+            '${AtSecondaryServerImpl.getInstance().currentAtSign}';
+  }
+
+  Future<void> savePasscode(String passcode,
+      {required int ttl, required bool isSpp}) async {
     await keyStore.put(
-        'private:$otp.$otpNamespace${AtSecondaryServerImpl.getInstance().currentAtSign}',
+        passcodeKey(passcode, isSpp: isSpp),
         AtData()
-          ..data = otp
-          ..metaData = (AtMetaData()..ttl = otpExpiryInMillis));
+          ..data = passcode
+          ..metaData = (AtMetaData()..ttl = ttl));
   }
 
   String _generateRandomAlphabet() {

packages/at_secondary_server/test/abstract_verb_handler_test.dart

@@ -97,7 +97,7 @@ class TestUpdateVerbHandler extends AbstractVerbHandler {
   }
 
   @override
-  Future<bool> isOTPValid(String? otp) {
+  Future<bool> isPasscodeValid(String? otp) {
     // TODO: implement isOTPValid
     throw UnimplementedError();
   }

packages/at_secondary_server/test/enroll_verb_test.dart

@@ -108,7 +108,7 @@ void main() {
           response, enrollmentRequestVerbParams, inboundConnection);
       String enrollmentId = jsonDecode(response.data!)['enrollmentId'];
       expect(enrollmentId, isNotNull);
-      expect(await enrollVerbHandler.isOTPValid(otp), false);
+      expect(await enrollVerbHandler.isPasscodeValid(otp), false);
     });
     tearDown(() async => await verbTestsTearDown());
   });

packages/at_secondary_server/test/monitor_verb_test.dart

@@ -11,7 +11,6 @@ import 'package:at_secondary/src/verb/handler/enroll_verb_handler.dart';
 import 'package:at_secondary/src/verb/handler/monitor_verb_handler.dart';
 import 'package:at_secondary/src/verb/handler/otp_verb_handler.dart';
 import 'package:at_server_spec/at_server_spec.dart';
-import 'package:test/expect.dart';
 import 'package:test/test.dart';
 import 'package:uuid/uuid.dart';
 
@@ -351,7 +350,7 @@ void main() {
         {required bool autoApprove}) async {
       OtpVerbHandler otpVH = OtpVerbHandler(secondaryKeyStore);
       String otp = otpVH.generateOTP();
-      await otpVH.saveOTP(otp, 5000);
+      await otpVH.savePasscode(otp, ttl: 5000, isSpp: false);
 
       EnrollVerbHandler enrollVerbHandler =
           EnrollVerbHandler(secondaryKeyStore);
@@ -361,7 +360,7 @@ void main() {
           ',"deviceName":"$deviceName"'
           ',"namespaces":${jsonEncode(namespaces)}'
           ',"apkamPublicKey":"dummy_apkam_public_key"'
-          ',"encryptedAPKAMSymmetricKey":"dummy_encrypted_apkam_symm_key"'
+          ',"encryptedAPKAMSymmetricKey":"dummy_encrypted_apkam_symmetric_key"'
           '}';
       HashMap<String, String?> enrollmentRequestVerbParams =
           getVerbParam(VerbSyntax.enroll, enrollmentRequest);
@@ -480,9 +479,9 @@ void main() {
       final valueJson = jsonDecode(notificationJson['value']);
       //TODO remove encryptedApkamSymmetricKey in the future
       expect(valueJson['encryptedApkamSymmetricKey'],
-          'dummy_encrypted_apkam_symm_key');
+          'dummy_encrypted_apkam_symmetric_key');
       expect(valueJson['encryptedAPKAMSymmetricKey'],
-          'dummy_encrypted_apkam_symm_key');
+          'dummy_encrypted_apkam_symmetric_key');
       expect(valueJson['appName'], 'mvt_app_2');
       expect(valueJson['deviceName'], 'mvt_dev_2');
       expect(valueJson['namespace'], equals({'app_2_namespace': 'rw'}));

packages/at_secondary_server/test/otp_verb_test.dart

@@ -90,7 +90,7 @@ void main() {
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
       String? otp = response.data;
-      expect(await otpVerbHandler.isOTPValid(otp), true);
+      expect(await otpVerbHandler.isPasscodeValid(otp), true);
     });
 
     test('A test to verify otp:get with TTL set expires after the TTL is met',
@@ -103,7 +103,7 @@ void main() {
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
       String? otp = response.data;
       await Future.delayed(Duration(seconds: 1));
-      expect(await otpVerbHandler.isOTPValid(otp), false);
+      expect(await otpVerbHandler.isPasscodeValid(otp), false);
     });
     tearDown(() async => await verbTestsTearDown());
   });
@@ -113,7 +113,7 @@ void main() {
       await verbTestsSetUp();
     });
     test(
-        'A test to verify UnAuthorizedException is thrown when opt verb is executed on an unauthenticated conn',
+        'A test to verify UnAuthorizedException is thrown when opt:get is executed on an unauthenticated conn',
         () {
       Response response = Response();
       HashMap<String, String?> verbParams =
@@ -124,8 +124,9 @@ void main() {
               response, verbParams, inboundConnection),
           throwsA(predicate((e) =>
               e is UnAuthenticatedException &&
-              e.message == 'otp:get requires authenticated connection')));
+              e.message == 'otp: requires authenticated connection')));
     });
+
     tearDown(() async => await verbTestsTearDown());
   });
 
@@ -141,7 +142,7 @@ void main() {
       inboundConnection.metaData.isAuthenticated = true;
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
-      expect(await otpVerbHandler.isOTPValid(response.data), true);
+      expect(await otpVerbHandler.isPasscodeValid(response.data), true);
     });
 
     test('A test to verify otp:validate returns invalid when OTP is expired',
@@ -154,7 +155,7 @@ void main() {
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
       String? otp = response.data;
       await Future.delayed(Duration(milliseconds: 2));
-      expect(await otpVerbHandler.isOTPValid(otp), false);
+      expect(await otpVerbHandler.isPasscodeValid(otp), false);
     });
 
     test('A test to verify default otp expiry not overwritten', () async {
@@ -182,7 +183,7 @@ void main() {
         () async {
       String otp = 'ABC123';
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
-      expect(await otpVerbHandler.isOTPValid(otp), false);
+      expect(await otpVerbHandler.isPasscodeValid(otp), false);
     });
 
     test('A test to verify otp is removed from the keystore after use',
@@ -194,8 +195,8 @@ void main() {
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
       String? otp = response.data;
-      expect(await otpVerbHandler.isOTPValid(otp), true);
-      expect(await otpVerbHandler.isOTPValid(otp), false);
+      expect(await otpVerbHandler.isPasscodeValid(otp), true);
+      expect(await otpVerbHandler.isPasscodeValid(otp), false);
     });
 
     test('validate backwards compatability with legacy otp key', () async {
@@ -209,7 +210,7 @@ void main() {
       await secondaryKeyStore.put(otpLegacyKey, value);
 
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
-      expect(await otpVerbHandler.isOTPValid(testOtp), true);
+      expect(await otpVerbHandler.isPasscodeValid(testOtp), true);
     });
     tearDown(() async => await verbTestsTearDown());
   });
@@ -227,6 +228,7 @@ void main() {
       await secondaryKeyStore.put(
           enrollmentKey, AtData()..data = jsonEncode(enrollDataStoreValue));
     });
+
     test('A test to set a pass code and verify isOTPValid returns true',
         () async {
       String passcode = 'abc123';
@@ -238,10 +240,65 @@ void main() {
           enrollmentId;
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
-      expect(await otpVerbHandler.isOTPValid(passcode), true);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+      // Adding expect again to ensure the Semi-permanent passcodes are not deleted
+      // after one time use.
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+    });
+
+    test('set spp with a ttl, check isOTPValid before ttl expires', () async {
+      String passcode = 'SppWithTtl50';
+      Response response = Response();
+      HashMap<String, String?> verbParams =
+          getVerbParam(VerbSyntax.otp, 'otp:put:$passcode:ttl:50');
+      inboundConnection.metaData.isAuthenticated = true;
+      (inboundConnection.metaData as InboundConnectionMetadata).enrollmentId =
+          enrollmentId;
+      OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
+      await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+      // Adding expect again to ensure the Semi-permanent passcodes are not deleted
+      // after one time use.
+      await Future.delayed(Duration(milliseconds: 10));
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+    });
+
+    test('set spp with a ttl, check isOTPValid before and after ttl expires',
+        () async {
+      String passcode = 'SppWithTtl50';
+      Response response = Response();
+      HashMap<String, String?> verbParams =
+          getVerbParam(VerbSyntax.otp, 'otp:put:$passcode:ttl:50');
+      inboundConnection.metaData.isAuthenticated = true;
+      (inboundConnection.metaData as InboundConnectionMetadata).enrollmentId =
+          enrollmentId;
+      OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
+      await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
       // Adding expect again to ensure the Semi-permanent passcodes are not deleted
       // after one time use.
-      expect(await otpVerbHandler.isOTPValid(passcode), true);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+
+      await Future.delayed(Duration(milliseconds: 51));
+      expect(await otpVerbHandler.isPasscodeValid(passcode), false);
+    });
+
+    test('set spp without a ttl, verify its ttl has been set to -1', () async {
+      String passcode = 'SppWithoutTtl';
+      Response response = Response();
+      HashMap<String, String?> verbParams =
+          getVerbParam(VerbSyntax.otp, 'otp:put:$passcode');
+      inboundConnection.metaData.isAuthenticated = true;
+      (inboundConnection.metaData as InboundConnectionMetadata).enrollmentId =
+          enrollmentId;
+      OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
+      await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
+      await Future.delayed(Duration(milliseconds: 2));
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
+
+      String sppKey = OtpVerbHandler.passcodeKey(passcode, isSpp: true);
+      var atData = await secondaryKeyStore.get(sppKey);
+      expect(atData?.metaData?.ttl, -1);
     });
 
     test('A test to verify pass code can be updated', () async {
@@ -254,14 +311,14 @@ void main() {
           enrollmentId;
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
-      expect(await otpVerbHandler.isOTPValid(passcode), true);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
       // Update the pass-code
       passcode = 'xyz987';
       response = Response();
       verbParams = getVerbParam(VerbSyntax.otp, 'otp:put:$passcode');
       otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
       await otpVerbHandler.processVerb(response, verbParams, inboundConnection);
-      expect(await otpVerbHandler.isOTPValid(passcode), true);
+      expect(await otpVerbHandler.isPasscodeValid(passcode), true);
     });
 
     test('validate backwards compatability with legacy ssp key', () async {
@@ -272,7 +329,7 @@ void main() {
       await secondaryKeyStore.put(otpLegacyKey, value);
 
       OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore);
-      expect(await otpVerbHandler.isOTPValid(testOtp), true);
+      expect(await otpVerbHandler.isPasscodeValid(testOtp), true);
     });
     tearDown(() async => await verbTestsTearDown());
   });