Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI - write without read for kv #6570

Merged
merged 19 commits into from
Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Contributor

@andaley andaley Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the user not having read access in their policy, are there any other scenarios which would cause the version to not be read from the server? If so, my comments above regarding the alert banner warning message aren't relevant. 😝

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it'd only be the policy missing (or an additional Sentinel policy on the path) that could prevent it.

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