From 16831c25c16a51ba719e014a0ef7dafd10f18fac Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 22:19:45 -0600 Subject: [PATCH 01/10] turns out sourcemaps are useful --- ui/ember-cli-build.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/ember-cli-build.js b/ui/ember-cli-build.js index a2a718ef97b1..500c14016e27 100644 --- 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, From 4aa32882ec103bb800123b6ab2785f21a06daa99 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 22:20:22 -0600 Subject: [PATCH 02/10] add test for restricted policy in kv v2 --- .../secrets/backend/kv/secret-test.js | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index 88f2cd91f4c9..9bd3137a63b6 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -9,11 +9,18 @@ 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); @@ -24,6 +31,7 @@ module('Acceptance | secrets/secret/create', function(hooks) { hooks.afterEach(function() { this.server.shutdown(); + return logout.visit(); }); test('it creates a secret and redirects', async function(assert) { @@ -44,11 +52,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 +105,30 @@ 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", "delete"] + } + '`; + await consoleComponent.runCommands([ + `write sys/mounts/${backend} type=kv options=version=2`, + `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`, + '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'); + }); }); From 2b368120e8c6d2b03e43c88ece8ab16280f3a5b1 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 22:37:55 -0600 Subject: [PATCH 03/10] only include version param on fetch if it's encoded in the id --- ui/app/adapters/secret-v2-version.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/app/adapters/secret-v2-version.js b/ui/app/adapters/secret-v2-version.js index 4f77992cdfec..1c9236470a9c 100644 --- 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() { From 7675dc921a8c523fb0d56ca69d9395b446361c59 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 22:40:32 -0600 Subject: [PATCH 04/10] rename some vars for clarity and use model.id when persisting a secret --- ui/app/components/secret-edit.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/ui/app/components/secret-edit.js b/ui/app/components/secret-edit.js index 59b5be8788fb..d814827524bc 100644 --- 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') || secretData.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); }); }, From 3b900782a9c6181388cb50c3ae76101638a209f4 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 23:52:44 -0600 Subject: [PATCH 05/10] fix delete attributes on the models --- ui/app/models/secret-v2.js | 2 +- ui/app/models/secret.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/models/secret-v2.js b/ui/app/models/secret-v2.js index d68f9e43535e..5a37b4eb8876 100644 --- 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'), }); diff --git a/ui/app/models/secret.js b/ui/app/models/secret.js index 36fc8eeeafc9..02d5a61c4ad4 100644 --- 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'), }); From c92ef8ecfd92f200e348f6a8113464c61659498b Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Thu, 29 Nov 2018 23:54:23 -0600 Subject: [PATCH 06/10] allow data edit when there's metadata access is disallowed --- .../cluster/secrets/backend/secret-edit.js | 63 +++++++++++++------ ui/app/serializers/secret-v2-version.js | 3 +- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js b/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js index 873fc305f6bb..4f73de9424b1 100644 --- 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 dummy 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 dummy model and won't be saving it + // because we don't have access to that endpoint + isDummy: 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), }); }, diff --git a/ui/app/serializers/secret-v2-version.js b/ui/app/serializers/secret-v2-version.js index 61a001821ff6..98ab30ee24b3 100644 --- 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.isDummy ? snapshot.attr('version') : secret.attr('currentVersion'); + version = version || 0; return { data: snapshot.attr('secretData'), options: { From c46ce2d9177e4be56e154704080acefbabc8239a Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Fri, 30 Nov 2018 10:25:47 -0600 Subject: [PATCH 07/10] add tests for edit with restricted policy --- ui/app/components/secret-edit.js | 2 +- .../templates/partials/secret-form-edit.hbs | 1 + .../secrets/backend/kv/secret-test.js | 40 +++++++++++++++++-- .../pages/secrets/backend/kv/edit-secret.js | 6 ++- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/ui/app/components/secret-edit.js b/ui/app/components/secret-edit.js index d814827524bc..fcb816f98553 100644 --- a/ui/app/components/secret-edit.js +++ b/ui/app/components/secret-edit.js @@ -183,7 +183,7 @@ export default Component.extend(FocusOnInsertMixin, { let secret = this.model; let secretData = this.modelForData; let isV2 = this.isV2; - let key = secretData.get('path') || secretData.id; + let key = secretData.get('path') || secret.id; if (key.startsWith('/')) { key = key.replace(/^\/+/g, ''); diff --git a/ui/app/templates/partials/secret-form-edit.hbs b/ui/app/templates/partials/secret-form-edit.hbs index c8b7cda98a4c..f1db5d5b4a6c 100644 --- a/ui/app/templates/partials/secret-form-edit.hbs +++ b/ui/app/templates/partials/secret-form-edit.hbs @@ -32,6 +32,7 @@