From 08f2f68a04e4d1aeeb2f1dcf1f79232375ce44f7 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Tue, 5 Sep 2023 14:35:16 +0530 Subject: [PATCH 01/21] fix: new field 'expiresAt' in EnrollDataStoreValue --- .../src/enroll/enroll_datastore_value.dart | 13 ++- .../src/enroll/enroll_datastore_value.g.dart | 4 +- .../lib/src/server/at_secondary_config.dart | 5 + .../src/verb/handler/enroll_verb_handler.dart | 97 +++++++++++++------ .../src/verb/handler/pkam_verb_handler.dart | 12 ++- 5 files changed, 99 insertions(+), 32 deletions(-) diff --git a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart index ea72ab422..a765af8ca 100644 --- a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart +++ b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart @@ -14,6 +14,7 @@ class EnrollDataStoreValue { late String apkamPublicKey; EnrollRequestType? requestType; EnrollApproval? approval; + DateTime? expiresAt; EnrollDataStoreValue( this.sessionId, this.appName, this.deviceName, this.apkamPublicKey); @@ -21,12 +22,22 @@ class EnrollDataStoreValue { _$EnrollDataStoreValueFromJson(json); Map toJson() => _$EnrollDataStoreValueToJson(this); + + bool isExpired() { + if (expiresAt != null && DateTime.now().toUtc().isAfter(expiresAt!)) { + return true; + } + return false; + } } class EnrollApproval { String state; + EnrollApproval(this.state); + EnrollApproval.fromJson(Map json) : state = json['state']; + Map toJson() => { 'state': state, }; @@ -37,6 +48,6 @@ class EnrollApproval { } } -enum EnrollStatus { pending, approved, denied, revoked } +enum EnrollStatus { pending, approved, denied, revoked, expired } enum EnrollRequestType { newEnrollment, changeEnrollment } diff --git a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart index be6ba5244..617932290 100644 --- a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart +++ b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart @@ -19,7 +19,8 @@ EnrollDataStoreValue _$EnrollDataStoreValueFromJson( $enumDecodeNullable(_$EnrollRequestTypeEnumMap, json['requestType']) ..approval = json['approval'] == null ? null - : EnrollApproval.fromJson(json['approval'] as Map); + : EnrollApproval.fromJson(json['approval'] as Map) + ..expiresAt = DateTime.parse(json['expiresAt']); Map _$EnrollDataStoreValueToJson( EnrollDataStoreValue instance) => @@ -31,6 +32,7 @@ Map _$EnrollDataStoreValueToJson( 'apkamPublicKey': instance.apkamPublicKey, 'requestType': _$EnrollRequestTypeEnumMap[instance.requestType], 'approval': instance.approval, + 'expiresAt': instance.expiresAt.toString() }; const _$EnrollRequestTypeEnumMap = { diff --git a/packages/at_secondary_server/lib/src/server/at_secondary_config.dart b/packages/at_secondary_server/lib/src/server/at_secondary_config.dart index bdee83da5..f82607a96 100644 --- a/packages/at_secondary_server/lib/src/server/at_secondary_config.dart +++ b/packages/at_secondary_server/lib/src/server/at_secondary_config.dart @@ -121,7 +121,10 @@ class AtSecondaryConfig { ? ConfigUtil.getPubspecConfig()!['version'] : null; + // specifies the time after which an enrollment is considered expired static final int _enrollmentExpiryInHours = 48; + // time after which the enrollment key is actually deleted from the keystore + static final int _enrollmentKeyTtlInDays = 14; static final Map _envVars = Platform.environment; @@ -129,6 +132,8 @@ class AtSecondaryConfig { static int get enrollmentExpiryInHours => _enrollmentExpiryInHours; + static int get enrollmentKeyTtlInDays => _enrollmentKeyTtlInDays; + // TODO: Medium priority: Most (all?) getters in this class return a default value but the signatures currently // allow for nulls. Should fix this as has been done for logLevel // TODO: Low priority: Lots of very similar boilerplate code here. Not necessarily bad in this particular case, but diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 898be210c..f2e3b4cc4 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -3,7 +3,6 @@ import 'dart:collection'; import 'dart:convert'; import 'package:at_commons/at_commons.dart'; import 'package:at_secondary/src/connection/inbound/inbound_connection_metadata.dart'; -import 'package:at_secondary/src/server/at_secondary_config.dart'; import 'package:at_secondary/src/server/at_secondary_impl.dart'; import 'package:at_secondary/src/constants/enroll_constants.dart'; import 'package:at_secondary/src/enroll/enroll_datastore_value.dart'; @@ -14,6 +13,7 @@ import 'package:at_server_spec/at_server_spec.dart'; import 'package:at_server_spec/at_verb_spec.dart'; import 'package:meta/meta.dart'; import 'package:uuid/uuid.dart'; +import '../../server/at_secondary_config.dart'; import 'abstract_verb_handler.dart'; import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart'; @@ -30,8 +30,10 @@ class EnrollVerbHandler extends AbstractVerbHandler { Verb getVerb() => enrollVerb; @visibleForTesting - int enrollmentExpiryInMills = - Duration(hours: AtSecondaryConfig.enrollmentExpiryInHours).inMilliseconds; + Duration enrollmentExpiry = Duration(seconds: 1); + + int enrollKeyDeletionAfterMillis = + Duration(days: AtSecondaryConfig.enrollmentKeyTtlInDays).inMilliseconds; @override Future processVerb( @@ -43,12 +45,12 @@ class EnrollVerbHandler extends AbstractVerbHandler { final operation = verbParams['operation']; final currentAtSign = AtSecondaryServerImpl.getInstance().currentAtSign; //Approve, deny, revoke or list enrollments only on authenticated connections - if (operation != 'request' && !atConnection.getMetaData().isAuthenticated) { - throw UnAuthenticatedException( - 'Cannot $operation enrollment without authentication'); - } + // if (operation != 'request' && !atConnection.getMetaData().isAuthenticated) { + // throw UnAuthenticatedException( + // 'Cannot $operation enrollment without authentication'); + // } try { - var enrollVerbParams; + EnrollParams? enrollVerbParams; if (verbParams[enrollParams] != null) { enrollVerbParams = EnrollParams.fromJson(jsonDecode(verbParams[enrollParams]!)); @@ -56,14 +58,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { switch (operation) { case 'request': await _handleEnrollmentRequest( - enrollVerbParams, currentAtSign, responseJson, atConnection); + enrollVerbParams!, currentAtSign, responseJson, atConnection); break; case 'approve': case 'deny': case 'revoke': await _handleEnrollmentPermissions( - enrollVerbParams, currentAtSign, operation, responseJson); + enrollVerbParams!, currentAtSign, operation, responseJson); break; case 'list': @@ -79,6 +81,12 @@ class EnrollVerbHandler extends AbstractVerbHandler { logger.severe('Exception: $e\n$stackTrace'); rethrow; } + if (responseJson['status'] == EnrollStatus.expired.name) { + response.isError = true; + response.errorMessage = 'enroll id: $enrollmentId is expired'; + response.errorCode = 'AT0028'; + return; + } response.data = jsonEncode(responseJson); } @@ -128,6 +136,10 @@ class EnrollVerbHandler extends AbstractVerbHandler { enrollParams.apkamPublicKey!); enrollmentValue.namespaces = enrollNamespaces; enrollmentValue.requestType = EnrollRequestType.newEnrollment; + // The enrollments will expire after configured + // expiry limit, beyond which any action (approve/deny/revoke) on an + // enrollment is forbidden + enrollmentValue.expiresAt = DateTime.now().toUtc().add(enrollmentExpiry); AtData enrollData; if (atConnection.getMetaData().authType != null && atConnection.getMetaData().authType == AuthType.cram) { @@ -153,11 +165,9 @@ class EnrollVerbHandler extends AbstractVerbHandler { responseJson['status'] = 'pending'; enrollData = AtData() ..data = jsonEncode(enrollmentValue.toJson()) - // Set TTL to the pending enrollments. - // The enrollments will expire after configured - // expiry limit, beyond which any action (approve/deny/revoke) on an - // enrollment is forbidden - ..metaData = (AtMetaData()..ttl = enrollmentExpiryInMills); + // Set TTL to the pending enrollments + // This configures when an enrollment key is deleted + ..metaData = (AtMetaData()..ttl = enrollKeyDeletionAfterMillis); } logger.finer('enrollData: $enrollData'); await keyStore.put('$key$currentAtSign', enrollData, skipCommit: true); @@ -183,9 +193,25 @@ class EnrollVerbHandler extends AbstractVerbHandler { // 2. Enrollment key is not active AtData enrollData = await _fetchEnrollmentDataFromKeyStore( enrollmentKey, currentAtSign, enrollmentIdFromParams); - var enrollDataStoreValue = + + EnrollDataStoreValue enrollDataStoreValue = EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data!)); + if (enrollDataStoreValue.approval!.state == EnrollStatus.expired.name) { + // case 1: enrollment expired and the enrollment keystore value is updated to expired + responseJson['status'] = EnrollStatus.expired.name; + responseJson['enrollmentId'] = enrollmentIdFromParams; + return; + } else if(enrollDataStoreValue.isExpired()){ + // case 2: enrollment expired and the enrollment keystore value is NOT updated to expired + enrollDataStoreValue.approval!.state = EnrollStatus.expired.name; + // update keystore value with approval state as expired + await _updateEnrollmentKey('$enrollmentKey$currentAtSign', + enrollDataStoreValue, enrollData.metaData); + responseJson['status'] = EnrollStatus.expired.name; + responseJson['enrollmentId'] = enrollmentIdFromParams; + return; + } // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state @@ -200,14 +226,12 @@ class EnrollVerbHandler extends AbstractVerbHandler { 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 keyStore.put( + await _updateEnrollmentKey( '$enrollmentKey$currentAtSign', - AtData() - ..data = jsonEncode(enrollDataStoreValue.toJson()) - ..metaData = (enrollData.metaData - ?..ttl = 0 - ..expiresAt = null), - skipCommit: true); + enrollDataStoreValue, + enrollData.metaData + ?..ttl = 0 + ..expiresAt = null); // when enrollment is approved store the apkamPublicKey of the enrollment if (operation == 'approve') { var apkamPublicKeyInKeyStore = @@ -280,11 +304,13 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (_doesEnrollmentHaveManageNamespace(enrollDataStoreValue)) { await _fetchAllEnrollments(enrollmentKeysList, enrollmentRequestsMap); } else { - enrollmentRequestsMap[enrollmentKey] = { - 'appName': enrollDataStoreValue.appName, - 'deviceName': enrollDataStoreValue.deviceName, - 'namespace': enrollDataStoreValue.namespaces - }; + if (!(enrollDataStoreValue.approval!.state == 'expired')) { + enrollmentRequestsMap[enrollmentKey] = { + 'appName': enrollDataStoreValue.appName, + 'deviceName': enrollDataStoreValue.deviceName, + 'namespace': enrollDataStoreValue.namespaces + }; + } } return jsonEncode(enrollmentRequestsMap); } @@ -294,6 +320,10 @@ class EnrollVerbHandler extends AbstractVerbHandler { for (var enrollmentKey in enrollmentKeysList) { EnrollDataStoreValue enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); + if (enrollDataStoreValue.expiresAt != null && + DateTime.now().toUtc().isAfter(enrollDataStoreValue.expiresAt!)) { + continue; + } enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -349,6 +379,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { throw AtEnrollmentException( 'enrollment id: $enrollmentId not found in keystore'); } + // If enrollment is not active, throw AtEnrollmentException if (!SecondaryUtil.isActiveKey(enrollData)) { throw AtEnrollmentException('The enrollment $enrollmentId is expired'); @@ -372,4 +403,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { 'Cannot revoke a ${enrollDataStoreValue.approval!.state} enrollment. Only approved enrollments can be revoked'); } } + + Future _updateEnrollmentKey(String key, + EnrollDataStoreValue enrollDataStoreValue, var metaData) async { + await keyStore.put( + key, + AtData() + ..data = jsonEncode(enrollDataStoreValue.toJson()) + ..metaData = metaData, + skipCommit: true); + } } diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index 601c977c1..27f785ff2 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -92,7 +92,10 @@ class PkamVerbHandler extends AbstractVerbHandler { EnrollDataStoreValue.fromJson(jsonDecode(atData)); EnrollStatus enrollStatus = EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); - + if (enrollDataStoreValue.isExpired()) { + enrollDataStoreValue.approval!.state = EnrollStatus.expired.name; + enrollStatus = EnrollStatus.expired; + } ApkamVerificationResult apkamResult = ApkamVerificationResult(); apkamResult.response = _getApprovalStatus( enrollStatus, enrollId, enrollDataStoreValue.approval!.state); @@ -125,6 +128,11 @@ class PkamVerbHandler extends AbstractVerbHandler { response.errorCode = 'AT0027'; response.errorMessage = 'enrollment_id: $enrollId is revoked'; break; + case EnrollStatus.expired: + response.isError = true; + response.errorCode = 'AT0028'; + response.errorMessage = 'enrollment_id: $enrollId is expired'; + break; default: response.isError = true; response.errorCode = 'AT0026'; @@ -143,7 +151,7 @@ class PkamVerbHandler extends AbstractVerbHandler { bool isValidSignature = false; var storedSecret = await keyStore.get('private:$sessionId$atSign'); storedSecret = storedSecret?.data; - if(signature == null || signature.isEmpty ){ + if (signature == null || signature.isEmpty) { logger.severe('inputSignature is null/empty'); return false; } From 55f2a71456ac70af344aab4eac46b29e776d45d4 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Tue, 5 Sep 2023 14:39:41 +0530 Subject: [PATCH 02/21] reformat: minor refactoring --- .../lib/src/verb/handler/enroll_verb_handler.dart | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index f2e3b4cc4..d316e7e95 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -30,9 +30,9 @@ class EnrollVerbHandler extends AbstractVerbHandler { Verb getVerb() => enrollVerb; @visibleForTesting - Duration enrollmentExpiry = Duration(seconds: 1); + Duration enrollmentExpiry = Duration(hours: AtSecondaryConfig.enrollmentExpiryInHours); - int enrollKeyDeletionAfterMillis = + int enrollmentKeyTtl = Duration(days: AtSecondaryConfig.enrollmentKeyTtlInDays).inMilliseconds; @override @@ -45,10 +45,10 @@ class EnrollVerbHandler extends AbstractVerbHandler { final operation = verbParams['operation']; final currentAtSign = AtSecondaryServerImpl.getInstance().currentAtSign; //Approve, deny, revoke or list enrollments only on authenticated connections - // if (operation != 'request' && !atConnection.getMetaData().isAuthenticated) { - // throw UnAuthenticatedException( - // 'Cannot $operation enrollment without authentication'); - // } + if (operation != 'request' && !atConnection.getMetaData().isAuthenticated) { + throw UnAuthenticatedException( + 'Cannot $operation enrollment without authentication'); + } try { EnrollParams? enrollVerbParams; if (verbParams[enrollParams] != null) { @@ -167,7 +167,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { ..data = jsonEncode(enrollmentValue.toJson()) // Set TTL to the pending enrollments // This configures when an enrollment key is deleted - ..metaData = (AtMetaData()..ttl = enrollKeyDeletionAfterMillis); + ..metaData = (AtMetaData()..ttl = enrollmentKeyTtl); } logger.finer('enrollData: $enrollData'); await keyStore.put('$key$currentAtSign', enrollData, skipCommit: true); From 5db077a503c48a65821f564358e632074797508a Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Wed, 6 Sep 2023 18:17:20 +0530 Subject: [PATCH 03/21] revert: introduction of expiresAt into EnrollDatastoreValue --- .../src/enroll/enroll_datastore_value.dart | 10 --- .../src/enroll/enroll_datastore_value.g.dart | 6 +- .../src/verb/handler/enroll_verb_handler.dart | 83 +++++-------------- 3 files changed, 23 insertions(+), 76 deletions(-) diff --git a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart index 3e61a52c7..5ff62f01d 100644 --- a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart +++ b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.dart @@ -15,7 +15,6 @@ class EnrollDataStoreValue { late String apkamPublicKey; EnrollRequestType? requestType; EnrollApproval? approval; - DateTime? expiresAt; EnrollDataStoreValue( this.sessionId, this.appName, this.deviceName, this.apkamPublicKey); @@ -24,13 +23,6 @@ class EnrollDataStoreValue { _$EnrollDataStoreValueFromJson(json); Map toJson() => _$EnrollDataStoreValueToJson(this); - - bool isExpired() { - if (expiresAt != null && DateTime.now().toUtc().isAfter(expiresAt!)) { - return true; - } - return false; - } } class EnrollApproval { @@ -50,6 +42,4 @@ class EnrollApproval { } } -enum EnrollStatus { pending, approved, denied, revoked, expired } - enum EnrollRequestType { newEnrollment, changeEnrollment } diff --git a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart index 617932290..bc646f562 100644 --- a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart +++ b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart @@ -19,8 +19,7 @@ EnrollDataStoreValue _$EnrollDataStoreValueFromJson( $enumDecodeNullable(_$EnrollRequestTypeEnumMap, json['requestType']) ..approval = json['approval'] == null ? null - : EnrollApproval.fromJson(json['approval'] as Map) - ..expiresAt = DateTime.parse(json['expiresAt']); + : EnrollApproval.fromJson(json['approval'] as Map); Map _$EnrollDataStoreValueToJson( EnrollDataStoreValue instance) => @@ -31,8 +30,7 @@ Map _$EnrollDataStoreValueToJson( 'namespaces': instance.namespaces, 'apkamPublicKey': instance.apkamPublicKey, 'requestType': _$EnrollRequestTypeEnumMap[instance.requestType], - 'approval': instance.approval, - 'expiresAt': instance.expiresAt.toString() + 'approval': instance.approval }; const _$EnrollRequestTypeEnumMap = { diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index dd5c4cdde..c1bf73f7c 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -3,6 +3,7 @@ import 'dart:collection'; import 'dart:convert'; import 'package:at_commons/at_commons.dart'; import 'package:at_secondary/src/connection/inbound/inbound_connection_metadata.dart'; +import 'package:at_secondary/src/server/at_secondary_config.dart'; import 'package:at_secondary/src/server/at_secondary_impl.dart'; import 'package:at_secondary/src/constants/enroll_constants.dart'; import 'package:at_secondary/src/enroll/enroll_datastore_value.dart'; @@ -13,7 +14,6 @@ import 'package:at_server_spec/at_server_spec.dart'; import 'package:at_server_spec/at_verb_spec.dart'; import 'package:meta/meta.dart'; import 'package:uuid/uuid.dart'; -import '../../server/at_secondary_config.dart'; import 'abstract_verb_handler.dart'; import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart'; @@ -30,10 +30,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { Verb getVerb() => enrollVerb; @visibleForTesting - Duration enrollmentExpiry = Duration(hours: AtSecondaryConfig.enrollmentExpiryInHours); - - int enrollmentKeyTtl = - Duration(days: AtSecondaryConfig.enrollmentKeyTtlInDays).inMilliseconds; + int enrollmentExpiryInMills = + Duration(hours: AtSecondaryConfig.enrollmentExpiryInHours).inMilliseconds; @override Future processVerb( @@ -81,12 +79,6 @@ class EnrollVerbHandler extends AbstractVerbHandler { logger.severe('Exception: $e\n$stackTrace'); rethrow; } - if (responseJson['status'] == EnrollStatus.expired.name) { - response.isError = true; - response.errorMessage = 'enroll id: $enrollmentId is expired'; - response.errorCode = 'AT0028'; - return; - } response.data = jsonEncode(responseJson); } @@ -136,10 +128,6 @@ class EnrollVerbHandler extends AbstractVerbHandler { enrollParams.apkamPublicKey!); enrollmentValue.namespaces = enrollNamespaces; enrollmentValue.requestType = EnrollRequestType.newEnrollment; - // The enrollments will expire after configured - // expiry limit, beyond which any action (approve/deny/revoke) on an - // enrollment is forbidden - enrollmentValue.expiresAt = DateTime.now().toUtc().add(enrollmentExpiry); AtData enrollData; if (atConnection.getMetaData().authType != null && atConnection.getMetaData().authType == AuthType.cram) { @@ -165,9 +153,11 @@ class EnrollVerbHandler extends AbstractVerbHandler { responseJson['status'] = 'pending'; enrollData = AtData() ..data = jsonEncode(enrollmentValue.toJson()) - // Set TTL to the pending enrollments - // This configures when an enrollment key is deleted - ..metaData = (AtMetaData()..ttl = enrollmentKeyTtl); + // Set TTL to the pending enrollments. + // The enrollments will expire after configured + // expiry limit, beyond which any action (approve/deny/revoke) on an + // enrollment is forbidden + ..metaData = (AtMetaData()..ttl = enrollmentExpiryInMills); } logger.finer('enrollData: $enrollData'); await keyStore.put('$key$currentAtSign', enrollData, skipCommit: true); @@ -193,25 +183,9 @@ class EnrollVerbHandler extends AbstractVerbHandler { // 2. Enrollment key is not active AtData enrollData = await _fetchEnrollmentDataFromKeyStore( enrollmentKey, currentAtSign, enrollmentIdFromParams); - - EnrollDataStoreValue enrollDataStoreValue = + var enrollDataStoreValue = EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data!)); - if (enrollDataStoreValue.approval!.state == EnrollStatus.expired.name) { - // case 1: enrollment expired and the enrollment keystore value is updated to expired - responseJson['status'] = EnrollStatus.expired.name; - responseJson['enrollmentId'] = enrollmentIdFromParams; - return; - } else if(enrollDataStoreValue.isExpired()){ - // case 2: enrollment expired and the enrollment keystore value is NOT updated to expired - enrollDataStoreValue.approval!.state = EnrollStatus.expired.name; - // update keystore value with approval state as expired - await _updateEnrollmentKey('$enrollmentKey$currentAtSign', - enrollDataStoreValue, enrollData.metaData); - responseJson['status'] = EnrollStatus.expired.name; - responseJson['enrollmentId'] = enrollmentIdFromParams; - return; - } // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state @@ -226,12 +200,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { 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 _updateEnrollmentKey( + await keyStore.put( '$enrollmentKey$currentAtSign', - enrollDataStoreValue, - enrollData.metaData - ?..ttl = 0 - ..expiresAt = null); + AtData() + ..data = jsonEncode(enrollDataStoreValue.toJson()) + ..metaData = (enrollData.metaData + ?..ttl = 0 + ..expiresAt = null), + skipCommit: true); // when enrollment is approved store the apkamPublicKey of the enrollment if (operation == 'approve') { var apkamPublicKeyInKeyStore = @@ -304,13 +280,11 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (_doesEnrollmentHaveManageNamespace(enrollDataStoreValue)) { await _fetchAllEnrollments(enrollmentKeysList, enrollmentRequestsMap); } else { - if (!(enrollDataStoreValue.approval!.state == 'expired')) { - enrollmentRequestsMap[enrollmentKey] = { - 'appName': enrollDataStoreValue.appName, - 'deviceName': enrollDataStoreValue.deviceName, - 'namespace': enrollDataStoreValue.namespaces - }; - } + enrollmentRequestsMap[enrollmentKey] = { + 'appName': enrollDataStoreValue.appName, + 'deviceName': enrollDataStoreValue.deviceName, + 'namespace': enrollDataStoreValue.namespaces + }; } return jsonEncode(enrollmentRequestsMap); } @@ -320,10 +294,6 @@ class EnrollVerbHandler extends AbstractVerbHandler { for (var enrollmentKey in enrollmentKeysList) { EnrollDataStoreValue enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); - if (enrollDataStoreValue.expiresAt != null && - DateTime.now().toUtc().isAfter(enrollDataStoreValue.expiresAt!)) { - continue; - } enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -379,7 +349,6 @@ class EnrollVerbHandler extends AbstractVerbHandler { throw AtEnrollmentException( 'enrollment id: $enrollmentId not found in keystore'); } - // If enrollment is not active, throw AtEnrollmentException if (!SecondaryUtil.isActiveKey(enrollData)) { throw AtEnrollmentException('The enrollment $enrollmentId is expired'); @@ -403,14 +372,4 @@ class EnrollVerbHandler extends AbstractVerbHandler { 'Cannot revoke a ${enrollDataStoreValue.approval!.state} enrollment. Only approved enrollments can be revoked'); } } - - Future _updateEnrollmentKey(String key, - EnrollDataStoreValue enrollDataStoreValue, var metaData) async { - await keyStore.put( - key, - AtData() - ..data = jsonEncode(enrollDataStoreValue.toJson()) - ..metaData = metaData, - skipCommit: true); - } } From 5a3eb16fe280296060ec003febce1cdf94918aaf Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Wed, 6 Sep 2023 18:18:08 +0530 Subject: [PATCH 04/21] fix: update error reponse handling for expired/invalid enrollmentId's --- .../src/verb/handler/pkam_verb_handler.dart | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index 27f785ff2..2b560b7b8 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -86,19 +86,23 @@ class PkamVerbHandler extends AbstractVerbHandler { String enrollId, String atSign) async { String enrollmentKey = '$enrollId.$newEnrollmentKeyPattern.$enrollManageNamespace$atSign'; - var enrollData = await keyStore.get(enrollmentKey); - final atData = enrollData.data; - final enrollDataStoreValue = - EnrollDataStoreValue.fromJson(jsonDecode(atData)); - EnrollStatus enrollStatus = - EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); - if (enrollDataStoreValue.isExpired()) { - enrollDataStoreValue.approval!.state = EnrollStatus.expired.name; - enrollStatus = EnrollStatus.expired; - } + late final EnrollDataStoreValue enrollDataStoreValue; ApkamVerificationResult apkamResult = ApkamVerificationResult(); - apkamResult.response = _getApprovalStatus( - enrollStatus, enrollId, enrollDataStoreValue.approval!.state); + try { + var enrollData = await keyStore.get(enrollmentKey); + enrollDataStoreValue = + EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data)); + EnrollStatus enrollStatus = + EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); + apkamResult.response = _getApprovalStatus( + enrollStatus, enrollId, enrollDataStoreValue.approval!.state); + } on KeyNotFoundException catch (e) { + logger.finer('Caught: $e'); + apkamResult.response.isError = true; + apkamResult.response.errorCode = 'AT0028'; + apkamResult.response.errorMessage = + 'enrollment_id: $enrollId is expired (or) invalid'; + } if (apkamResult.response.isError) { return apkamResult; } @@ -128,11 +132,6 @@ class PkamVerbHandler extends AbstractVerbHandler { response.errorCode = 'AT0027'; response.errorMessage = 'enrollment_id: $enrollId is revoked'; break; - case EnrollStatus.expired: - response.isError = true; - response.errorCode = 'AT0028'; - response.errorMessage = 'enrollment_id: $enrollId is expired'; - break; default: response.isError = true; response.errorCode = 'AT0026'; From da281eb13b5609223cf493f4ecd7189c9ed2c051 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Wed, 6 Sep 2023 18:22:07 +0530 Subject: [PATCH 05/21] revert: changed related to enrollmentExpiry in AtSecondaryConfig --- .../lib/src/enroll/enroll_datastore_value.g.dart | 2 +- .../lib/src/server/at_secondary_config.dart | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart index bc646f562..be6ba5244 100644 --- a/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart +++ b/packages/at_secondary_server/lib/src/enroll/enroll_datastore_value.g.dart @@ -30,7 +30,7 @@ Map _$EnrollDataStoreValueToJson( 'namespaces': instance.namespaces, 'apkamPublicKey': instance.apkamPublicKey, 'requestType': _$EnrollRequestTypeEnumMap[instance.requestType], - 'approval': instance.approval + 'approval': instance.approval, }; const _$EnrollRequestTypeEnumMap = { diff --git a/packages/at_secondary_server/lib/src/server/at_secondary_config.dart b/packages/at_secondary_server/lib/src/server/at_secondary_config.dart index eb8c62ea2..5b004bb75 100644 --- a/packages/at_secondary_server/lib/src/server/at_secondary_config.dart +++ b/packages/at_secondary_server/lib/src/server/at_secondary_config.dart @@ -121,10 +121,7 @@ class AtSecondaryConfig { ? ConfigUtil.getPubspecConfig()!['version'] : null; - // specifies the time after which an enrollment is considered expired static final int _enrollmentExpiryInHours = 48; - // time after which the enrollment key is actually deleted from the keystore - static final int _enrollmentKeyTtlInDays = 14; static final Map _envVars = Platform.environment; @@ -132,8 +129,6 @@ class AtSecondaryConfig { static int get enrollmentExpiryInHours => _enrollmentExpiryInHours; - static int get enrollmentKeyTtlInDays => _enrollmentKeyTtlInDays; - // TODO: Medium priority: Most (all?) getters in this class return a default value but the signatures currently // allow for nulls. Should fix this as has been done for logLevel // TODO: Low priority: Lots of very similar boilerplate code here. Not necessarily bad in this particular case, but From fbdafe4e55ca0a130f7e004f143d7f74d7437942 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 7 Sep 2023 01:00:18 +0530 Subject: [PATCH 06/21] fix: respond with errorCode:AT0028 when enrollment is expired --- .../src/verb/handler/enroll_verb_handler.dart | 47 ++++++++++++++----- .../src/verb/handler/pkam_verb_handler.dart | 25 ++++++---- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index c1bf73f7c..58d7af79c 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -3,8 +3,8 @@ import 'dart:collection'; import 'dart:convert'; import 'package:at_commons/at_commons.dart'; import 'package:at_secondary/src/connection/inbound/inbound_connection_metadata.dart'; -import 'package:at_secondary/src/server/at_secondary_config.dart'; import 'package:at_secondary/src/server/at_secondary_impl.dart'; +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'; @@ -47,8 +47,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { throw UnAuthenticatedException( 'Cannot $operation enrollment without authentication'); } + EnrollParams? enrollVerbParams; try { - EnrollParams? enrollVerbParams; if (verbParams[enrollParams] != null) { enrollVerbParams = EnrollParams.fromJson(jsonDecode(verbParams[enrollParams]!)); @@ -79,6 +79,12 @@ class EnrollVerbHandler extends AbstractVerbHandler { logger.severe('Exception: $e\n$stackTrace'); rethrow; } + if (responseJson.containsKey('isError')) { + response.isError = true; + response.errorMessage = responseJson['errorMessage']; + response.errorCode = responseJson['errorCode']; + return; + } response.data = jsonEncode(responseJson); } @@ -177,14 +183,21 @@ class EnrollVerbHandler extends AbstractVerbHandler { '$enrollmentIdFromParams.$newEnrollmentKeyPattern.$enrollManageNamespace'; logger.finer( 'Enrollment key: $enrollmentKey$currentAtSign | Enrollment operation: $operation'); + AtData? enrollData; // Fetch and returns enrollment data from the keystore. // Throw AtEnrollmentException, IF // 1. Enrollment key is not present in keystore // 2. Enrollment key is not active - AtData enrollData = await _fetchEnrollmentDataFromKeyStore( - enrollmentKey, currentAtSign, enrollmentIdFromParams); + try { + enrollData = await _fetchEnrollmentDataFromKeyStore( + enrollmentKey, currentAtSign, enrollmentIdFromParams, responseJson); + } on AtEnrollmentException catch (e) { + // Incase of + logger.finer('Caught: $e'); + return; + } var enrollDataStoreValue = - EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data!)); + EnrollDataStoreValue.fromJson(jsonDecode(enrollData!.data!)); // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from @@ -337,21 +350,29 @@ class EnrollVerbHandler extends AbstractVerbHandler { } } - Future _fetchEnrollmentDataFromKeyStore( - String enrollmentKey, currentAtSign, String? enrollmentId) async { - AtData enrollData; + Future _fetchEnrollmentDataFromKeyStore(String enrollmentKey, + currentAtSign, String? enrollmentId, responseJson) async { // KeyStore.get will not return null. If the value is null, keyStore.get - // throws KeyNotFoundException. - // So, enrollData will NOT be null. + // throws KeyNotFoundException which will be rethrown as an EnrollmentException + AtData? enrollData; try { enrollData = await keyStore.get('$enrollmentKey$currentAtSign'); - } on KeyNotFoundException { + } on KeyNotFoundException catch (e) { + responseJson['isError'] = 'true'; + responseJson['errorCode'] = 'AT0028'; + responseJson['errorMessage'] = + 'enrollment_id: $enrollmentId is expired or invalid'; throw AtEnrollmentException( - 'enrollment id: $enrollmentId not found in keystore'); + 'enrollmentId: $enrollmentId is not an active key'); } + // If enrollment is not active, throw AtEnrollmentException if (!SecondaryUtil.isActiveKey(enrollData)) { - throw AtEnrollmentException('The enrollment $enrollmentId is expired'); + responseJson['isError'] = 'true'; + responseJson['errorCode'] = 'AT0028'; + responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired'; + throw AtEnrollmentException( + 'enrollmentId: $enrollmentId is not an active key'); } return enrollData; } diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index 2b560b7b8..4309caf91 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -12,6 +12,7 @@ import 'package:at_persistence_secondary_server/at_persistence_secondary_server. import 'package:at_chops/at_chops.dart'; import 'package:at_server_spec/at_server_spec.dart'; import 'package:at_secondary/src/constants/enroll_constants.dart'; +import 'package:at_secondary/src/utils/secondary_util.dart'; import 'package:meta/meta.dart'; class PkamVerbHandler extends AbstractVerbHandler { @@ -88,20 +89,20 @@ class PkamVerbHandler extends AbstractVerbHandler { '$enrollId.$newEnrollmentKeyPattern.$enrollManageNamespace$atSign'; late final EnrollDataStoreValue enrollDataStoreValue; ApkamVerificationResult apkamResult = ApkamVerificationResult(); + AtData? enrollData; try { - var enrollData = await keyStore.get(enrollmentKey); + enrollData = await keyStore.get(enrollmentKey); enrollDataStoreValue = - EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data)); + EnrollDataStoreValue.fromJson(jsonDecode(enrollData!.data!)); EnrollStatus enrollStatus = EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); - apkamResult.response = _getApprovalStatus( - enrollStatus, enrollId, enrollDataStoreValue.approval!.state); + apkamResult.response = _getApprovalStatus(enrollStatus, enrollId); } on KeyNotFoundException catch (e) { logger.finer('Caught: $e'); - apkamResult.response.isError = true; - apkamResult.response.errorCode = 'AT0028'; - apkamResult.response.errorMessage = - 'enrollment_id: $enrollId is expired (or) invalid'; + apkamResult.response = _getApprovalStatus(EnrollStatus.expired, enrollId); + } + if(!SecondaryUtil.isActiveKey(enrollData)){ + apkamResult.response = _getApprovalStatus(EnrollStatus.expired, enrollId); } if (apkamResult.response.isError) { return apkamResult; @@ -110,8 +111,7 @@ class PkamVerbHandler extends AbstractVerbHandler { return apkamResult; } - Response _getApprovalStatus( - EnrollStatus enrollStatus, enrollId, approvalState) { + Response _getApprovalStatus(EnrollStatus enrollStatus, enrollId) { Response response = Response(); switch (enrollStatus) { case EnrollStatus.denied: @@ -132,6 +132,11 @@ class PkamVerbHandler extends AbstractVerbHandler { response.errorCode = 'AT0027'; response.errorMessage = 'enrollment_id: $enrollId is revoked'; break; + case EnrollStatus.expired: + response.isError = true; + response.errorCode = 'AT0028'; + response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; + break; default: response.isError = true; response.errorCode = 'AT0026'; From 48ffeb66916e13d9261144d7050b6a03a81e4309 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 7 Sep 2023 01:00:34 +0530 Subject: [PATCH 07/21] test: update tests --- .../test/enroll_verb_test.dart | 37 +++++++++---------- .../test/pkam_verb_test.dart | 12 ++++++ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/packages/at_secondary_server/test/enroll_verb_test.dart b/packages/at_secondary_server/test/enroll_verb_test.dart index bad17ebbb..bcef11ed0 100644 --- a/packages/at_secondary_server/test/enroll_verb_test.dart +++ b/packages/at_secondary_server/test/enroll_verb_test.dart @@ -341,7 +341,8 @@ void main() { test( 'A test to verify revoke operations thrown exception when given enrollmentId is not in keystore', () async { - String enrollmentRequest = 'enroll:revoke:{"enrollmentId":"123"}'; + String enrollmentId = '123'; + String enrollmentRequest = 'enroll:revoke:{"enrollmentId":"$enrollmentId"}'; HashMap verbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = true; @@ -351,12 +352,11 @@ void main() { Response response = Response(); EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); - expect( - () async => await enrollVerbHandler.processVerb( - response, verbParams, inboundConnection), - throwsA(predicate((dynamic e) => - e is AtEnrollmentException && - e.message == 'enrollment id: 123 not found in keystore'))); + await enrollVerbHandler.processVerb(response, verbParams, inboundConnection); + expect(response.isError, true); + expect(response.errorMessage, isNotNull); + assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + expect(response.errorCode, 'AT0028'); }); tearDown(() async => await verbTestsTearDown()); }); @@ -500,12 +500,12 @@ void main() { getVerbParam(VerbSyntax.enroll, approveEnrollmentCommand); inboundConnection.getMetaData().isAuthenticated = true; inboundConnection.getMetaData().sessionID = 'dummy_session_id'; - expect( - () async => await enrollVerbHandler.processVerb( - response, enrollVerbParams, inboundConnection), - throwsA(predicate((dynamic e) => - e is AtEnrollmentException && - e.message == 'The enrollment $enrollmentId is expired'))); + await enrollVerbHandler.processVerb( + response, enrollVerbParams, inboundConnection); + expect(response.isError, true); + expect(response.errorMessage, isNotNull); + assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + expect(response.errorCode, 'AT0028'); }); test('A test to verify expired enrollment cannot be denied', () async { @@ -532,12 +532,11 @@ void main() { getVerbParam(VerbSyntax.enroll, approveEnrollmentCommand); inboundConnection.getMetaData().isAuthenticated = true; inboundConnection.getMetaData().sessionID = 'dummy_session_id'; - expect( - () async => await enrollVerbHandler.processVerb( - response, enrollVerbParams, inboundConnection), - throwsA(predicate((dynamic e) => - e is AtEnrollmentException && - e.message == 'The enrollment $enrollmentId is expired'))); + await enrollVerbHandler.processVerb(response, enrollVerbParams, inboundConnection); + expect(response.isError, true); + expect(response.errorMessage, isNotNull); + assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + expect(response.errorCode, 'AT0028'); }); test('A test to verify TTL on approved enrollment is reset', () async { diff --git a/packages/at_secondary_server/test/pkam_verb_test.dart b/packages/at_secondary_server/test/pkam_verb_test.dart index 58f3e89f8..9d93d013e 100644 --- a/packages/at_secondary_server/test/pkam_verb_test.dart +++ b/packages/at_secondary_server/test/pkam_verb_test.dart @@ -131,6 +131,18 @@ void main() { 'enrollment_id: enrollId is denied'); }); + test('verify apkam behaviour - case: enrollment expired ', () async { + enrollData.approval = EnrollApproval('denied'); + when(() => mockKeyStore.get(any())) + .thenThrow(KeyNotFoundException('key not found')); + + var apkamResult = + await pkamVerbHandler.handleApkamVerification('enrollId', '@alice'); + expect(apkamResult.response.isError, true); + expect(apkamResult.response.errorCode, 'AT0028'); + expect(apkamResult.response.errorMessage, + 'enrollment_id: enrollId is expired or invalid'); + }); tearDownAll(() async => await tearDownFunc()); }); From c617c0254496d677716ea9c30f2136f666e7f3ca Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 7 Sep 2023 01:01:08 +0530 Subject: [PATCH 08/21] build: add a dependency override --- packages/at_secondary_server/pubspec.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/at_secondary_server/pubspec.yaml b/packages/at_secondary_server/pubspec.yaml index 5569e42c6..f3b71c78b 100644 --- a/packages/at_secondary_server/pubspec.yaml +++ b/packages/at_secondary_server/pubspec.yaml @@ -34,6 +34,13 @@ dependencies: yaml: 3.1.2 logging: 1.2.0 +dependency_overrides: + at_commons: + git: + url: https://github.com/atsign-foundation/at_libraries.git + path: packages/at_commons + ref: apkam_new_error_code + dev_dependencies: test: ^1.24.4 coverage: ^1.6.1 From ff6ec9a4c6d132817ae4ba35de0aca12cce8599e Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 7 Sep 2023 01:16:27 +0530 Subject: [PATCH 09/21] fix: exclude expired enrollments in enroll:list --- .../src/verb/handler/enroll_verb_handler.dart | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 58d7af79c..b9ce21b53 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -293,11 +293,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (_doesEnrollmentHaveManageNamespace(enrollDataStoreValue)) { await _fetchAllEnrollments(enrollmentKeysList, enrollmentRequestsMap); } else { - enrollmentRequestsMap[enrollmentKey] = { - 'appName': enrollDataStoreValue.appName, - 'deviceName': enrollDataStoreValue.deviceName, - 'namespace': enrollDataStoreValue.namespaces - }; + if (!(enrollDataStoreValue.approval!.state == + EnrollStatus.expired.name)) { + enrollmentRequestsMap[enrollmentKey] = { + 'appName': enrollDataStoreValue.appName, + 'deviceName': enrollDataStoreValue.deviceName, + 'namespace': enrollDataStoreValue.namespaces + }; + } } return jsonEncode(enrollmentRequestsMap); } @@ -307,11 +310,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { for (var enrollmentKey in enrollmentKeysList) { EnrollDataStoreValue enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); - enrollmentRequestsMap[enrollmentKey] = { - 'appName': enrollDataStoreValue.appName, - 'deviceName': enrollDataStoreValue.deviceName, - 'namespace': enrollDataStoreValue.namespaces - }; + if (!(enrollDataStoreValue.approval!.state == + EnrollStatus.expired.name)) { + enrollmentRequestsMap[enrollmentKey] = { + 'appName': enrollDataStoreValue.appName, + 'deviceName': enrollDataStoreValue.deviceName, + 'namespace': enrollDataStoreValue.namespaces + }; + } } } From 1556d62beba675b5de83c44a61a93a37ebb12087 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 7 Sep 2023 01:17:33 +0530 Subject: [PATCH 10/21] fix: getEnrollData() checks for expiry and sets appropriate response --- .../lib/src/verb/handler/abstract_verb_handler.dart | 4 ++++ 1 file changed, 4 insertions(+) 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 85f87a2ee..55c320d0c 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 @@ -11,6 +11,7 @@ import 'package:at_secondary/src/verb/manager/response_handler_manager.dart'; import 'package:at_server_spec/at_server_spec.dart'; import 'package:at_server_spec/at_verb_spec.dart'; import 'package:at_utils/at_logger.dart'; +import 'package:at_secondary/src/utils/secondary_util.dart'; final String paramFullCommandAsReceived = 'FullCommandAsReceived'; @@ -89,6 +90,9 @@ abstract class AbstractVerbHandler implements VerbHandler { AtData enrollData = await keyStore.get(enrollmentKey); EnrollDataStoreValue enrollDataStoreValue = EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data!)); + if(!SecondaryUtil.isActiveKey(enrollData)){ + enrollDataStoreValue.approval?.state = EnrollStatus.expired.name; + } return enrollDataStoreValue; } on KeyNotFoundException { logger.severe('$enrollmentKey does not exist in the keystore'); From 04c08a2a6c3c5185aaccc7f02f1cdac7ea55d1eb Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sat, 9 Sep 2023 22:17:38 +0530 Subject: [PATCH 11/21] fix: address review comments --- .../src/verb/handler/enroll_verb_handler.dart | 23 ++++++++++--------- .../src/verb/handler/pkam_verb_handler.dart | 8 ++++--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index b9ce21b53..7a4c5625e 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -86,6 +86,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { return; } response.data = jsonEncode(responseJson); + return; } /// Enrollment requests details are persisted in the keystore and are excluded from @@ -191,9 +192,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { try { enrollData = await _fetchEnrollmentDataFromKeyStore( enrollmentKey, currentAtSign, enrollmentIdFromParams, responseJson); - } on AtEnrollmentException catch (e) { - // Incase of - logger.finer('Caught: $e'); + } on AtEnrollmentException { + // When an enrollment key is expired or invalid AtEnrollmentException is thrown return; } var enrollDataStoreValue = @@ -293,8 +293,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (_doesEnrollmentHaveManageNamespace(enrollDataStoreValue)) { await _fetchAllEnrollments(enrollmentKeysList, enrollmentRequestsMap); } else { - if (!(enrollDataStoreValue.approval!.state == - EnrollStatus.expired.name)) { + if (enrollDataStoreValue.approval!.state != + EnrollStatus.expired.name) { enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -310,8 +310,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { for (var enrollmentKey in enrollmentKeysList) { EnrollDataStoreValue enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); - if (!(enrollDataStoreValue.approval!.state == - EnrollStatus.expired.name)) { + if (enrollDataStoreValue.approval!.state != + EnrollStatus.expired.name) { enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -365,20 +365,21 @@ class EnrollVerbHandler extends AbstractVerbHandler { enrollData = await keyStore.get('$enrollmentKey$currentAtSign'); } on KeyNotFoundException catch (e) { responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0028'; + responseJson['errorCode'] = 'AT0029'; responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired or invalid'; + logger.finer('Caught while fetching enrollment key: $e'); throw AtEnrollmentException( - 'enrollmentId: $enrollmentId is not an active key'); + 'enrollmentId: $enrollmentId is expired or invalid'); } // If enrollment is not active, throw AtEnrollmentException if (!SecondaryUtil.isActiveKey(enrollData)) { responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0028'; + responseJson['errorCode'] = 'AT0029'; responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired'; throw AtEnrollmentException( - 'enrollmentId: $enrollmentId is not an active key'); + 'enrollmentId: $enrollmentId is expired'); } return enrollData; } diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index 4309caf91..447b87060 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -98,8 +98,10 @@ class PkamVerbHandler extends AbstractVerbHandler { EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); apkamResult.response = _getApprovalStatus(enrollStatus, enrollId); } on KeyNotFoundException catch (e) { - logger.finer('Caught: $e'); - apkamResult.response = _getApprovalStatus(EnrollStatus.expired, enrollId); + logger.finer('Caught exception trying to fetch enrollment key: $e'); + apkamResult.response.isError = true; + apkamResult.response.errorCode = 'AT0029'; + apkamResult.response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; } if(!SecondaryUtil.isActiveKey(enrollData)){ apkamResult.response = _getApprovalStatus(EnrollStatus.expired, enrollId); @@ -134,7 +136,7 @@ class PkamVerbHandler extends AbstractVerbHandler { break; case EnrollStatus.expired: response.isError = true; - response.errorCode = 'AT0028'; + response.errorCode = 'AT0029'; response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; break; default: From d451144e477d4ace76e786f5f1b58755d9ee58da Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sat, 9 Sep 2023 22:20:29 +0530 Subject: [PATCH 12/21] build: replace at_commons dep_override with latest version --- packages/at_secondary_server/pubspec.yaml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/at_secondary_server/pubspec.yaml b/packages/at_secondary_server/pubspec.yaml index a9611a24b..c22bee9ab 100644 --- a/packages/at_secondary_server/pubspec.yaml +++ b/packages/at_secondary_server/pubspec.yaml @@ -18,7 +18,7 @@ dependencies: collection: 1.18.0 basic_utils: 5.6.1 ecdsa: 0.0.4 - at_commons: 3.0.54 + at_commons: 3.0.55 at_utils: 3.0.15 at_chops: 1.0.4 at_lookup: 3.0.40 @@ -34,13 +34,6 @@ dependencies: yaml: 3.1.2 logging: 1.2.0 -dependency_overrides: - at_commons: - git: - url: https://github.com/atsign-foundation/at_libraries.git - path: packages/at_commons - ref: apkam_new_error_code - dev_dependencies: test: ^1.24.4 coverage: ^1.6.1 From a3a47fbb983e74462776f617d28032ebf25d9d73 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sat, 9 Sep 2023 22:51:06 +0530 Subject: [PATCH 13/21] fix: change apkam expiry error code from AT0029 -> AT0028 --- .../src/verb/handler/enroll_verb_handler.dart | 4 +-- .../src/verb/handler/pkam_verb_handler.dart | 4 +-- .../test/enroll_verb_test.dart | 25 ++++++++----------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 7a4c5625e..22a6aad1f 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -365,7 +365,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { enrollData = await keyStore.get('$enrollmentKey$currentAtSign'); } on KeyNotFoundException catch (e) { responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0029'; + responseJson['errorCode'] = 'AT0028'; responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired or invalid'; logger.finer('Caught while fetching enrollment key: $e'); @@ -376,7 +376,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { // If enrollment is not active, throw AtEnrollmentException if (!SecondaryUtil.isActiveKey(enrollData)) { responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0029'; + responseJson['errorCode'] = 'AT0028'; responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired'; throw AtEnrollmentException( 'enrollmentId: $enrollmentId is expired'); diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index 447b87060..e3ac6043b 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -100,7 +100,7 @@ class PkamVerbHandler extends AbstractVerbHandler { } on KeyNotFoundException catch (e) { logger.finer('Caught exception trying to fetch enrollment key: $e'); apkamResult.response.isError = true; - apkamResult.response.errorCode = 'AT0029'; + apkamResult.response.errorCode = 'AT0028'; apkamResult.response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; } if(!SecondaryUtil.isActiveKey(enrollData)){ @@ -136,7 +136,7 @@ class PkamVerbHandler extends AbstractVerbHandler { break; case EnrollStatus.expired: response.isError = true; - response.errorCode = 'AT0029'; + response.errorCode = 'AT0028'; response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; break; default: diff --git a/tests/at_functional_test/test/enroll_verb_test.dart b/tests/at_functional_test/test/enroll_verb_test.dart index d77a878c5..1b6c4df5c 100644 --- a/tests/at_functional_test/test/enroll_verb_test.dart +++ b/tests/at_functional_test/test/enroll_verb_test.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'dart:io'; +import 'package:at_commons/at_commons.dart'; import 'package:at_demo_data/at_demo_data.dart' as at_demos; import 'package:at_functional_test/conf/config_util.dart'; import 'package:test/test.dart'; @@ -119,10 +120,8 @@ void main() { await socket_writer(socketConnection1!, approveEnrollCommand); var approveEnrollResponse = await read(); approveEnrollResponse = approveEnrollResponse.replaceFirst('error:', ''); - expect( - approveEnrollResponse.contains( - 'enrollment id: $dummyEnrollmentId not found in keystore'), - true); + expect(approveEnrollResponse, + 'AT0028:enrollment_id: $enrollmentId is expired or invalid'); }); test( @@ -141,10 +140,8 @@ void main() { await socket_writer(socketConnection1!, denyEnrollCommand); var denyEnrollResponse = await read(); denyEnrollResponse = denyEnrollResponse.replaceFirst('error:', ''); - expect( - denyEnrollResponse.contains( - 'enrollment id: $dummyEnrollmentId not found in keystore'), - true); + expect(denyEnrollResponse, + 'AT0028:enrollment_id: $enrollmentId is expired or invalid'); }); test('enroll request on unauthenticated connection without otp', () async { @@ -695,8 +692,8 @@ void main() { expect(jsonDecode(enrollmentResponse)['status'], 'denied'); expect(jsonDecode(enrollmentResponse)['enrollmentId'], enrollmentId); // Approve enrollment - await socket_writer( - socketConnection1!, 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); + await socket_writer(socketConnection1!, + 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); enrollmentResponse = (await read()).replaceAll('error:', ''); expect( jsonDecode(enrollmentResponse)['errorDescription'], @@ -728,8 +725,8 @@ void main() { // Approve enrollment await _connect(); await prepare(socketConnection1!, firstAtsign); - await socket_writer( - socketConnection1!, 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); + await socket_writer(socketConnection1!, + 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); enrollmentResponse = (await read()).replaceAll('data:', ''); expect(jsonDecode(enrollmentResponse)['status'], 'approved'); expect(jsonDecode(enrollmentResponse)['enrollmentId'], enrollmentId); @@ -740,8 +737,8 @@ void main() { expect(jsonDecode(enrollmentResponse)['status'], 'revoked'); expect(jsonDecode(enrollmentResponse)['enrollmentId'], enrollmentId); // Approve a revoked enrollment - await socket_writer( - socketConnection1!, 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); + await socket_writer(socketConnection1!, + 'enroll:approve:{"enrollmentId":"$enrollmentId"}'); enrollmentResponse = (await read()).replaceAll('error:', ''); expect( jsonDecode(enrollmentResponse)['errorDescription'], From f8c0393b104c3be52dc4aabee30a029ea6296b11 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sun, 10 Sep 2023 01:57:12 +0530 Subject: [PATCH 14/21] fix: EnrollVerb Refactoring + use getEnrollDataStoreValue() in AbstractVerb instead of Individual methods to fetch enrollment value. --- .../src/verb/handler/enroll_verb_handler.dart | 122 ++++++++---------- .../src/verb/handler/pkam_verb_handler.dart | 12 +- .../test/enroll_verb_test.dart | 3 +- 3 files changed, 56 insertions(+), 81 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 22a6aad1f..17554b276 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/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/utils/secondary_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'; @@ -62,8 +61,8 @@ class EnrollVerbHandler extends AbstractVerbHandler { case 'approve': case 'deny': case 'revoke': - await _handleEnrollmentPermissions( - enrollVerbParams!, currentAtSign, operation, responseJson); + await _handleEnrollmentPermissions(enrollVerbParams!, currentAtSign, + operation, responseJson, response); break; case 'list': @@ -72,19 +71,9 @@ class EnrollVerbHandler extends AbstractVerbHandler { return; } } catch (e, stackTrace) { - response.isError = true; - response.errorMessage = e.toString(); - responseJson['status'] = 'exception'; - responseJson['reason'] = e.toString(); logger.severe('Exception: $e\n$stackTrace'); rethrow; } - if (responseJson.containsKey('isError')) { - response.isError = true; - response.errorMessage = responseJson['errorMessage']; - response.errorCode = responseJson['errorCode']; - return; - } response.data = jsonEncode(responseJson); return; } @@ -178,32 +167,36 @@ class EnrollVerbHandler extends AbstractVerbHandler { EnrollParams enrollParams, currentAtSign, String? operation, - Map responseJson) async { + Map responseJson, + Response response) async { final enrollmentIdFromParams = enrollParams.enrollmentId; String enrollmentKey = '$enrollmentIdFromParams.$newEnrollmentKeyPattern.$enrollManageNamespace'; logger.finer( 'Enrollment key: $enrollmentKey$currentAtSign | Enrollment operation: $operation'); - AtData? enrollData; + EnrollDataStoreValue? enrollDataStoreValue; + EnrollStatus? enrollStatus; // Fetch and returns enrollment data from the keystore. // Throw AtEnrollmentException, IF // 1. Enrollment key is not present in keystore // 2. Enrollment key is not active try { - enrollData = await _fetchEnrollmentDataFromKeyStore( - enrollmentKey, currentAtSign, enrollmentIdFromParams, responseJson); - } on AtEnrollmentException { - // When an enrollment key is expired or invalid AtEnrollmentException is thrown + enrollDataStoreValue = await getEnrollDataStoreValue('$enrollmentKey$currentAtSign'); + } on KeyNotFoundException { + // When an enrollment key is expired or invalid + enrollStatus = EnrollStatus.expired; + } + enrollStatus ??= getEnrollStatusFromString(enrollDataStoreValue!.approval!.state); + // Validates if enrollment is not expired + _validateEnrollmentValidity(enrollStatus, enrollmentIdFromParams!, response); + if(response.isError){ return; } - var enrollDataStoreValue = - EnrollDataStoreValue.fromJson(jsonDecode(enrollData!.data!)); - // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state - _verifyEnrollmentStateBeforeAction(operation, enrollDataStoreValue); - enrollDataStoreValue.approval!.state = _getEnrollStatusEnum(operation).name; + _verifyEnrollmentStateBeforeAction(operation, enrollStatus); + enrollDataStoreValue!.approval!.state = _getEnrollStatusEnum(operation).name; responseJson['status'] = _getEnrollStatusEnum(operation).name; // If an enrollment is approved, we need the enrollment to be active @@ -213,24 +206,17 @@ class EnrollVerbHandler extends AbstractVerbHandler { 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 keyStore.put( - '$enrollmentKey$currentAtSign', - AtData() - ..data = jsonEncode(enrollDataStoreValue.toJson()) - ..metaData = (enrollData.metaData - ?..ttl = 0 - ..expiresAt = null), - skipCommit: true); + await _updateEnrollmentValueAndResetTTL( + '$enrollmentKey$currentAtSign', enrollDataStoreValue); // when enrollment is approved store the apkamPublicKey of the enrollment if (operation == 'approve') { var apkamPublicKeyInKeyStore = 'public:${enrollDataStoreValue.appName}.${enrollDataStoreValue.deviceName}.pkam.$pkamNamespace.__public_keys$currentAtSign'; - var valueJson = {}; - valueJson[apkamPublicKey] = enrollDataStoreValue.apkamPublicKey; + var valueJson = {'apkamPublicKey':enrollDataStoreValue.apkamPublicKey}; var atData = AtData()..data = jsonEncode(valueJson); await keyStore.put(apkamPublicKeyInKeyStore, atData); await _storeEncryptionKeys( - enrollmentIdFromParams!, enrollParams, currentAtSign); + enrollmentIdFromParams, enrollParams, currentAtSign); } responseJson['enrollmentId'] = enrollmentIdFromParams; } @@ -293,8 +279,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (_doesEnrollmentHaveManageNamespace(enrollDataStoreValue)) { await _fetchAllEnrollments(enrollmentKeysList, enrollmentRequestsMap); } else { - if (enrollDataStoreValue.approval!.state != - EnrollStatus.expired.name) { + if (enrollDataStoreValue.approval!.state != EnrollStatus.expired.name) { enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -310,8 +295,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { for (var enrollmentKey in enrollmentKeysList) { EnrollDataStoreValue enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); - if (enrollDataStoreValue.approval!.state != - EnrollStatus.expired.name) { + if (enrollDataStoreValue.approval!.state != EnrollStatus.expired.name) { enrollmentRequestsMap[enrollmentKey] = { 'appName': enrollDataStoreValue.appName, 'deviceName': enrollDataStoreValue.deviceName, @@ -355,49 +339,47 @@ class EnrollVerbHandler extends AbstractVerbHandler { 'Error while storing notification key $enrollmentId. Error $e. Trace $trace'); } } - - Future _fetchEnrollmentDataFromKeyStore(String enrollmentKey, - currentAtSign, String? enrollmentId, responseJson) async { - // KeyStore.get will not return null. If the value is null, keyStore.get - // throws KeyNotFoundException which will be rethrown as an EnrollmentException - AtData? enrollData; - try { - enrollData = await keyStore.get('$enrollmentKey$currentAtSign'); - } on KeyNotFoundException catch (e) { - responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0028'; - responseJson['errorMessage'] = - 'enrollment_id: $enrollmentId is expired or invalid'; - logger.finer('Caught while fetching enrollment key: $e'); - throw AtEnrollmentException( - 'enrollmentId: $enrollmentId is expired or invalid'); - } - - // If enrollment is not active, throw AtEnrollmentException - if (!SecondaryUtil.isActiveKey(enrollData)) { - responseJson['isError'] = 'true'; - responseJson['errorCode'] = 'AT0028'; - responseJson['errorMessage'] = 'enrollment_id: $enrollmentId is expired'; - throw AtEnrollmentException( - 'enrollmentId: $enrollmentId is expired'); + + void _validateEnrollmentValidity( + EnrollStatus enrollStatus, String enrollmentId, Response response) { + if (EnrollStatus.expired == enrollStatus) { + response.isError = true; + response.errorCode = 'AT0028'; + response.errorMessage = 'enrollment_id: $enrollmentId is expired or invalid'; + return; } - return enrollData; } /// Verifies whether the enrollment state matches the intended state. /// Throws AtEnrollmentException: If the enrollment state is different /// from the intended state. void _verifyEnrollmentStateBeforeAction( - String? operation, EnrollDataStoreValue enrollDataStoreValue) { + String? operation, EnrollStatus enrollStatus) { if (operation == 'approve' && - enrollDataStoreValue.approval!.state != EnrollStatus.pending.name) { + EnrollStatus.pending != enrollStatus) { throw AtEnrollmentException( - 'Cannot approve a ${enrollDataStoreValue.approval!.state} enrollment. Only pending enrollments can be approved'); + 'Cannot approve a ${enrollStatus.name} enrollment. Only pending enrollments can be approved'); } if (operation == 'revoke' && - enrollDataStoreValue.approval!.state != EnrollStatus.approved.name) { + EnrollStatus.approved != enrollStatus) { throw AtEnrollmentException( - 'Cannot revoke a ${enrollDataStoreValue.approval!.state} enrollment. Only approved enrollments can be revoked'); + 'Cannot revoke a ${enrollStatus.name} enrollment. Only approved enrollments can be revoked'); } } + + Future _updateEnrollmentValueAndResetTTL( + String enrollmentKey, EnrollDataStoreValue enrollDataStoreValue) async { + // Fetch the existing data + AtData? enrollData = await keyStore.get(enrollmentKey); + // Update key with new data + // only update ttl, expiresAt in metadata to conserve all the other valid data fields + await keyStore.put( + enrollmentKey, + AtData() + ..data = jsonEncode(enrollDataStoreValue.toJson()) + ..metaData = (enrollData?.metaData + ?..ttl = 0 + ..expiresAt = null), + skipCommit: true); + } } diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index e3ac6043b..bae8ff308 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -12,7 +12,6 @@ import 'package:at_persistence_secondary_server/at_persistence_secondary_server. import 'package:at_chops/at_chops.dart'; import 'package:at_server_spec/at_server_spec.dart'; import 'package:at_secondary/src/constants/enroll_constants.dart'; -import 'package:at_secondary/src/utils/secondary_util.dart'; import 'package:meta/meta.dart'; class PkamVerbHandler extends AbstractVerbHandler { @@ -89,13 +88,9 @@ class PkamVerbHandler extends AbstractVerbHandler { '$enrollId.$newEnrollmentKeyPattern.$enrollManageNamespace$atSign'; late final EnrollDataStoreValue enrollDataStoreValue; ApkamVerificationResult apkamResult = ApkamVerificationResult(); - AtData? enrollData; try { - enrollData = await keyStore.get(enrollmentKey); - enrollDataStoreValue = - EnrollDataStoreValue.fromJson(jsonDecode(enrollData!.data!)); - EnrollStatus enrollStatus = - EnrollStatus.values.byName(enrollDataStoreValue.approval!.state); + enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); + EnrollStatus enrollStatus = getEnrollStatusFromString(enrollDataStoreValue.approval!.state); apkamResult.response = _getApprovalStatus(enrollStatus, enrollId); } on KeyNotFoundException catch (e) { logger.finer('Caught exception trying to fetch enrollment key: $e'); @@ -103,9 +98,6 @@ class PkamVerbHandler extends AbstractVerbHandler { apkamResult.response.errorCode = 'AT0028'; apkamResult.response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; } - if(!SecondaryUtil.isActiveKey(enrollData)){ - apkamResult.response = _getApprovalStatus(EnrollStatus.expired, enrollId); - } if (apkamResult.response.isError) { return apkamResult; } diff --git a/packages/at_secondary_server/test/enroll_verb_test.dart b/packages/at_secondary_server/test/enroll_verb_test.dart index bcef11ed0..dfb1904cd 100644 --- a/packages/at_secondary_server/test/enroll_verb_test.dart +++ b/packages/at_secondary_server/test/enroll_verb_test.dart @@ -609,7 +609,7 @@ void main() { HashMap enrollVerbParams; setUp(() async { await verbTestsSetUp(); - // Fetch TOTP + // Fetch OTP String totpCommand = 'otp:get'; HashMap totpVerbParams = getVerbParam(VerbSyntax.otp, totpCommand); @@ -632,6 +632,7 @@ void main() { String status = jsonDecode(response.data!)['status']; expect(status, 'pending'); }); + test('A test to verify denied enrollment cannot be approved', () async { //deny enrollment String denyEnrollmentCommand = From 4a31b09dbd737ce4e47a1689899da750f82601bb Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sun, 10 Sep 2023 02:08:35 +0530 Subject: [PATCH 15/21] fix: improve error handling in pkamVerb --- .../lib/src/verb/handler/pkam_verb_handler.dart | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart index bae8ff308..96b28aae2 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/pkam_verb_handler.dart @@ -88,16 +88,15 @@ class PkamVerbHandler extends AbstractVerbHandler { '$enrollId.$newEnrollmentKeyPattern.$enrollManageNamespace$atSign'; late final EnrollDataStoreValue enrollDataStoreValue; ApkamVerificationResult apkamResult = ApkamVerificationResult(); + EnrollStatus? enrollStatus; try { enrollDataStoreValue = await getEnrollDataStoreValue(enrollmentKey); - EnrollStatus enrollStatus = getEnrollStatusFromString(enrollDataStoreValue.approval!.state); - apkamResult.response = _getApprovalStatus(enrollStatus, enrollId); + enrollStatus = getEnrollStatusFromString(enrollDataStoreValue.approval!.state); } on KeyNotFoundException catch (e) { logger.finer('Caught exception trying to fetch enrollment key: $e'); - apkamResult.response.isError = true; - apkamResult.response.errorCode = 'AT0028'; - apkamResult.response.errorMessage = 'enrollment_id: $enrollId is expired or invalid'; + enrollStatus = EnrollStatus.expired; } + apkamResult.response = _getApprovalStatus(enrollStatus, enrollId); if (apkamResult.response.isError) { return apkamResult; } From 89f2247d1a9abe6556fcaf4f5624866a32df27ab Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sun, 10 Sep 2023 02:12:19 +0530 Subject: [PATCH 16/21] fix: ttl not being updated in enroll_verb_handler.dart --- .../src/verb/handler/enroll_verb_handler.dart | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 17554b276..37d81d51d 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -181,22 +181,26 @@ class EnrollVerbHandler extends AbstractVerbHandler { // 1. Enrollment key is not present in keystore // 2. Enrollment key is not active try { - enrollDataStoreValue = await getEnrollDataStoreValue('$enrollmentKey$currentAtSign'); + enrollDataStoreValue = + await getEnrollDataStoreValue('$enrollmentKey$currentAtSign'); } on KeyNotFoundException { // When an enrollment key is expired or invalid enrollStatus = EnrollStatus.expired; } - enrollStatus ??= getEnrollStatusFromString(enrollDataStoreValue!.approval!.state); + enrollStatus ??= + getEnrollStatusFromString(enrollDataStoreValue!.approval!.state); // Validates if enrollment is not expired - _validateEnrollmentValidity(enrollStatus, enrollmentIdFromParams!, response); - if(response.isError){ + _validateEnrollmentValidity( + enrollStatus, enrollmentIdFromParams!, response); + if (response.isError) { return; } // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state _verifyEnrollmentStateBeforeAction(operation, enrollStatus); - enrollDataStoreValue!.approval!.state = _getEnrollStatusEnum(operation).name; + enrollDataStoreValue!.approval!.state = + _getEnrollStatusEnum(operation).name; responseJson['status'] = _getEnrollStatusEnum(operation).name; // If an enrollment is approved, we need the enrollment to be active @@ -212,7 +216,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { if (operation == 'approve') { var apkamPublicKeyInKeyStore = 'public:${enrollDataStoreValue.appName}.${enrollDataStoreValue.deviceName}.pkam.$pkamNamespace.__public_keys$currentAtSign'; - var valueJson = {'apkamPublicKey':enrollDataStoreValue.apkamPublicKey}; + var valueJson = {'apkamPublicKey': enrollDataStoreValue.apkamPublicKey}; var atData = AtData()..data = jsonEncode(valueJson); await keyStore.put(apkamPublicKeyInKeyStore, atData); await _storeEncryptionKeys( @@ -339,13 +343,14 @@ class EnrollVerbHandler extends AbstractVerbHandler { 'Error while storing notification key $enrollmentId. Error $e. Trace $trace'); } } - + void _validateEnrollmentValidity( EnrollStatus enrollStatus, String enrollmentId, Response response) { if (EnrollStatus.expired == enrollStatus) { response.isError = true; response.errorCode = 'AT0028'; - response.errorMessage = 'enrollment_id: $enrollmentId is expired or invalid'; + response.errorMessage = + 'enrollment_id: $enrollmentId is expired or invalid'; return; } } @@ -355,31 +360,29 @@ class EnrollVerbHandler extends AbstractVerbHandler { /// from the intended state. void _verifyEnrollmentStateBeforeAction( String? operation, EnrollStatus enrollStatus) { - if (operation == 'approve' && - EnrollStatus.pending != enrollStatus) { + if (operation == 'approve' && EnrollStatus.pending != enrollStatus) { throw AtEnrollmentException( 'Cannot approve a ${enrollStatus.name} enrollment. Only pending enrollments can be approved'); } - if (operation == 'revoke' && - EnrollStatus.approved != enrollStatus) { + if (operation == 'revoke' && EnrollStatus.approved != enrollStatus) { throw AtEnrollmentException( 'Cannot revoke a ${enrollStatus.name} enrollment. Only approved enrollments can be revoked'); } } - + Future _updateEnrollmentValueAndResetTTL( String enrollmentKey, EnrollDataStoreValue enrollDataStoreValue) async { // Fetch the existing data - AtData? enrollData = await keyStore.get(enrollmentKey); + AtMetaData? enrollMetaData = await keyStore.getMeta(enrollmentKey); // Update key with new data - // only update ttl, expiresAt in metadata to conserve all the other valid data fields + // only update ttl, expiresAt in metadata to preserve all the other valid data fields + enrollMetaData?.ttl = 0; + enrollMetaData?.expiresAt = null; await keyStore.put( enrollmentKey, AtData() ..data = jsonEncode(enrollDataStoreValue.toJson()) - ..metaData = (enrollData?.metaData - ?..ttl = 0 - ..expiresAt = null), + ..metaData = enrollMetaData, skipCommit: true); } } From 4137f5eb500f2352968d51bb53bed5ff00ff521d Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sun, 10 Sep 2023 02:13:30 +0530 Subject: [PATCH 17/21] test: fix failing functional tests --- tests/at_functional_test/test/enroll_verb_test.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/at_functional_test/test/enroll_verb_test.dart b/tests/at_functional_test/test/enroll_verb_test.dart index 1b6c4df5c..c6b2bf7ad 100644 --- a/tests/at_functional_test/test/enroll_verb_test.dart +++ b/tests/at_functional_test/test/enroll_verb_test.dart @@ -3,7 +3,6 @@ import 'dart:convert'; import 'dart:io'; -import 'package:at_commons/at_commons.dart'; import 'package:at_demo_data/at_demo_data.dart' as at_demos; import 'package:at_functional_test/conf/config_util.dart'; import 'package:test/test.dart'; @@ -121,7 +120,7 @@ void main() { var approveEnrollResponse = await read(); approveEnrollResponse = approveEnrollResponse.replaceFirst('error:', ''); expect(approveEnrollResponse, - 'AT0028:enrollment_id: $enrollmentId is expired or invalid'); + 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid'); }); test( @@ -141,7 +140,7 @@ void main() { var denyEnrollResponse = await read(); denyEnrollResponse = denyEnrollResponse.replaceFirst('error:', ''); expect(denyEnrollResponse, - 'AT0028:enrollment_id: $enrollmentId is expired or invalid'); + 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid'); }); test('enroll request on unauthenticated connection without otp', () async { From 55e99bb43aaf7e9ab6cab7b1f0b3f68df28a8938 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Sun, 10 Sep 2023 02:21:01 +0530 Subject: [PATCH 18/21] test: fix failing functional tests --- tests/at_functional_test/test/enroll_verb_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/at_functional_test/test/enroll_verb_test.dart b/tests/at_functional_test/test/enroll_verb_test.dart index c6b2bf7ad..2868b6704 100644 --- a/tests/at_functional_test/test/enroll_verb_test.dart +++ b/tests/at_functional_test/test/enroll_verb_test.dart @@ -120,7 +120,7 @@ void main() { var approveEnrollResponse = await read(); approveEnrollResponse = approveEnrollResponse.replaceFirst('error:', ''); expect(approveEnrollResponse, - 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid'); + 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid\n'); }); test( @@ -140,7 +140,7 @@ void main() { var denyEnrollResponse = await read(); denyEnrollResponse = denyEnrollResponse.replaceFirst('error:', ''); expect(denyEnrollResponse, - 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid'); + 'AT0028:enrollment_id: $dummyEnrollmentId is expired or invalid\n'); }); test('enroll request on unauthenticated connection without otp', () async { From 25eb563f94d1815b2af8171185a22e3496ded925 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Mon, 11 Sep 2023 15:15:09 +0530 Subject: [PATCH 19/21] test: fix: do not use shared response object for tests --- .../verb/handler/abstract_verb_handler.dart | 3 +- .../src/verb/handler/enroll_verb_handler.dart | 1 - .../test/enroll_verb_test.dart | 75 ++++++++++++------- 3 files changed, 48 insertions(+), 31 deletions(-) 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 55c320d0c..5e0c52b50 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 @@ -90,7 +90,8 @@ abstract class AbstractVerbHandler implements VerbHandler { AtData enrollData = await keyStore.get(enrollmentKey); EnrollDataStoreValue enrollDataStoreValue = EnrollDataStoreValue.fromJson(jsonDecode(enrollData.data!)); - if(!SecondaryUtil.isActiveKey(enrollData)){ + if (!SecondaryUtil.isActiveKey(enrollData) && + enrollDataStoreValue.approval!.state != EnrollStatus.approved.name) { enrollDataStoreValue.approval?.state = EnrollStatus.expired.name; } return enrollDataStoreValue; diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 37d81d51d..d51c8d9a5 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -351,7 +351,6 @@ class EnrollVerbHandler extends AbstractVerbHandler { response.errorCode = 'AT0028'; response.errorMessage = 'enrollment_id: $enrollmentId is expired or invalid'; - return; } } diff --git a/packages/at_secondary_server/test/enroll_verb_test.dart b/packages/at_secondary_server/test/enroll_verb_test.dart index dfb1904cd..6e48d943d 100644 --- a/packages/at_secondary_server/test/enroll_verb_test.dart +++ b/packages/at_secondary_server/test/enroll_verb_test.dart @@ -341,8 +341,9 @@ void main() { test( 'A test to verify revoke operations thrown exception when given enrollmentId is not in keystore', () async { - String enrollmentId = '123'; - String enrollmentRequest = 'enroll:revoke:{"enrollmentId":"$enrollmentId"}'; + String enrollmentId = '123'; + String enrollmentRequest = + 'enroll:revoke:{"enrollmentId":"$enrollmentId"}'; HashMap verbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = true; @@ -352,10 +353,12 @@ void main() { Response response = Response(); EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); - await enrollVerbHandler.processVerb(response, verbParams, inboundConnection); + await enrollVerbHandler.processVerb( + response, verbParams, inboundConnection); expect(response.isError, true); expect(response.errorMessage, isNotNull); - assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + assert(response.errorMessage! + .contains('enrollment_id: $enrollmentId is expired')); expect(response.errorCode, 'AT0028'); }); tearDown(() async => await verbTestsTearDown()); @@ -378,12 +381,12 @@ void main() { inboundConnection.getMetaData().sessionID = 'dummy_session'; (inboundConnection.getMetaData() as InboundConnectionMetadata) .enrollmentId = '123'; - Response response = Response(); + Response responseObject = Response(); EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); await enrollVerbHandler.processVerb( - response, verbParams, inboundConnection); - Map enrollmentResponse = jsonDecode(response.data!); + responseObject, verbParams, inboundConnection); + Map enrollmentResponse = jsonDecode(responseObject.data!); expect(enrollmentResponse['enrollmentId'], isNotNull); expect(enrollmentResponse['status'], 'approved'); // Commit log @@ -464,7 +467,7 @@ void main() { }); group('A group of tests related to enrollment request expiry', () { - Response response = Response(); + String? otp; setUp(() async { await verbTestsSetUp(); // Fetch TOTP @@ -473,16 +476,19 @@ void main() { getVerbParam(VerbSyntax.otp, totpCommand); OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore); inboundConnection.getMetaData().isAuthenticated = true; + Response defaultResponse = Response(); await otpVerbHandler.processVerb( - response, totpVerbParams, inboundConnection); + defaultResponse, totpVerbParams, inboundConnection); + otp = defaultResponse.data; }); test('A test to verify expired enrollment cannot be approved', () async { + Response response = Response(); // Enroll a request on an unauthenticated connection which will expire in 1 millisecond EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); enrollVerbHandler.enrollmentExpiryInMills = 1; String enrollmentRequest = - 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}'; + 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}'; HashMap enrollVerbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = false; @@ -504,21 +510,23 @@ void main() { response, enrollVerbParams, inboundConnection); expect(response.isError, true); expect(response.errorMessage, isNotNull); - assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + assert(response.errorMessage! + .contains('enrollment_id: $enrollmentId is expired')); expect(response.errorCode, 'AT0028'); }); test('A test to verify expired enrollment cannot be denied', () async { + Response response = Response(); // Enroll a request on an unauthenticated connection which will expire in 1 millisecond EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); enrollVerbHandler.enrollmentExpiryInMills = 1; String enrollmentRequest = - 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}'; + 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}'; HashMap enrollVerbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = false; - inboundConnection.getMetaData().sessionID = 'dummy_session_id'; + inboundConnection.getMetaData().sessionID = 'dummy_session_id1'; await enrollVerbHandler.processVerb( response, enrollVerbParams, inboundConnection); String enrollmentId = jsonDecode(response.data!)['enrollmentId']; @@ -532,20 +540,23 @@ void main() { getVerbParam(VerbSyntax.enroll, approveEnrollmentCommand); inboundConnection.getMetaData().isAuthenticated = true; inboundConnection.getMetaData().sessionID = 'dummy_session_id'; - await enrollVerbHandler.processVerb(response, enrollVerbParams, inboundConnection); + await enrollVerbHandler.processVerb( + response, enrollVerbParams, inboundConnection); expect(response.isError, true); expect(response.errorMessage, isNotNull); - assert(response.errorMessage!.contains('enrollment_id: $enrollmentId is expired')); + assert(response.errorMessage! + .contains('enrollment_id: $enrollmentId is expired')); expect(response.errorCode, 'AT0028'); }); test('A test to verify TTL on approved enrollment is reset', () async { + Response response = Response(); // Enroll a request on an unauthenticated connection which will expire in 1 minute EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); - enrollVerbHandler.enrollmentExpiryInMills = 60000; + enrollVerbHandler.enrollmentExpiryInMills = 600000; String enrollmentRequest = - 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}'; + 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}'; HashMap enrollVerbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = false; @@ -555,11 +566,12 @@ void main() { String enrollmentId = jsonDecode(response.data!)['enrollmentId']; String status = jsonDecode(response.data!)['status']; expect(status, 'pending'); + String enrollmentKey = + '$enrollmentId.$newEnrollmentKeyPattern.$enrollManageNamespace$alice'; // Verify TTL is added to the enrollment - AtData? enrollmentData = await secondaryKeyStore.get( - '$enrollmentId.$newEnrollmentKeyPattern.$enrollManageNamespace$alice'); + AtData? enrollmentData = await secondaryKeyStore.get(enrollmentKey); expect(enrollmentData!.metaData!.expiresAt, isNotNull); - expect(enrollmentData.metaData!.ttl, 60000); + expect(enrollmentData.metaData!.ttl, 600000); //Approve enrollment String approveEnrollmentCommand = 'enroll:approve:{"enrollmentId":"$enrollmentId"}'; @@ -570,8 +582,7 @@ void main() { await enrollVerbHandler.processVerb( response, enrollVerbParams, inboundConnection); // Verify TTL is reset - enrollmentData = await secondaryKeyStore.get( - '$enrollmentId.$newEnrollmentKeyPattern.$enrollManageNamespace$alice'); + enrollmentData = await secondaryKeyStore.get(enrollmentKey); expect(enrollmentData!.metaData!.expiresAt, null); expect(enrollmentData.metaData!.ttl, 0); }); @@ -579,10 +590,11 @@ void main() { test( 'A test to verify TTL is not set for enrollment requested on an authenticated connection', () async { + Response response = Response(); EnrollVerbHandler enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); String enrollmentRequest = - 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}'; + 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}'; HashMap enrollVerbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = true; @@ -603,10 +615,11 @@ void main() { }); group('A group of tests related to approve enrollment', () { - Response response = Response(); + String? otp; late String enrollmentId; late EnrollVerbHandler enrollVerbHandler; HashMap enrollVerbParams; + Response defaultResponse = Response(); setUp(() async { await verbTestsSetUp(); // Fetch OTP @@ -616,24 +629,26 @@ void main() { OtpVerbHandler otpVerbHandler = OtpVerbHandler(secondaryKeyStore); inboundConnection.getMetaData().isAuthenticated = true; await otpVerbHandler.processVerb( - response, totpVerbParams, inboundConnection); + defaultResponse, totpVerbParams, inboundConnection); + otp = defaultResponse.data; // Enroll a request on an unauthenticated connection which will expire in 1 minute enrollVerbHandler = EnrollVerbHandler(secondaryKeyStore); enrollVerbHandler.enrollmentExpiryInMills = 60000; String enrollmentRequest = - 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"${response.data}","apkamPublicKey":"dummy_apkam_public_key"}'; + 'enroll:request:{"appName":"wavi","deviceName":"mydevice","namespaces":{"wavi":"r"},"otp":"$otp","apkamPublicKey":"dummy_apkam_public_key"}'; HashMap enrollVerbParams = getVerbParam(VerbSyntax.enroll, enrollmentRequest); inboundConnection.getMetaData().isAuthenticated = false; inboundConnection.getMetaData().sessionID = 'dummy_session_id'; await enrollVerbHandler.processVerb( - response, enrollVerbParams, inboundConnection); - enrollmentId = jsonDecode(response.data!)['enrollmentId']; - String status = jsonDecode(response.data!)['status']; + defaultResponse, enrollVerbParams, inboundConnection); + enrollmentId = jsonDecode(defaultResponse.data!)['enrollmentId']; + String status = jsonDecode(defaultResponse.data!)['status']; expect(status, 'pending'); }); test('A test to verify denied enrollment cannot be approved', () async { + Response response = Response(); //deny enrollment String denyEnrollmentCommand = 'enroll:deny:{"enrollmentId":"$enrollmentId"}'; @@ -661,6 +676,7 @@ void main() { }); test('A test to verify revoked enrollment cannot be approved', () async { + Response response = Response(); //approve enrollment String approveEnrollmentCommand = 'enroll:approve:{"enrollmentId":"$enrollmentId"}'; @@ -691,6 +707,7 @@ void main() { }); test('A test to verify pending enrollment cannot be revoked', () async { + Response response = Response(); //revoke enrollment String denyEnrollmentCommand = 'enroll:revoke:{"enrollmentId":"$enrollmentId"}'; From 2ce182065eb49268bbd4f8e97616c2a1ed798f78 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Mon, 11 Sep 2023 17:35:17 +0530 Subject: [PATCH 20/21] fix: rename verifyEnrollmentStateBeforeAction -> verifyEnrollmentState and merge validateEnrollmentValidity() into parent --- .../src/verb/handler/enroll_verb_handler.dart | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index d51c8d9a5..02fd0c7b2 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -48,20 +48,21 @@ class EnrollVerbHandler extends AbstractVerbHandler { } EnrollParams? enrollVerbParams; try { - if (verbParams[enrollParams] != null) { + if (verbParams[enrollParams] == null) { + throw IllegalArgumentException('Verb parameters not provided for EnrollVerb'); + } enrollVerbParams = EnrollParams.fromJson(jsonDecode(verbParams[enrollParams]!)); - } switch (operation) { case 'request': await _handleEnrollmentRequest( - enrollVerbParams!, currentAtSign, responseJson, atConnection); + enrollVerbParams, currentAtSign, responseJson, atConnection); break; case 'approve': case 'deny': case 'revoke': - await _handleEnrollmentPermissions(enrollVerbParams!, currentAtSign, + await _handleEnrollmentPermissions(enrollVerbParams, currentAtSign, operation, responseJson, response); break; @@ -190,15 +191,19 @@ class EnrollVerbHandler extends AbstractVerbHandler { enrollStatus ??= getEnrollStatusFromString(enrollDataStoreValue!.approval!.state); // Validates if enrollment is not expired - _validateEnrollmentValidity( - enrollStatus, enrollmentIdFromParams!, response); + if (EnrollStatus.expired == enrollStatus) { + response.isError = true; + response.errorCode = 'AT0028'; + response.errorMessage = + 'enrollment_id: $enrollmentId is expired or invalid'; + } if (response.isError) { return; } // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state - _verifyEnrollmentStateBeforeAction(operation, enrollStatus); + _verifyEnrollmentState(operation, enrollStatus); enrollDataStoreValue!.approval!.state = _getEnrollStatusEnum(operation).name; responseJson['status'] = _getEnrollStatusEnum(operation).name; @@ -220,7 +225,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { var atData = AtData()..data = jsonEncode(valueJson); await keyStore.put(apkamPublicKeyInKeyStore, atData); await _storeEncryptionKeys( - enrollmentIdFromParams, enrollParams, currentAtSign); + enrollmentIdFromParams!, enrollParams, currentAtSign); } responseJson['enrollmentId'] = enrollmentIdFromParams; } @@ -344,20 +349,10 @@ class EnrollVerbHandler extends AbstractVerbHandler { } } - void _validateEnrollmentValidity( - EnrollStatus enrollStatus, String enrollmentId, Response response) { - if (EnrollStatus.expired == enrollStatus) { - response.isError = true; - response.errorCode = 'AT0028'; - response.errorMessage = - 'enrollment_id: $enrollmentId is expired or invalid'; - } - } - /// Verifies whether the enrollment state matches the intended state. /// Throws AtEnrollmentException: If the enrollment state is different /// from the intended state. - void _verifyEnrollmentStateBeforeAction( + void _verifyEnrollmentState( String? operation, EnrollStatus enrollStatus) { if (operation == 'approve' && EnrollStatus.pending != enrollStatus) { throw AtEnrollmentException( From 4ef1efb0c490d5ec2459a5d7768fa917a0b28ab8 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Mon, 11 Sep 2023 19:52:53 +0530 Subject: [PATCH 21/21] fix: ensure enrollParams are not null in EnrollVerb --- .../src/verb/handler/enroll_verb_handler.dart | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart index 02fd0c7b2..49e85eacf 100644 --- a/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart +++ b/packages/at_secondary_server/lib/src/verb/handler/enroll_verb_handler.dart @@ -48,21 +48,28 @@ class EnrollVerbHandler extends AbstractVerbHandler { } EnrollParams? enrollVerbParams; try { + // Ensure that enrollParams are present for all enroll operation + // Exclude operation 'list' which does not have enrollParams if (verbParams[enrollParams] == null) { - throw IllegalArgumentException('Verb parameters not provided for EnrollVerb'); - } + if (operation != 'list') { + logger.severe( + 'Enroll params is empty | EnrollParams: ${verbParams[enrollParams]}'); + throw IllegalArgumentException('Enroll parameters not provided'); + } + } else { enrollVerbParams = EnrollParams.fromJson(jsonDecode(verbParams[enrollParams]!)); + } switch (operation) { case 'request': await _handleEnrollmentRequest( - enrollVerbParams, currentAtSign, responseJson, atConnection); + enrollVerbParams!, currentAtSign, responseJson, atConnection); break; case 'approve': case 'deny': case 'revoke': - await _handleEnrollmentPermissions(enrollVerbParams, currentAtSign, + await _handleEnrollmentPermissions(enrollVerbParams!, currentAtSign, operation, responseJson, response); break; @@ -195,7 +202,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { response.isError = true; response.errorCode = 'AT0028'; response.errorMessage = - 'enrollment_id: $enrollmentId is expired or invalid'; + 'enrollment_id: $enrollmentIdFromParams is expired or invalid'; } if (response.isError) { return; @@ -203,7 +210,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { // Verifies whether the enrollment state matches the intended state // Throws AtEnrollmentException, if the enrollment state is different from // the intended state - _verifyEnrollmentState(operation, enrollStatus); + _verifyEnrollmentStateBeforeAction(operation, enrollStatus); enrollDataStoreValue!.approval!.state = _getEnrollStatusEnum(operation).name; responseJson['status'] = _getEnrollStatusEnum(operation).name; @@ -352,7 +359,7 @@ class EnrollVerbHandler extends AbstractVerbHandler { /// Verifies whether the enrollment state matches the intended state. /// Throws AtEnrollmentException: If the enrollment state is different /// from the intended state. - void _verifyEnrollmentState( + void _verifyEnrollmentStateBeforeAction( String? operation, EnrollStatus enrollStatus) { if (operation == 'approve' && EnrollStatus.pending != enrollStatus) { throw AtEnrollmentException(