Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix login redirect for expired sessions #57157

Merged
merged 7 commits into from
Feb 16, 2020
Merged

Fix login redirect for expired sessions #57157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
48aced7
Fix login redirect for expired sessions
jportner Feb 7, 2020
e74e931
Merge branch 'master' into issue-57113-fix-login-redirect
jportner Feb 11, 2020
516191c
PR review feedback
jportner Feb 11, 2020
d8ea466
Fix type check and unit test
jportner Feb 11, 2020
3b839ed
Merge branch 'master' into issue-57113-fix-login-redirect
elasticmachine Feb 14, 2020
4d631da
Merge branch 'master' into issue-57113-fix-login-redirect
jportner Feb 15, 2020
843f18d
More PR review feedback
jportner Feb 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions x-pack/legacy/plugins/security/public/services/auto_logout.js
View file
Open in desktop
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}`;
jportner marked this conversation as resolved.
Show resolved Hide resolved
return Promise.halt();
};
});
7 changes: 4 additions & 3 deletions x-pack/plugins/security/public/plugin.tsx
View file
Open in desktop
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
View file
Open in desktop
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', () => {
jportner marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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}`);
jportner marked this conversation as resolved.
Show resolved Hide resolved
}
}
8 changes: 4 additions & 4 deletions x-pack/plugins/security/public/session/unauthorized_response_http_interceptor.test.ts
View file
Open in desktop
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