Skip to content

Commit 1faa3bf

Browse files
jportnerjportner
committed
Fix login redirect for expired sessions (#57157)
1 parent 9319120 commit 1faa3bf

5 files changed

Lines changed: 76 additions & 82 deletions

File tree

‎x-pack/legacy/plugins/security/public/services/auto_logout.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,26 @@ import { uiModules } from 'ui/modules';
88
import chrome from 'ui/chrome';
99

1010
const module = uiModules.get('security');
11+
12+
const getNextParameter = () => {
13+
const { location } = window;
14+
const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`);
15+
return `&next=${next}`;
16+
};
17+
18+
const getProviderParameter = tenant => {
19+
const key = `${tenant}/session_provider`;
20+
const providerName = sessionStorage.getItem(key);
21+
return providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
22+
};
23+
1124
module.service('autoLogout', ($window, Promise) => {
1225
return () => {
13-
const next = chrome.removeBasePath(`${window.location.pathname}${window.location.hash}`);
14-
$window.location.href = chrome.addBasePath(
15-
`/logout?next=${encodeURIComponent(next)}&msg=SESSION_EXPIRED`
16-
);
26+
const logoutUrl = chrome.getInjected('logoutUrl');
27+
const tenant = `${chrome.getInjected('session.tenant', '')}`;
28+
const next = getNextParameter();
29+
const provider = getProviderParameter(tenant);
30+
$window.location.href = `${logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`;
1731
return Promise.halt();
1832
};
1933
});

‎x-pack/plugins/security/public/plugin.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,14 @@ export class SecurityPlugin implements Plugin<SecurityPluginSetup, SecurityPlugi
3030

3131
public setup(core: CoreSetup, { licensing }: PluginSetupDependencies) {
3232
const { http, notifications, injectedMetadata } = core;
33-
const { basePath, anonymousPaths } = http;
33+
const { anonymousPaths } = http;
3434
anonymousPaths.register('/login');
3535
anonymousPaths.register('/logout');
3636
anonymousPaths.register('/logged_out');
3737

38-
const tenant = `${injectedMetadata.getInjectedVar('session.tenant', '')}`;
39-
const sessionExpired = new SessionExpired(basePath, tenant);
38+
const tenant = injectedMetadata.getInjectedVar('session.tenant', '') as string;
39+
const logoutUrl = injectedMetadata.getInjectedVar('logoutUrl') as string;
40+
const sessionExpired = new SessionExpired(logoutUrl, tenant);
4041
http.intercept(new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths));
4142
this.sessionTimeout = new SessionTimeout(notifications, sessionExpired, http, tenant);
4243
http.intercept(new SessionTimeoutHttpInterceptor(this.sessionTimeout, anonymousPaths));

‎x-pack/plugins/security/public/session/session_expired.test.ts‎

Lines changed: 34 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { coreMock } from 'src/core/public/mocks';
87
import { SessionExpired } from './session_expired';
98

10-
describe('Session Expiration', () => {
9+
describe('#logout', () => {
1110
const mockGetItem = jest.fn().mockReturnValue(null);
11+
const CURRENT_URL = '/foo/bar?baz=quz#quuz';
12+
const LOGOUT_URL = '/logout';
13+
const TENANT = '/some-basepath';
14+
15+
let newUrlPromise: Promise<string>;
1216

1317
beforeAll(() => {
1418
Object.defineProperty(window, 'sessionStorage', {
@@ -19,69 +23,42 @@ describe('Session Expiration', () => {
1923
});
2024
});
2125

26+
beforeEach(() => {
27+
window.history.pushState({}, '', CURRENT_URL);
28+
mockGetItem.mockReset();
29+
newUrlPromise = new Promise<string>(resolve => {
30+
jest.spyOn(window.location, 'assign').mockImplementation(url => {
31+
resolve(url);
32+
});
33+
});
34+
});
35+
2236
afterAll(() => {
2337
delete (window as any).sessionStorage;
2438
});
2539

26-
describe('logout', () => {
27-
const mockCurrentUrl = (url: string) => window.history.pushState({}, '', url);
28-
const tenant = '';
29-
30-
it('redirects user to "/logout" when there is no basePath', async () => {
31-
const { basePath } = coreMock.createSetup().http;
32-
mockCurrentUrl('/foo/bar?baz=quz#quuz');
33-
const sessionExpired = new SessionExpired(basePath, tenant);
34-
const newUrlPromise = new Promise<string>(resolve => {
35-
jest.spyOn(window.location, 'assign').mockImplementation(url => {
36-
resolve(url);
37-
});
38-
});
39-
40-
sessionExpired.logout();
40+
it(`redirects user to the logout URL with 'msg' and 'next' parameters`, async () => {
41+
const sessionExpired = new SessionExpired(LOGOUT_URL, TENANT);
42+
sessionExpired.logout();
4143

42-
const url = await newUrlPromise;
43-
expect(url).toBe(
44-
`/logout?next=${encodeURIComponent('/foo/bar?baz=quz#quuz')}&msg=SESSION_EXPIRED`
45-
);
46-
});
47-
48-
it('adds a provider parameter when an auth provider is saved in sessionStorage', async () => {
49-
const { basePath } = coreMock.createSetup().http;
50-
mockCurrentUrl('/foo/bar?baz=quz#quuz');
51-
const sessionExpired = new SessionExpired(basePath, tenant);
52-
const newUrlPromise = new Promise<string>(resolve => {
53-
jest.spyOn(window.location, 'assign').mockImplementation(url => {
54-
resolve(url);
55-
});
56-
});
57-
mockGetItem.mockReturnValueOnce('basic');
58-
59-
sessionExpired.logout();
44+
const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
45+
await expect(newUrlPromise).resolves.toBe(`${LOGOUT_URL}?msg=SESSION_EXPIRED${next}`);
46+
});
6047

61-
const url = await newUrlPromise;
62-
expect(url).toBe(
63-
`/logout?next=${encodeURIComponent(
64-
'/foo/bar?baz=quz#quuz'
65-
)}&msg=SESSION_EXPIRED&provider=basic`
66-
);
67-
});
48+
it(`adds 'provider' parameter when sessionStorage contains the provider name for this tenant`, async () => {
49+
const providerName = 'basic';
50+
mockGetItem.mockReturnValueOnce(providerName);
6851

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

79-
sessionExpired.logout();
55+
expect(mockGetItem).toHaveBeenCalledTimes(1);
56+
expect(mockGetItem).toHaveBeenCalledWith(`${TENANT}/session_provider`);
8057

81-
const url = await newUrlPromise;
82-
expect(url).toBe(
83-
`/foo/logout?next=${encodeURIComponent('/bar?baz=quz#quuz')}&msg=SESSION_EXPIRED`
84-
);
85-
});
58+
const next = `&next=${encodeURIComponent(CURRENT_URL)}`;
59+
const provider = `&provider=${providerName}`;
60+
await expect(newUrlPromise).resolves.toBe(
61+
`${LOGOUT_URL}?msg=SESSION_EXPIRED${next}${provider}`
62+
);
8663
});
8764
});

‎x-pack/plugins/security/public/session/session_expired.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,28 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { HttpSetup } from 'src/core/public';
8-
97
export interface ISessionExpired {
108
logout(): void;
119
}
1210

11+
const getNextParameter = () => {
12+
const { location } = window;
13+
const next = encodeURIComponent(`${location.pathname}${location.search}${location.hash}`);
14+
return `&next=${next}`;
15+
};
16+
17+
const getProviderParameter = (tenant: string) => {
18+
const key = `${tenant}/session_provider`;
19+
const providerName = sessionStorage.getItem(key);
20+
return providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
21+
};
22+
1323
export class SessionExpired {
14-
constructor(private basePath: HttpSetup['basePath'], private tenant: string) {}
24+
constructor(private logoutUrl: string, private tenant: string) {}
1525

1626
logout() {
17-
const next = this.basePath.remove(
18-
`${window.location.pathname}${window.location.search}${window.location.hash}`
19-
);
20-
const key = `${this.tenant}/session_provider`;
21-
const providerName = sessionStorage.getItem(key);
22-
const provider = providerName ? `&provider=${encodeURIComponent(providerName)}` : '';
23-
window.location.assign(
24-
this.basePath.prepend(
25-
`/logout?next=${encodeURIComponent(next)}&msg=SESSION_EXPIRED${provider}`
26-
)
27-
);
27+
const next = getNextParameter();
28+
const provider = getProviderParameter(this.tenant);
29+
window.location.assign(`${this.logoutUrl}?msg=SESSION_EXPIRED${next}${provider}`);
2830
}
2931
}

‎x-pack/plugins/security/public/session/unauthorized_response_http_interceptor.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ afterEach(() => {
3333

3434
it(`logs out 401 responses`, async () => {
3535
const http = setupHttp('/foo');
36-
const sessionExpired = new SessionExpired(http.basePath, tenant);
36+
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
3737
const logoutPromise = new Promise(resolve => {
3838
jest.spyOn(sessionExpired, 'logout').mockImplementation(() => resolve());
3939
});
@@ -59,7 +59,7 @@ it(`ignores anonymous paths`, async () => {
5959
const http = setupHttp('/foo');
6060
const { anonymousPaths } = http;
6161
anonymousPaths.register('/bar');
62-
const sessionExpired = new SessionExpired(http.basePath, tenant);
62+
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
6363
const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, anonymousPaths);
6464
http.intercept(interceptor);
6565
fetchMock.mock('*', 401);
@@ -70,7 +70,7 @@ it(`ignores anonymous paths`, async () => {
7070

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

8282
it(`ignores requests which omit credentials`, async () => {
8383
const http = setupHttp('/foo');
84-
const sessionExpired = new SessionExpired(http.basePath, tenant);
84+
const sessionExpired = new SessionExpired(`${http.basePath}/logout`, tenant);
8585
const interceptor = new UnauthorizedResponseHttpInterceptor(sessionExpired, http.anonymousPaths);
8686
http.intercept(interceptor);
8787
fetchMock.mock('*', 401);

0 commit comments

Comments
 (0)