Skip to content

Commit

Permalink
Adding Global Access Agreement (#139217)
Browse files Browse the repository at this point in the history
* Adding Global Access Agreement

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Adding docs

* PR Review changes

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* PR Review changes

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Fixing global access agreement redirect logic

* Fixing unit test

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Changing verbiage

* Changing local -> provider specific

* Update x-pack/plugins/security/server/routes/views/access_agreement.ts

Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>

* PR Feedback

* removing `only`

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Changing the accessAgreement to be optional

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Adding docker config

* Update docs/user/security/access-agreement.asciidoc

Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

* Update docs/user/security/access-agreement.asciidoc

Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

* Adding PR Review feedback

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
  • Loading branch information
4 people authored Sep 6, 2022
1 parent d11ee88 commit c6fb0bc
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 25 deletions.
11 changes: 11 additions & 0 deletions docs/settings/security-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ Adds a message accessible at the login UI with additional help information for t
xpack.security.authc.selector.enabled {ess-icon}::
Determines if the login selector UI should be enabled. By default, this setting is set to `true` if more than one authentication provider is configured.

[float]
[[authentication-access-agreement-settings]]
==== Configure a default access agreement

You can configure the following settings in the `kibana.yml` file.

xpack.security.accessAgreement.message {ess-icon}::
This setting specifies the access agreement text in Markdown format that will be used as the default access agreement for all providers that do not
specify a value for `xpack.security.authc.providers.<provider-type>.<provider-name>.accessAgreement.message`.
For more information, refer to <<xpack-security-access-agreement>>.

[float]
[[security-session-and-cookie-settings]]
==== Session and cookie security settings
Expand Down
3 changes: 3 additions & 0 deletions docs/user/security/access-agreement.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ Access agreement is a https://www.elastic.co/subscriptions[subscription feature]
agreement before accessing {kib}. The agreement text supports Markdown format and can be specified using the
`xpack.security.authc.providers.<provider-type>.<provider-name>.accessAgreement.message` setting.

You can specify a default access agreement using the `xpack.security.accessAgreement.message` setting.
This message will be used for each provider who doesn't specify an access agreement.

[NOTE]
============================================================================
You need to acknowledge the access agreement only once per session, and {kib} reports the acknowledgement in the audit logs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ kibana_vars=(
xpack.reporting.roles.allow
xpack.reporting.roles.enabled
xpack.ruleRegistry.write.enabled
xpack.security.accessAgreement.message
xpack.security.audit.appender.fileName
xpack.security.audit.appender.layout.highlight
xpack.security.audit.appender.layout.pattern
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,10 @@ export class AuthenticationService {
loggers,
clusterClient,
basePath: http.basePath,
config: { authc: config.authc },
config: {
authc: config.authc,
accessAgreement: config.accessAgreement,
},
getCurrentUser,
featureUsageService,
userProfileService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,21 @@ function getMockOptions({
providers,
http = {},
selector,
accessAgreementMessage,
}: {
providers?: Record<string, unknown> | string[];
http?: Partial<AuthenticatorOptions['config']['authc']['http']>;
selector?: AuthenticatorOptions['config']['authc']['selector'];
accessAgreementMessage?: string;
} = {}) {
const auditService = auditServiceMock.create();
auditLogger = auditLoggerMock.create();
auditService.asScoped.mockReturnValue(auditLogger);

const accessAgreementObj = accessAgreementMessage
? { accessAgreement: { message: accessAgreementMessage } }
: null;

return {
audit: auditService,
getCurrentUser: jest.fn(),
Expand All @@ -65,7 +72,10 @@ function getMockOptions({
loggers: loggingSystemMock.create(),
getServerBaseURL: jest.fn(),
config: createConfig(
ConfigSchema.validate({ authc: { selector, providers, http } }),
ConfigSchema.validate({
authc: { selector, providers, http },
...accessAgreementObj,
}),
loggingSystemMock.create().get(),
{ isTLSEnabled: false }
),
Expand Down Expand Up @@ -1921,6 +1931,39 @@ describe('Authenticator', () => {
);
expect(auditLogger.log).not.toHaveBeenCalled();
});

it('redirects to global Access Agreement when provider specific Access Agreement is not configured.', async () => {
mockOptions = getMockOptions({
providers: {
basic: { basic1: { order: 0 } },
},
accessAgreementMessage: 'Foo',
});

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

authenticator = new Authenticator(mockOptions);

mockOptions.session.get.mockResolvedValue(mockSessVal);
mockOptions.session.extend.mockResolvedValue(mockSessVal);

mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
AuthenticationResult.succeeded(mockUser, {
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
})
);

const request = httpServerMock.createKibanaRequest();
await expect(authenticator.authenticate(request)).resolves.toEqual(
AuthenticationResult.redirectTo(
'/mock-server-basepath/security/access_agreement?next=%2Fmock-server-basepath%2Fpath',
{ user: mockUser, authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' } }
)
);
expect(auditLogger.log).not.toHaveBeenCalled();
});
});

describe('with Overwritten Session', () => {
Expand Down
39 changes: 23 additions & 16 deletions x-pack/plugins/security/server/authentication/authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export interface AuthenticatorOptions {
featureUsageService: SecurityFeatureUsageServiceStart;
userProfileService: UserProfileServiceStartInternal;
getCurrentUser: (request: KibanaRequest) => AuthenticatedUser | null;
config: Pick<ConfigType, 'authc'>;
config: Pick<ConfigType, 'authc' | 'accessAgreement'>;
basePath: IBasePath;
license: SecurityLicense;
loggers: LoggerFactory;
Expand Down Expand Up @@ -831,21 +831,27 @@ export class Authenticator {
* @param sessionValue Current session value if any.
*/
private shouldRedirectToAccessAgreement(sessionValue: SessionValue | null) {
// Request should be redirected to Access Agreement UI only if all following conditions are met:
// 1. Request can be redirected (not API call)
// 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. Current license allows access agreement
// 5. And it's not a request to the Access Agreement UI itself
return (
sessionValue != null &&
!sessionValue.accessAgreementAcknowledged &&
(this.options.config.authc.providers as Record<string, any>)[sessionValue.provider.type]?.[
sessionValue.provider.name
]?.accessAgreement &&
this.options.license.getFeatures().allowAccessAgreement
);
// If user doesn't have an active session or if they already acknowledged
// access agreement (based on the flag we store in the session) - bail out.
if (sessionValue == null || sessionValue.accessAgreementAcknowledged) {
return false;
}

// If access agreement is neither enabled globally (for all providers)
// nor for the provider that authenticated user request - bail out.
const providerConfig = (this.options.config.authc.providers as Record<string, any>)[
sessionValue.provider.type
]?.[sessionValue.provider.name];

if (
!this.options.config.accessAgreement?.message &&
!providerConfig?.accessAgreement?.message
) {
return false;
}

// Check if the current license allows access agreement.
return this.options.license.getFeatures().allowAccessAgreement;
}

/**
Expand Down Expand Up @@ -874,6 +880,7 @@ export class Authenticator {
const isUpdatedSessionAuthenticated = isSessionAuthenticated(sessionUpdateResult?.value);

let preAccessRedirectURL;

if (isUpdatedSessionAuthenticated && sessionUpdateResult?.overwritten) {
this.logger.debug('Redirecting user to the overwritten session UI.');
preAccessRedirectURL = `${this.options.basePath.serverBasePath}${OVERWRITTEN_SESSION_ROUTE}`;
Expand Down
26 changes: 26 additions & 0 deletions x-pack/plugins/security/server/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2204,4 +2204,30 @@ describe('createConfig()', () => {
`);
});
});

describe('Global Access Agreement', () => {
it('should require `message` for globally configured `accessAgreement`', () => {
expect(() => {
createConfig(
ConfigSchema.validate({
accessAgreement: {},
}),
loggingSystemMock.create().get(),
{ isTLSEnabled: true }
);
}).toThrow('[accessAgreement.message]: expected value of type [string] but got [undefined]');
});

it('should accept string `message` for globally configured `accessAgreement`', () => {
expect(
createConfig(
ConfigSchema.validate({
accessAgreement: { message: 'Foo' },
}),
loggingSystemMock.create().get(),
{ isTLSEnabled: true }
)?.accessAgreement?.message
).toEqual('Foo');
});
});
});
11 changes: 10 additions & 1 deletion x-pack/plugins/security/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function getUniqueProviderSchema<TProperties extends Record<string, Type<any>>>(
}

type ProvidersConfigType = TypeOf<typeof providersConfigSchema>;

const providersConfigSchema = schema.object(
{
basic: getUniqueProviderSchema('basic', {
Expand Down Expand Up @@ -235,6 +236,7 @@ export const ConfigSchema = schema.object({
hostname: schema.maybe(schema.string({ hostname: true })),
port: schema.maybe(schema.number({ min: 0, max: 65535 })),
}),
accessAgreement: schema.maybe(schema.object({ message: schema.string() })),
authc: schema.object({
selector: schema.object({ enabled: schema.maybe(schema.boolean()) }),
providers: schema.oneOf([schema.arrayOf(schema.string()), providersConfigSchema], {
Expand Down Expand Up @@ -306,6 +308,7 @@ export function createConfig(
}

let secureCookies = config.secureCookies;

if (!isTLSEnabled) {
if (secureCookies) {
logger.warn(
Expand All @@ -322,6 +325,7 @@ export function createConfig(
}

const isUsingLegacyProvidersFormat = Array.isArray(config.authc.providers);

const providers = (
isUsingLegacyProvidersFormat
? [...new Set(config.authc.providers as Array<keyof ProvidersConfigType>)].reduce(
Expand All @@ -332,6 +336,7 @@ export function createConfig(
? { enabled: true, showInSelector: true, order, ...config.authc[providerType] }
: { enabled: true, showInSelector: true, order },
};

return legacyProviders;
},
{} as Record<string, unknown>
Expand All @@ -346,16 +351,19 @@ export function createConfig(
order: number;
hasAccessAgreement: boolean;
}> = [];

for (const [type, providerGroup] of Object.entries(providers)) {
for (const [name, { enabled, order, accessAgreement }] of Object.entries(providerGroup ?? {})) {
if (!enabled) {
delete providerGroup![name];
} else {
const hasAccessAgreement: boolean = !!accessAgreement?.message;

sortedProviders.push({
type: type as any,
name,
order,
hasAccessAgreement: !!accessAgreement?.message,
hasAccessAgreement,
});
}
}
Expand Down Expand Up @@ -391,6 +399,7 @@ export function createConfig(
max: 10,
},
} as AppenderConfigType);

return {
...config,
audit: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('Access agreement view routes', () => {
let session: jest.Mocked<PublicMethodsOf<Session>>;
let license: jest.Mocked<SecurityLicense>;
let mockContext: SecurityRequestHandlerContext;

beforeEach(() => {
const routeParamsMock = routeDefinitionParamsMock.create();
router = routeParamsMock.router;
Expand Down Expand Up @@ -138,7 +139,7 @@ describe('Access agreement view routes', () => {
});
});

it('returns non-empty `accessAgreement` only if it is configured.', async () => {
it('returns non-empty provider specific `accessAgreement` only if it is configured.', async () => {
const request = httpServerMock.createKibanaRequest();

config.authc = routeDefinitionParamsMock.create({
Expand Down Expand Up @@ -172,5 +173,42 @@ describe('Access agreement view routes', () => {
});
}
});

it('returns global `accessAgreement` when provider specific `accessAgreement` is not configured', async () => {
const request = httpServerMock.createKibanaRequest();

config.authc = routeDefinitionParamsMock.create({
authc: {
providers: {
basic: { basic1: { order: 0 } },
saml: {
saml1: {
order: 1,
realm: 'realm1',
accessAgreement: { message: 'Some access agreement' },
},
},
},
},
}).config.authc;

config.accessAgreement = { message: 'Foo' };

const cases: Array<[AuthenticationProvider, string]> = [
[{ type: 'basic', name: 'basic1' }, 'Foo'],
[{ type: 'saml', name: 'saml1' }, 'Some access agreement'],
[{ type: 'unknown-type', name: 'unknown-name' }, 'Foo'],
];

for (const [sessionProvider, expectedAccessAgreement] of cases) {
session.get.mockResolvedValue(sessionMock.createValue({ provider: sessionProvider }));

await expect(routeHandler(mockContext, request, kibanaResponseFactory)).resolves.toEqual({
options: { body: { accessAgreement: expectedAccessAgreement } },
payload: { accessAgreement: expectedAccessAgreement },
status: 200,
});
}
});
});
});
19 changes: 15 additions & 4 deletions x-pack/plugins/security/server/routes/views/access_agreement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,23 @@ export function defineAccessAgreementRoutes({
// authenticated with the help of HTTP authentication), that means we should safely check if
// we have it and can get a corresponding configuration.
const sessionValue = await getSession().get(request);
const accessAgreement =
(sessionValue &&

let accessAgreement = '';

if (sessionValue) {
const providerSpecificAccessAgreement =
config.authc.providers[
sessionValue.provider.type as keyof ConfigType['authc']['providers']
]?.[sessionValue.provider.name]?.accessAgreement?.message) ||
'';
]?.[sessionValue.provider.name]?.accessAgreement?.message;

const globalAccessAgreement = config.accessAgreement?.message;

if (providerSpecificAccessAgreement) {
accessAgreement = providerSpecificAccessAgreement;
} else if (globalAccessAgreement) {
accessAgreement = globalAccessAgreement;
}
}

return response.ok({ body: { accessAgreement } });
})
Expand Down
Loading

0 comments on commit c6fb0bc

Please sign in to comment.