Skip to content

Commit

Permalink
Using ES list privileges API to determine the authorization mode (ela…
Browse files Browse the repository at this point in the history
…stic#24211)

* Making it easier and more terse to specify the user for a test

* Using ES list privileges API to determine the authorization mode

This let's us correct use RBAC authorization for the proper users when
security is enabled, and spaces is disabled to detect whether they have
privileges of any kind and if so use RBAC.

* Fixing authorization service test

* Fixing tests referencing wrong expects

* Putting create test back

* Update x-pack/plugins/security/server/lib/authorization/mode.js

* Update x-pack/plugins/security/server/lib/authorization/mode.js
  • Loading branch information
kobelb committed Oct 18, 2018
1 parent 612bd74 commit 49168a9
Show file tree
Hide file tree
Showing 14 changed files with 259 additions and 286 deletions.
43 changes: 11 additions & 32 deletions x-pack/plugins/security/server/lib/authorization/mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,32 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { GLOBAL_RESOURCE } from '../../../common/constants';
import { spaceApplicationPrivilegesSerializer } from './space_application_privileges_serializer';

const hasAnyPrivileges = privileges => {
return Object.values(privileges).some(hasPrivilege => hasPrivilege === true);
};

const hasAnyResourcePrivileges = resourcePrivileges => {
return Object.values(resourcePrivileges).some(resource => hasAnyPrivileges(resource));
};

export function authorizationModeFactory(
actions,
checkPrivilegesWithRequest,
application,
config,
log,
plugins,
savedObjects,
shieldClient,
xpackInfoFeature,
) {
const useRbacForRequestCache = new WeakMap();

// TODO: This logic will change once we have the ES API to list all privileges
// and is not covered by unit tests currently
const shouldUseRbacForRequest = async (request) => {
if (!config.get('xpack.security.authorization.legacyFallback.enabled')) {
return true;
}

const adminCluster = plugins.elasticsearch.getCluster('admin');
const { callWithInternalUser } = adminCluster;
const { callWithRequest } = shieldClient;

const internalSavedObjectsRepository = savedObjects.getSavedObjectsRepository(
callWithInternalUser
);
const getUserPrivilegesResponse = await callWithRequest(request, 'shield.getUserPrivileges');

const checkPrivileges = checkPrivilegesWithRequest(request);
if (!plugins.spaces) {
const { privileges } = await checkPrivileges.globally(actions.login);
return hasAnyPrivileges(privileges);
}
// Superusers have `*` and all other roles will have the explicit application.
// We aren't using wildcards at this time, so if the user somehow specifies them
// using the ES apis directly (which is documented as unsupported) they won't work here.
const result = getUserPrivilegesResponse.applications
.some(entry => entry.application === '*' || entry.application === application);

const { saved_objects: spaceSavedObjects } = await internalSavedObjectsRepository.find({ type: 'space' });
const spaceResources = spaceSavedObjects.map(space => spaceApplicationPrivilegesSerializer.resource.serialize(space.id));
const allResources = [GLOBAL_RESOURCE, ...spaceResources];
const { resourcePrivileges } = await checkPrivileges.atResources(allResources, actions.login);
return hasAnyResourcePrivileges(resourcePrivileges);
return result;
};

const isRbacEnabled = () => xpackInfoFeature.getLicenseCheckResults().allowRbac;
Expand All @@ -62,7 +41,7 @@ export function authorizationModeFactory(
}

if (!isRbacEnabled()) {
useRbacForRequestCache.set(request, true);
useRbacForRequestCache.set(request, false);
return;
}

Expand Down
127 changes: 122 additions & 5 deletions x-pack/plugins/security/server/lib/authorization/mode.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { authorizationModeFactory } from './mode';

const application = 'kibana-.kibana';

const createMockConfig = (settings) => {
const mockConfig = {
get: jest.fn()
Expand All @@ -30,12 +32,16 @@ const createMockXpackInfoFeature = (allowRbac) => {
};
};

const createMockShieldClient = (getUserPrivilegesResponse) => ({
callWithRequest: jest.fn().mockReturnValue(getUserPrivilegesResponse)
});

describe(`#initialize`, () => {
test(`can't be initialized twice for the same request`, async () => {
const mockConfig = createMockConfig();
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature();
const mode = authorizationModeFactory({}, {}, mockConfig, mockLogger, {}, {}, mockXpackInfoFeature);
const mode = authorizationModeFactory(application, mockConfig, mockLogger, null, mockXpackInfoFeature);
const request = {};

await mode.initialize(request);
Expand All @@ -50,7 +56,7 @@ describe(`#useRbacForRequest`, () => {
const mockConfig = createMockConfig();
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature();
const mode = authorizationModeFactory({}, {}, mockConfig, mockLogger, {}, {}, mockXpackInfoFeature);
const mode = authorizationModeFactory(application, mockConfig, mockLogger, null, mockXpackInfoFeature);
const request = {};

const result = mode.useRbacForRequest(request);
Expand All @@ -63,13 +69,124 @@ describe(`#useRbacForRequest`, () => {
'xpack.security.authorization.legacyFallback.enabled': false,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature();
const mode = authorizationModeFactory({}, {}, mockConfig, mockLogger, {}, {}, mockXpackInfoFeature);
const mockXpackInfoFeature = createMockXpackInfoFeature(true);
const mode = authorizationModeFactory(application, mockConfig, mockLogger, null, mockXpackInfoFeature);
const request = {};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(true);
expect(mockLogger).not.toHaveBeenCalled();
});

test(`returns false if xpackInfoFeature.getLicenseCheckResults().allowRbac is false`, async () => {
const mockConfig = createMockConfig({
'xpack.security.authorization.legacyFallback.enabled': true,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature(false);
const mode = authorizationModeFactory(application, mockConfig, mockLogger, null, mockXpackInfoFeature);
const request = {};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(false);
});

test(`returns false if shieldClient getUserPrivileges returns no applications`, async () => {
const mockConfig = createMockConfig({
'xpack.security.authorization.legacyFallback.enabled': true,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature(true);
const mockShieldClient = createMockShieldClient({
applications: []
});
const mode = authorizationModeFactory(application, mockConfig, mockLogger, mockShieldClient, mockXpackInfoFeature);
const request = {
headers: {
foo: 'bar'
}
};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(false);
expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.getUserPrivileges');
});

test(`returns false if shieldClient getUserPrivileges returns incorrect application`, async () => {
const mockConfig = createMockConfig({
'xpack.security.authorization.legacyFallback.enabled': true,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature(true);
const mockShieldClient = createMockShieldClient({
applications: [{
application: 'kibana-.kibana-marketing'
}]
});
const mode = authorizationModeFactory(application, mockConfig, mockLogger, mockShieldClient, mockXpackInfoFeature);
const request = {
headers: {
foo: 'bar'
}
};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(false);
expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.getUserPrivileges');
});

test(`returns true if shieldClient getUserPrivileges returns * and incorrect application`, async () => {
const mockConfig = createMockConfig({
'xpack.security.authorization.legacyFallback.enabled': true,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature(true);
const mockShieldClient = createMockShieldClient({
applications: [{
application: 'kibana-.kibana-marketing'
}, {
application: '*'
}]
});
const mode = authorizationModeFactory(application, mockConfig, mockLogger, mockShieldClient, mockXpackInfoFeature);
const request = {
headers: {
foo: 'bar'
}
};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(true);
expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.getUserPrivileges');
});

test(`returns true if shieldClient getUserPrivileges returns matching application and incorrect application`, async () => {
const mockConfig = createMockConfig({
'xpack.security.authorization.legacyFallback.enabled': true,
});
const mockLogger = createMockLogger();
const mockXpackInfoFeature = createMockXpackInfoFeature(true);
const mockShieldClient = createMockShieldClient({
applications: [{
application: 'kibana-.kibana-marketing'
}, {
application
}]
});
const mode = authorizationModeFactory(application, mockConfig, mockLogger, mockShieldClient, mockXpackInfoFeature);
const request = {
headers: {
foo: 'bar'
}
};

await mode.initialize(request);
const result = mode.useRbacForRequest(request);
expect(result).toBe(true);
expect(mockShieldClient.callWithRequest).toHaveBeenCalledWith(request, 'shield.getUserPrivileges');
});
});
8 changes: 3 additions & 5 deletions x-pack/plugins/security/server/lib/authorization/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ export function createAuthorizationService(server, xpackInfoFeature) {
const application = `kibana-${config.get('kibana.index')}`;
const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(actions, application, shieldClient);
const mode = authorizationModeFactory(
actions,
checkPrivilegesWithRequest,
application,
config,
(...args) => server.log(...args),
server.plugins,
server.savedObjects,
xpackInfoFeature
shieldClient,
xpackInfoFeature,
);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,10 @@ test(`calls server.expose with exposed services`, () => {
expect(actionsFactory).toHaveBeenCalledWith(mockConfig);
expect(checkPrivilegesWithRequestFactory).toHaveBeenCalledWith(mockActions, application, mockShieldClient);
expect(authorizationModeFactory).toHaveBeenCalledWith(
mockActions,
mockCheckPrivilegesWithRequest,
application,
mockConfig,
expect.any(Function),
mockServer.plugins,
mockServer.savedObjects,
mockShieldClient,
mockXpackInfoFeature,
);
});
13 changes: 13 additions & 0 deletions x-pack/server/lib/esjs_shield_plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,19 @@
method: 'PUT'
});

/**
* Perform a [shield.getUserPrivileges](Retrieve a user's list of privileges) request
*
*/
shield.getUserPrivileges = ca({
params: {},
urls: [
{
fmt: '/_xpack/security/user/_privileges'
}
]
});

/**
* Asks Elasticsearch to prepare SAML authentication request to be sent to
* the 3rd-party SAML identity provider.
Expand Down
20 changes: 13 additions & 7 deletions x-pack/test/saved_object_api_integration/common/suites/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,22 @@ export function getTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>)
});
};

const createExpectSpaceAwareNotFound = (spaceId = DEFAULT_SPACE_ID) => {
return createExpectNotFound(spaceAwareId, spaceId);
};

const createExpectSpaceAwareRbacForbidden = () => (resp: { [key: string]: any }) => {
const createExpectRbacForbidden = (type: string) => (resp: { [key: string]: any }) => {
expect(resp.body).to.eql({
error: 'Forbidden',
message: `Unable to get visualization, missing action:saved_objects/visualization/get`,
message: `Unable to get ${type}, missing action:saved_objects/${type}/get`,
statusCode: 403,
});
};

const createExpectSpaceAwareNotFound = (spaceId = DEFAULT_SPACE_ID) => {
return createExpectNotFound(spaceAwareId, spaceId);
};

const expectSpaceAwareRbacForbidden = createExpectRbacForbidden('visualization');
const expectNotSpaceAwareRbacForbidden = createExpectRbacForbidden('globaltype');
const expectDoesntExistRbacForbidden = createExpectRbacForbidden('visualization');

const createExpectSpaceAwareResults = (spaceId = DEFAULT_SPACE_ID) => (resp: {
[key: string]: any;
}) => {
Expand Down Expand Up @@ -174,8 +178,10 @@ export function getTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>)
createExpectNotSpaceAwareRbacForbidden,
createExpectNotSpaceAwareResults,
createExpectSpaceAwareNotFound,
createExpectSpaceAwareRbacForbidden,
createExpectSpaceAwareResults,
expectSpaceAwareRbacForbidden,
expectNotSpaceAwareRbacForbidden,
expectDoesntExistRbacForbidden,
getTest,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ export default function({ getService }: TestInvoker) {
const {
createExpectDoesntExistNotFound,
createExpectLegacyForbidden,
createExpectSpaceAwareRbacForbidden,
createExpectSpaceAwareResults,
createExpectNotSpaceAwareResults,
createExpectNotSpaceAwareRbacForbidden,
expectSpaceAwareRbacForbidden,
expectNotSpaceAwareRbacForbidden,
expectDoesntExistRbacForbidden,
getTest,
} = getTestSuiteFactory(esArchiver, supertest);

Expand Down Expand Up @@ -255,15 +256,15 @@ export default function({ getService }: TestInvoker) {
tests: {
spaceAware: {
statusCode: 403,
response: createExpectSpaceAwareRbacForbidden(),
response: expectSpaceAwareRbacForbidden,
},
notSpaceAware: {
statusCode: 403,
response: createExpectNotSpaceAwareRbacForbidden(),
response: expectNotSpaceAwareRbacForbidden,
},
doesntExist: {
statusCode: 403,
response: createExpectSpaceAwareRbacForbidden(),
response: expectDoesntExistRbacForbidden,
},
},
});
Expand Down
Loading

0 comments on commit 49168a9

Please sign in to comment.