Skip to content

Commit

Permalink
Fix login redirect for expired sessions (elastic#57157) (elastic#57756)
Browse files Browse the repository at this point in the history
  • Loading branch information
jportner authored Feb 16, 2020
1 parent 76157e1 commit cb40502
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 82 deletions.
22 changes: 18 additions & 4 deletions x-pack/legacy/plugins/security/public/services/auto_logout.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,26 @@ import { uiModules } from 'ui/modules';
import chrome from 'ui/chrome';

const module = uiModules.get('security');

const getNextParameter = () => {
const { location } = window;
const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`);
return `&next=${next}`;
};

const getProviderParameter = tenant => {
const key = `${tenant}/session_provider`;
const providerName = sessionStorage.getItem(key);
return providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
};

module.service('autoLogout', ($window, Promise) => {
return () => {
const next = chrome.removeBasePath(`${window.location.pathname}${window.location.hash}`);
$window.location.href = chrome.addBasePath(
`/logout?next=${encodeURIComponent(next)}&msg=SESSION_EXPIRED`
);
const logoutUrl = chrome.getInjected('logoutUrl');
const tenant = `${chrome.getInjected('session.tenant', '')}`;
const next = getNextParameter();
const provider = getProviderParameter(tenant);
$window.location.href = `${logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`;
return Promise.halt();
};
});
7 changes: 4 additions & 3 deletions x-pack/plugins/security/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,14 @@ export class SecurityPlugin
{ home, licensing, management }: PluginSetupDependencies
) {
const { http, notifications, injectedMetadata } = core;
const { basePath, anonymousPaths } = http;
const { anonymousPaths } = http;
anonymousPaths.register('/login');
anonymousPaths.register('/logout');
anonymousPaths.register('/logged_out');

const tenant = `${injectedMetadata.getInjectedVar('session.tenant', '')}`;
const sessionExpired = new SessionExpired(basePath, tenant);
const tenant = injectedMetadata.getInjectedVar('session.tenant', '') as string;
const logoutUrl = injectedMetadata.getInjectedVar('logoutUrl') as string;
const sessionExpired = new SessionExpired(logoutUrl, tenant);
http.intercept(new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths));
this.sessionTimeout = new SessionTimeout(notifications, sessionExpired, http, tenant);
http.intercept(new SessionTimeoutHttpInterceptor(this.sessionTimeout, anonymousPaths));
Expand Down
91 changes: 34 additions & 57 deletions x-pack/plugins/security/public/session/session_expired.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { coreMock } from 'src/core/public/mocks';
import { SessionExpired } from './session_expired';

describe('Session Expiration', () => {
describe('#logout', () => {
const mockGetItem = jest.fn().mockReturnValue(null);
const CURRENT_URL = '/foo/bar?baz=quz#quuz';
const LOGOUT_URL = '/logout';
const TENANT = '/some-basepath';

let newUrlPromise: Promise<string>;

beforeAll(() => {
Object.defineProperty(window, 'sessionStorage', {
Expand All @@ -19,69 +23,42 @@ describe('Session Expiration', () => {
});
});

beforeEach(() => {
window.history.pushState({}, '', CURRENT_URL);
mockGetItem.mockReset();
newUrlPromise = new Promise<string>(resolve => {
jest.spyOn(window.location, 'assign').mockImplementation(url => {
resolve(url);
});
});
});

afterAll(() => {
delete (window as any).sessionStorage;
});

describe('logout', () => {
const mockCurrentUrl = (url: string) => window.history.pushState({}, '', url);
const tenant = '';

it('redirects user to "/logout" when there is no basePath', async () => {
const { basePath } = coreMock.createSetup().http;
mockCurrentUrl('/foo/bar?baz=quz#quuz');
const sessionExpired = new SessionExpired(basePath, tenant);
const newUrlPromise = new Promise<string>(resolve => {
jest.spyOn(window.location, 'assign').mockImplementation(url => {
resolve(url);
});
});

sessionExpired.logout();
it(`redirects user to the logout URL with 'msg' and 'next' parameters`, async () => {
const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
sessionExpired.logout();

const url = await newUrlPromise;
expect(url).toBe(
`/logout?next=${encodeURIComponent('/foo/bar?baz=quz#quuz')}&msg=SESSION_EXPIRED`
);
});

it('adds a provider parameter when an auth provider is saved in sessionStorage', async () => {
const { basePath } = coreMock.createSetup().http;
mockCurrentUrl('/foo/bar?baz=quz#quuz');
const sessionExpired = new SessionExpired(basePath, tenant);
const newUrlPromise = new Promise<string>(resolve => {
jest.spyOn(window.location, 'assign').mockImplementation(url => {
resolve(url);
});
});
mockGetItem.mockReturnValueOnce('basic');

sessionExpired.logout();
const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
await expect(newUrlPromise).resolves.toBe(`${LOGOUT_URL}?msg=SESSION_EXPIRED${next}`);
});

const url = await newUrlPromise;
expect(url).toBe(
`/logout?next=${encodeURIComponent(
'/foo/bar?baz=quz#quuz'
)}&msg=SESSION_EXPIRED&provider=basic`
);
});
it(`adds 'provider' parameter when sessionStorage contains the provider name for this tenant`, async () => {
const providerName = 'basic';
mockGetItem.mockReturnValueOnce(providerName);

it('redirects user to "/${basePath}/logout" and removes basePath from next parameter when there is a basePath', async () => {
const { basePath } = coreMock.createSetup({ basePath: '/foo' }).http;
mockCurrentUrl('/foo/bar?baz=quz#quuz');
const sessionExpired = new SessionExpired(basePath, tenant);
const newUrlPromise = new Promise<string>(resolve => {
jest.spyOn(window.location, 'assign').mockImplementation(url => {
resolve(url);
});
});
const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
sessionExpired.logout();

sessionExpired.logout();
expect(mockGetItem).toHaveBeenCalledTimes(1);
expect(mockGetItem).toHaveBeenCalledWith(`${TENANT}/session_provider`);

const url = await newUrlPromise;
expect(url).toBe(
`/foo/logout?next=${encodeURIComponent('/bar?baz=quz#quuz')}&msg=SESSION_EXPIRED`
);
});
const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
const provider = `&provider=${providerName}`;
await expect(newUrlPromise).resolves.toBe(
`${LOGOUT_URL}?msg=SESSION_EXPIRED${next}${provider}`
);
});
});
30 changes: 16 additions & 14 deletions x-pack/plugins/security/public/session/session_expired.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,28 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { HttpSetup } from 'src/core/public';

export interface ISessionExpired {
logout(): void;
}

const getNextParameter = () => {
const { location } = window;
const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`);
return `&next=${next}`;
};

const getProviderParameter = (tenant: string) => {
const key = `${tenant}/session_provider`;
const providerName = sessionStorage.getItem(key);
return providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
};

export class SessionExpired {
constructor(private basePath: HttpSetup['basePath'], private tenant: string) {}
constructor(private logoutUrl: string, private tenant: string) {}

logout() {
const next = this.basePath.remove(
`${window.location.pathname}${window.location.search}${window.location.hash}`
);
const key = `${this.tenant}/session_provider`;
const providerName = sessionStorage.getItem(key);
const provider = providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
window.location.assign(
this.basePath.prepend(
`/logout?next=${encodeURIComponent(next)}&msg=SESSION_EXPIRED${provider}`
)
);
const next = getNextParameter();
const provider = getProviderParameter(this.tenant);
window.location.assign(`${this.logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ afterEach(() => {

it(`logs out 401 responses`, async () => {
const http = setupHttp('/foo');
const sessionExpired = new SessionExpired(http.basePath, tenant);
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
const logoutPromise = new Promise(resolve => {
jest.spyOn(sessionExpired, 'logout').mockImplementation(() => resolve());
});
Expand All @@ -59,7 +59,7 @@ it(`ignores anonymous paths`, async () => {
const http = setupHttp('/foo');
const { anonymousPaths } = http;
anonymousPaths.register('/bar');
const sessionExpired = new SessionExpired(http.basePath, tenant);
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths);
http.intercept(interceptor);
fetchMock.mock('*', 401);
Expand All @@ -70,7 +70,7 @@ it(`ignores anonymous paths`, async () => {

it(`ignores errors which don't have a response, for example network connectivity issues`, async () => {
const http = setupHttp('/foo');
const sessionExpired = new SessionExpired(http.basePath, tenant);
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, http.anonymousPaths);
http.intercept(interceptor);
fetchMock.mock('*', new Promise((resolve, reject) => reject(new Error('Network is down'))));
Expand All @@ -81,7 +81,7 @@ it(`ignores errors which don't have a response, for example network connectivity

it(`ignores requests which omit credentials`, async () => {
const http = setupHttp('/foo');
const sessionExpired = new SessionExpired(http.basePath, tenant);
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, http.anonymousPaths);
http.intercept(interceptor);
fetchMock.mock('*', 401);
Expand Down

0 comments on commit cb40502

Please sign in to comment.