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

Public Role APIs #20732

Merged
merged 28 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c2474b2
Beginning to work on external role management APIs
kobelb Jul 9, 2018
50e4fc1
Refactoring GET tests and adding more permutations
kobelb Jul 10, 2018
366df0f
Adding test for excluding other resources
kobelb Jul 10, 2018
a03ac1b
Adding get role tests
kobelb Jul 10, 2018
55f59b3
Splitting out the endpoints, or else it's gonna get overwhelming
kobelb Jul 10, 2018
25ddb7b
Splitting out the post and delete actions
kobelb Jul 10, 2018
35a7def
Beginning to work on POST and the tests
kobelb Jul 11, 2018
c9a64c6
Posting the updated role
kobelb Jul 11, 2018
70862b2
Adding update tests
kobelb Jul 12, 2018
da8d5db
Modifying the UI to use the new public APIs
kobelb Jul 12, 2018
0abe3b6
Removing internal roles API
kobelb Jul 12, 2018
7c22748
Moving the rbac api integration setup tests to use the public role apis
kobelb Jul 12, 2018
e9f8a73
Testing field_security and query
kobelb Jul 12, 2018
3b1ddc3
Adding create role tests
kobelb Jul 12, 2018
cff2b89
We can't update the transient_metadata...
kobelb Jul 12, 2018
5ee5d23
Removing debugger
kobelb Jul 12, 2018
2072348
Update and delete tests
kobelb Jul 12, 2018
3e9e8bd
Returning a 204 when POSTing a Role.
kobelb Jul 12, 2018
fa03390
Switching POST to PUT and roles to role
kobelb Jul 12, 2018
2566043
We don't need the rbacApplication client-side anymore
kobelb Jul 12, 2018
77ab30b
Adding delete route tests
kobelb Jul 12, 2018
694ed25
Using not found instead of not acceptable, as that's more likely
kobelb Jul 12, 2018
4096ae0
Only allowing us to PUT known Kibana privileges
kobelb Jul 13, 2018
941cf2b
Removing transient_metadata
kobelb Jul 13, 2018
7bcaac7
Removing one letter variable names
kobelb Jul 13, 2018
f00cb3e
Merge remote-tracking branch 'upstream/security-app-privs' into exter…
kobelb Jul 13, 2018
b599fda
Using PUT instead of POST when saving roles
kobelb Jul 13, 2018
8b7cb3b
Fixing broken tests
kobelb Jul 13, 2018
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
6 changes: 2 additions & 4 deletions x-pack/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { resolve } from 'path';
import { getUserProvider } from './server/lib/get_user';
import { initAuthenticateApi } from './server/routes/api/v1/authenticate';
import { initUsersApi } from './server/routes/api/v1/users';
import { initRolesApi } from './server/routes/api/v1/roles';
import { initPublicRolesApi } from './server/routes/api/public/roles';
import { initIndicesApi } from './server/routes/api/v1/indices';
import { initLoginView } from './server/routes/views/login';
import { initLogoutView } from './server/routes/views/logout';
Expand Down Expand Up @@ -76,11 +76,9 @@ export const security = (kibana) => new kibana.Plugin({
injectDefaultVars: function (server) {
const config = server.config();

const { authorization } = server.plugins.security;
return {
secureCookies: config.get('xpack.security.secureCookies'),
sessionTimeout: config.get('xpack.security.sessionTimeout'),
rbacApplication: authorization.application,
};
}
},
Expand Down Expand Up @@ -152,7 +150,7 @@ export const security = (kibana) => new kibana.Plugin({
await initAuthenticator(server);
initAuthenticateApi(server);
initUsersApi(server);
initRolesApi(server);
initPublicRolesApi(server);
initIndicesApi(server);
initPrivilegesApi(server);
initLoginView(server, xpackMainPlugin);
Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/security/public/services/shield_role.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@
*/

import 'angular-resource';
import { omit } from 'lodash';
import angular from 'angular';
import { uiModules } from 'ui/modules';

const module = uiModules.get('security', ['ngResource']);
module.service('ShieldRole', ($resource, chrome) => {
return $resource(chrome.addBasePath('/api/security/v1/roles/:name'), {
return $resource(chrome.addBasePath('/api/security/role/:name'), {
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a precedent for how to version our public APIs yet? I'm concerned that not including a version number will prevent us from iterating in minor versions. Or at the other side of the spectrum, these public APIs could inadvertently become fluid like our plugin API, where users have to tie to a specific minor/patch release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want our public APIs to be versioned with our "product version" so that when we do a major release, we are able to make breaking changes to the APIs. If we had to do a breaking change in a minor, we'd have to put it in the release notes and have a very good reason for doing so. We don't want "multiple versions" for stuff like this.

name: '@name'
}, {
save: {
method: 'POST',
transformRequest(data) {
return angular.toJson(omit(data, 'name', 'transient_metadata', '_unrecognized_applications'));
}
}
});
});
14 changes: 7 additions & 7 deletions x-pack/plugins/security/public/views/management/edit_role.html
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ <h1 class="kuiTitle">
<input
class="kuiCheckBox"
type="checkbox"
ng-checked="includes(role.cluster, privilege)"
ng-click="toggle(role.cluster, privilege)"
ng-checked="includes(role.elasticsearch.cluster, privilege)"
ng-click="toggle(role.elasticsearch.cluster, privilege)"
ng-disabled="role.metadata._reserved || !isRoleEnabled(role)"
/>
<span class="kuiOptionLabel">{{privilege}}</span>
Expand All @@ -116,12 +116,12 @@ <h1 class="kuiTitle">
Kibana Privileges
</label>

<div ng-repeat="(key, value) in kibanaPrivileges">
<div ng-repeat="(key, value) in kibanaPrivilegesViewModel">
<label>
<input
class="kuiCheckBox"
type="checkbox"
ng-model="kibanaPrivileges[key]"
ng-model="kibanaPrivilegesViewModel[key]"
ng-disabled="role.metadata._reserved || !isRoleEnabled(role)"
/>
<span class="kuiOptionLabel">{{key}}</span>
Expand All @@ -136,7 +136,7 @@ <h1 class="kuiTitle">
</label>
<ui-select
multiple
ng-model="role.run_as"
ng-model="role.elasticsearch.run_as"
ng-disabled="role.metadata._reserved || !isRoleEnabled(role)"
>
<ui-select-match placeholder="Add a user...">
Expand All @@ -152,7 +152,7 @@ <h1 class="kuiTitle">
<div class="kuiFormSection">
<kbn-index-privileges-form
is-new-role="editRole.isNewRole"
indices="role.indices"
indices="role.elasticsearch.indices"
index-patterns="indexPatterns"
privileges="privileges"
field-options="editRole.fieldOptions"
Expand All @@ -173,7 +173,7 @@ <h1 class="kuiTitle">
class="kuiButton kuiButton--primary"
ng-click="saveRole(role)"
ng-if="!role.metadata._reserved && isRoleEnabled(role)"
ng-disabled="form.$invalid || !areIndicesValid(role.indices)"
ng-disabled="form.$invalid || !areIndicesValid(role.elasticsearch.indices)"
data-test-subj="roleFormSaveButton"
>
Save
Expand Down
76 changes: 29 additions & 47 deletions x-pack/plugins/security/public/views/management/edit_role.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import _ from 'lodash';
import chrome from 'ui/chrome';
import routes from 'ui/routes';
import { fatalError, toastNotifications } from 'ui/notify';
import { toggle } from 'plugins/security/lib/util';
Expand All @@ -22,60 +21,41 @@ import { IndexPatternsProvider } from 'ui/index_patterns/index_patterns';
import { XPackInfoProvider } from 'plugins/xpack_main/services/xpack_info';
import { checkLicenseError } from 'plugins/security/lib/check_license_error';
import { EDIT_ROLES_PATH, ROLES_PATH } from './management_urls';
import { ALL_RESOURCE } from '../../../common/constants';

const getKibanaPrivileges = (applicationPrivileges, roleApplications, application) => {
const kibanaPrivileges = applicationPrivileges.reduce((acc, p) => {
const getKibanaPrivilegesViewModel = (applicationPrivileges, roleKibanaPrivileges) => {
const viewModel = applicationPrivileges.reduce((acc, p) => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: avoid one letter variables

acc[p.name] = false;
return acc;
}, {});

if (!roleApplications || roleApplications.length === 0) {
return kibanaPrivileges;
if (!roleKibanaPrivileges || roleKibanaPrivileges.length === 0) {
return viewModel;
}

// we're filtering out privileges for non-all resources incase the roles were created in a future version
const applications = roleApplications
.filter(roleApplication => roleApplication.application === application)
.filter(roleApplication => !roleApplication.resources.some(resource => resource !== ALL_RESOURCE));

const assigned = _.uniq(_.flatten(_.pluck(applications, 'privileges')));
const assigned = _.uniq(_.flatten(_.pluck(roleKibanaPrivileges, 'privileges')));
assigned.forEach(a => {
// we don't want to display privileges that aren't in our expected list of privileges
if (a in kibanaPrivileges) {
kibanaPrivileges[a] = true;
if (a in viewModel) {
viewModel[a] = true;
}
});

return kibanaPrivileges;
return viewModel;
};

const getRoleApplications = (kibanaPrivileges, currentRoleApplications = [], application) => {
// we keep any other applications
const newRoleApplications = currentRoleApplications.filter(roleApplication => {
return roleApplication.application !== application;
});

const selectedPrivileges = Object.keys(kibanaPrivileges).filter(key => kibanaPrivileges[key]);
const getKibanaPrivileges = (kibanaPrivilegesViewModel) => {
const selectedPrivileges = Object.keys(kibanaPrivilegesViewModel).filter(key => kibanaPrivilegesViewModel[key]);

// if we have any selected privileges, add a single application entry
if (selectedPrivileges.length > 0) {
newRoleApplications.push({
application,
privileges: selectedPrivileges,
resources: [ALL_RESOURCE]
});
}

return newRoleApplications;
};

const getOtherApplications = (roleApplications, application) => {
if (!roleApplications || roleApplications.length === 0) {
return [];
return [
{
privileges: selectedPrivileges
}
];
}

return roleApplications.map(roleApplication => roleApplication.application).filter(app =>app !== application);
return [];
};

routes.when(`${EDIT_ROLES_PATH}/:name?`, {
Expand All @@ -98,10 +78,13 @@ routes.when(`${EDIT_ROLES_PATH}/:name?`, {
});
}
return new ShieldRole({
cluster: [],
indices: [],
run_as: [],
applications: []
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [],
_unrecognized_applications: []
});
},
applicationPrivileges(ApplicationPrivileges, kbnUrl, Promise, Private) {
Expand All @@ -128,7 +111,6 @@ routes.when(`${EDIT_ROLES_PATH}/:name?`, {
const Private = $injector.get('Private');
const confirmModal = $injector.get('confirmModal');
const shieldIndices = $injector.get('shieldIndices');
const rbacApplication = chrome.getInjected('rbacApplication');

$scope.role = $route.current.locals.role;
$scope.users = $route.current.locals.users;
Expand All @@ -137,8 +119,8 @@ routes.when(`${EDIT_ROLES_PATH}/:name?`, {

const applicationPrivileges = $route.current.locals.applicationPrivileges;
const role = $route.current.locals.role;
$scope.kibanaPrivileges = getKibanaPrivileges(applicationPrivileges, role.applications, rbacApplication);
$scope.otherApplications = getOtherApplications(role.applications, rbacApplication);
$scope.kibanaPrivilegesViewModel = getKibanaPrivilegesViewModel(applicationPrivileges, role.kibana);
$scope.otherApplications = role._unrecognized_applications;

$scope.rolesHref = `#${ROLES_PATH}`;

Expand All @@ -162,10 +144,10 @@ routes.when(`${EDIT_ROLES_PATH}/:name?`, {
};

$scope.saveRole = (role) => {
role.indices = role.indices.filter((index) => index.names.length);
role.indices.forEach((index) => index.query || delete index.query);
role.elasticsearch.indices = role.elasticsearch.indices.filter((index) => index.names.length);
role.elasticsearch.indices.forEach((index) => index.query || delete index.query);

role.applications = getRoleApplications($scope.kibanaPrivileges, role.applications, rbacApplication);
role.kibana = getKibanaPrivileges($scope.kibanaPrivilegesViewModel);

return role.$save()
.then(() => toastNotifications.addSuccess('Updated role'))
Expand Down Expand Up @@ -203,7 +185,7 @@ routes.when(`${EDIT_ROLES_PATH}/:name?`, {
$scope.allowDocumentLevelSecurity = xpackInfo.get('features.security.allowRoleDocumentLevelSecurity');
$scope.allowFieldLevelSecurity = xpackInfo.get('features.security.allowRoleFieldLevelSecurity');

$scope.$watch('role.indices', (indices) => {
$scope.$watch('role.elasticsearch.indices', (indices) => {
if (!indices.length) $scope.addIndex(indices);
else indices.forEach($scope.fetchFieldOptions);
}, true);
Expand Down
33 changes: 33 additions & 0 deletions x-pack/plugins/security/server/routes/api/public/roles/delete.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import _ from 'lodash';
import Joi from 'joi';
import { wrapError } from '../../../../lib/errors';

export function initDeleteRolesApi(server, callWithRequest, routePreCheckLicenseFn) {
server.route({
method: 'DELETE',
path: '/api/security/role/{name}',
handler(request, reply) {
const name = request.params.name;
Copy link
Member

Choose a reason for hiding this comment

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

Edit - I'm withdrawing this comment, but leaving it here for posterity.

Since this is a public API, I feel like we may want to have more robust error handling.

In my testing, executing DELETE /api/security/roles/this-does-not-exist correctly returns a 404 from Elasticsearch:

{"statusCode":404,"error":"Not Found","message":"Not Found"}

But from the Kibana API perspective, it's ambiguous. Is this saying that the API endpoint wasn't found, or that the role wasn't found?

return callWithRequest(request, 'shield.deleteRole', { name }).then(
() => reply().code(204),
_.flow(wrapError, reply));
},
config: {
validate: {
params: Joi.object()
.keys({
name: Joi.string()
.required(),
})
.required(),
},
pre: [routePreCheckLicenseFn]
}
});
}
Loading