Skip to content

Commit

Permalink
Decouple Authorization subsystem from Legacy API. (#52638)
Browse files Browse the repository at this point in the history
  • Loading branch information
azasypkin committed Dec 11, 2019
1 parent 666817e commit 7dd8280
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 37 deletions.
1 change: 0 additions & 1 deletion x-pack/legacy/plugins/security/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ export const security = (kibana) => new kibana.Plugin({
server.plugins.kibana.systemApi
),
cspRules: createCSPRuleString(config.get('csp.rules')),
kibanaIndexName: config.get('kibana.index'),
});

// Legacy xPack Info endpoint returns whatever we return in a callback for `registerLicenseCheckResultsGenerator`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('#atSpace', () => {
const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(
mockActions,
mockClusterClient,
() => application
application
);
const request = httpServerMock.createKibanaRequest();
const checkPrivileges = checkPrivilegesWithRequest(request);
Expand Down Expand Up @@ -291,7 +291,7 @@ describe('#atSpaces', () => {
const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(
mockActions,
mockClusterClient,
() => application
application
);
const request = httpServerMock.createKibanaRequest();
const checkPrivileges = checkPrivilegesWithRequest(request);
Expand Down Expand Up @@ -772,7 +772,7 @@ describe('#globally', () => {
const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(
mockActions,
mockClusterClient,
() => application
application
);
const request = httpServerMock.createKibanaRequest();
const checkPrivileges = checkPrivilegesWithRequest(request);
Expand Down
11 changes: 6 additions & 5 deletions x-pack/plugins/security/server/authorization/check_privileges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface CheckPrivileges {
export function checkPrivilegesWithRequestFactory(
actions: CheckPrivilegesActions,
clusterClient: IClusterClient,
getApplicationName: () => string
applicationName: string
) {
const hasIncompatibleVersion = (
applicationPrivilegesResponse: HasPrivilegesResponseApplication
Expand All @@ -81,23 +81,24 @@ export function checkPrivilegesWithRequestFactory(
: [privilegeOrPrivileges];
const allApplicationPrivileges = uniq([actions.version, actions.login, ...privileges]);

const application = getApplicationName();
const hasPrivilegesResponse = (await clusterClient
.asScoped(request)
.callAsCurrentUser('shield.hasPrivileges', {
body: {
applications: [{ application, resources, privileges: allApplicationPrivileges }],
applications: [
{ application: applicationName, resources, privileges: allApplicationPrivileges },
],
},
})) as HasPrivilegesResponse;

validateEsPrivilegeResponse(
hasPrivilegesResponse,
application,
applicationName,
allApplicationPrivileges,
resources
);

const applicationPrivilegesResponse = hasPrivilegesResponse.application[application];
const applicationPrivilegesResponse = hasPrivilegesResponse.application[applicationName];

if (hasIncompatibleVersion(applicationPrivilegesResponse)) {
throw new Error(
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/security/server/authorization/index.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@ import { Actions } from '.';
import { AuthorizationMode } from './mode';

export const authorizationMock = {
create: ({ version = 'mock-version' }: { version?: string } = {}) => ({
create: ({
version = 'mock-version',
applicationName = 'mock-application',
}: { version?: string; applicationName?: string } = {}) => ({
actions: new Actions(version),
checkPrivilegesWithRequest: jest.fn(),
checkPrivilegesDynamicallyWithRequest: jest.fn(),
checkSavedObjectsPrivilegesWithRequest: jest.fn(),
getApplicationName: jest.fn().mockReturnValue('mock-application'),
applicationName,
mode: { useRbacForRequest: jest.fn() } as jest.Mocked<AuthorizationMode>,
privileges: { get: jest.fn() },
registerPrivilegesWithCluster: jest.fn(),
Expand Down
7 changes: 3 additions & 4 deletions x-pack/plugins/security/server/authorization/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,27 @@ test(`returns exposed services`, () => {
.fn()
.mockReturnValue({ getSpaceId: jest.fn(), namespaceToSpaceId: jest.fn() });
const mockFeaturesService = { getFeatures: () => [] };
const mockGetLegacyAPI = () => ({ kibanaIndexName });
const mockLicense = licenseMock.create();

const authz = setupAuthorization({
http: coreMock.createSetup().http,
clusterClient: mockClusterClient,
license: mockLicense,
loggers: loggingServiceMock.create(),
getLegacyAPI: mockGetLegacyAPI,
kibanaIndexName,
packageVersion: 'some-version',
featuresService: mockFeaturesService,
getSpacesService: mockGetSpacesService,
});

expect(authz.actions.version).toBe('version:some-version');
expect(authz.getApplicationName()).toBe(application);
expect(authz.applicationName).toBe(application);

expect(authz.checkPrivilegesWithRequest).toBe(mockCheckPrivilegesWithRequest);
expect(checkPrivilegesWithRequestFactory).toHaveBeenCalledWith(
authz.actions,
mockClusterClient,
authz.getApplicationName
authz.applicationName
);

expect(authz.checkPrivilegesDynamicallyWithRequest).toBe(
Expand Down
16 changes: 8 additions & 8 deletions x-pack/plugins/security/server/authorization/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
IClusterClient,
} from '../../../../../src/core/server';

import { FeaturesService, LegacyAPI, SpacesService } from '../plugin';
import { FeaturesService, SpacesService } from '../plugin';
import { Actions } from './actions';
import { CheckPrivilegesWithRequest, checkPrivilegesWithRequestFactory } from './check_privileges';
import {
Expand Down Expand Up @@ -43,7 +43,7 @@ interface SetupAuthorizationParams {
license: SecurityLicense;
loggers: LoggerFactory;
featuresService: FeaturesService;
getLegacyAPI(): Pick<LegacyAPI, 'kibanaIndexName'>;
kibanaIndexName: string;
getSpacesService(): SpacesService | undefined;
}

Expand All @@ -52,7 +52,7 @@ export interface Authorization {
checkPrivilegesWithRequest: CheckPrivilegesWithRequest;
checkPrivilegesDynamicallyWithRequest: CheckPrivilegesDynamicallyWithRequest;
checkSavedObjectsPrivilegesWithRequest: CheckSavedObjectsPrivilegesWithRequest;
getApplicationName: () => string;
applicationName: string;
mode: AuthorizationMode;
privileges: PrivilegesService;
disableUnauthorizedCapabilities: (
Expand All @@ -69,23 +69,23 @@ export function setupAuthorization({
license,
loggers,
featuresService,
getLegacyAPI,
kibanaIndexName,
getSpacesService,
}: SetupAuthorizationParams): Authorization {
const actions = new Actions(packageVersion);
const mode = authorizationModeFactory(license);
const getApplicationName = () => `${APPLICATION_PREFIX}${getLegacyAPI().kibanaIndexName}`;
const applicationName = `${APPLICATION_PREFIX}${kibanaIndexName}`;
const checkPrivilegesWithRequest = checkPrivilegesWithRequestFactory(
actions,
clusterClient,
getApplicationName
applicationName
);
const privileges = privilegesFactory(actions, featuresService);
const logger = loggers.get('authorization');

const authz = {
actions,
getApplicationName,
applicationName,
checkPrivilegesWithRequest,
checkPrivilegesDynamicallyWithRequest: checkPrivilegesDynamicallyWithRequestFactory(
checkPrivilegesWithRequest,
Expand Down Expand Up @@ -123,7 +123,7 @@ export function setupAuthorization({
registerPrivilegesWithCluster: async () => {
validateFeaturePrivileges(actions, featuresService.getFeatures());

await registerPrivilegesWithCluster(logger, privileges, getApplicationName(), clusterClient);
await registerPrivilegesWithCluster(logger, privileges, applicationName, clusterClient);
},
};

Expand Down
10 changes: 6 additions & 4 deletions x-pack/plugins/security/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Subscription } from 'rxjs';
import { Subscription, combineLatest } from 'rxjs';
import { first } from 'rxjs/operators';
import {
IClusterClient,
Expand Down Expand Up @@ -43,7 +43,6 @@ export type FeaturesService = Pick<FeaturesSetupContract, 'getFeatures'>;
export interface LegacyAPI {
serverConfig: { protocol: string; hostname: string; port: number };
isSystemAPIRequest: (request: KibanaRequest) => boolean;
kibanaIndexName: string;
cspRules: string;
savedObjects: SavedObjectsLegacyService<KibanaRequest | LegacyRequest>;
auditLogger: {
Expand Down Expand Up @@ -122,7 +121,10 @@ export class Plugin {
core: CoreSetup,
{ features, licensing }: PluginSetupDependencies
): Promise<RecursiveReadonly<PluginSetupContract>> {
const config = await createConfig$(this.initializerContext, core.http.isTlsEnabled)
const [config, legacyConfig] = await combineLatest([
createConfig$(this.initializerContext, core.http.isTlsEnabled),
this.initializerContext.config.legacy.globalConfig$,
])
.pipe(first())
.toPromise();

Expand All @@ -149,7 +151,7 @@ export class Plugin {
clusterClient: this.clusterClient,
license,
loggers: this.initializerContext.logger,
getLegacyAPI: this.getLegacyAPI,
kibanaIndexName: legacyConfig.kibana.index,
packageVersion: this.initializerContext.env.packageInfo.version,
getSpacesService: this.getSpacesService,
featuresService: features,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('GET role', () => {
) => {
test(description, async () => {
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application);
mockRouteDefinitionParams.authz.applicationName = application;

const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function defineGetRolesRoutes({ router, authz, clusterClient }: RouteDefi
body: transformElasticsearchRoleToRole(
elasticsearchRole,
request.params.name,
authz.getApplicationName()
authz.applicationName
),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('GET all roles', () => {
) => {
test(description, async () => {
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application);
mockRouteDefinitionParams.authz.applicationName = application;

const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockRouteDefinitionParams.clusterClient.asScoped.mockReturnValue(mockScopedClusterClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ export function defineGetAllRolesRoutes({ router, authz, clusterClient }: RouteD
return response.ok({
body: Object.entries(elasticsearchRoles)
.map(([roleName, elasticsearchRole]) =>
transformElasticsearchRoleToRole(
elasticsearchRole,
roleName,
authz.getApplicationName()
)
transformElasticsearchRoleToRole(elasticsearchRole, roleName, authz.applicationName)
)
.sort((roleA, roleB) => {
if (roleA.name < roleB.name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const putRoleTest = (
) => {
test(description, async () => {
const mockRouteDefinitionParams = routeDefinitionParamsMock.create();
mockRouteDefinitionParams.authz.getApplicationName.mockReturnValue(application);
mockRouteDefinitionParams.authz.applicationName = application;
mockRouteDefinitionParams.authz.privileges.get.mockReturnValue(privilegeMap);

const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function definePutRolesRoutes({ router, authz, clusterClient }: RouteDefi

const body = transformPutPayloadToElasticsearchRole(
request.body,
authz.getApplicationName(),
authz.applicationName,
rawRoles[name] ? rawRoles[name].applications : []
);

Expand Down

0 comments on commit 7dd8280

Please sign in to comment.