Merge pull request #1420 from atsign-foundation/feat-1403

feat: relax restrictions on device name

packages/dart/noports_core/CHANGELOG.md

@@ -1,3 +1,5 @@
+# 6.2.0
+- feat: allow hyphens in device name
 # 6.1.1
 - build[deps]: upgrade: \
   at_client to 3.2.2 | at_onboarding_cli to 1.6.4 | at_utils to 3.0.19 | at_commons to 5.0.0

packages/dart/noports_core/lib/src/common/default_args.dart

@@ -24,11 +24,16 @@ class DefaultArgs {
   static const bool authenticateDeviceToRvd = true;
   static const bool encryptRvdTraffic = true;
 
-  /// How long a client should wait for response after pinging a daemon
+  /// How long a client should wait for response after pinging a NoPorts daemon
   static const int daemonPingTimeoutSeconds = 20;
   static const Duration daemonPingTimeoutDuration =
       Duration(seconds: daemonPingTimeoutSeconds);
 
+  /// How long a client should wait for response from a NoPorts relay
+  static const int relayResponseTimeoutSeconds = 20;
+  static const Duration relayResponseTimeoutDuration =
+      Duration(seconds: relayResponseTimeoutSeconds);
+
   /// How long srv should stay running if SocketConnector has no connections
   static const int srvTimeoutInSeconds = 30;
   static const Duration srvTimeout = Duration(seconds: srvTimeoutInSeconds);

packages/dart/noports_core/lib/src/common/validation_utils.dart

@@ -8,13 +8,21 @@ import 'package:noports_core/src/common/file_system_utils.dart';
 import 'package:noports_core/src/common/io_types.dart';
 import 'package:path/path.dart' as path;
 
-const String sshnpDeviceNameRegex = r'[a-z0-9_]{1,36}';
+const String sshnpDeviceNameRegex = r'[a-z0-9][a-z0-9_\-]{1,35}';
 const String invalidDeviceNameMsg = 'Device name must be alphanumeric'
-    ' snake case, max length 36';
-const String deviceNameFormatHelp = 'Alphanumeric snake case, max length 36.';
+    ' snake case, max length 36. First char must be a-z or 0-9.';
+const String deviceNameFormatHelp = 'Alphanumeric snake case, max length 36. First char must be a-z or 0-9.';
 const String invalidSshKeyPermissionsMsg =
     'Detected newline characters in the ssh public key permissions which malforms the authorized_keys file.';
 
+/// Returns deviceName with uppercase latin replaced by lowercase, and
+/// whitespace replaced with underscores. Note that multiple consecutive
+/// whitespace characters will be replaced by a single underscore.
+String snakifyDeviceName(String deviceName) {
+  return deviceName.toLowerCase().replaceAll(RegExp(r'\s+'), '_');
+}
+
+/// Returns false if the device name does not match [sshnpDeviceNameRegex]
 bool invalidDeviceName(String test) {
   return RegExp(sshnpDeviceNameRegex).allMatches(test).first.group(0) != test;
 }

packages/dart/noports_core/lib/src/sshnp/models/sshnp_params.dart

@@ -299,12 +299,14 @@ class SshnpParams extends ClientParamsBase
               'srvdAtSign is mandatory, unless list-devices is passed.'));
     }
 
+    String device = partial.device ?? DefaultSshnpArgs.device;
+    device = snakifyDeviceName(device);
     return SshnpParams(
       profileName: partial.profileName,
       clientAtSign: partial.clientAtSign!,
       sshnpdAtSign: partial.sshnpdAtSign ?? "",
       srvdAtSign: partial.srvdAtSign ?? "",
-      device: partial.device ?? DefaultSshnpArgs.device,
+      device: device,
       localPort: partial.localPort ?? DefaultSshnpArgs.localPort,
       identityFile: partial.identityFile,
       identityPassphrase: partial.identityPassphrase,

packages/dart/noports_core/lib/src/sshnp/util/srvd_channel/srvd_channel.dart

@@ -130,7 +130,8 @@ abstract class SrvdChannel<T> with AsyncInitialization, AtClientBindings {
 
   @protected
   @visibleForTesting
-  Future<void> getHostAndPortFromSrvd() async {
+  Future<void> getHostAndPortFromSrvd(
+      {Duration timeout = DefaultArgs.relayResponseTimeoutDuration}) async {
     srvdAck = SrvdAck.notAcknowledged;
     subscribe(regex: '$sessionId.${Srvd.namespace}@', shouldDecrypt: true)
         .listen((notification) async {
@@ -201,13 +202,16 @@ abstract class SrvdChannel<T> with AsyncInitialization, AtClientBindings {
     );
 
     int counter = 1;
+    int t = DateTime.now().add(timeout).millisecondsSinceEpoch;
     while (srvdAck == SrvdAck.notAcknowledged) {
-      if (counter % 20 == 0) {
+      // we'll log a message every two seconds while we're waiting
+      // (40 loops, 50 milliseconds sleep per loop)
+      if (counter % 40 == 0) {
         logger.info('Still waiting for srvd response');
       }
-      await Future.delayed(Duration(milliseconds: 100));
+      await Future.delayed(Duration(milliseconds: 50));
       counter++;
-      if (counter > 150) {
+      if (DateTime.now().millisecondsSinceEpoch > t) {
         logger.warning('Timed out waiting for srvd response');
         throw TimeoutException(
             'Connection timeout to srvd ${params.srvdAtSign} service');

packages/dart/noports_core/lib/src/sshnpd/sshnpd_params.dart

@@ -78,14 +78,15 @@ class SshnpdParams {
     }
     String homeDirectory = getHomeDirectory()!;
 
-    // Do we have a device ?
-    String device = r['device'];
-
     SupportedSshClient sshClient = SupportedSshClient.values.firstWhere(
         (c) => c.toString() == r['ssh-client'],
         orElse: () => DefaultSshnpdArgs.sshClient);
 
-    // Do we have an ASCII ?
+    // Do we have a valid device name?
+    String device = r['device'];
+    // First of all let's snakify it
+    device = snakifyDeviceName(device);
+    // and now check it against desired regex
     if (invalidDeviceName(device)) {
       throw ArgumentError(invalidDeviceNameMsg);
     }
@@ -109,7 +110,7 @@ class SshnpdParams {
       permitOpen = '*:*';
     }
     return SshnpdParams(
-      device: r['device'],
+      device: device,
       username: getUserName(throwIfNull: true)!,
       homeDirectory: homeDirectory,
       managerAtsigns: managerAtsigns,
@@ -133,7 +134,7 @@ class SshnpdParams {
               homeDirectory: homeDirectory,
               atSign: deviceAtsign,
               progName: '.sshnpd',
-              uniqueID: r['device']),
+              uniqueID: device),
       permitOpen: permitOpen,
     );
   }

packages/dart/noports_core/pubspec.yaml

@@ -2,7 +2,7 @@ name: noports_core
 description: Core library code for sshnoports
 homepage: https://docs.atsign.com/
 
-version: 6.1.1
+version: 6.2.0
 
 environment:
   sdk: ">=3.0.0 <4.0.0"

packages/dart/noports_core/test/sshnp/models/sshnp_arg_test.dart

@@ -5,7 +5,7 @@ import 'package:test/test.dart';
 void main() {
   group('ParserType', () {
     test('public API test', () {
-      // abitrary values
+      // arbitrary values
       ParserType parserType = ParserType.all;
       ParseWhen parseWhen = ParseWhen.always;
 

packages/dart/noports_core/test/sshnp/models/sshnp_params_test.dart

@@ -92,6 +92,16 @@ void main() {
             clientAtSign: '', sshnpdAtSign: '', srvdAtSign: '@my_srvd');
         expect(params.srvdAtSign, equals('@my_srvd'));
       });
+      test('Test snakifyDeviceName', () {
+        expect(snakifyDeviceName('ABCDEF'), 'abcdef');
+        expect(snakifyDeviceName('Ab_cd_Ef'), 'ab_cd_ef');
+        expect(snakifyDeviceName('Ab-cd-Ef'), 'ab-cd-ef');
+        expect(snakifyDeviceName('Ab cD-Ef'), 'ab_cd-ef');
+        expect(snakifyDeviceName('Ab-cD Ef'), 'ab-cd_ef');
+        expect(snakifyDeviceName('Ab cD Ef'), 'ab_cd_ef');
+        expect(snakifyDeviceName('Ab\tcD\nEf'), 'ab_cd_ef');
+        expect(snakifyDeviceName('Ab \t\n cD      Ef'), 'ab_cd_ef');
+      });
       test('SshnpParams.device invalid with uppercase test', () {
         expect(
             () => SshnpParams(
@@ -107,7 +117,7 @@ void main() {
                 clientAtSign: '',
                 sshnpdAtSign: '',
                 srvdAtSign: '',
-                device: 'my-device-name'),
+                device: 'my#device#name'),
             throwsA(TypeMatcher<ArgumentError>()));
       });
       test('SshnpParams.device invalid too long test', () {
@@ -119,7 +129,16 @@ void main() {
                 device: 'abcde_12345_abcde_12345_abcde_12345_X'),
             throwsA(TypeMatcher<ArgumentError>()));
       });
-      test('SshnpParams.device test', () {
+      test('SshnpParams.device invalid must start with a-z or 0-9', () {
+        expect(
+            () => SshnpParams(
+                clientAtSign: '',
+                sshnpdAtSign: '',
+                srvdAtSign: '',
+                device: '_abcde-12345-abcde_12345_abcde_12345'),
+            throwsA(TypeMatcher<ArgumentError>()));
+      });
+      test('SshnpParams.device test pure snake case', () {
         String deviceName = 'my_device_name_12345';
         final params = SshnpParams(
             clientAtSign: '',
@@ -128,6 +147,15 @@ void main() {
             device: deviceName);
         expect(params.device, equals(deviceName));
       });
+      test('SshnpParams.device test with hyphens', () {
+        String deviceName = 'my-device-name_12345';
+        final params = SshnpParams(
+            clientAtSign: '',
+            sshnpdAtSign: '',
+            srvdAtSign: '',
+            device: deviceName);
+        expect(params.device, equals(deviceName));
+      });
       test('SshnpParams.localPort test', () {
         final params = SshnpParams(
             clientAtSign: '',

packages/dart/noports_core/test/sshnp/util/srvd_channel/srvd_channel_test.dart

@@ -330,7 +330,9 @@ void main() {
           sessionId: sessionId);
 
       expect(
-          () async => await srvdDartBindPortChannel.getHostAndPortFromSrvd(),
+          () async => await srvdDartBindPortChannel.getHostAndPortFromSrvd(
+              // set timeout to something short so unit test runs quickly
+              timeout: Duration(milliseconds: 50)),
           throwsA(predicate((dynamic e) =>
               e is TimeoutException &&
               e.message == 'Connection timeout to srvd @srvd service')));

packages/dart/sshnoports/bin/npt.dart

@@ -233,6 +233,14 @@ void main(List<String> args) async {
       bool quiet = parsedArgs[quietFlag];
       bool keepAlive = parsedArgs['keep-alive'];
 
+      // Do we have a valid device name?
+      // First of all let's snakify it
+      device = snakifyDeviceName(device);
+      // and now check it against desired regex
+      if (invalidDeviceName(device)) {
+        throw ArgumentError(invalidDeviceNameMsg);
+      }
+
       // A listen progress listener for the CLI
       // Will only log if verbose is false, since if verbose is true
       // there will already be a boatload of log messages.

packages/dart/sshnoports/pubspec.lock

@@ -593,7 +593,7 @@ packages:
       path: "../noports_core"
       relative: true
     source: path
-    version: "6.1.1"
+    version: "6.2.0"
   openssh_ed25519:
     dependency: transitive
     description:

packages/dart/sshnoports/pubspec.yaml

@@ -1,15 +1,15 @@
 name: sshnoports
 publish_to: none
 
-version: 5.6.2
+version: 5.6.3
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
 
 dependencies:
   noports_core:
     path: "../noports_core"
-    version: 6.1.1
+    version: 6.2.0
   at_onboarding_cli: 1.6.4
   at_cli_commons: ^1.1.0
   at_client: ^3.2.2