Skip to content

Commit

Permalink
UI - write without read for kv (#6570)
Browse files Browse the repository at this point in the history
* wait for all hash promises to be settled

* skeleton tests with policies for write without read

* adjust what gets returned from the model hook

* refactor secret-edit model hook to use async/await

* return a stub version if we can't read secret data

* return a stub model for v1 kv

* tweak tests to make re-runs friendlier

* allow write without CAS if both v2 models cannot be read

* show warnings on edit pages for different write without read scenarios

* add no read empty states on secret show pages

* review feedback

* make message language consistent

* use version models from metadata if we can read it

* refresh route on delete / undelete / destroy

* hide controls in the toolbar when you can't read the secret data

* show deleted / destroyed messaging over cannot read messaging on the show page

* fix test with model stub

* refactor large model hook into several functions

* comment clarifications
  • Loading branch information
meirish authored Apr 16, 2019
1 parent 538c3f5 commit e6ec125
Show file tree
Hide file tree
Showing 15 changed files with 314 additions and 69 deletions.
2 changes: 1 addition & 1 deletion ui/app/adapters/secret-v2-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default ApplicationAdapter.extend({
return this.ajax(this._url(backend, path, deleteType), 'POST', { data: { versions: [version] } }).then(
() => {
let model = store.peekRecord('secret-v2-version', id);
return model && model.reload();
return model && model.rollbackAttributes() && model.reload();
}
);
},
Expand Down
16 changes: 16 additions & 0 deletions ui/app/components/secret-edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,22 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, {
return this.secretDataIsAdvanced || this.preferAdvancedEdit;
}),

isWriteWithoutRead: computed(
'model.{failedServerRead,selectedVersion.failedServerRead}',
'isV2',
function() {
// if the version couldn't be read from the server
if (this.isV2 && this.model.selectedVersion.failedServerRead) {
return true;
}
// if the model couldn't be read from the server
if (!this.isV2 && this.model.failedServerRead) {
return true;
}
return false;
}
),

transitionToRoute() {
return this.router.transitionTo(...arguments);
},
Expand Down
4 changes: 3 additions & 1 deletion ui/app/components/secret-version-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default Component.extend({
store: service(),
version: null,
useDefaultTrigger: false,
onRefresh() {},

deleteVersionPath: maybeQueryRecord(
'capabilities',
Expand Down Expand Up @@ -52,7 +53,8 @@ export default Component.extend({
deleteVersion(deleteType = 'destroy') {
return this.store
.adapterFor('secret-v2-version')
.v2DeleteOperation(this.store, this.version.id, deleteType);
.v2DeleteOperation(this.store, this.version.id, deleteType)
.then(this.onRefresh);
},
},
});
2 changes: 1 addition & 1 deletion ui/app/models/secret.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default DS.Model.extend(KeyMixin, {

isAdvancedFormat: computed('secretData', function() {
const data = this.get('secretData');
return Object.keys(data).some(key => typeof data[key] !== 'string');
return data && Object.keys(data).some(key => typeof data[key] !== 'string');
}),

helpText: attr('string'),
Expand Down
186 changes: 128 additions & 58 deletions ui/app/routes/vault/cluster/secrets/backend/secret-edit.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { set } from '@ember/object';
import { hash, resolve } from 'rsvp';
import { resolve } from 'rsvp';
import { inject as service } from '@ember/service';
import DS from 'ember-data';
import Route from '@ember/routing/route';
Expand All @@ -21,12 +21,11 @@ export default Route.extend(UnloadModelRoute, {
capabilities(secret) {
const backend = this.enginePathParam();
let backendModel = this.modelFor('vault.cluster.secrets.backend');
let backendType = backendModel.get('engineType');
if (backendType === 'kv' || backendType === 'cubbyhole' || backendType === 'generic') {
return resolve({});
}
let backendType = backendModel.engineType;
let path;
if (backendType === 'transit') {
if (backendModel.isV2KV) {
path = `${backend}/data/${secret}`;
} else if (backendType === 'transit') {
path = backend + '/keys/' + secret;
} else if (backendType === 'ssh' || backendType === 'aws') {
path = backend + '/roles/' + secret;
Expand All @@ -43,9 +42,6 @@ export default Route.extend(UnloadModelRoute, {
templateName: 'vault/cluster/secrets/backend/secretEditLayout',

beforeModel() {
// currently there is no recursive delete for folders in vault, so there's no need to 'edit folders'
// perhaps in the future we could recurse _for_ users, but for now, just kick them
// back to the list
let secret = this.secretParam();
return this.buildModel(secret).then(() => {
const parentKey = utils.parentKeyForKey(secret);
Expand Down Expand Up @@ -86,10 +82,106 @@ export default Route.extend(UnloadModelRoute, {
return types[type];
},

model(params) {
let secret = this.secretParam();
getTargetVersion(currentVersion, paramsVersion) {
if (currentVersion) {
// we have the secret metadata, so we can read the currentVersion but give priority to any
// version passed in via the url
return parseInt(paramsVersion || currentVersion, 10);
} else {
// we've got a stub model because don't have read access on the metadata endpoint
return paramsVersion ? parseInt(paramsVersion, 10) : null;
}
},

async fetchV2Models(capabilities, secretModel, params) {
let backend = this.enginePathParam();
let backendModel = this.modelFor('vault.cluster.secrets.backend', backend);
let targetVersion = this.getTargetVersion(secretModel.currentVersion, params.version);

// if we have the metadata, a list of versions are part of the payload
let version = secretModel.versions && secretModel.versions.findBy('version', targetVersion);
// if it didn't fail the server read, and the version is not attached to the metadata,
// this should 404
if (!version && secretModel.failedServerRead !== true) {
let error = new DS.AdapterError();
set(error, 'httpStatus', 404);
throw error;
}
// manually set the related model
secretModel.set('engine', backendModel);

secretModel.set(
'selectedVersion',
await this.fetchV2VersionModel(capabilities, secretModel, version, targetVersion)
);
return secretModel;
},

async fetchV2VersionModel(capabilities, secretModel, version, targetVersion) {
let secret = this.secretParam();
let backend = this.enginePathParam();

// v2 versions have a composite ID, we generated one here if we need to manually set it
// after a failed fetch later;
let versionId = targetVersion ? [backend, secret, targetVersion] : [backend, secret];

let versionModel;
try {
if (secretModel.failedServerRead) {
// we couldn't read metadata, so we want to directly fetch the version
versionModel = await this.store.findRecord('secret-v2-version', JSON.stringify(versionId), {
reload: true,
});
} else {
// we may have previously errored, so roll it back here
version.rollbackAttributes();
// if metadata read was successful, the version we have is only a partial model
// trigger reload to fetch the whole version model
versionModel = await version.reload();
}
} catch (error) {
// cannot read the version data, but can write according to capabilities-self endpoint
if (error.httpStatus === 403 && capabilities.get('canUpdate')) {
// versionModel is then a partial model from the metadata (if we have read there), or
// we need to create one on the client
versionModel = version || this.store.createRecord('secret-v2-version');
versionModel.setProperties({
failedServerRead: true,
});
// if it was created on the client we need to trigger an event via ember-data
// so that it won't try to create the record on save
if (versionModel.isNew) {
versionModel.set('id', JSON.stringify(versionId));
//TODO make this a util to better show what's happening
// this is because we want the ember-data model save to call update instead of create
// in the adapter so we have to force the frontend model to a "saved" state
versionModel.send('pushedData');
}
} else {
throw error;
}
}
return versionModel;
},

handleSecretModelError(capabilities, secret, modelType, error) {
// can't read the path and don't have update capability, so re-throw
if (!capabilities.get('canUpdate') && modelType === 'secret') {
throw error;
}
// don't have access to the metadata for v2 or the secret for v1,
// so we make a stub model and mark it as `failedServerRead`
let secretModel = this.store.createRecord(modelType);
secretModel.setProperties({
id: secret,
failedServerRead: true,
});
return secretModel;
},

async model(params) {
let secret = this.secretParam();
let backend = this.enginePathParam();
let modelType = this.modelType(backend, secret);

if (!secret) {
Expand All @@ -98,53 +190,31 @@ export default Route.extend(UnloadModelRoute, {
if (modelType === 'pki-certificate') {
secret = secret.replace('cert/', '');
}
return hash({
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(() => {
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),
});
let secretModel;

let capabilities = this.capabilities(secret);
try {
secretModel = await this.store.queryRecord(modelType, { id: secret, backend });
} catch (err) {
// we've failed the read request, but if it's a kv-type backend, we want to
// do additional checks of the capabilities
if (err.httpStatus === 403 && (modelType === 'secret-v2' || modelType === 'secret')) {
await capabilities;
secretModel = this.handleSecretModelError(capabilities, secret, modelType, err);
} else {
throw err;
}
}
await capabilities;
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);
}
return {
secret: secretModel,
capabilities,
};
},

setupController(controller, model) {
Expand Down
8 changes: 7 additions & 1 deletion ui/app/serializers/secret-v2-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ export default ApplicationSerializer.extend({
},
serialize(snapshot) {
let secret = snapshot.belongsTo('secret');
let version = secret.record.isStub ? snapshot.attr('version') : secret.attr('currentVersion');
// if both models failed to read from the server, we need to write without CAS
if (secret.record.failedServerRead && snapshot.record.failedServerRead) {
return {
data: snapshot.attr('secretData'),
};
}
let version = secret.record.failedServerRead ? snapshot.attr('version') : secret.attr('currentVersion');
version = version || 0;
return {
data: snapshot.attr('secretData'),
Expand Down
4 changes: 2 additions & 2 deletions ui/app/templates/components/empty-state.hbs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<div class="empty-state">
<div class="empty-state" ...attributes>
<div class="empty-state-content">
<h3 class="empty-state-title">
{{title}}
</h3>
{{#if message}}
<p class="empty-state-message">
<p class="empty-state-message" data-test-empty-state-message>
{{message}}
</p>
{{/if}}
Expand Down
27 changes: 26 additions & 1 deletion ui/app/templates/components/secret-edit-display.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.isStub))}}
{{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.failedServerRead))}}
<div data-test-metadata-fields class="form-section box is-shadowless is-fullwidth">
<label class="title is-5">
Secret metadata
Expand All @@ -9,6 +9,31 @@
</div>
{{/if}}

{{#if @showWriteWithoutReadWarning}}
{{#if (and @isV2 @model.failedServerRead)}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading metadata for this secret and the current version's data. Creating a new version of the secret with this form will not be able to use the check-and-set mechanism. If this is required on the secret, then you will need access to read the secret's metadata."
@class="is-marginless"
data-test-v2-no-cas-warning
/>
{{else if @isV2}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret version. Saving this form will create a new version of the secret and will utilize the available check-and-set mechanism."
@class="is-marginless"
data-test-v2-write-without-read
/>
{{else}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret data. Saving using this form will overwrite the existing values."
@class="is-marginless"
data-test-v1-write-without-read
/>
{{/if}}
{{/if}}

{{#if @showAdvancedMode}}
<div class="form-section">
<label class="title is-5">
Expand Down
11 changes: 8 additions & 3 deletions ui/app/templates/components/secret-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
</p.levelRight>
</PageHeader>
<div class="secret-control-bar">
{{#unless (and (eq mode 'show') isWriteWithoutRead)}}
<div class="control">
<input
data-test-secret-json-toggle=true
Expand All @@ -52,9 +53,10 @@
/>
<label for="json" class="has-text-grey">JSON</label>
</div>
{{/unless}}
{{#if (and (eq mode 'show') (or canEditV2Secret canEdit))}}
{{#let (concat 'vault.cluster.secrets.backend.' (if (eq mode 'show') 'edit' 'show')) as |targetRoute|}}
{{#unless (and isV2 (or modelForData.destroyed modelForData.deleted))}}
{{#unless (and isV2 (or isWriteWithoutRead modelForData.destroyed modelForData.deleted))}}
<div class="control">
<BasicDropdown
@class="popup-menu"
Expand Down Expand Up @@ -134,9 +136,12 @@
</div>
{{/let}}
{{/if}}
{{#if (and (eq @mode "show") this.isV2)}}
{{#if (and (eq @mode "show") this.isV2 (not @model.failedServerRead))}}
<div class="control">
<SecretVersionMenu @version={{this.modelForData}} />
<SecretVersionMenu
@version={{this.modelForData}}
@onRefresh={{action 'refresh'}}
/>
</div>
<div class="control">
<BasicDropdown
Expand Down
3 changes: 2 additions & 1 deletion ui/app/templates/partials/secret-form-edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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 (and (not model.isStub) (not-eq model.selectedVersion.version model.currentVersion))}}
{{#if (and (not model.failedServerRead) (not model.selectedVersion.failedServerRead) (not-eq model.selectedVersion.version model.currentVersion))}}
<div class="form-section">
<AlertBanner
@type="warning"
Expand All @@ -17,6 +17,7 @@
@secretData={{secretData}}
@isV2={{isV2}}
@canEditV2Secret={{canEditV2Secret}}
@showWriteWithoutReadWarning={{isWriteWithoutRead}}
@model={{model}}
@editActions={{hash
codemirrorUpdated=(action "codemirrorUpdated")
Expand Down
Loading

0 comments on commit e6ec125

Please sign in to comment.