diff --git a/packages/at_secondary_server/lib/src/verb/handler/abstract_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/abstract_verb_handler.dart index 12b943118..e687c949e 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/abstract_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/abstract_verb_handler.dart @@ -274,39 +274,43 @@ abstract class AbstractVerbHandler implements VerbHandler { if (otp == 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(otp, 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 (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(otp, 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}'; } + AtData? otpAtData; try { otpAtData ??= await keyStore.get(otpKey); diff --git a/packages/at_secondary_server/lib/src/verb/handler/otp_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/otp_verb_handler.dart index b73690d69..68f6e35bc 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/otp_verb_handler.dart +++ b/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 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 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() { diff --git a/packages/at_secondary_server/test/monitor_verb_test.dart b/packages/at_secondary_server/test/monitor_verb_test.dart index a1c66b5d4..73e12fd63 100644 --- a/packages/at_secondary_server/test/monitor_verb_test.dart +++ b/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 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'})); diff --git a/packages/at_secondary_server/test/otp_verb_test.dart b/packages/at_secondary_server/test/otp_verb_test.dart index fdbb85c4e..c22b18c08 100644 --- a/packages/at_secondary_server/test/otp_verb_test.dart +++ b/packages/at_secondary_server/test/otp_verb_test.dart @@ -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 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()); }); @@ -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'; @@ -244,6 +246,42 @@ void main() { expect(await otpVerbHandler.isOTPValid(passcode), true); }); + test('set spp with a ttl, check isOTPValid before ttl expires', () async { + String passcode = 'SppWithTtl50'; + Response response = Response(); + HashMap 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.isOTPValid(passcode), true); + // Adding expect again to ensure the Semi-permanent passcodes are not deleted + // after one time use. + expect(await otpVerbHandler.isOTPValid(passcode), true); + }); + + test('set spp with a ttl, check isOTPValid before and after ttl expires', + () async { + String passcode = 'SppWithTtl50'; + Response response = Response(); + HashMap 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.isOTPValid(passcode), true); + // Adding expect again to ensure the Semi-permanent passcodes are not deleted + // after one time use. + expect(await otpVerbHandler.isOTPValid(passcode), true); + + await Future.delayed(Duration(milliseconds: 51)); + expect(await otpVerbHandler.isOTPValid(passcode), false); + }); + test('A test to verify pass code can be updated', () async { String passcode = 'abc123'; Response response = Response();