UI: add allow_empty_principals to ssh engine (fixes failing test) (#…

…28484)
hashicorp · Sep 24, 2024 · 8567b75 · 8567b75
1 parent c8c51b1
commit 8567b75
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 110 deletions.
diff --git a/ui/app/components/secret-engine/configure-ssh.hbs b/ui/app/components/secret-engine/configure-ssh.hbs
@@ -3,84 +3,73 @@
   SPDX-License-Identifier: BUSL-1.1
 ~}}
 
-<form {{on "submit" (perform this.save)}} aria-label="save ssh creds" data-test-configure-form>
-  <div class="box is-fullwidth is-shadowless is-marginless">
-    <NamespaceReminder @mode="save" @noun="configuration" />
-    <MessageError @errorMessage={{this.errorMessage}} />
-    {{#unless @model.isNew}}
-      <p class="has-text-grey-dark">
-        NOTE: You must delete your existing certificate and key before saving new values.
-      </p>
-    {{/unless}}
-  </div>
+<form {{on "submit" (perform this.save)}} aria-label="save ssh creds" class="has-top-padding-m" data-test-configure-form>
+  <NamespaceReminder @mode="save" @noun="configuration" />
+  <MessageError @errorMessage={{this.errorMessage}} />
   {{#if @model.isNew}}
-    <div class="box is-fullwidth is-sideless">
-      {{#each @model.formFields as |attr|}}
-        <FormField @attr={{attr}} @model={{@model}} @modelValidations={{this.modelValidations}} />
-      {{/each}}
-    </div>
-    <div class="box is-fullwidth is-bottomless">
-      <div class="control">
-        <Hds::Button
-          @text="Save"
-          @icon={{if this.save.isRunning "loading"}}
-          type="submit"
-          disabled={{this.save.isRunning}}
-          data-test-configure-save-button
-        />
-        <Hds::Button
-          @text="Cancel"
-          @color="secondary"
-          class="has-left-margin-s"
-          disabled={{this.save.isRunning}}
-          {{on "click" this.onCancel}}
-          data-test-cancel-button
-        />
-      </div>
-      {{#if this.invalidFormAlert}}
-        <AlertInline
-          data-test-invalid-form-alert
-          class="has-top-padding-s"
-          @type="danger"
-          @message={{this.invalidFormAlert}}
-        />
-      {{/if}}
-    </div>
+    {{#each @model.formFields as |attr|}}
+      <FormField @attr={{attr}} @model={{@model}} @modelValidations={{this.modelValidations}} />
+    {{/each}}
+    <hr class="has-background-gray-300" />
+    <Hds::ButtonSet>
+      <Hds::Button
+        @text="Save"
+        @icon={{if this.save.isRunning "loading"}}
+        type="submit"
+        disabled={{this.save.isRunning}}
+        data-test-configure-save-button
+      />
+      <Hds::Button
+        @text="Cancel"
+        @color="secondary"
+        disabled={{this.save.isRunning}}
+        {{on "click" this.onCancel}}
+        data-test-cancel-button
+      />
+    </Hds::ButtonSet>
+    {{#if this.invalidFormAlert}}
+      <AlertInline
+        data-test-invalid-form-alert
+        class="has-top-padding-s"
+        @type="danger"
+        @message={{this.invalidFormAlert}}
+      />
+    {{/if}}
   {{else}}
     {{! Model is not new and keys have already been created. Require user deletes the keys before creating new ones }}
-    <div class="box is-fullwidth is-sideless is-marginless" data-test-edit-config-section>
-      <div class="field">
-        <label for="publicKey" class="is-label">
-          Public key
-        </label>
-        <div class="control">
-          <MaskedInput
-            @name="publickey"
-            @id="publicKey"
-            @value={{@model.publicKey}}
-            @displayOnly={{true}}
-            @allowCopy={{true}}
-            data-test-input="public-key"
-          />
-        </div>
-      </div>
-    </div>
-    <div class="field is-grouped-split box is-fullwidth is-bottomless">
-      <Hds::ButtonSet>
-        <Hds::Copy::Button
-          @text="Copy"
-          @textToCopy={{@model.publicKey}}
-          @onError={{fn (set-flash-message "Clipboard copy failed. The Clipboard API requires a secure context." "danger")}}
-          class="primary"
-        />
-        <ConfirmAction
-          @buttonText="Delete"
-          @buttonColor="secondary"
-          @confirmMessage="Confirming will remove the CA certificate information."
-          @onConfirmAction={{this.deleteCaConfig}}
-          data-test-delete-public-key
+    <p class="has-text-grey-dark has-top-bottom-margin">
+      NOTE: You must delete your existing certificate and key before saving new values.
+    </p>
+
+    <div class="box is-fullwidth is-sideless" data-test-edit-config-section>
+      <label for="publicKey" class="is-label">
+        Public key
+      </label>
+      <div class="control">
+        <MaskedInput
+          @name="publickey"
+          @id="publicKey"
+          @value={{@model.publicKey}}
+          @displayOnly={{true}}
+          @allowCopy={{true}}
+          data-test-input="public-key"
         />
-      </Hds::ButtonSet>
+      </div>
     </div>
+    <Hds::ButtonSet>
+      <Hds::Copy::Button
+        @text="Copy"
+        @textToCopy={{@model.publicKey}}
+        @onError={{fn (set-flash-message "Clipboard copy failed. The Clipboard API requires a secure context." "danger")}}
+        class="primary"
+      />
+      <ConfirmAction
+        @buttonText="Delete"
+        @buttonColor="secondary"
+        @confirmMessage="Confirming will remove the CA certificate information."
+        @onConfirmAction={{this.deleteCaConfig}}
+        data-test-delete-public-key
+      />
+    </Hds::ButtonSet>
   {{/if}}
 </form>
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',
@@ -118,6 +119,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
@@ -76,48 +76,41 @@
       <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-save
-        />
-        <Hds::Button
-          @text="Cancel"
-          @color="secondary"
-          @route="vault.cluster.secrets.backend.list-root"
-          @model={{this.backend.id}}
-          data-test-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-save />
+      <Hds::Button
+        @text="Cancel"
+        @color="secondary"
+        @route="vault.cluster.secrets.backend.list-root"
+        @model={{this.backend.id}}
+        data-test-cancel
+      />
+    </Hds::ButtonSet>
   </form>
 {{/if}}
diff --git a/ui/tests/acceptance/secrets/backend/ssh/roles-test.js b/ui/tests/acceptance/secrets/backend/ssh/roles-test.js
@@ -47,6 +47,9 @@ module('Acceptance | ssh | roles', function (hooks) {
       credsRoute: 'vault.cluster.secrets.backend.sign',
       async fillInCreate() {
         await click(GENERAL.inputByAttr('allowUserCertificates'));
+        await click(GENERAL.toggleGroup('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(GENERAL.inputByAttr('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: