Skip to content

Commit

Permalink
do not swallow ControlGroupErrors when viewing or editing kvv2 secrets (
Browse files Browse the repository at this point in the history
#7504)

* do not swallow ControlGroupErrors when viewing or editing kvv2 secrets

* test kv v2 control group workflow

* do not manually clearModelCache when logging out since this already happens when leaving the logout route

* remove pauseTest

* update comments

* wip - looking into why restricted user can see the control group protected secret after it has already been unwrapped once

* strip version from query params so we can unwrap a secret after it is authorized

* use attachCapabilities instead of lazyCapabilities to ensure models are cleaned up properly

* remove comment

* make ControlGroupError extend AdapterError

* fix broken redirect_to test

* one day i will remember to remove my debugger statements; today is not that day

* no need to check for a ControlGroupError since it extends an AdapterError

* see if using EmberError instead of AdapterError fixes the browserstack tests

* Revert "see if using EmberError instead of AdapterError fixes the browserstack tests"

This reverts commit 14ddd67.
  • Loading branch information
Noelle Daley authored Oct 4, 2019
1 parent 0c98996 commit f8370d6
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 25 deletions.
5 changes: 3 additions & 2 deletions ui/app/adapters/secret-v2-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { get } from '@ember/object';
import ApplicationAdapter from './application';
import DS from 'ember-data';
import { encodePath } from 'vault/utils/path-encoding-helpers';
import ControlGroupError from 'vault/lib/control-group-error';

export default ApplicationAdapter.extend({
namespace: 'v1',
Expand All @@ -27,8 +28,8 @@ export default ApplicationAdapter.extend({

findRecord() {
return this._super(...arguments).catch(errorOrModel => {
// if it's a real 404, this will be an error, if not
// it will be the body of a deleted / destroyed version
// if the response is a real 404 or if the secret is gated by a control group this will be an error,
// otherwise the response will be the body of a deleted / destroyed version
if (errorOrModel instanceof DS.AdapterError) {
throw errorOrModel;
}
Expand Down
4 changes: 2 additions & 2 deletions ui/app/lib/control-group-error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import EmberError from '@ember/error';
import DS from 'ember-data';

export default class ControlGroupError extends EmberError {
export default class ControlGroupError extends DS.AdapterError {
constructor(wrapInfo) {
let { accessor, creation_path, creation_time, token, ttl } = wrapInfo;
super();
Expand Down
15 changes: 8 additions & 7 deletions ui/app/models/identity/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { computed } from '@ember/object';
import { alias } from '@ember/object/computed';
import IdentityModel from './_base';
import DS from 'ember-data';
import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities';
import identityCapabilities from 'vault/macros/identity-capabilities';
import apiPath from 'vault/utils/api-path';
import attachCapabilities from 'vault/lib/attach-capabilities';

const { attr, hasMany } = DS;

export default IdentityModel.extend({
let Model = IdentityModel.extend({
formFields: computed(function() {
return ['name', 'disabled', 'policies', 'metadata'];
}),
Expand Down Expand Up @@ -43,12 +43,13 @@ export default IdentityModel.extend({
inheritedGroupIds: attr({
readOnly: true,
}),

updatePath: identityCapabilities(),
canDelete: alias('updatePath.canDelete'),
canEdit: alias('updatePath.canUpdate'),
canRead: alias('updatePath.canRead'),

aliasPath: lazyCapabilities(apiPath`identity/entity-alias`),
canAddAlias: alias('aliasPath.canCreate'),
});

export default attachCapabilities(Model, {
updatePath: apiPath`identity/${'identityType'}/id/${'id'}`,
aliasPath: apiPath`identity/entity-alias`,
});
1 change: 0 additions & 1 deletion ui/app/routes/vault/cluster/logout.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export default Route.extend(ModelBoundaryRoute, {
this.namespaceService.reset();
this.console.set('isOpen', false);
this.console.clearLog(true);
this.clearModelCache();
this.flashMessages.clearMessages();
this.permissions.reset();
this.replaceWith('vault.cluster.auth');
Expand Down
4 changes: 3 additions & 1 deletion ui/app/services/control-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Service, { inject as service } from '@ember/service';
import RSVP from 'rsvp';
import ControlGroupError from 'vault/lib/control-group-error';
import getStorage from 'vault/lib/token-storage';
import parseURL from 'core/utils/parse-url';

const CONTROL_GROUP_PREFIX = 'vault:cg-';
const TOKEN_SEPARATOR = '☃';
Expand Down Expand Up @@ -71,7 +72,8 @@ export default Service.extend({
if (this.get('version.isOSS')) {
return null;
}
let pathForUrl = url.replace('/v1/', '');
let pathForUrl = parseURL(url).pathname;
pathForUrl = pathForUrl.replace('/v1/', '');
let tokenInfo = this.get('tokenToUnwrap');
if (tokenInfo && tokenInfo.creation_path === pathForUrl) {
let { token, accessor, creation_time } = tokenInfo;
Expand Down
50 changes: 39 additions & 11 deletions ui/tests/acceptance/enterprise-control-groups-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import authForm from 'vault/tests/pages/components/auth-form';
import controlGroup from 'vault/tests/pages/components/control-group';
import controlGroupSuccess from 'vault/tests/pages/components/control-group-success';
import authPage from 'vault/tests/pages/auth';
import logout from 'vault/tests/pages/logout';
import editPage from 'vault/tests/pages/secrets/backend/kv/edit-secret';
import listPage from 'vault/tests/pages/secrets/backend/list';

const consoleComponent = create(consoleClass);
const authFormComponent = create(authForm);
Expand All @@ -23,10 +24,6 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
return authPage.login();
});

hooks.afterEach(function() {
return logout.visit();
});

const POLICY = `
path "kv/foo" {
capabilities = ["create", "read", "update", "delete", "list"]
Expand All @@ -40,6 +37,23 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
}
}
}
path "kv-v2-mount/data/foo" {
capabilities = ["create", "read", "update", "list"]
control_group = {
max_ttl = "24h"
factor "ops_manager" {
identity {
group_names = ["managers"]
approvals = 1
}
}
}
}
path "kv-v2-mount/*" {
capabilities = ["list"]
}
`;

const AUTHORIZER_POLICY = `
Expand All @@ -59,9 +73,10 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
await visit('/vault/secrets');
await consoleComponent.toggle();
await consoleComponent.runCommands([
//enable kv mount and write some data
//enable kv-v1 mount and write a secret
'write sys/mounts/kv type=kv',
'write kv/foo bar=baz',

//enable userpass, create user and associated entity
'write sys/auth/userpass type=userpass',
`write auth/userpass/users/${ADMIN_USER} password=${ADMIN_PASSWORD} policies=default`,
Expand All @@ -72,7 +87,9 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
// read out mount to get the accessor
'read -field=accessor sys/internal/ui/mounts/auth/userpass',
]);

userpassAccessor = consoleComponent.lastTextOutput;

await consoleComponent.runCommands([
// lookup entity id for our authorizer
`write -field=id identity/lookup/entity name=${ADMIN_USER}`,
Expand All @@ -86,14 +103,25 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
'write -field=client_token auth/token/create policies=kv-control-group',
]);
context.userToken = consoleComponent.lastLogOutput;
await logout.visit();

await authPage.login(context.userToken);
return this;
};

test('it redirects you if you try to navigate to a Control Group restricted path', async function(assert) {
const writeSecret = async function(backend, path, key, val) {
await listPage.visitRoot({ backend });
await listPage.create();
await editPage.createSecret(path, key, val);
};

test('for v2 secrets it redirects you if you try to navigate to a Control Group restricted path', async function(assert) {
await consoleComponent.runCommands([
'write sys/mounts/kv-v2-mount type=kv-v2',
'delete kv-v2-mount/metadata/foo',
]);
await writeSecret('kv-v2-mount', 'foo', 'bar', 'baz');
await setupControlGroup(this);
await visit('/vault/secrets/kv/show/foo');
await visit('/vault/secrets/kv-v2-mount/show/foo');
assert.equal(
currentRouteName(),
'vault.cluster.access.control-group-accessor',
Expand All @@ -112,7 +140,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
await visit(url);
accessor = controlGroupComponent.accessor;
controlGroupToken = controlGroupComponent.token;
await logout.visit();
await authPage.logout();

// log in as the admin, navigate to the accessor page,
// and authorize the control group request
Expand All @@ -123,7 +151,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
await visit(`/vault/access/control-groups/${accessor}`);
await controlGroupComponent.authorize();
assert.equal(controlGroupComponent.bannerPrefix, 'Thanks!', 'text display changes');
await logout.visit();
await authPage.logout();

await authPage.login(context.userToken);

Expand Down
1 change: 1 addition & 0 deletions ui/tests/acceptance/redirect-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ module('Acceptance | redirect_to query param functionality', function(hooks) {
redirect_to: url,
wrapped_token: wrappedToken,
});

assert.equal(currentURL(), url, 'authenticates then navigates to the redirect_to url after auth');
});
});
2 changes: 1 addition & 1 deletion ui/tests/pages/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export default create({
submit: clickable('[data-test-auth-submit]'),
tokenInput: fillable('[data-test-token]'),
login: async function(token) {
// make sure we're always logged out and logged back in :w
// make sure we're always logged out and logged back in
await this.logout();
await this.visit({ with: 'token' });
if (token) {
Expand Down

0 comments on commit f8370d6

Please sign in to comment.