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

Beginning to use the ES APIs to insert/check privileges #18645

Merged
merged 29 commits into from
May 16, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d243802
Beginning to use the ES APIs to insert/check privileges
kobelb Apr 27, 2018
d9c93b7
Removing todo comment, I think we're good with the current check
kobelb Apr 27, 2018
5aaf5bc
Adding ability to edit kibana application privileges
kobelb Apr 30, 2018
05c594f
Introducing DEFAULT_RESOURCE constant
kobelb Apr 30, 2018
4d60ca1
Removing unused arguments when performing saved objects auth check
kobelb Apr 30, 2018
03c429a
Performing bulkCreate auth more efficiently
kobelb May 8, 2018
1d2b564
Throwing error in SavedObjectClient.find if type isn't provided
kobelb May 8, 2018
a9fda18
Fixing Reporting and removing errant console.log
kobelb May 8, 2018
80494b8
Introducing a separate hasPrivileges "service"
kobelb May 9, 2018
6567941
Adding tests and fleshing out the has privileges "service"
kobelb May 11, 2018
d93b758
Fixing error message
kobelb May 14, 2018
442ee73
You can now edit whatever roles you want
kobelb May 14, 2018
3019006
We're gonna throw the find error in another PR
kobelb May 14, 2018
5a16dea
Changing conflicting version detection to work when user has no
kobelb May 14, 2018
92ab282
Throwing correct error when user is forbidden
kobelb May 14, 2018
bb87576
Removing unused interceptor
kobelb May 14, 2018
6f83da2
Adding warning if they're editing a role with application privileges we
kobelb May 14, 2018
1ea7cb9
Fixing filter...
kobelb May 14, 2018
dddccb6
Beginning to only update privileges when they need to be
kobelb May 15, 2018
f0f705e
More tests
kobelb May 15, 2018
98c9d8c
One more test...
kobelb May 15, 2018
19dd2bc
Restricting the rbac application name that can be chosen
kobelb May 15, 2018
c35e759
Removing DEFAULT_RESOURCE check
kobelb May 15, 2018
4385765
Supporting 1024 characters for the role name
kobelb May 15, 2018
e621440
Renaming some variables, fixing issue with role w/ no kibana privileges
kobelb May 15, 2018
4096be9
Throwing decorated general error when appropriate
kobelb May 15, 2018
22a22ac
Fixing test description
kobelb May 16, 2018
9d73c4b
Dedent does nothing...
kobelb May 16, 2018
ab2d9ff
Renaming some functions
kobelb May 16, 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
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { getClient } from '../../../../../server/lib/get_client_shield';


const createRoleIfDoesntExist = async (callCluster, name) => {
const createRoleIfDoesntExist = async (callCluster, { name, application, privilege }) => {
try {
await callCluster('shield.getRole', { name });
} catch (err) {
Expand All @@ -23,7 +23,13 @@ const createRoleIfDoesntExist = async (callCluster, name) => {
body: {
cluster: [],
index: [],
// application: [ { "privileges": [ "kibana:all" ], "resources": [ "*" ] } ]
applications: [
{
application,
privileges: [ privilege ],
resources: [ 'default' ]
Copy link
Member

Choose a reason for hiding this comment

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

Do you have any thoughts yet on how we'd make this space aware? We don't need a solution for phase 1, but what do you think about parameterizing or abstracting this piece?

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've been delaying that for the moment, and have 'default' hard-coded a few places. We can introduce a constant that we use for this at the moment, a bunch of stuff is going to likely change once we become space aware but we'll likely still need the constant around because we'll be treating the default space "special" when it comes to routing and behaving without spaces.

}
]
}
});
}
Expand All @@ -40,8 +46,17 @@ export async function createDefaultRoles(server) {

const callCluster = getClient(server).callWithInternalUser;

const createKibanaUserRole = createRoleIfDoesntExist(callCluster, `${application}_rbac_user`);
const createKibanaDashboardOnlyRole = createRoleIfDoesntExist(callCluster, `${application}_rbac_dashboard_only_user`);
const createKibanaUserRole = createRoleIfDoesntExist(callCluster, {
name: `${application}_rbac_user`,
application,
privilege: 'all'
});

const createKibanaDashboardOnlyRole = createRoleIfDoesntExist(callCluster, {
name: `${application}_rbac_dashboard_only_user`,
application,
privilege: 'read'
});

await Promise.all([createKibanaUserRole, createKibanaDashboardOnlyRole]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import { buildPrivilegeMap } from './privileges';
import { getClient } from '../../../../../server/lib/get_client_shield';

export async function registerPrivilegesWithCluster(server) {
return;

const config = server.config();
const kibanaVersion = config.get('pkg.version');
const application = config.get('xpack.security.rbac.application');
Expand All @@ -23,7 +21,6 @@ export async function registerPrivilegesWithCluster(server) {

const callCluster = getClient(server).callWithInternalUser;

// TODO(legrego) - non-working stub
await callCluster('shield.postPrivileges', {
body: {
[application]: privilegeActionMapping
Expand Down
48 changes: 16 additions & 32 deletions x-pack/plugins/security/server/lib/privileges/privileges.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,33 @@
* Licensed under the Elastic License; you may not use this file except in compliance with the Elastic License. */

export function buildPrivilegeMap(application, kibanaVersion) {
const readSavedObjectsPrivileges = buildSavedObjectsReadPrivileges('');
const readSavedObjectsPrivileges = buildSavedObjectsReadPrivileges();

const commonMetadata = {
kibanaVersion
};

function getActionName(...nameParts) {
return nameParts.join('/');
}
const privilegeActions = {};

const privilegeActionMap = [];

privilegeActionMap.push({
privilegeActions.all = {
application,
name: 'all',
actions: [getActionName('*')],
metadata: {
...commonMetadata,
displayName: 'all'
}
});

privilegeActionMap.push({
actions: [`version:${kibanaVersion}`, 'action:*'],
};

privilegeActions.read = {
application,
name: 'read',
actions: [...readSavedObjectsPrivileges],
metadata: {
...commonMetadata,
displayName: 'read'
}
});

return privilegeActionMap;
actions: [`version:${kibanaVersion}`, ...readSavedObjectsPrivileges],
};

return privilegeActions;
}

function buildSavedObjectsReadPrivileges(prefix) {
function buildSavedObjectsReadPrivileges() {
const readActions = ['get', 'mget', 'search'];
return buildSavedObjectsPrivileges(prefix, readActions);
return buildSavedObjectsPrivileges(readActions);
}

function buildSavedObjectsPrivileges(prefix, actions) {
const objectTypes = ['dashboard', 'visualization', 'search'];
function buildSavedObjectsPrivileges(actions) {
const objectTypes = ['config', 'dashboard', 'index-pattern', 'search', 'visualization'];
return objectTypes
.map(type => actions.map(action => `${prefix}/${type}/${action}`))
.map(type => actions.map(action => `action:saved-objects/${type}/${action}`))
.reduce((acc, types) => [...acc, ...types], []);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ export function secureSavedObjectsClientOptionsBuilder(server, options) {
const adminCluster = server.plugins.elasticsearch.getCluster('admin');
const { callWithInternalUser } = adminCluster;

const config = server.config();

return {
...options,
application: server.config().get('xpack.security.rbac.application'),
application: config.get('xpack.security.rbac.application'),
kibanaVersion: config.get('pkg.version'),
callCluster: callWithInternalUser
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ export class SecureSavedObjectsClient {
request,
baseClient,
application,
kibanaVersion,
} = options;

this.errors = baseClient.errors;

this._client = baseClient;
this._application = application;
this._kibanaVersion = kibanaVersion;
this._callCluster = getClient(server).callWithRequest;
this._request = request;
}
Expand All @@ -48,8 +50,7 @@ export class SecureSavedObjectsClient {
}

async find(options = {}) {
// TODO(legrego) - need to constrain which types users can search for...
await this._performAuthorizationCheck(null, 'search', null, options);
await this._performAuthorizationCheck(options.type, 'search', null, options);
Copy link
Member

Choose a reason for hiding this comment

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

Is options.type a required search property? If not, what happens within this._performAuthorizationCheck when a type isn't provided?

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 still need to track down all the usages, the basic features that I've tested provide the options.type, and if they don't it's not going to authorize them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you come across any where we aren't providing this?


return await this._client.find(options);
}
Expand All @@ -75,22 +76,21 @@ export class SecureSavedObjectsClient {
}

async _performAuthorizationCheck(type, action, attributes = {}, options = {}) { // eslint-disable-line no-unused-vars
Copy link
Member

Choose a reason for hiding this comment

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

We might be able to remove attributes and options now. It doesn't look like we'll need them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, I'll remove it for the time being and we can add it back if we need to.

return;
// TODO(legrego) use ES Custom Privilege API once implemented.
const kibanaAction = `saved-objects/${type}/${action}`;
const version = `version:${this._kibanaVersion}`;
const kibanaAction = `action:saved-objects/${type}/${action}`;

const privilegeCheck = await this._callCluster(this._request, 'shield.hasPrivileges', {
body: {
applications: [{
application: this._application,
resources: ['default'],
privileges: [kibanaAction]
privileges: [version, kibanaAction]
}]
}
});

if (!privilegeCheck.has_all_requested) {
throw Boom.unauthorized(`User ${privilegeCheck.username} is not authorized to ${action} objects of type ${type}`);
throw Boom.forbidden(`User ${privilegeCheck.username} is not authorized to ${action} objects of type ${type}`);
}
}
}
4 changes: 2 additions & 2 deletions x-pack/plugins/security/server/routes/api/v1/privileges.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export function initPrivilegesApi(server) {
method: 'GET',
path: '/api/security/v1/privileges',
handler(request, reply) {

reply(buildPrivilegeMap(application, kibanaVersion));
const privileges = buildPrivilegeMap(application, kibanaVersion);
reply(Object.values(privileges));
}
});
}