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

change capabilities logic for sudo prefixes #5647

Merged
merged 6 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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/models/capabilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const computedCapability = function(capability) {
return false;
}
// if the path is sudo protected, they'll need sudo + the appropriate capability
if (SUDO_PATHS.includes(path) || SUDO_PATH_PREFIXES.find(item => item.startsWith(path))) {
if (SUDO_PATHS.includes(path) || SUDO_PATH_PREFIXES.find(item => path.startsWith(item))) {
return capabilities.includes('sudo') && capabilities.includes(capability);
}
return capabilities.includes(capability);
Expand Down
42 changes: 42 additions & 0 deletions ui/tests/unit/models/capabilities-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,46 @@ module('Unit | Model | capabilities', function(hooks) {
assert.notOk(model.get('canDelete'));
assert.notOk(model.get('canList'));
});

test('it does not require sudo on sys/leases/revoke if update capability is present and path is not fully a sudo prefix', function(assert) {
let model = run(() =>
this.owner.lookup('service:store').createRecord('capabilities', {
path: 'sys/leases/revoke',
capabilities: ['update', 'read'],
})
);
assert.ok(model.get('canRead'));
assert.notOk(model.get('canCreate'), 'sudo requires the capability to be set as well');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this doesn't have to do with sudo, just that create wasn't in the capabilities list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's in every test so I just left it

assert.ok(model.get('canUpdate'), 'should not require sudo if it has update');
assert.notOk(model.get('canDelete'));
assert.notOk(model.get('canList'));
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add another test for the paths in the SUDO_PREFIX arg too to make sure they are requiring sudo?

So for path: 'sys/leases/revoke-prefix/aws' with capabilities "update", canUpdate should be false, but with "update", "sudo", canUpdate should be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a couple. Do they look right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yassss - perfect!


test('it requires sudo on prefix path even if capability is present', function(assert) {
let model = run(() =>
this.owner.lookup('service:store').createRecord('capabilities', {
path: SUDO_PATH_PREFIXES[0] + '/aws',
capabilities: ['update', 'read'],
})
);
assert.notOk(model.get('canRead'));
assert.notOk(model.get('canCreate'));
assert.notOk(model.get('canUpdate'), 'should still require sudo');
assert.notOk(model.get('canDelete'));
assert.notOk(model.get('canList'));
});

test('it does not require sudo on prefix path if both update and sudo capabilities are present', function(assert) {
let model = run(() =>
this.owner.lookup('service:store').createRecord('capabilities', {
path: SUDO_PATH_PREFIXES[0] + '/aws',
capabilities: ['sudo', 'update', 'read'],
})
);
assert.ok(model.get('canRead'));
assert.notOk(model.get('canCreate'));
assert.ok(model.get('canUpdate'), 'should not require sudo');
assert.notOk(model.get('canDelete'));
assert.notOk(model.get('canList'));
});
});