Backport 1.17.x: add allow_empty_principals to ssh engine (fixes fail…

…ing test) (#28493) * add ssh allowEmptyPrincipals to openapi * add attrs to ssh sign form * oops, allowEmptyPrincipals should be on the role, not the signing form * update test
hashicorp · Sep 24, 2024 · 65debed · 65debed
1 parent 56a9038
commit 65debed
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 36 deletions.
diff --git a/ui/app/models/role-ssh.js b/ui/app/models/role-ssh.js
@@ -40,6 +40,7 @@ const CA_FIELDS = [
   'defaultExtensions',
   'allowBareDomains',
   'allowSubdomains',
+  'allowEmptyPrincipals',
   'allowUserKeyIds',
   'keyIdFormat',
   'notBeforeDuration',
@@ -122,6 +123,10 @@ export default Model.extend({
     helpText:
       'Specifies if host certificates that are requested are allowed to be subdomains of those listed in Allowed Domains',
   }),
+  allowEmptyPrincipals: attr('boolean', {
+    helpText:
+      'Allow signing certificates with no valid principals (e.g. any valid principal). For backwards compatibility only. The default of false is highly recommended.',
+  }),
   allowUserKeyIds: attr('boolean', {
     helpText: 'Specifies if users can override the key ID for a signed certificate with the "key_id" field',
   }),

diff --git a/ui/app/models/ssh-sign.js b/ui/app/models/ssh-sign.js
@@ -30,7 +30,10 @@ export default Model.extend({
     label: 'TTL',
     editType: 'ttl',
   }),
-  validPrincipals: attr('string'),
+  validPrincipals: attr('string', {
+    helpText:
+      'Specifies valid principals, either usernames or hostnames, that the certificate should be signed for. Required unless the role has specified allow_empty_principals.',
+  }),
   certType: attr('string', {
     defaultValue: 'user',
     label: 'Certificate Type',

diff --git a/ui/app/templates/vault/cluster/secrets/backend/sign.hbs b/ui/app/templates/vault/cluster/secrets/backend/sign.hbs
@@ -81,48 +81,47 @@
       <MessageError @model={{this.model}} />
       <NamespaceReminder @mode="sign" @noun="SSH key" />
       {{#if this.model.attrs}}
-        {{#each (take 1 this.model.attrs) as |attr|}}
-          <FormFieldFromModel
-            @attr={{attr}}
-            @model={{this.model}}
-            @updateTtl={{action "updateTtl" attr.name}}
-            @emptyData={{this.emptyData}}
-            @codemirrorUpdated={{action "codemirrorUpdated" attr.name}}
-          />
-        {{/each}}
+        {{#let (find-by "name" "publicKey" this.model.attrs) as |attr|}}
+          <FormFieldFromModel @attr={{attr}} @model={{this.model}} />
+        {{/let}}
+        {{! valid_principals is required unless allow_empty_principals is true (not recommended) }}
+        {{#let (find-by "name" "validPrincipals" this.model.attrs) as |attr|}}
+          <FormFieldFromModel @attr={{attr}} @model={{this.model}} />
+        {{/let}}
         <ToggleButton @isOpen={{this.showOptions}} @onClick={{fn (mut this.showOptions)}} data-test-toggle-button />
         {{#if this.showOptions}}
           <div class="box is-marginless">
-            {{#each (drop 1 this.model.attrs) as |attr|}}
-              <FormFieldFromModel
-                @attr={{attr}}
-                @model={{this.model}}
-                @updateTtl={{action "updateTtl" attr.name}}
-                @emptyData={{this.emptyData}}
-                @codemirrorUpdated={{action "codemirrorUpdated" attr.name}}
-              />
+            {{#each this.model.attrs as |attr|}}
+              {{! These attrs render above, outside of the "More options" toggle }}
+              {{#if (not (includes attr.name (array "publicKey" "validPrincipals")))}}
+                <FormFieldFromModel
+                  @attr={{attr}}
+                  @model={{this.model}}
+                  @updateTtl={{action "updateTtl" attr.name}}
+                  @emptyData={{this.emptyData}}
+                  @codemirrorUpdated={{action "codemirrorUpdated" attr.name}}
+                />
+              {{/if}}
             {{/each}}
           </div>
         {{/if}}
       {{/if}}
     </div>
-    <div class="field is-grouped box is-fullwidth is-bottomless">
-      <Hds::ButtonSet>
-        <Hds::Button
-          @text="Sign"
-          @icon={{if this.loading "loading"}}
-          type="submit"
-          disabled={{this.loading}}
-          data-test-secret-generate
-        />
-        <Hds::Button
-          @text="Cancel"
-          @color="secondary"
-          @route="vault.cluster.secrets.backend.list-root"
-          @model={{this.backend.id}}
-          data-test-secret-generate-cancel
-        />
-      </Hds::ButtonSet>
-    </div>
+    <Hds::ButtonSet class="has-top-bottom-margin">
+      <Hds::Button
+        @text="Sign"
+        @icon={{if this.loading "loading"}}
+        type="submit"
+        disabled={{this.loading}}
+        data-test-secret-generate
+      />
+      <Hds::Button
+        @text="Cancel"
+        @color="secondary"
+        @route="vault.cluster.secrets.backend.list-root"
+        @model={{this.backend.id}}
+        data-test-secret-generate-cancel
+      />
+    </Hds::ButtonSet>
   </form>
 {{/if}}
diff --git a/ui/tests/acceptance/ssh-test.js b/ui/tests/acceptance/ssh-test.js
@@ -8,6 +8,7 @@ import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
 
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
 import authPage from 'vault/tests/pages/auth';
 import enablePage from 'vault/tests/pages/settings/mount-secret-backend';
 import { setRunOptions } from 'ember-a11y-testing/test-support';
@@ -28,6 +29,9 @@ module('Acceptance | ssh secret backend', function (hooks) {
       name: 'carole',
       async fillInCreate() {
         await click('[data-test-input="allowUserCertificates"]');
+        await click('[data-test-toggle-group="Options"]');
+        // it's recommended to keep allow_empty_principals false, check for testing so we don't have to input an extra field when signing a key
+        await click(GENERAL.inputByAttr('allowEmptyPrincipals'));
       },
       async fillInGenerate() {
         await fillIn('[data-test-input="publicKey"]', PUB_KEY);

diff --git a/ui/tests/helpers/openapi/expected-secret-attrs.js b/ui/tests/helpers/openapi/expected-secret-attrs.js
@@ -34,6 +34,13 @@ const ssh = {
       fieldGroup: 'default',
       type: 'boolean',
     },
+    allowEmptyPrincipals: {
+      editType: 'boolean',
+      fieldGroup: 'default',
+      helpText:
+        'Whether to allow issuing certificates with no valid principals (meaning any valid principal). Exists for backwards compatibility only, the default of false is highly recommended.',
+      type: 'boolean',
+    },
     allowHostCertificates: {
       editType: 'boolean',
       helpText: