Skip to content

Commit

Permalink
Add route config option to avoid extending the user's session timeout
Browse files Browse the repository at this point in the history
This is necessary for any API that the client may call automatically
(e.g., without user interaction). We don't want those APIs to cause
the user's session to be extended indefinitely.
  • Loading branch information
jportner committed Nov 5, 2019
1 parent 3edd0bc commit e143e48
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 6 deletions.
4 changes: 3 additions & 1 deletion src/core/server/http/http_server.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface RequestFixtureOptions {
path?: string;
method?: RouteMethod;
socket?: Socket;
route?: Record<string, any>;
}

function createKibanaRequestMock({
Expand All @@ -49,6 +50,7 @@ function createKibanaRequestMock({
query = {},
method = 'get',
socket = new Socket(),
route = { settings: {} },
}: RequestFixtureOptions = {}) {
const queryString = querystring.stringify(query);
return KibanaRequest.from(
Expand All @@ -64,7 +66,7 @@ function createKibanaRequestMock({
query: queryString,
search: queryString ? `?${queryString}` : queryString,
},
route: { settings: {} },
route,
raw: {
req: { socket },
},
Expand Down
1 change: 1 addition & 0 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ test('exposes route details of incoming request to a route handler', async () =>
path: '/',
options: {
authRequired: true,
extendsSession: true,
tags: [],
},
});
Expand Down
4 changes: 3 additions & 1 deletion src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,15 @@ export class HttpServer {
for (const router of this.registeredRouters) {
for (const route of router.getRoutes()) {
this.log.debug(`registering route handler for [${route.path}]`);
const { authRequired = true, tags } = route.options;
const { authRequired = true, extendsSession = true, tags } = route.options;
// Hapi provides the "server.options.app" property to pass in application-specific configuration
this.server.route({
handler: route.handler,
method: route.method,
path: route.path,
options: {
auth: authRequired ? undefined : false,
app: { extendsSession },
tags: tags ? Array.from(tags) : undefined,
},
});
Expand Down
12 changes: 11 additions & 1 deletion src/core/server/http/router/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { Url } from 'url';
import { Request } from 'hapi';
import { Request, RouteOptionsApp } from 'hapi';

import { ObjectType, TypeOf } from '@kbn/config-schema';

Expand Down Expand Up @@ -150,11 +150,13 @@ export class KibanaRequest<Params = unknown, Query = unknown, Body = unknown> {

private getRouteInfo() {
const request = this[requestSymbol];
const app = request.route.settings.app as KibanaRouteOptionsApp;
return {
path: request.path,
method: request.method,
options: {
authRequired: request.route.settings.auth !== false,
extendsSession: !app || app.extendsSession !== false,
tags: request.route.settings.tags || [],
},
};
Expand Down Expand Up @@ -187,3 +189,11 @@ function isRequest(request: any): request is LegacyRequest {
export function isRealRequest(request: unknown): request is KibanaRequest | LegacyRequest {
return isKibanaRequest(request) || isRequest(request);
}

/**
* Extend the Hapi interface for application-specific configuration on routes
* @internal
*/
interface KibanaRouteOptionsApp extends RouteOptionsApp {
extendsSession: boolean;
}
9 changes: 9 additions & 0 deletions src/core/server/http/router/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ export interface RouteConfigOptions {
*/
authRequired?: boolean;

/**
* A flag shows that authentication for a route:
* Extends the user's session when true
* Does not extend the user's session when false
*
* Enabled by default.
*/
extendsSession?: boolean;

/**
* Additional metadata tag strings to attach to the route.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,54 @@ describe('Authenticator', () => {
expect(mockSessionStorage.clear).not.toHaveBeenCalled();
});

it('does not extend session expiration for routes that opt out.', async () => {
const user = mockAuthenticatedUser();
const state = { authorization: 'Basic xxx' };
const options = { route: { settings: { app: { extendsSession: false } } } };
const request = httpServerMock.createKibanaRequest(options);
const currentDate = new Date(Date.UTC(2019, 10, 10)).valueOf();

// Create new authenticator with non-null session `idleTimeout`.
const idleTimeout = 3600 * 24;
mockOptions = getMockOptions({
session: {
idleTimeout,
lifespan: null,
},
authc: { providers: ['basic'], oidc: {}, saml: {} },
});

mockSessionStorage = sessionStorageMock.create();
mockSessionStorage.get.mockResolvedValue({
expires: currentDate + idleTimeout / 2,
maxExpires: null,
state,
provider: 'basic',
});
mockOptions.sessionStorageFactory.asScoped.mockReturnValue(mockSessionStorage);

authenticator = new Authenticator(mockOptions);

mockBasicAuthenticationProvider.authenticate.mockResolvedValue(
AuthenticationResult.succeeded(user)
);

jest.spyOn(Date, 'now').mockImplementation(() => currentDate);

const authenticationResult = await authenticator.authenticate(request);
expect(authenticationResult.succeeded()).toBe(true);
expect(authenticationResult.user).toEqual(user);

expect(mockSessionStorage.set).toHaveBeenCalledTimes(1);
expect(mockSessionStorage.set).toHaveBeenCalledWith({
expires: currentDate + idleTimeout / 2,
maxExpires: null,
state,
provider: 'basic',
});
expect(mockSessionStorage.clear).not.toHaveBeenCalled();
});

it('does not extend session expiration past the defined maximum session lifespan.', async () => {
const user = mockAuthenticatedUser();
const state = { authorization: 'Basic xxx' };
Expand Down
18 changes: 15 additions & 3 deletions x-pack/plugins/security/server/authentication/authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ export class Authenticator {
if (existingSession && shouldClearSession) {
sessionStorage.clear();
} else if (!attempt.stateless && authenticationResult.shouldUpdateState()) {
const { expires, maxExpires } = this.calculateExpiry(existingSession);
const shouldExtendSession = request.route.options.extendsSession;
const { expires, maxExpires } = this.calculateExpiry(existingSession, shouldExtendSession);
sessionStorage.set({
state: authenticationResult.state,
provider: attempt.provider,
Expand Down Expand Up @@ -302,11 +303,13 @@ export class Authenticator {
ownsSession ? existingSession!.state : null
);

const shouldExtendSession = request.route.options.extendsSession;
this.updateSessionValue(sessionStorage, {
providerType,
isSystemAPIRequest: this.options.isSystemAPIRequest(request),
authenticationResult,
existingSession: ownsSession ? existingSession : null,
shouldExtendSession,
});

if (
Expand Down Expand Up @@ -397,11 +400,13 @@ export class Authenticator {
providerType,
authenticationResult,
existingSession,
shouldExtendSession,
isSystemAPIRequest,
}: {
providerType: string;
authenticationResult: AuthenticationResult;
existingSession: ProviderSession | null;
shouldExtendSession: boolean;
isSystemAPIRequest: boolean;
}
) {
Expand All @@ -425,7 +430,7 @@ export class Authenticator {
) {
sessionStorage.clear();
} else if (sessionCanBeUpdated) {
const { expires, maxExpires } = this.calculateExpiry(existingSession);
const { expires, maxExpires } = this.calculateExpiry(existingSession, shouldExtendSession);
sessionStorage.set({
state: authenticationResult.shouldUpdateState()
? authenticationResult.state
Expand All @@ -438,8 +443,15 @@ export class Authenticator {
}

private calculateExpiry(
existingSession: ProviderSession | null
existingSession: ProviderSession | null,
shouldExtendSession: boolean
): { expires: number | null; maxExpires: number | null } {
// if we should not extend the session expiration, and there is an existing session,
// then just return the values for the existing session
if (!shouldExtendSession && existingSession) {
return existingSession;
}

const maxExpires = existingSession
? existingSession.maxExpires
: this.lifespan && Date.now() + this.lifespan;
Expand Down

0 comments on commit e143e48

Please sign in to comment.