PKI role non-default options #17393

Monkeychip · 2022-10-03T17:30:09Z

2 of 3 PRs needed to finish the PKI Roles Create View

This PR handles the three Toggle Options: Domain handling, Key parameters, and Key usage.

Some notes:

Domain handling needed another kind of formField option that only showed a more-info type callout.
Key Parameters two special cases:
a. it needed a subText formField with no input and no label.

b. the keyBits param's possibleValues are conditional depending on what keyType the user selected.
https://user-images.githubusercontent.com/6618863/194140779-529f7d70-08e1-4182-90bf-6090d6189b9d.mov
Key Usage needed to use CSS Grid to match the designs AND all those checkboxes are just list items, not actually API params, so I append them to the ext_key_usage and key_usage parameters in a component.

You can see why I stopped at this PR instead of finishing the view. This custom functionality is outside our normal patterns so a lot of custom work and modifications to formFields, etc had to be made, but in the end it makes PKI a LOT easier for users to navigate.

At this point designs and the work do not exactly match. There are a lot of comments on the designs, but even then, if something doesn't exactly match, but makes sense, then it's probably been a confirmed change in a conversation between backend and design.

TODO:

in future PR remove the helpText on string-list components. There is some refactoring that should happen in that component, so moving outside the scope of this PR.

…nd conditionals

ui/app/models/pki/pki-role-engine.js

Monkeychip · 2022-10-05T18:51:09Z

ui/lib/pki/addon/components/pki-role-form.hbs

-    >
-      <RadioSelectTtlOrString @model={{@model}} @attr={{attr}} />
-    </FormFieldGroupsLoop>
+    {{#each @model.fieldGroups as |fieldGroup|}}


ended up not being able to use FormFieldGroupsLoop for two main reasons:

because there is a lot of custom work required to make the CSS grid for Key Usage work, and it didn't make sense to apply that logic to a widely used component where PKI might be a unique situation where CSS grid is needed.

I needed to pass in a conditional attr when dealing with key_bits, this complicated a whole mess of things and again is a one off situation specific to pki.

…should be it's own pr.

zofskeez

Wow lots of work here good job. I left some comments but mainly want to discuss the serializer work further.

ui/app/adapters/pki/pki-role-engine.js

ui/app/models/pki/pki-role-engine.js

zofskeez · 2022-10-05T21:55:19Z

ui/lib/pki/addon/components/pki-role-form.hbs

+                  {{#each fields as |attr|}}
+                    <FormField
+                      data-test-field={{true}}
+                      @attr={{if (eq attr.name "keyBits") @model.keyBitsConditional attr}}


I wonder if it would be tidier to do this in the fieldGroups getter on the model instead? After doing fieldToAttrs you could replace the keyBits attr with the result from the keyBitsConditional getter.

I couldn't get that to work. And it doesn't feel cleaner in the model because I have to really dig into the _fieldToAttrsGroups.

// inside the model on row 256, just before I return this_fieldToAttrsGroups this._fieldToAttrsGroups[2]['Key parameters'][2] = this.keyBitsConditional;

ui/app/models/pki/pki-role-engine.js

ui/app/serializers/pki/pki-role-engine.js

ui/app/styles/core/helpers.scss

zofskeez · 2022-10-05T22:33:39Z

ui/lib/core/addon/components/form-field-groups-loop.js

+import Component from '@glimmer/component';
+
+/**
+ * @module FormFieldGroupsLoop
+ * FormFieldGroupsLoop components loop through the "groups" set on a model and display them either as default or behind toggle components. 
+ *
+ * @example
+ * ```js
+ <FormFieldGroupsLoop @model={{this.model}} @mode={{if @model.isNew "create" "update"}}/>
+ * ```
+ * @param {class} model - The routes model class.
+ * @param {string} mode - "create" or "update" used to hide the name form field. TODO: not ideal, would prefer to disable it to follow new design patterns.
+ * @param {function} [modelValidations] - Passed through to formField.
+ * @param {boolean} [showHelpText] - Passed through to formField.
+ */
+/* eslint ember/no-empty-glimmer-component-classes: 'warn' */
+export default class FormFieldGroupsLoop extends Component {}


ui/lib/core/addon/components/form-field.hbs

ui/lib/pki/addon/components/pki-key-usage.hbs

hellobontempo

Left some comments! Kind of have wonky end of day brain, but I'll revisit tomorrow!

ui/app/models/pki/pki-role-engine.js

ui/lib/core/addon/components/form-field.hbs

ui/lib/core/addon/components/string-list.hbs

ui/lib/pki/addon/components/pki-key-usage.hbs

hellobontempo

Great work on this so far! It's so nice seeing the PKI work come together 😄

hellobontempo · 2022-10-10T19:27:15Z

ui/lib/pki/addon/components/pki-key-usage.hbs

+  />
+  {{#if (get @model prop)}}
+    <div class="box is-tall is-marginless">
+      <FormFieldLabel


ui/lib/pki/addon/components/pki-key-usage.hbs

hellobontempo · 2022-10-10T19:29:03Z

ui/lib/pki/addon/components/pki-key-usage.js

+ * @param {string} group - The name of the group created in the model. In this case, it's the "Key usage" group.
+ */
+
+const KEY_USAGE_FIELDS = {


Nice! I think this is much clearer with the label attr removed 😄

hellobontempo · 2022-10-10T19:33:42Z

ui/lib/pki/addon/components/pki-key-usage.js

+  CrlSign: { label: 'CRL Sign' },
+  KeyAgreement: {
+    label: 'Key Agreement',
+    value: true,


I think setting these values here means that they will always be true. So when editing a role, these will always change back to true regardless of what the @model value is..

Starting to think that writing component tests earlier than later for these forms will save a bunch of time/energy speculating the various what ifs for this form 🤔

When they change, the model updates the keyUsage list. But yep, working on the tests in the next PR.

So by default are all the true values checked? And then what happens when editing and the model has false values instead?

By default the true values are checked. On any click for any of the checkboxes it updates the keyUsage list. So if it has a false value it's removed from the list. In the screenshot only Key Encipherment is checked, so it's the only one in the list.

hellobontempo · 2022-10-10T19:35:18Z

ui/lib/pki/addon/components/pki-key-usage.js

+  CodeSigning: { label: 'Code Signing' },
+};
+
+export default class PkiKeyUsage extends Component {


I think a test for this component would be 😍

ui/app/models/pki/pki-role-engine.js

ui/lib/core/addon/components/form-field.hbs

ui/app/models/pki/pki-role-engine.js

hellobontempo

Just a few small cleanup comments. Really great work on this! All of that custom form work is 🤯

hellobontempo · 2022-10-12T16:33:38Z

ui/app/models/pki/pki-role-engine.js

+      ed25519: [0],
+      any: [0],
+    };
+    const attrs = expandAttributeMeta(this, ['keyBits']);


ui/app/models/pki/pki-role-engine.js

ui/app/styles/core/box.scss

hellobontempo · 2022-10-12T16:48:20Z

ui/lib/core/addon/components/form-field.hbs

@@ -9,15 +9,15 @@
      )
    )
  }}
-    {{#unless (eq @attr.type "object")}}
+    {{#if (not (or (eq @attr.type "object") (eq @attr.options.editType "moreInfo")))}}


I think we can revert this back and remove moreInfo

hellobontempo · 2022-10-12T16:49:18Z

ui/lib/core/addon/components/form-field.js

@@ -21,7 +21,7 @@ import { dasherize } from 'vault/helpers/dasherize';
 *     label: "Foo", // custom label to be shown, otherwise attr.name will be displayed
 *     defaultValue: "", // default value to display if model value is not present
 *     fieldValue: "bar", // used for value lookup on model over attr.name
- *     editType: "ttl", type of field to use. List of editTypes:boolean, file, json, kv, optionalText, mountAccessor, password, radio, regex, searchSelect, stringArray,textarea, ttl, yield.
+ *     editType: "ttl", type of field to use. List of editTypes:boolean, file, json, kv, optionalText, mountAccessor, moreInfo, password, radio, regex, searchSelect, stringArray, textarea, ttl, yield.


moreInfo can be removed here, too?

Ug, I know I did a whole search for moreInfo, but I've been working off this branch for testing and I think I messed up a push 🤦🏻‍♀️

hellobontempo · 2022-10-12T16:52:49Z

ui/lib/pki/addon/components/pki-key-usage.js

+  CrlSign: { label: 'CRL Sign' },
+  KeyAgreement: {
+    label: 'Key Agreement',
+    value: true,


So by default are all the true values checked? And then what happens when editing and the model has false values instead?

hellobontempo · 2022-10-12T16:59:56Z

ui/lib/pki/addon/components/pki-key-usage.js

+    const value = event.target['checked'];
+    type === 'keyUsage'
+      ? this.args.model.set('keyUsage', this._amendList(checkboxName, value, type))
+      : this.args.model.set('extKeyUsage', this._amendList(checkboxName, value, type));


if type is the attr name on the model could the conditional be removed and this change to:

Suggested change

: this.args.model.set('extKeyUsage', this._amendList(checkboxName, value, type));

this.args.model.set(type, this._amendList(checkboxName, value, type));

or, since this component is glimmerized, do we need to use `.set` here?

Suggested change

: this.args.model.set('extKeyUsage', this._amendList(checkboxName, value, type));

this.args.model[type] = this._amendList(checkboxName, value, type)

Nice, yep. This works: this.args.model.set(type, this._amendList(checkboxName, value, type));

hellobontempo · 2022-10-12T17:03:40Z

ui/lib/pki/addon/components/pki-role-form.hbs

+            />
+            {{#if (get @model prop)}}
+              <div class="box is-marginless">
+                {{#let (get @model.fieldGroupsInfo group) as |groupInfo|}}


* dynamically render the secretlistheader in the parent route. * start getting form setup even without openAPi working * add in create and cancel * making openAPI work * add default openAPI params * wip for new component with two radio options a ttl and input * handle createRecord on pki-roles-form * remove tooltips and cleanup * move formfieldgroupsloop back to non addon * cleanup * move secretListHeader * broadcast from radioSelectTtlOrString to parent * cleanup * hide tooltips * pass through sub text to stringArray * Add conditional for keybits and keyType * set defaults for keyBits ... 🤮 * fix some small issues * more info form field typ * show only label and subText * wip context switch 🤮 * fix dontShowLabel * getting css grid setup * more on flex groups * adding the second chunk to key usage * serialize the post for key_usage * finish for ext_key_usage * clean up * fix snack_case issue * commit for working state, next trying to remove form-field-group-loops because it's causing issues. * remove usage of formfieldgroupsloop because of issues with css grid and conditionals * clean up * remove string-list helpText changes for tooltip removal because that should be it's own pr. * clarification from design and backend. * small cleanup * pull key_usage and ext_key_usage out of the model and into a component * clean up * clean up * restructure css grid: * clean up * broke some things * fix error when roles list returned 404 * claires feedback * cleanup * clean up

Monkeychip added 18 commits September 21, 2022 10:24

dynamically render the secretlistheader in the parent route.

2452118

start getting form setup even without openAPi working

fdc67ad

add in create and cancel

f3ee281

making openAPI work

2e9cf32

add default openAPI params

7afe1b1

wip for new component with two radio options a ttl and input

182795e

handle createRecord on pki-roles-form

8055710

remove tooltips and cleanup

d42991c

move formfieldgroupsloop back to non addon

b2a89d6

cleanup

de014a4

move secretListHeader

2627050

broadcast from radioSelectTtlOrString to parent

49f4586

cleanup

d205296

hide tooltips

3e0665d

pass through sub text to stringArray

e1f1631

Add conditional for keybits and keyType

051aacf

set defaults for keyBits ... 🤮

daf60a3

merge in main

0176c3e

Monkeychip added ui pr/no-changelog labels Oct 3, 2022

Monkeychip added this to the 1.13.0-rc1 milestone Oct 3, 2022

Monkeychip added 9 commits October 3, 2022 11:32

fix some small issues

d0a64c0

more info form field typ

d408413

show only label and subText

c8a08ac

wip context switch 🤮

6c03ac2

fix dontShowLabel

b1e98fa

getting css grid setup

820c134

more on flex groups

8bbfa81

adding the second chunk to key usage

8d0c506

serialize the post for key_usage

54d27cd

remove usage of formfieldgroupsloop because of issues with css grid a…

45fd243

…nd conditionals

Monkeychip commented Oct 5, 2022

View reviewed changes

ui/app/models/pki/pki-role-engine.js Show resolved Hide resolved

Monkeychip commented Oct 5, 2022

View reviewed changes

clean up

3376346

Monkeychip marked this pull request as ready for review October 5, 2022 19:04

Monkeychip added 2 commits October 5, 2022 13:36

remove string-list helpText changes for tooltip removal because that …

6e3f57c

…should be it's own pr.

clarification from design and backend.

74d1489

zofskeez reviewed Oct 5, 2022

View reviewed changes

Monkeychip added 3 commits October 5, 2022 19:25

small cleanup

fa7445a

pull key_usage and ext_key_usage out of the model and into a component

dae56ee

clean up

0a4920e

Monkeychip commented Oct 6, 2022

View reviewed changes

ui/lib/pki/addon/components/pki-key-usage.hbs Show resolved Hide resolved

clean up

f349ef6

hellobontempo reviewed Oct 7, 2022

View reviewed changes

Monkeychip added 2 commits October 7, 2022 12:56

restructure css grid:

4595bb8

clean up

1aab410

Monkeychip enabled auto-merge (squash) October 7, 2022 19:16

hellobontempo reviewed Oct 10, 2022

View reviewed changes

Monkeychip added 4 commits October 11, 2022 11:11

broke some things

36a8060

fix error when roles list returned 404

2834b55

claires feedback

5ed7380

cleanup

878854d

hellobontempo reviewed Oct 12, 2022

View reviewed changes

ui/app/models/pki/pki-role-engine.js Outdated Show resolved Hide resolved

hellobontempo disabled auto-merge October 12, 2022 17:06

hellobontempo approved these changes Oct 12, 2022

View reviewed changes

clean up

4ae6033

Monkeychip merged commit 8188328 into main Oct 12, 2022

Monkeychip deleted the ui/VAULT-6521/pki-role-create-2 branch October 12, 2022 18:56

hashishaw mentioned this pull request Jan 27, 2023

UI: PKI Sign Intermediate #18842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PKI role non-default options #17393

PKI role non-default options #17393

Monkeychip commented Oct 3, 2022 •

edited

Monkeychip Oct 5, 2022

zofskeez left a comment

zofskeez Oct 5, 2022

Monkeychip Oct 6, 2022 •

edited

Monkeychip Oct 6, 2022

zofskeez Oct 5, 2022

hellobontempo left a comment

hellobontempo left a comment

hellobontempo Oct 10, 2022

hellobontempo Oct 10, 2022

hellobontempo Oct 10, 2022

Monkeychip Oct 11, 2022

hellobontempo Oct 12, 2022

Monkeychip Oct 12, 2022

hellobontempo Oct 10, 2022

hellobontempo left a comment

hellobontempo Oct 12, 2022

hellobontempo Oct 12, 2022

hellobontempo Oct 12, 2022

Monkeychip Oct 12, 2022

hellobontempo Oct 12, 2022

hellobontempo Oct 12, 2022

Monkeychip Oct 12, 2022

hellobontempo Oct 12, 2022

	: this.args.model.set('extKeyUsage', this._amendList(checkboxName, value, type));
	this.args.model.set(type, this._amendList(checkboxName, value, type));

	: this.args.model.set('extKeyUsage', this._amendList(checkboxName, value, type));
	this.args.model[type] = this._amendList(checkboxName, value, type)

PKI role non-default options #17393

PKI role non-default options #17393

Conversation

Monkeychip commented Oct 3, 2022 • edited

Choose a reason for hiding this comment

zofskeez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip Oct 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellobontempo left a comment

Choose a reason for hiding this comment

hellobontempo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellobontempo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip commented Oct 3, 2022 •

edited

Monkeychip Oct 6, 2022 •

edited