Repository: atsign-foundation/noports (dart)

Issue:
Better defaults for sshnpd's --permit-open
### Is your feature request related to a problem? Please describe.

Currently permitOpen defaults to `localhost:22,localhost:3389` which is overly restrictive for a typical 'bastion'-like daemon deployment.

### Describe the solution you'd like

IF a --permit-open is not specified by the user
AND a policy atSign IS specified
THEN default permitOpen to `*:*`


### Describe alternatives you've considered

Considered just defaulting to '*:*' in all cases but that might be overly permissive. In discussion, @cpswan and I felt that this proposed solution has the best balance between considerations of user experience and security.

### Additional context

_No response_

Pull request:
**- What I did**
- resolves #1295 

**- How I did it**
- added logic in SshnpdParams for dealing with permitOpen in the specific situation described in #1295
- added the permitOpen default to SshnpdDefaultArgs
- added unit tests

**- How to verify it**
Tests pass

Patch:
diff --git a/packages/dart/noports_core/lib/src/common/default_args.dart b/packages/dart/noports_core/lib/src/common/default_args.dart
index 3086da642..9f3e8fe7b 100644
--- a/packages/dart/noports_core/lib/src/common/default_args.dart
+++ b/packages/dart/noports_core/lib/src/common/default_args.dart
@@ -49,4 +49,5 @@ class DefaultSshnpdArgs {
   static const String deviceGroupName = '__none__';
   static const String sshPublicKeyPermissions = "";
   static const Duration policyHeartbeatFrequency = Duration(minutes: 5);
+  static const String permitOpen ='localhost:22,localhost:3389';
 }
diff --git a/packages/dart/noports_core/lib/src/sshnpd/sshnpd_params.dart b/packages/dart/noports_core/lib/src/sshnpd/sshnpd_params.dart
index a7625ce10..ac25e881c 100644
--- a/packages/dart/noports_core/lib/src/sshnpd/sshnpd_params.dart
+++ b/packages/dart/noports_core/lib/src/sshnpd/sshnpd_params.dart
@@ -101,6 +101,13 @@ class SshnpdParams {
       throw ArgumentError(invalidSshKeyPermissionsMsg);
     }
 
+    var permitOpen = r['permit-open'];
+    // #1295 - if we have a policy-manager arg and no permit-open arg
+    // then we set permit-open '*:*' (since the policy manager will
+    // be taking care of decisions in any event)
+    if (r.wasParsed('policy-manager') && !r.wasParsed('permit-open')) {
+      permitOpen = '*:*';
+    }
     return SshnpdParams(
       device: r['device'],
       username: getUserName(throwIfNull: true)!,
@@ -127,7 +134,7 @@ class SshnpdParams {
               atSign: deviceAtsign,
               progName: '.sshnpd',
               uniqueID: r['device']),
-      permitOpen: r['permit-open'],
+      permitOpen: permitOpen,
     );
   }
 
@@ -288,7 +295,7 @@ class SshnpdParams {
       'permit-open',
       aliases: ['po'],
       mandatory: false,
-      defaultsTo: 'localhost:22,localhost:3389',
+      defaultsTo: DefaultSshnpdArgs.permitOpen,
       help: 'Comma separated-list of host:port to which the daemon will permit'
           ' a connection from an authorized client. Hosts may be dns names or'
           ' ip addresses.',


Test patch:
diff --git a/packages/dart/noports_core/test/sshnpd/sshnpd_params_test.dart b/packages/dart/noports_core/test/sshnpd/sshnpd_params_test.dart
new file mode 100644
index 000000000..dabc885bd
--- /dev/null
+++ b/packages/dart/noports_core/test/sshnpd/sshnpd_params_test.dart
@@ -0,0 +1,60 @@
+import 'package:noports_core/src/common/default_args.dart';
+import 'package:noports_core/src/sshnpd/sshnpd_params.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('test sshnpd params defaults', () {
+    test('require at least one of managers and policyManager options', () async {
+      List<String> args = '-a @daemon'.split(' ');
+      await expectLater(
+              () => SshnpdParams.fromArgs(args),
+          throwsA(TypeMatcher<ArgumentError>()));
+    });
+    test('just managers option supplied', () async {
+      List<String> args = '-a @daemon -m @bob'.split(' ');
+      final p = await SshnpdParams.fromArgs(args);
+      expect(p.managerAtsigns, ['@bob']);
+    });
+    test('just policyManager option supplied', () async {
+      List<String> args = '-a @daemon -p @bob'.split(' ');
+      final p = await SshnpdParams.fromArgs(args);
+      expect(p.policyManagerAtsign, '@bob');
+      expect(p.managerAtsigns, []);
+    });
+    test('both managers and policyManager options supplied', () async {
+      List<String> args = '-a @daemon -m @bob,@chuck -p @policy'.split(' ');
+      final p = await SshnpdParams.fromArgs(args);
+      expect(p.deviceAtsign, '@daemon');
+      expect(p.policyManagerAtsign, '@policy');
+      expect(p.managerAtsigns, ['@bob','@chuck']);
+    });
+    test('test permitOpen default without policyManager', () async {
+      List<String> args = '-a @daemon -m @bob'.split(' ');
+      SshnpdParams p = await SshnpdParams.fromArgs(args);
+      expect(p.permitOpen, DefaultSshnpdArgs.permitOpen);
+    });
+    test('test permitOpen default with policyManager', () async {
+      List<String> args = '-a @daemon -p @policy'.split(' ');
+      SshnpdParams p = await SshnpdParams.fromArgs(args);
+      expect(p.permitOpen, '*:*');
+    });
+    test('test permitOpen provided without policyManager', () async {
+      final po = 'host1.sub.net:12345,host2.sub.net:34567,localhost:3000';
+      List<String> args = '-a @daemon -m @bob --permit-open $po'.split(' ');
+      SshnpdParams p = await SshnpdParams.fromArgs(args);
+      expect(p.permitOpen, po);
+    });
+    test('test permitOpen provided with policyManager', () async {
+      final po = 'host1.sub.net:12345,host2.sub.net:34567,localhost:3000';
+      List<String> args = '-a @daemon -p @policy --permit-open $po'.split(' ');
+      SshnpdParams p = await SshnpdParams.fromArgs(args);
+      expect(p.permitOpen, po);
+    });
+    // TODO add unit tests for other mildly complicated options
+    // device
+    // storage-path
+    // key-file
+    // local-sshd-port
+    // sshpublickey-permissions
+  });
+}
\ No newline at end of file