KV: handle various metadata permissions (#12673)

* fix delete issue when no read on metadata

* show create button

* fix navigation on metadata

* dont show search unless its version 2

* need to query because can't be certain the model will have loaded

* fix issue with no read metadata and create new version

* cleanup the delete reload issues

* cleanup modal refresh issues

* extra conditional on delete

* test coverage

* add more test coverage

* some pr comments but also fix soft delete

* test cleanup

* fix soft delete issue

ui/app/adapters/secret-v2-version.js

@@ -36,6 +36,13 @@ export default ApplicationAdapter.extend({
     });
   },
 
+  async getSecretDataVersion(backend, id) {
+    // used in secret-edit route when you don't have current version and you need it for pulling the correct secret-v2-version record
+    let url = this._url(backend, id);
+    let response = await this.ajax(this._url(backend, id), 'GET');
+    return response.data.metadata.version;
+  },
+
   queryRecord(id, options) {
     return this.ajax(this.urlForQueryRecord(id), 'GET', options).then(resp => {
       if (options.wrapTTL) {
@@ -67,28 +74,72 @@ export default ApplicationAdapter.extend({
     return this._url(backend, path);
   },
 
-  v2DeleteOperation(store, id, deleteType = 'delete') {
+  async deleteLatestVersion(backend, path) {
+    try {
+      await this.ajax(this._url(backend, path, 'data'), 'DELETE');
+      let model = this.store.peekRecord('secret-v2-version', path);
+      await model.reload();
+      return model && model.rollbackAttributes();
+    } catch (e) {
+      return e;
+    }
+  },
+
+  async undeleteVersion(backend, path, currentVersionForNoReadMetadata) {
+    try {
+      await this.ajax(this._url(backend, path, 'undelete'), 'POST', {
+        data: { versions: [currentVersionForNoReadMetadata] },
+      });
+      let model = this.store.peekRecord('secret-v2-version', path);
+      await model.reload();
+      return model && model.rollbackAttributes();
+    } catch (e) {
+      return e;
+    }
+  },
+
+  async softDelete(backend, path, version) {
+    try {
+      await this.ajax(this._url(backend, path, 'delete'), 'POST', {
+        data: { versions: [version] },
+      });
+      let model = this.store.peekRecord('secret-v2-version', path);
+      await model.reload();
+      return model && model.rollbackAttributes();
+    } catch (e) {
+      return e;
+    }
+  },
+
+  async deleteByDeleteType(backend, path, deleteType, version) {
+    try {
+      await this.ajax(this._url(backend, path, deleteType), 'POST', {
+        data: { versions: [version] },
+      });
+      let model = this.store.peekRecord('secret-v2-version', path);
+      await model.reload();
+      return model && model.rollbackAttributes();
+    } catch (e) {
+      return e;
+    }
+  },
+
+  v2DeleteOperation(store, id, deleteType = 'delete', currentVersionForNoReadMetadata) {
     let [backend, path, version] = JSON.parse(id);
     // deleteType should be 'delete', 'destroy', 'undelete', 'delete-latest-version', 'destroy-version'
-    if ((!version && deleteType === 'delete') || deleteType === 'delete-latest-version') {
-      return this.ajax(this._url(backend, path, 'data'), 'DELETE')
-        .then(() => {
-          let model = store.peekRecord('secret-v2-version', id);
-          return model && model.rollbackAttributes() && model.reload();
-        })
-        .catch(e => {
-          return e;
-        });
+    if (
+      (currentVersionForNoReadMetadata && deleteType === 'delete') ||
+      deleteType === 'delete-latest-version'
+    ) {
+      // moved to async to away model reload which is a promise
+      return this.deleteLatestVersion(backend, path);
+    } else if (deleteType === 'undelete' && !version) {
+      // happens when no read access to metadata
+      return this.undeleteVersion(backend, path, currentVersionForNoReadMetadata);
+    } else if (deleteType === 'soft-delete') {
+      return this.softDelete(backend, path, version);
     } else {
-      return this.ajax(this._url(backend, path, deleteType), 'POST', { data: { versions: [version] } })
-        .then(() => {
-          let model = store.peekRecord('secret-v2-version', id);
-          // potential that model.reload() is never called.
-          return model && model.rollbackAttributes() && model.reload();
-        })
-        .catch(e => {
-          return e;
-        });
+      return this.deleteByDeleteType(backend, path, deleteType, version);
     }
   },
 

ui/app/components/secret-create-or-update.js

@@ -117,6 +117,8 @@ export default class SecretCreateOrUpdate extends Component {
       key = key.replace(/^\/+/g, '');
       secretData.set(secretData.pathAttr, key);
     }
+    let changed = secret.changedAttributes();
+    let changedKeys = Object.keys(changed);
 
     return secretData
       .save()
@@ -125,7 +127,9 @@ export default class SecretCreateOrUpdate extends Component {
           if (isV2) {
             secret.set('id', key);
           }
-          if (isV2 && Object.keys(secret.changedAttributes()).length > 0) {
+          // this secret.save() saves to the metadata endpoint. Only saved if metadata has been added
+          // and if the currentVersion attr changed that's because we added it (only happens if they don't have read access to metadata on mode = update which does not allow you to change metadata)
+          if (isV2 && changedKeys.length > 0 && changedKeys[0] !== 'currentVersion') {
             // save secret metadata
             secret
               .save()
@@ -134,7 +138,7 @@ export default class SecretCreateOrUpdate extends Component {
               })
               .catch(e => {
                 // when mode is not create the metadata error is handled in secret-edit-metadata
-                if (this.mode === 'create') {
+                if (this.args.mode === 'create') {
                   this.error = e.errors.join(' ');
                 }
                 return;

ui/app/components/secret-delete-menu.js

@@ -17,20 +17,6 @@ export default class SecretDeleteMenu extends Component {
 
   @tracked showDeleteModal = false;
 
-  @maybeQueryRecord(
-    'capabilities',
-    context => {
-      if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return;
-      let [backend, id] = JSON.parse(context.args.modelForData.id);
-      return {
-        id: `${backend}/delete/${id}`,
-      };
-    },
-    'model.id'
-  )
-  deleteVersionPath;
-  @alias('deleteVersionPath.canUpdate') canDeleteAnyVersion;
-
   @maybeQueryRecord(
     'capabilities',
     context => {
@@ -97,7 +83,29 @@ export default class SecretDeleteMenu extends Component {
   secretDataPath;
   @alias('secretDataPath.canDelete') canDeleteSecretData;
 
+  @maybeQueryRecord(
+    'capabilities',
+    context => {
+      if (!context.args.model || context.args.mode === 'create') {
+        return;
+      }
+      let backend = context.args.isV2 ? context.args.model.engine.id : context.args.model.backend;
+      let id = context.args.model.id;
+      let path = context.args.isV2 ? `${backend}/delete/${id}` : `${backend}/${id}`;
+      return {
+        id: path,
+      };
+    },
+    'isV2',
+    'model',
+    'model.id',
+    'mode'
+  )
+  secretSoftDataPath;
+  @alias('secretSoftDataPath.canUpdate') canSoftDeleteSecretData;
+
   get isLatestVersion() {
+    // must have metadata access.
     let { model } = this.args;
     if (!model) return false;
     let latestVersion = model.currentVersion;
@@ -115,17 +123,19 @@ export default class SecretDeleteMenu extends Component {
       return;
     }
     if (deleteType === 'destroy-all-versions' || deleteType === 'v1') {
-      let { id } = this.args.model;
       this.args.model.destroyRecord().then(() => {
-        if (deleteType === 'v1') {
-          return this.router.transitionTo('vault.cluster.secrets.backend.list-root');
-        }
-        this.args.navToNearestAncestor.perform(id);
+        return this.router.transitionTo('vault.cluster.secrets.backend.list-root');
       });
     } else {
+      // if they do not have read access on the metadata endpoint we need to pull the version from modelForData so they can perform delete and undelete operations
+      // only perform if no access to metatdata otherwise it will only delete latest version for any deleteType === delete
+      let currentVersionForNoReadMetadata;
+      if (!this.args.canReadSecretMetadata) {
+        currentVersionForNoReadMetadata = this.args.modelForData?.version;
+      }
       return this.store
         .adapterFor('secret-v2-version')
-        .v2DeleteOperation(this.store, this.args.modelForData.id, deleteType)
+        .v2DeleteOperation(this.store, this.args.modelForData.id, deleteType, currentVersionForNoReadMetadata)
         .then(resp => {
           if (Ember.testing) {
             return;

ui/app/components/secret-edit.js

@@ -90,6 +90,7 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, {
     'mode'
   ),
   canUpdateSecretData: alias('checkSecretCapabilities.canUpdate'),
+  canReadSecretData: alias('checkSecretCapabilities.canRead'),
 
   checkMetadataCapabilities: maybeQueryRecord(
     'capabilities',

ui/app/routes/vault/cluster/secrets/backend/metadata.js

@@ -12,6 +12,7 @@ export default class MetadataShow extends Route {
 
   model(params) {
     let { secret } = params;
+    this.id = secret;
     return this.store
       .queryRecord('secret-v2', {
         backend: this.backend,
@@ -28,6 +29,7 @@ export default class MetadataShow extends Route {
 
   setupController(controller, model) {
     controller.set('backend', this.backend); // for backendCrumb
+    controller.set('id', this.id); // for navigation on tabs
     controller.set('model', model);
     controller.set('noReadAccess', this.noReadAccess);
   }

ui/app/routes/vault/cluster/secrets/backend/secret-edit.js

@@ -251,7 +251,20 @@ export default Route.extend(UnloadModelRoute, {
     if (modelType === 'secret-v2') {
       // after the the base model fetch, kv-v2 has a second associated
       // version model that contains the secret data
-      secretModel = await this.fetchV2Models(capabilities, secretModel, params);
+
+      // if no read access to metadata, return current Version from secret data.
+      if (!secretModel.currentVersion) {
+        let adapter = this.store.adapterFor('secret-v2-version');
+        try {
+          secretModel.currentVersion = await adapter.getSecretDataVersion(backend, secret);
+        } catch {
+          // will get error if you have deleted the secret
+          // if this is the case do nothing
+        }
+        secretModel = await this.fetchV2Models(capabilities, secretModel, params);
+      } else {
+        secretModel = await this.fetchV2Models(capabilities, secretModel, params);
+      }
     }
     return {
       secret: secretModel,
@@ -308,6 +321,14 @@ export default Route.extend(UnloadModelRoute, {
       let version = model.get('selectedVersion');
       let changed = model.changedAttributes();
       let changedKeys = Object.keys(changed);
+
+      // when you don't have read access on metadata we add currentVersion to the model
+      // this makes it look like you have unsaved changes and prompts a browser warning
+      // here we are specifically ignoring it.
+      if (mode === 'edit' && (changedKeys.length && changedKeys[0] === 'currentVersion')) {
+        version && version.rollbackAttributes();
+        return true;
+      }
       // until we have time to move `backend` on a v1 model to a relationship,
       // it's going to dirty the model state, so we need to look for it
       // and explicity ignore it here

ui/app/templates/components/secret-create-or-update.hbs

@@ -139,15 +139,14 @@
 {{!-- no metadata option because metadata is version agnostic --}}
   <form onsubmit={{action "createOrUpdateKey" "edit"}}>
     <div class="box is-sideless is-fullwidth is-marginless padding-top">
-      {{#if @model.canReadSecretData}}
-        <MessageError @model={{@modelForData}} @errorMessage={{this.error}} />
-      {{else}}
+      <MessageError @model={{@modelForData}} @errorMessage={{this.error}} />
+      {{#unless @canReadSecretData}}
         <AlertBanner
           @type="warning"
           @message="You do not have read permissions. If a secret exists here creating a new secret will overwrite it."
           data-test-warning-no-read-permissions
         />
-      {{/if}}
+      {{/unless}}
       <NamespaceReminder @mode="edit" @noun="secret" />
       {{#if this.isCreateNewVersionFromOldVersion}}
         <div class="form-section">

ui/app/templates/components/secret-delete-menu.hbs

@@ -33,12 +33,12 @@
       </button>
       <div class="toolbar-separator"/>
     {{else}}
-      {{#if (or this.canDeleteAnyVersion (and this.isLatestVersion this.canDeleteSecretData))}}
+      {{#if (or (and this.isLatestVersion this.canDeleteSecretData) this.canSoftDeleteSecretData)}}
         <ConfirmAction
           @buttonClasses="toolbar-link"
           @confirmTitle="Delete"
           @confirmMessage="Deleting this secret removes read access, but the underlying data will not be removed and can be undeleted later."
-          @onConfirmAction={{action "handleDelete" (if canDeleteAnyVersion "delete" "delete-latest-version") }}
+          @onConfirmAction={{action "handleDelete" (if this.canSoftDeleteSecretData "soft-delete" "delete-latest-version")}}
           data-test-secret-v2-delete="true"
         >
           Delete
@@ -59,10 +59,10 @@
       <p class="has-bottom-margin-s"><strong>How would you like to proceed?</strong></p>
       {{#unless @modelForData.destroyed}}
         {{#unless @modelForData.deleted}}
-          {{#if this.canDeleteSecretData}}
+          {{#if (or this.canSoftDeleteSecretData this.canDeleteSecretData)}}
             <div class="modal-radio-button" data-test-delete-modal="delete-version">
               <RadioButton
-                @value="delete"
+                @value={{if this.canSoftDeleteSecretData "soft-delete" "delete-latest-version"}}
                 @radioClass="radio"
                 @groupValue={{this.deleteType}}
                 @changed={{action (mut this.deleteType)}}
@@ -70,7 +70,7 @@
                 @radioId="delete-version"
               />
               <div class="helper-text">
-                <label for="delete-version" data-test-replication-type-select="delete-version"><strong>Delete this version</strong></label>
+                <label for="delete-version" data-test-type-select="delete-version"><strong>Delete this version</strong></label>
                 <p>This deletes Version {{@modelForData.version}} of the secret. It can be un-deleted later.</p>
               </div>
             </div>
@@ -87,7 +87,7 @@
               @radioId="destroy-version"
             />
             <div class="helper-text">
-              <label for="destroy-version" data-test-replication-type-select="destroy-version"><strong>Destroy this version</strong></label>
+              <label for="destroy-version" data-test-type-select="destroy-version"><strong>Destroy this version</strong></label>
               <p>Version {{@modelForData.version}} is permanently destroyed and cannot be read or recovered later.</p>
             </div>
           </div>
@@ -116,6 +116,7 @@
         class="button has-text-danger"
         {{on "click" (fn this.handleDelete this.deleteType (action (mut this.showDeleteModal) false))}}
         disabled={{if this.deleteType false true}}
+        data-test-modal-delete
       >
         Delete
       </button>

ui/app/templates/components/secret-edit-toolbar.hbs

@@ -21,6 +21,7 @@
         @navToNearestAncestor={{@navToNearestAncestor}}
         @isV2={{@isV2}}
         @refresh={{action @editActions.refresh}}
+        @canReadSecretMetadata={{@canReadSecretMetadata}}
       />
     {{/if}}
     {{#if (and (eq @mode 'show') @canUpdateSecretData)}}

ui/app/templates/components/secret-edit.hbs

@@ -49,6 +49,7 @@
   @modelForData={{modelForData}}
   @navToNearestAncestor={{navToNearestAncestor}}
   @canUpdateSecretData={{canUpdateSecretData}}
+  @canReadSecretMetadata={{canReadSecretMetadata}}
   @codemirrorString={{codemirrorString}}
   @editActions={{hash
     toggleAdvanced=(action "toggleAdvanced")
@@ -67,6 +68,8 @@
     @secretData={{secretData}}
     @buttonDisabled={{buttonDisabled}}
     @canUpdateSecretMetadata={{canUpdateSecretMetadata}}
+    @canReadSecretData={{canReadSecretData}}
+    @canReadSecretMetadata={{canReadSecretMetadata}}
   />
 {{else if (eq mode "show")}}
   <SecretFormShow

ui/app/templates/vault/cluster/secrets/backend/list.hbs

@@ -5,7 +5,21 @@
   @backendCrumb={{backendCrumb}}
   @filter={{filter}}
 />
-{{#if this.noMetadataPermissions}}
+{{#if (and this.noMetadataPermissions backendModel.isV2KV)}}
+{{!-- Show the create button only in the toolbar. Cannot conditional hide this because they could have a specific policy that allows them to create secrets by a certain name --}}
+  <Toolbar>
+    <ToolbarActions>
+      <ToolbarSecretLink
+        @secret=''
+        @mode="create"
+        @type="add"
+        @queryParams={{query-params initialKey=(or filter baseKey.id) itemType=tab}}
+        @data-test-secret-create=true
+      >
+        Create Secret
+      </ToolbarSecretLink>
+    </ToolbarActions>
+  </Toolbar>
   <div class="box is-fullwidth is-shadowless has-tall-padding">
     <div class="selectable-card-container one-card">
       <GetCredentialsCard

ui/app/templates/vault/cluster/secrets/backend/metadata.hbs

@@ -1,7 +1,7 @@
 <PageHeader as |p|>
   <p.top>
     <KeyValueHeader
-      @baseKey={{hash id=model.id}}
+      @baseKey={{hash id=this.id}}
       @path="vault.cluster.secrets.backend.show"
       @mode="show"
       @showCurrent={{true}} 
@@ -10,7 +10,7 @@
   </p.top>
   <p.levelLeft>
     <h1 class="title is-3">
-      {{this.model.id}}
+      {{this.id}}
     </h1>
   </p.levelLeft>
 </PageHeader>
@@ -19,11 +19,11 @@
   <nav class="tabs">
     <ul>
       <LinkTo @route="vault.cluster.secrets.backend.show" @tagName="li" @activeClass="is-active">
-        <LinkTo @route="vault.cluster.secrets.backend.show" @model={{this.model.id}}>
+        <LinkTo @route="vault.cluster.secrets.backend.show" @model={{this.id}} data-test-secret-tab>
           Secret
         </LinkTo>
       </LinkTo>
-      <LinkTo @route="vault.cluster.secrets.backend.metadata" @model={{this.model.id}} @tagName="li" @activeClass="is-active">
+      <LinkTo @route="vault.cluster.secrets.backend.metadata" @model={{this.id}} @tagName="li" @activeClass="is-active">
         <LinkTo @route="vault.cluster.secrets.backend.metadata">
           Metadata
         </LinkTo>
@@ -36,7 +36,7 @@
   {{!-- You must have update on metadata, create is not enough. --}}
   {{#if this.model.canUpdateMetadata}}
     <ToolbarActions>  
-      <ToolbarLink @params={{array 'vault.cluster.secrets.backend.edit-metadata' this.model.id }}>
+      <ToolbarLink @params={{array 'vault.cluster.secrets.backend.edit-metadata' this.id }}>
         Edit metadata
       </ToolbarLink>
     </ToolbarActions>

ui/tests/acceptance/secrets/backend/kv/secret-test.js

@@ -530,14 +530,14 @@ module('Acceptance | secrets/secret/create', function(hooks) {
     assert.dom('[data-test-secret-metadata-tab]').doesNotExist('does not show metadata tab');
   });
 
-  test('version 2: with metadata no read or list access but access to the data endpoint', async function(assert) {
+  test('version 2: with metadata no read or list but with delete access and full access to the data endpoint', async function(assert) {
     let backend = 'no-metadata-read';
     let V2_POLICY_NO_LIST = `
       path "${backend}/metadata/*" {
-        capabilities = ["create","update"]
+        capabilities = ["update","delete"]
       }
       path "${backend}/data/*" {
-        capabilities = ["create", "read", "update"]
+        capabilities = ["create", "read", "update", "delete"]
       }
     `;
     await consoleComponent.runCommands([
@@ -575,7 +575,18 @@ module('Acceptance | secrets/secret/create', function(hooks) {
     await assert
       .dom('[data-test-value-div="secret-key"]')
       .exists('secret view page and info table row with secret-key value');
-    // check metadata
+    // check you can create new version
+    await click('[data-test-secret-edit="true"]');
+    await settled();
+    await fillIn('[data-test-secret-key]', 'version2');
+    await editPage.save();
+    await settled();
+    assert.dom('[data-test-row-label="version2"]').exists('the current version displayed is the new version');
+    assert
+      .dom('[data-test-popup-menu-trigger="version"]')
+      .doesNotExist('the version drop down menu does not show');
+
+    // check metadata tab
     await click('[data-test-secret-metadata-tab]');
     await settled();
     assert
@@ -583,6 +594,21 @@ module('Acceptance | secrets/secret/create', function(hooks) {
       .hasText(
         'In order to edit secret metadata access, the UI requires read permissions; otherwise, data may be deleted. Edits can still be made via the API and CLI.'
       );
+    // destroy the version
+    await click('[data-test-secret-tab]');
+    await settled();
+    await click('[data-test-delete-open-modal]');
+    await settled();
+    assert.dom('[data-test-delete-modal="destroy-all-versions"]').exists(); // we have a if Ember.testing catch in the delete action because it breaks things in testing
+    // we can however destroy the versions
+    await click('#destroy-all-versions');
+    await settled();
+    await click('[data-test-modal-delete]');
+    await settled();
+    assert.equal(currentURL(), '/vault/secrets/no-metadata-read/list', 'brings you back to the list page');
+    await visit('/vault/secrets/no-metadata-read/show/secret-path');
+    await settled();
+    assert.dom('[data-test-secret-not-found]').exists('secret no longer found');
   });
 
   // KV delete operations testing