Skip to content

Commit

Permalink
Allow access acknowledgement only if license is Gold+.
Browse files Browse the repository at this point in the history
  • Loading branch information
azasypkin committed Apr 28, 2020
1 parent be9a7f1 commit de10976
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 28 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/security/common/licensing/license_features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export interface SecurityLicenseFeatures {
*/
readonly showRoleMappingsManagement: boolean;

/**
* Indicates whether we allow users to access agreement UI and acknowledge it.
*/
readonly allowAccessAgreement: boolean;

/**
* Indicates whether we allow users to define document level security in roles.
*/
Expand Down
14 changes: 11 additions & 3 deletions x-pack/plugins/security/common/licensing/license_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('license features', function() {
allowLogin: false,
showLinks: false,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
layout: 'error-es-unavailable',
Expand All @@ -37,6 +38,7 @@ describe('license features', function() {
allowLogin: false,
showLinks: false,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
layout: 'error-xpack-unavailable',
Expand All @@ -60,6 +62,7 @@ describe('license features', function() {
expect(subscriptionHandler.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"allowAccessAgreement": false,
"allowLogin": false,
"allowRbac": false,
"allowRoleDocumentLevelSecurity": false,
Expand All @@ -78,6 +81,7 @@ describe('license features', function() {
expect(subscriptionHandler.mock.calls[1]).toMatchInlineSnapshot(`
Array [
Object {
"allowAccessAgreement": true,
"allowLogin": true,
"allowRbac": true,
"allowRoleDocumentLevelSecurity": true,
Expand All @@ -94,7 +98,7 @@ describe('license features', function() {
}
});

it('should show login page and other security elements, allow RBAC but forbid role mappings, DLS, and sub-feature privileges if license is basic.', () => {
it('should show login page and other security elements, allow RBAC but forbid role mappings, access agreement, DLS, and sub-feature privileges if license is basic.', () => {
const mockRawLicense = licensingMock.createLicense({
features: { security: { isEnabled: true, isAvailable: true } },
});
Expand All @@ -109,6 +113,7 @@ describe('license features', function() {
allowLogin: true,
showLinks: true,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
allowRbac: true,
Expand All @@ -131,14 +136,15 @@ describe('license features', function() {
allowLogin: false,
showLinks: false,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
allowRbac: false,
allowSubFeaturePrivileges: false,
});
});

it('should allow role mappings and sub-feature privileges, but not DLS/FLS if license = gold', () => {
it('should allow role mappings, access agreement and sub-feature privileges, but not DLS/FLS if license = gold', () => {
const mockRawLicense = licensingMock.createLicense({
license: { mode: 'gold', type: 'gold' },
features: { security: { isEnabled: true, isAvailable: true } },
Expand All @@ -152,14 +158,15 @@ describe('license features', function() {
allowLogin: true,
showLinks: true,
showRoleMappingsManagement: true,
allowAccessAgreement: true,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
allowRbac: true,
allowSubFeaturePrivileges: true,
});
});

it('should allow to login, allow RBAC, role mappings, sub-feature privileges, and DLS if license >= platinum', () => {
it('should allow to login, allow RBAC, role mappings, access agreement, sub-feature privileges, and DLS if license >= platinum', () => {
const mockRawLicense = licensingMock.createLicense({
license: { mode: 'platinum', type: 'platinum' },
features: { security: { isEnabled: true, isAvailable: true } },
Expand All @@ -173,6 +180,7 @@ describe('license features', function() {
allowLogin: true,
showLinks: true,
showRoleMappingsManagement: true,
allowAccessAgreement: true,
allowRoleDocumentLevelSecurity: true,
allowRoleFieldLevelSecurity: true,
allowRbac: true,
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/security/common/licensing/license_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class SecurityLicenseService {
allowLogin: false,
showLinks: false,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
allowRbac: false,
Expand All @@ -88,6 +89,7 @@ export class SecurityLicenseService {
allowLogin: false,
showLinks: false,
showRoleMappingsManagement: false,
allowAccessAgreement: false,
allowRoleDocumentLevelSecurity: false,
allowRoleFieldLevelSecurity: false,
allowRbac: false,
Expand All @@ -102,6 +104,7 @@ export class SecurityLicenseService {
allowLogin: true,
showLinks: true,
showRoleMappingsManagement: isLicenseGoldOrBetter,
allowAccessAgreement: isLicenseGoldOrBetter,
allowSubFeaturePrivileges: isLicenseGoldOrBetter,
// Only platinum and trial licenses are compliant with field- and document-level security.
allowRoleDocumentLevelSecurity: isLicensePlatinumOrBetter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import {
elasticsearchServiceMock,
sessionStorageMock,
} from '../../../../../src/core/server/mocks';
import { licenseMock } from '../../common/licensing/index.mock';
import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock';
import { securityAuditLoggerMock } from '../audit/index.mock';
import { SecurityLicenseFeatures } from '../../common/licensing';
import { ConfigSchema, createConfig } from '../config';
import { AuthenticationResult } from './authentication_result';
import { Authenticator, AuthenticatorOptions, ProviderSession } from './authenticator';
Expand All @@ -44,6 +46,7 @@ function getMockOptions({
getCurrentUser: jest.fn(),
clusterClient: elasticsearchServiceMock.createClusterClient(),
basePath: httpServiceMock.createSetupContract().basePath,
license: licenseMock.create(),
loggers: loggingServiceMock.create(),
config: createConfig(
ConfigSchema.validate({ session, authc: { selector, providers, http } }),
Expand Down Expand Up @@ -1121,6 +1124,9 @@ describe('Authenticator', () => {
},
});
mockOptions.sessionStorageFactory.asScoped.mockReturnValue(mockSessionStorage);
mockOptions.license.getFeatures.mockReturnValue({
allowAccessAgreement: true,
} as SecurityLicenseFeatures);

mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
AuthenticationResult.succeeded(mockUser)
Expand Down Expand Up @@ -1220,6 +1226,18 @@ describe('Authenticator', () => {
);
});

it('does not redirect to Access Agreement if license doesnt allow it.', async () => {
const request = httpServerMock.createKibanaRequest();
mockSessionStorage.get.mockResolvedValue(mockSessVal);
mockOptions.license.getFeatures.mockReturnValue({
allowAccessAgreement: false,
} as SecurityLicenseFeatures);

await expect(authenticator.authenticate(request)).resolves.toEqual(
AuthenticationResult.succeeded(mockUser)
);
});

it('redirects to Access Agreement when needed.', async () => {
mockSessionStorage.get.mockResolvedValue(mockSessVal);

Expand Down Expand Up @@ -1416,6 +1434,9 @@ describe('Authenticator', () => {
};
mockSessionStorage.get.mockResolvedValue(mockSessionValue);
mockOptions.getCurrentUser.mockReturnValue(mockAuthenticatedUser());
mockOptions.license.getFeatures.mockReturnValue({
allowAccessAgreement: true,
} as SecurityLicenseFeatures);

authenticator = new Authenticator(mockOptions);
});
Expand Down Expand Up @@ -1444,6 +1465,20 @@ describe('Authenticator', () => {
expect(mockSessionStorage.set).not.toHaveBeenCalled();
});

it('fails if license doesn allow access agreement acknowledgement', async () => {
mockOptions.license.getFeatures.mockReturnValue({
allowAccessAgreement: false,
} as SecurityLicenseFeatures);

await expect(
authenticator.acknowledgeAccessAgreement(httpServerMock.createKibanaRequest())
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Current license does not allow access agreement acknowledgement."`
);

expect(mockSessionStorage.set).not.toHaveBeenCalled();
});

it('properly acknowledges access agreement for the authenticated user', async () => {
await authenticator.acknowledgeAccessAgreement(httpServerMock.createKibanaRequest());

Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/security/server/authentication/authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
HttpServiceSetup,
IClusterClient,
} from '../../../../../src/core/server';
import { SecurityLicense } from '../../common/licensing';
import { AuthenticatedUser } from '../../common/model';
import { AuthenticationProvider, SessionInfo } from '../../common/types';
import { SecurityAuditLogger } from '../audit';
Expand Down Expand Up @@ -96,6 +97,7 @@ export interface AuthenticatorOptions {
getCurrentUser: (request: KibanaRequest) => AuthenticatedUser | null;
config: Pick<ConfigType, 'session' | 'authc'>;
basePath: HttpServiceSetup['basePath'];
license: SecurityLicense;
loggers: LoggerFactory;
clusterClient: IClusterClient;
sessionStorageFactory: SessionStorageFactory<ProviderSession>;
Expand Down Expand Up @@ -490,6 +492,10 @@ export class Authenticator {
throw new Error('Cannot acknowledge access agreement for unauthenticated user.');
}

if (!this.options.license.getFeatures().allowAccessAgreement) {
throw new Error('Current license does not allow access agreement acknowledgement.');
}

sessionStorage.set({ ...existingSession, accessAgreementAcknowledged: true });

this.options.auditLogger.accessAgreementAcknowledged(
Expand Down Expand Up @@ -682,14 +688,16 @@ export class Authenticator {
// 2. Request is authenticated, but user hasn't acknowledged access agreement in the current
// session yet (based on the flag we store in the session)
// 3. Request is authenticated by the provider that has `accessAgreement` configured
// 4. And it's not a request to the Access Agreement UI itself
// 4. Current license allows access agreement
// 5. And it's not a request to the Access Agreement UI itself
return (
canRedirectRequest(request) &&
session != null &&
!session.accessAgreementAcknowledged &&
(this.options.config.authc.providers as Record<string, any>)[session.provider.type]?.[
session.provider.name
]?.accessAgreement &&
this.options.license.getFeatures().allowAccessAgreement &&
request.url.pathname !== ACCESS_AGREEMENT_ROUTE
);
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export async function setupAuthentication({
clusterClient,
basePath: http.basePath,
config: { session: config.session, authc: config.authc },
license,
loggers,
sessionStorageFactory: await http.createCookieSessionStorageFactory({
encryptionKey: config.encryptionKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
RequestHandlerContext,
RouteConfig,
} from '../../../../../../src/core/server';
import { SecurityLicense, SecurityLicenseFeatures } from '../../../common/licensing';
import {
Authentication,
AuthenticationResult,
Expand All @@ -28,11 +29,13 @@ import { routeDefinitionParamsMock } from '../index.mock';
describe('Common authentication routes', () => {
let router: jest.Mocked<IRouter>;
let authc: jest.Mocked<Authentication>;
let license: jest.Mocked<SecurityLicense>;
let mockContext: RequestHandlerContext;
beforeEach(() => {
const routeParamsMock = routeDefinitionParamsMock.create();
router = routeParamsMock.router;
authc = routeParamsMock.authc;
license = routeParamsMock.license;

mockContext = ({
licensing: {
Expand Down Expand Up @@ -442,6 +445,10 @@ describe('Common authentication routes', () => {
([{ path }]) => path === '/internal/security/access_agreement/acknowledge'
)!;

license.getFeatures.mockReturnValue({
allowAccessAgreement: true,
} as SecurityLicenseFeatures);

routeConfig = acsRouteConfig;
routeHandler = acsRouteHandler;
});
Expand All @@ -451,6 +458,19 @@ describe('Common authentication routes', () => {
expect(routeConfig.validate).toBe(false);
});

it('returns 404 if current license doesnt allow access agreement acknowledgement.', async () => {
license.getFeatures.mockReturnValue({
allowAccessAgreement: false,
} as SecurityLicenseFeatures);

const request = httpServerMock.createKibanaRequest();
await expect(routeHandler(mockContext, request, kibanaResponseFactory)).resolves.toEqual({
status: 404,
payload: 'Not Found',
options: {},
});
});

it('returns 500 if acknowledge throws unhandled exception.', async () => {
const unhandledException = new Error('Something went wrong.');
authc.acknowledgeAccessAgreement.mockRejectedValue(unhandledException);
Expand Down
18 changes: 15 additions & 3 deletions x-pack/plugins/security/server/routes/authentication/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import { RouteDefinitionParams } from '..';
/**
* Defines routes that are common to various authentication mechanisms.
*/
export function defineCommonRoutes({ router, authc, basePath, logger }: RouteDefinitionParams) {
export function defineCommonRoutes({
router,
authc,
basePath,
license,
logger,
}: RouteDefinitionParams) {
// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
for (const path of ['/api/security/logout', '/api/security/v1/logout']) {
router.get(
Expand Down Expand Up @@ -138,7 +144,13 @@ export function defineCommonRoutes({ router, authc, basePath, logger }: RouteDef

router.post(
{ path: '/internal/security/access_agreement/acknowledge', validate: false },
async (context, request, response) => {
createLicensedRouteHandler(async (context, request, response) => {
// If license doesn't allow access agreement we shouldn't handle request.
if (!license.getFeatures().allowAccessAgreement) {
logger.warn(`Attempted to acknowledge access agreement when license doesn allow it.`);
return response.notFound();
}

try {
await authc.acknowledgeAccessAgreement(request);
} catch (err) {
Expand All @@ -147,6 +159,6 @@ export function defineCommonRoutes({ router, authc, basePath, logger }: RouteDef
}

return response.noContent();
}
})
);
}
18 changes: 15 additions & 3 deletions x-pack/plugins/security/server/routes/licensed_route_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,22 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { RequestHandler } from 'kibana/server';
import { KibanaResponseFactory, RequestHandler, RouteMethod } from 'kibana/server';

export const createLicensedRouteHandler = <P, Q, B>(handler: RequestHandler<P, Q, B>) => {
const licensedRouteHandler: RequestHandler<P, Q, B> = (context, request, responseToolkit) => {
export const createLicensedRouteHandler = <
P,
Q,
B,
M extends RouteMethod,
R extends KibanaResponseFactory
>(
handler: RequestHandler<P, Q, B, M, R>
) => {
const licensedRouteHandler: RequestHandler<P, Q, B, M, R> = (
context,
request,
responseToolkit
) => {
const { license } = context.licensing;
const licenseCheck = license.check('security', 'basic');
if (licenseCheck.state === 'unavailable' || licenseCheck.state === 'invalid') {
Expand Down
Loading

0 comments on commit de10976

Please sign in to comment.