UI - kv v2 graceful degrade (#5879)

* turns out sourcemaps are useful * add test for restricted policy in kv v2 * only include version param on fetch if it's encoded in the id * rename some vars for clarity and use model.id when persisting a secret * fix delete attributes on the models * allow data edit when there's metadata access is disallowed * add tests for edit with restricted policy * hide metadata fields if you can't edit them
2018-12-03 08:22:13 -06:00 · 2018-12-03 08:22:13 -06:00 · af8eda9322
parent c2e87c20d8
commit af8eda9322
11 changed files with 145 additions and 46 deletions
--- a/ui/app/adapters/secret-v2-version.js
+++ b/ui/app/adapters/secret-v2-version.js
@ -16,12 +16,12 @@ export default ApplicationAdapter.extend({

  urlForFindRecord(id) {
    let [backend, path, version] = JSON.parse(id);
-    return this._url(backend, path) + `?version=${version}`;
+    let base = this._url(backend, path);
+    return version ? base + `?version=${version}` : base;
  },

  urlForQueryRecord(id) {
-    let [backend, path, version] = JSON.parse(id);
-    return this._url(backend, path) + `?version=${version}`;
+    return this.urlForFindRecord(id);
  },

  findRecord() {
--- a/ui/app/components/secret-edit.js
+++ b/ui/app/components/secret-edit.js
@ -109,7 +109,7 @@ export default Component.extend(FocusOnInsertMixin, {
    'model.id',
    'mode'
  ),
-  canDelete: alias('updatePath.canDelete'),
+  canDelete: alias('model.canDelete'),
  canEdit: alias('updatePath.canUpdate'),

  v2UpdatePath: maybeQueryRecord(
@ -181,19 +181,21 @@ export default Component.extend(FocusOnInsertMixin, {
  // successCallback is called in the context of the component
  persistKey(successCallback) {
    let secret = this.model;
-    let model = this.modelForData;
+    let secretData = this.modelForData;
    let isV2 = this.isV2;
-    let key = model.get('path') || model.id;
+    let key = secretData.get('path') || secret.id;

    if (key.startsWith('/')) {
      key = key.replace(/^\/+/g, '');
-      model.set(model.pathAttr, key);
+      secretData.set(secretData.pathAttr, key);
    }

-    return model.save().then(() => {
-      if (!model.isError) {
-        if (isV2 && Object.keys(secret.changedAttributes()).length) {
+    return secretData.save().then(() => {
+      if (!secretData.isError) {
+        if (isV2) {
          secret.set('id', key);
+        }
+        if (isV2 && Object.keys(secret.changedAttributes()).length) {
          // save secret metadata
          secret
            .save()
@ -296,8 +298,8 @@ export default Component.extend(FocusOnInsertMixin, {
        return;
      }

-      this.persistKey(key => {
-        this.transitionToRoute(SHOW_ROUTE, key);
+      this.persistKey(() => {
+        this.transitionToRoute(SHOW_ROUTE, this.model.id);
      });
    },

--- a/ui/app/models/secret-v2.js
+++ b/ui/app/models/secret-v2.js
@ -33,6 +33,6 @@ export default Model.extend(KeyMixin, {
  secretPath: lazyCapabilities(apiPath`${'engineId'}/metadata/${'id'}`, 'engineId', 'id'),

  canEdit: alias('versionPath.canUpdate'),
-  canDelete: alias('secretPath.canUpdate'),
+  canDelete: alias('secretPath.canDelete'),
  canRead: alias('secretPath.canRead'),
 });
--- a/ui/app/models/secret.js
+++ b/ui/app/models/secret.js
@ -27,6 +27,6 @@ export default DS.Model.extend(KeyMixin, {
  backend: attr('string'),
  secretPath: lazyCapabilities(apiPath`${'backend'}/${'id'}`, 'backend', 'id'),
  canEdit: alias('secretPath.canUpdate'),
-  canDelete: alias('secretPath.canUpdate'),
+  canDelete: alias('secretPath.canDelete'),
  canRead: alias('secretPath.canRead'),
 });
--- a/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js
+++ b/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js
@ -64,6 +64,7 @@ export default Route.extend(UnloadModelRoute, {
  model(params) {
    let { secret } = params;
    const { backend } = this.paramsFor('vault.cluster.secrets.backend');
+    let backendModel = this.modelFor('vault.cluster.secrets.backend', backend);
    const modelType = this.modelType(backend, secret);

    if (!secret) {
@ -73,26 +74,50 @@ export default Route.extend(UnloadModelRoute, {
      secret = secret.replace('cert/', '');
    }
    return hash({
-      secret: this.store.queryRecord(modelType, { id: secret, backend }).then(resp => {
-        if (modelType === 'secret-v2') {
-          let backendModel = this.modelFor('vault.cluster.secrets.backend', backend);
-          let targetVersion = parseInt(params.version || resp.currentVersion, 10);
-          let version = resp.versions.findBy('version', targetVersion);
-          // 404 if there's no version
-          if (!version) {
-            let error = new DS.AdapterError();
-            set(error, 'httpStatus', 404);
-            throw error;
-          }
-          resp.set('engine', backendModel);
+      secret: this.store
+        .queryRecord(modelType, { id: secret, backend })
+        .then(secretModel => {
+          if (modelType === 'secret-v2') {
+            let targetVersion = parseInt(params.version || secretModel.currentVersion, 10);
+            let version = secretModel.versions.findBy('version', targetVersion);
+            // 404 if there's no version
+            if (!version) {
+              let error = new DS.AdapterError();
+              set(error, 'httpStatus', 404);
+              throw error;
+            }
+            secretModel.set('engine', backendModel);

-          return version.reload().then(() => {
-            resp.set('selectedVersion', version);
-            return resp;
-          });
-        }
-        return resp;
-      }),
+            return version.reload().then(() => {
+              secretModel.set('selectedVersion', version);
+              return secretModel;
+            });
+          }
+          return secretModel;
+        })
+        .catch(err => {
+          //don't have access to the metadata, so we'll make
+          //a stub metadata model and try to load the version
+          if (modelType === 'secret-v2' && err.httpStatus === 403) {
+            let secretModel = this.store.createRecord('secret-v2');
+            secretModel.setProperties({
+              engine: backendModel,
+              id: secret,
+              // so we know it's a stub model and won't be saving it
+              // because we don't have access to that endpoint
+              isStub: true,
+            });
+            let targetVersion = params.version ? parseInt(params.version, 10) : null;
+            let versionId = targetVersion ? [backend, secret, targetVersion] : [backend, secret];
+            return this.store
+              .findRecord('secret-v2-version', JSON.stringify(versionId), { reload: true })
+              .then(versionModel => {
+                secretModel.set('selectedVersion', versionModel);
+                return secretModel;
+              });
+          }
+          throw err;
+        }),
      capabilities: this.capabilities(secret),
    });
  },
--- a/ui/app/serializers/secret-v2-version.js
+++ b/ui/app/serializers/secret-v2-version.js
@ -17,7 +17,8 @@ export default ApplicationSerializer.extend({
  },
  serialize(snapshot) {
    let secret = snapshot.belongsTo('secret');
-    let version = secret.attr('currentVersion') || 0;
+    let version = secret.record.isStub ? snapshot.attr('version') : secret.attr('currentVersion');
+    version = version || 0;
    return {
      data: snapshot.attr('secretData'),
      options: {
--- a/ui/app/templates/components/secret-edit-display.hbs
+++ b/ui/app/templates/components/secret-edit-display.hbs
@ -1,5 +1,5 @@
-  {{#if (and (or @model.isNew @canEditV2Secret) @isV2)}}
-    <div class="form-section box is-shadowless is-fullwidth">
+  {{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.isStub))}}
+    <div data-test-metadata-fields class="form-section box is-shadowless is-fullwidth">
      <label class="title is-5">
        Secret Metadata
      </label>
--- a/ui/app/templates/partials/secret-form-edit.hbs
+++ b/ui/app/templates/partials/secret-form-edit.hbs
@ -2,7 +2,7 @@
  <div class="box is-sideless is-fullwidth is-marginless is-paddingless">
    <MessageError @model={{model}} @errorMessage={{error}} />
    <NamespaceReminder @mode="edit" @noun="secret" />
-    {{#if (not-eq model.selectedVersion.version model.currentVersion)}}
+    {{#if (and (not model.isStub) (not-eq model.selectedVersion.version model.currentVersion))}}
      <div class="form-section">
        <AlertBanner
          @type="warning"
@ -32,6 +32,7 @@
    <div class="field is-grouped">
      <div class="control">
        <button
+          data-test-secret-save
          type="submit"
          disabled={{buttonDisabled}}
          class="button is-primary"
--- a/ui/ember-cli-build.js
+++ b/ui/ember-cli-build.js
@ -22,7 +22,7 @@ module.exports = function(defaults) {
    hinting: isTest,
    tests: isTest,
    sourcemaps: {
-      enabled: false,
+      enabled: !isProd,
    },
    sassOptions: {
      sourceMap: false,
--- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js
+++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js
@ -9,16 +9,24 @@ import listPage from 'vault/tests/pages/secrets/backend/list';
 import mountSecrets from 'vault/tests/pages/settings/mount-secret-backend';
 import apiStub from 'vault/tests/helpers/noop-all-api-requests';
 import authPage from 'vault/tests/pages/auth';
+import logout from 'vault/tests/pages/logout';
 import withFlash from 'vault/tests/helpers/with-flash';
 import consoleClass from 'vault/tests/pages/components/console/ui-panel';

 const consoleComponent = create(consoleClass);

+let writeSecret = async function(backend, path, key, val) {
+  await listPage.visitRoot({ backend });
+  await listPage.create();
+  return editPage.createSecret(path, key, val);
+};
+
 module('Acceptance | secrets/secret/create', function(hooks) {
  setupApplicationTest(hooks);

-  hooks.beforeEach(function() {
+  hooks.beforeEach(async function() {
    this.server = apiStub({ usePassthrough: true });
+    await logout.visit();
    return authPage.login();
  });

@ -32,6 +40,7 @@ module('Acceptance | secrets/secret/create', function(hooks) {
    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.list-root', 'navigates to the list page');

    await listPage.create();
+    assert.ok(editPage.hasMetadataFields, 'shows the metadata form');
    await editPage.createSecret(path, 'foo', 'bar');

    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.show', 'redirects to the show page');
@ -44,11 +53,7 @@ module('Acceptance | secrets/secret/create', function(hooks) {
    await mountSecrets.visit();
    await mountSecrets.enable('kv', enginePath);
    await consoleComponent.runCommands(`write ${enginePath}/config cas_required=true`);
-
-    await listPage.visitRoot({ backend: enginePath });
-    await listPage.create();
-    await editPage.createSecret(secretPath, 'foo', 'bar');
-
+    await writeSecret(enginePath, secretPath, 'foo', 'bar');
    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.show', 'redirects to the show page');
    assert.ok(showPage.editIsPresent, 'shows the edit button');
  });
@ -101,4 +106,64 @@ module('Acceptance | secrets/secret/create', function(hooks) {
      'saves the content'
    );
  });
+
+  test('version 2 with restricted policy still allows creation', async function(assert) {
+    let backend = 'kv-v2';
+    const V2_POLICY = `'
+      path "kv-v2/metadata/*" {
+        capabilities = ["list"]
+      }
+      path "kv-v2/data/secret" {
+        capabilities = ["create", "read", "update"]
+      }
+    '`;
+    await consoleComponent.runCommands([
+      `write sys/mounts/${backend} type=kv options=version=2`,
+      `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`,
+      // delete any kv previously written here so that tests can be re-run
+      'delete kv-v2/metadata/secret',
+      'write -field=client_token auth/token/create policies=kv-v2-degrade',
+    ]);
+
+    let userToken = consoleComponent.lastLogOutput;
+    await logout.visit();
+    await authPage.login(userToken);
+
+    await writeSecret(backend, 'secret', 'foo', 'bar');
+    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.show', 'redirects to the show page');
+    assert.ok(showPage.editIsPresent, 'shows the edit button');
+    await logout.visit();
+  });
+
+  test('version 2 with restricted policy still allows edit', async function(assert) {
+    let backend = 'kv-v2';
+    const V2_POLICY = `'
+      path "kv-v2/metadata/*" {
+        capabilities = ["list"]
+      }
+      path "kv-v2/data/secret" {
+        capabilities = ["create", "read", "update"]
+      }
+    '`;
+    await consoleComponent.runCommands([
+      `write sys/mounts/${backend} type=kv options=version=2`,
+      `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`,
+      // delete any kv previously written here so that tests can be re-run
+      'delete kv-v2/metadata/secret',
+      'write -field=client_token auth/token/create policies=kv-v2-degrade',
+    ]);
+
+    let userToken = consoleComponent.lastLogOutput;
+    await writeSecret(backend, 'secret', 'foo', 'bar');
+    await logout.visit();
+    await authPage.login(userToken);
+
+    await editPage.visitEdit({ backend, id: 'secret' });
+    assert.notOk(editPage.hasMetadataFields, 'hides the metadata form');
+    await editPage.editSecret('bar', 'baz');
+
+    assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.show', 'redirects to the show page');
+    assert.ok(showPage.editIsPresent, 'shows the edit button');
+    await logout.visit();
+  });
 });
--- a/ui/tests/pages/secrets/backend/kv/edit-secret.js
+++ b/ui/tests/pages/secrets/backend/kv/edit-secret.js
@ -1,5 +1,5 @@
 import { Base } from '../create';
-import { clickable, visitable, create, fillable } from 'ember-cli-page-object';
+import { isPresent, clickable, visitable, create, fillable } from 'ember-cli-page-object';
 import { codeFillable } from 'vault/tests/pages/helpers/codemirror';
 export default create({
  ...Base,
@ -12,17 +12,22 @@ export default create({
  visitEdit: visitable('/vault/secrets/:backend/edit/:id'),
  visitEditRoot: visitable('/vault/secrets/:backend/edit'),
  toggleJSON: clickable('[data-test-secret-json-toggle]'),
+  hasMetadataFields: isPresent('[data-test-metadata-fields]'),
  editor: {
    fillIn: codeFillable('[data-test-component="json-editor"]'),
  },
  deleteSecret() {
    return this.deleteBtn().confirmBtn();
  },
-
  createSecret: async function(path, key, value) {
    return this.path(path)
      .secretKey(key)
      .secretValue(value)
      .save();
  },
+  editSecret: async function(key, value) {
+    return this.secretKey(key)
+      .secretValue(value)
+      .save();
+  },
 });