Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 1.17.x: add allow_empty_principals to ssh engine (fixes failing test) #28493

Merged
merged 5 commits into from
Sep 24, 2024
Merged

Backport 1.17.x: add allow_empty_principals to ssh engine (fixes failing test) #28493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ea3387
add ssh allowEmptyPrincipals to openapi
hellobontempo Sep 24, 2024
56ed5c1
add attrs to ssh sign form
hellobontempo Sep 24, 2024
c26d023
oops, allowEmptyPrincipals should be on the role, not the signing form
hellobontempo Sep 24, 2024
aefd1e2
update test
hellobontempo Sep 24, 2024
4e1405c
Merge branch 'release/1.17.x' into ui/backport-28484-1.17.x
hellobontempo Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ui/app/models/role-ssh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const CA_FIELDS = [
'defaultExtensions',
'allowBareDomains',
'allowSubdomains',
'allowEmptyPrincipals',
'allowUserKeyIds',
'keyIdFormat',
'notBeforeDuration',
Expand Down Expand Up @@ -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', {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-09-24 at 9 56 29 AM

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',
}),
Expand Down
5 changes: 4 additions & 1 deletion ui/app/models/ssh-sign.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
69 changes: 34 additions & 35 deletions ui/app/templates/vault/cluster/secrets/backend/sign.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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|}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-09-24 at 9 55 01 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-09-24 at 9 55 19 AM

<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}}
4 changes: 4 additions & 0 deletions ui/tests/acceptance/ssh-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions ui/tests/helpers/openapi/expected-secret-attrs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading