UI aws engine tweaks (#5294)

* allow passing a path for options so that it can be extracted from the model

* add cred type selector for the aws generate form

* style hint text on generate creds form

* add tests for aws-credential adapter

* allow for the case where we might have zero ttl

* show error for TTL picker if a non-number is entered for the duration part of the TTL

* fix positioning of tooltips

* fix ttl rendering with invalid input for initialValue

ui/app/adapters/aws-credential.js

@@ -4,15 +4,21 @@ export default ApplicationAdapter.extend({
   createRecord(store, type, snapshot) {
     let ttl = snapshot.attr('ttl');
     let roleArn = snapshot.attr('roleArn');
+    let roleType = snapshot.attr('credentialType');
+    let method = 'POST';
+    let options;
     let data = {};
-    if (ttl) {
-      data.ttl = ttl;
+    if (roleType === 'iam_user') {
+      method = 'GET';
+    } else {
+      if (ttl !== undefined) {
+        data.ttl = ttl;
+      }
+      if (roleType === 'assumed_role' && roleArn) {
+        data.role_arn = roleArn;
+      }
+      options = data.ttl || data.role_arn ? { data } : {};
     }
-    if (roleArn) {
-      data.role_arn = roleArn;
-    }
-    let method = ttl || roleArn ? 'POST' : 'GET';
-    let options = ttl || roleArn ? { data } : {};
     let role = snapshot.attr('role');
     let url = `/v1/${role.backend}/creds/${role.name}`;
 

ui/app/components/ttl-picker.js

@@ -1,9 +1,11 @@
 import { typeOf } from '@ember/utils';
 import EmberError from '@ember/error';
 import Component from '@ember/component';
-import { set, get, computed } from '@ember/object';
+import { set, computed } from '@ember/object';
 import Duration from 'Duration.js';
 
+const ERROR_MESSAGE = 'TTLs must be specified in whole number increments, please enter a whole number.';
+
 export default Component.extend({
   'data-test-component': 'ttl-picker',
   classNames: 'field',
@@ -14,6 +16,7 @@ export default Component.extend({
   time: 30,
   unit: 'm',
   initialValue: null,
+  errorMessage: null,
   unitOptions: computed(function() {
     return [
       { label: 'seconds', value: 's' },
@@ -46,27 +49,32 @@ export default Component.extend({
     return outputSeconds ? this.convertToSeconds(time, unit) : timeString;
   }),
 
-  didRender() {
+  didInsertElement() {
     this._super(...arguments);
-    if (get(this, 'setDefaultValue') === false) {
+    if (this.setDefaultValue === false) {
       return;
     }
-    get(this, 'onChange')(get(this, 'TTL'));
+    this.onChange(this.TTL);
   },
 
   init() {
     this._super(...arguments);
-    if (!get(this, 'onChange')) {
+    if (!this.onChange) {
       throw new EmberError('`onChange` handler is a required attr in `' + this.toString() + '`.');
     }
-    if (get(this, 'initialValue') != undefined) {
+    if (this.initialValue != undefined) {
       this.parseAndSetTime();
     }
   },
 
   parseAndSetTime() {
-    const value = get(this, 'initialValue');
-    const seconds = typeOf(value) === 'number' ? value : Duration.parse(value).seconds();
+    let value = this.initialValue;
+    let seconds = typeOf(value) === 'number' ? value : 30;
+    try {
+      seconds = Duration.parse(value).seconds();
+    } catch (e) {
+      // if parsing fails leave as default 30
+    }
 
     this.set('time', seconds);
     this.set('unit', 's');
@@ -74,10 +82,18 @@ export default Component.extend({
 
   actions: {
     changedValue(key, event) {
-      const { type, value, checked } = event.target;
-      const val = type === 'checkbox' ? checked : value;
+      let { type, value, checked } = event.target;
+      let val = type === 'checkbox' ? checked : value;
+      if (val && key === 'time') {
+        val = parseInt(val, 10);
+        if (Number.isNaN(val)) {
+          this.set('errorMessage', ERROR_MESSAGE);
+          return;
+        }
+      }
+      this.set('errorMessage', null);
       set(this, key, val);
-      get(this, 'onChange')(get(this, 'TTL'));
+      this.onChange(this.TTL);
     },
   },
 });

ui/app/helpers/path-or-array.js

@@ -0,0 +1,10 @@
+import { helper as buildHelper } from '@ember/component/helper';
+
+export function pathOrArray([maybeArray, target]) {
+  if (Array.isArray(maybeArray)) {
+    return maybeArray;
+  }
+  return target.get(maybeArray);
+}
+
+export default buildHelper(pathOrArray);

ui/app/models/aws-credential.js

@@ -2,24 +2,45 @@ import { computed } from '@ember/object';
 import DS from 'ember-data';
 import { expandAttributeMeta } from 'vault/utils/field-to-attrs';
 const { attr } = DS;
-const CREATE_FIELDS = ['ttl', 'roleArn'];
+const CREDENTIAL_TYPES = [
+  {
+    value: 'iam_user',
+    displayName: 'IAM User',
+  },
+  {
+    value: 'assumed_role',
+    displayName: 'Assumed Role',
+  },
+  {
+    value: 'federation_token',
+    displayName: 'Federation Token',
+  },
+];
 
 const DISPLAY_FIELDS = ['accessKey', 'secretKey', 'securityToken', 'leaseId', 'renewable', 'leaseDuration'];
 export default DS.Model.extend({
   helpText:
-    'For Vault roles that have a credential type of iam_user, these attributes are optional and you may simply submit the form.',
+    'For Vault roles of credential type iam_user, there are no inputs, just submit the form. Choose a type to change the input options.',
   role: attr('object', {
     readOnly: true,
   }),
 
+  credentialType: attr('string', {
+    defaultValue: 'iam_user',
+    possibleValues: CREDENTIAL_TYPES,
+    readOnly: true,
+  }),
+
   roleArn: attr('string', {
     label: 'Role ARN',
     helpText:
       'The ARN of the role to assume if credential_type on the Vault role is assumed_role. Optional if the role has a single role ARN; required otherwise.',
   }),
+
   ttl: attr({
     editType: 'ttl',
     defaultValue: '3600s',
+    setDefault: true,
     label: 'TTL',
     helpText:
       'Specifies the TTL for the use of the STS token. Valid only when credential_type is assumed_role or federation_token.',
@@ -31,10 +52,17 @@ export default DS.Model.extend({
   secretKey: attr('string'),
   securityToken: attr('string'),
 
-  attrs: computed('accessKey', 'securityToken', function() {
-    let keys =
-      this.get('accessKey') || this.get('securityToken') ? DISPLAY_FIELDS.slice(0) : CREATE_FIELDS.slice(0);
-    return expandAttributeMeta(this, keys);
+  attrs: computed('credentialType', 'accessKey', 'securityToken', function() {
+    let type = this.get('credentialType');
+    let fieldsForType = {
+      iam_user: ['credentialType'],
+      assumed_role: ['credentialType', 'ttl', 'roleArn'],
+      federation_token: ['credentialType', 'ttl'],
+    };
+    if (this.get('accessKey') || this.get('securityToken')) {
+      return expandAttributeMeta(this, DISPLAY_FIELDS.slice(0));
+    }
+    return expandAttributeMeta(this, fieldsForType[type].slice(0));
   }),
 
   toCreds: computed('accessKey', 'secretKey', 'securityToken', 'leaseId', function() {

ui/app/styles/components/tool-tip.scss

@@ -12,23 +12,29 @@
   }
   @include css-top-arrow(8px, $grey, 1px, $grey-dark, 20px);
   &.ember-basic-dropdown-content--below.ember-basic-dropdown--transitioning-in {
-    animation: drop-fade-above .15s;
+    animation: drop-fade-above 0.15s;
   }
   &.ember-basic-dropdown-content--below.ember-basic-dropdown--transitioning-out {
-    animation: drop-fade-above .15s reverse;
+    animation: drop-fade-above 0.15s reverse;
   }
   &.ember-basic-dropdown-content--above.ember-basic-dropdown--transitioning-in {
-    animation: drop-fade-below .15s;
+    animation: drop-fade-below 0.15s;
   }
   &.ember-basic-dropdown-content--above.ember-basic-dropdown--transitioning-out {
-    animation: drop-fade-below .15s reverse;
+    animation: drop-fade-below 0.15s reverse;
   }
 }
 
 .ember-basic-dropdown-content--left.tool-tip {
   margin: 8px 0 0 -11px;
 }
+.ember-basic-dropdown-content--right.tool-tip {
+  margin: 8px -11px;
+}
 
+.ember-basic-dropdown-content--below.ember-basic-dropdown-content--left.tool-tip {
+  @include css-top-arrow(8px, $grey, 1px, $grey-dark, calc(100% - 20px));
+}
 .ember-basic-dropdown-content--below.ember-basic-dropdown-content--right.tool-tip {
   @include css-top-arrow(8px, $grey, 1px, $grey-dark, calc(100% - 20px));
 }
@@ -47,7 +53,7 @@
   box-shadow: none;
   cursor: pointer;
   padding: 0;
-  olor: $grey-dark;
+  color: $grey-dark;
   margin-left: 8px;
 }
 

ui/app/styles/core/box.scss

@@ -8,6 +8,9 @@
 .box.no-padding-bottom {
   padding-bottom: 0;
 }
+.box.no-padding-top {
+  padding-top: 0;
+}
 .box.has-slim-padding {
   padding: 9px 0;
 }

ui/app/styles/core/helpers.scss

@@ -113,6 +113,11 @@
 .is-size-9 {
   font-size: $size-9 !important;
 }
+.is-hint {
+  color: $grey;
+  font-size: $size-8;
+  padding: $size-8 0;
+}
 @each $name, $shade in $shades {
   .has-background-#{$name} {
     background: $shade !important;

ui/app/templates/components/form-field.hbs

@@ -19,7 +19,7 @@
         onchange={{action (action "setAndBroadcast" valuePath) value="target.value"}}
         data-test-input={{attr.name}}
       >
-        {{#each attr.options.possibleValues as |val|}}
+        {{#each (path-or-array attr.options.possibleValues model) as |val|}}
           <option selected={{eq (get model valuePath) (or val.value val)}} value={{or val.value val}}>
             {{or val.displayName val}}
           </option>
@@ -77,7 +77,7 @@
     initialValue=(or (get model valuePath) attr.options.defaultValue)
     labelText=labelString
     warning=attr.options.warning
-    setDefaultValue=false
+    setDefaultValue=(or attr.options.setDefault false)
     onChange=(action (action "setAndBroadcast" valuePath))
   }}
 {{else if (eq attr.options.editType 'stringArray')}}

ui/app/templates/components/generate-credentials.hbs

@@ -93,11 +93,11 @@
   </div>
 {{else}}
   <form {{action "create" on="submit"}} data-test-secret-generate-form="true">
-    <div class="box is-sideless is-fullwidth is-marginless">
+    <div class="box is-sideless no-padding-top is-fullwidth is-marginless">
       <NamespaceReminder @mode="generate" @noun="credential" />
       {{message-error model=model}}
       {{#if model.helpText}}
-        <p>{{model.helpText}}</p>
+        <p class="is-hint">{{model.helpText}}</p>
       {{/if}}
       {{#if model.fieldGroups}}
         {{partial "partials/form-field-groups-loop"}}

ui/app/templates/components/info-tooltip.hbs

@@ -1,7 +1,6 @@
 {{#tool-tip as |d|}}
   {{#d.trigger tagName="button" type="button" class=(concat "tool-tip-trigger button") data-test-tool-tip-trigger=true}}
-    {{i-con
-      glyph="information-reversed"
+    {{i-con glyph="information-reversed"
       class="auto-width"
       size=16
       aria-label="help"
@@ -13,4 +12,4 @@
       {{yield}}
     </div>
   {{/d.content}}
-{{/tool-tip}}
+{{/tool-tip}}

ui/app/templates/components/tool-tip.hbs

@@ -1,13 +1,13 @@
-{{#basic-dropdown-hover
-  onOpen=onOpen
-  onClose=onClose
-  renderInPlace=renderInPlace
+{{#basic-dropdown-hover renderInPlace=renderInPlace
   verticalPosition=verticalPosition
   horizontalPosition=horizontalPosition
   matchTriggerWidth=matchTriggerWidth
   triggerComponent=triggerComponent
   contentComponent=contentComponent
   calculatePosition=calculatePosition
+  onOpen=onOpen
+  onClose=onClose
+  onFocus=onFocus
   calculateInPlacePosition=calculateInPlacePosition as |dd|}}
   {{yield (assign
     dd
@@ -15,15 +15,11 @@
       trigger=(component dd.trigger
         onMouseDown=(action "prevent")
         onMouseEnter=(action "open")
-        onMouseLeave=(action "close")
-        onBlur=(action "close")
-        )
+        onMouseLeave=(action "close"))
       content=(component dd.content
         onMouseEnter=(action "open")
         onMouseLeave=(action "close")
-        onFocus=(action "open")
-        onBlur=(action "close")
       )
     )
   )}}
-{{/basic-dropdown-hover}}
+{{/basic-dropdown-hover}}

ui/app/templates/components/ttl-picker.hbs

@@ -1,30 +1,19 @@
 <label for="time-{{elementId}}" class="is-label {{labelClass}}">{{labelText}}</label>
+<MessageError @errorMessage={{errorMessage}} data-test-ttl-error />
 <div class="field is-grouped">
   <div class="control is-expanded">
-    <input
-       data-test-ttl-value
-       value={{time}}
-       id="time-{{elementId}}"
-       type="text"
-       name="time"
-       class="input"
-       oninput={{action 'changedValue' 'time'}}
-       />
+    <input data-test-ttl-value value={{time}} id="time-{{elementId}}" type="text" name="time" class="input" oninput={{action 'changedValue' 'time'}}
+      pattern="[0-9]*" />
   </div>
   <div class="control is-expanded">
     <div class="select is-fullwidth">
-      <select
-         data-test-ttl-unit
-         name="unit"
-         id="unit"
-         onchange={{action 'changedValue' 'unit'}}
-         >
-         {{#each unitOptions as |unitOption|}}
-           <option selected={{eq unit unitOption.value}} value={{unitOption.value}}>
-             {{unitOption.label}}
-           </option>
-         {{/each}}
+      <select data-test-ttl-unit name="unit" id="unit" onchange={{action 'changedValue' 'unit'}}>
+        {{#each unitOptions as |unitOption|}}
+          <option selected={{eq unit unitOption.value}} value={{unitOption.value}}>
+            {{unitOption.label}}
+          </option>
+        {{/each}}
       </select>
     </div>
   </div>
-</div>
+</div>

ui/tests/integration/components/ttl-picker-test.js

@@ -0,0 +1,31 @@
+import { module, test } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+import { render, fillIn } from '@ember/test-helpers';
+import hbs from 'htmlbars-inline-precompile';
+import sinon from 'sinon';
+
+module('Integration | Component | ttl picker', function(hooks) {
+  setupRenderingTest(hooks);
+
+  hooks.beforeEach(function() {
+    this.changeSpy = sinon.spy();
+    this.set('onChange', this.changeSpy);
+  });
+
+  test('it renders error on non-number input', async function(assert) {
+    await render(hbs`<TtlPicker @onChange={{onChange}} />`);
+
+    let callCount = this.changeSpy.callCount;
+    await fillIn('[data-test-ttl-value]', 'foo');
+    assert.equal(this.changeSpy.callCount, callCount, "it did't call onChange again");
+    assert.dom('[data-test-ttl-error]').includesText('Error', 'renders the error box');
+
+    await fillIn('[data-test-ttl-value]', '33');
+    assert.dom('[data-test-ttl-error]').doesNotIncludeText('Error', 'removes the error box');
+  });
+
+  test('it shows 30s for invalid duration initialValue input', async function(assert) {
+    await render(hbs`<TtlPicker @onChange={{onChange}} @initialValue={{'invalid'}} />`);
+    assert.dom('[data-test-ttl-value]').hasValue('30', 'sets 30 as the default');
+  });
+});

ui/tests/unit/adapters/aws-credential-test.js

@@ -0,0 +1,80 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import apiStub from 'vault/tests/helpers/noop-all-api-requests';
+
+module('Unit | Adapter | aws credential', function(hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(function() {
+    this.server = apiStub();
+  });
+
+  hooks.afterEach(function() {
+    this.server.shutdown();
+  });
+
+  const storeStub = {
+    pushPayload() {},
+    serializerFor() {
+      return {
+        serializeIntoHash() {},
+      };
+    },
+  };
+
+  let makeSnapshot = obj => {
+    obj.role = {
+      backend: 'aws',
+      name: 'foo',
+    };
+    obj.attr = attr => obj[attr];
+    return obj;
+  };
+
+  const type = {
+    modelName: 'aws-credential',
+  };
+
+  const cases = [
+    ['iam_user type', [storeStub, type, makeSnapshot({ credentialType: 'iam_user', ttl: '3h' })], 'GET'],
+    [
+      'federation_token type with ttl',
+      [storeStub, type, makeSnapshot({ credentialType: 'federation_token', ttl: '3h', roleArn: 'arn' })],
+      'POST',
+      { ttl: '3h' },
+    ],
+    [
+      'federation_token type no ttl',
+      [storeStub, type, makeSnapshot({ credentialType: 'federation_token', roleArn: 'arn' })],
+      'POST',
+    ],
+    [
+      'assumed_role type no arn, no ttl',
+      [storeStub, type, makeSnapshot({ credentialType: 'assumed_role' })],
+      'POST',
+    ],
+    [
+      'assumed_role type no arn',
+      [storeStub, type, makeSnapshot({ credentialType: 'assumed_role', ttl: '3h' })],
+      'POST',
+      { ttl: '3h' },
+    ],
+    [
+      'assumed_role type',
+      [storeStub, type, makeSnapshot({ credentialType: 'assumed_role', roleArn: 'arn', ttl: '3h' })],
+      'POST',
+      { ttl: '3h', role_arn: 'arn' },
+    ],
+  ];
+  cases.forEach(([description, args, expectedMethod, expectedRequestBody]) => {
+    test(`aws-credential: ${description}`, function(assert) {
+      assert.expect(3);
+      let adapter = this.owner.lookup('adapter:aws-credential');
+      adapter.createRecord(...args);
+      let { method, url, requestBody } = this.server.handledRequests[0];
+      assert.equal(url, '/v1/aws/creds/foo', `calls the correct url`);
+      assert.equal(method, expectedMethod, `${description} uses the correct http verb: ${expectedMethod}`);
+      assert.equal(requestBody, JSON.stringify(expectedRequestBody));
+    });
+  });
+});