-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
Implement Server-Side sessions #68117
Changes from 8 commits
bfd42f0
fc0ab5d
194b06e
176fe3b
3f85d63
50a09bb
ada2b9f
a30e49b
f033f72
38f15bb
959a535
9d3273b
7133991
e915c3f
21bb805
11b3c4e
8bd08e5
3fe0d46
ccbcec1
04801a1
b6f3987
3f1fa9c
b91f050
09acb02
38f41cf
e98dbd0
a86884c
6acf005
d518e99
a8d46f1
d3c8454
bcbd9c2
fb47b91
b40adf6
9548184
f03d978
2d3ea32
8ee6b90
21d24ed
efdbef2
92437c0
d69116a
ed6944d
ca64e87
4d0ee0d
378c239
a4cdb60
b178201
3ded572
5bc9502
1d27f02
8ea6939
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { AppMount, ScopedHistory } from 'src/core/public'; | ||
import { captureURLApp } from './capture_url_app'; | ||
|
||
import { coreMock, scopedHistoryMock } from '../../../../../../src/core/public/mocks'; | ||
|
||
describe('captureURLApp', () => { | ||
beforeAll(() => { | ||
Object.defineProperty(window, 'location', { | ||
value: { href: 'https://some-host' }, | ||
writable: true, | ||
}); | ||
}); | ||
|
||
it('properly registers application', () => { | ||
const coreSetupMock = coreMock.createSetup(); | ||
|
||
captureURLApp.create(coreSetupMock); | ||
|
||
expect(coreSetupMock.http.anonymousPaths.register).toHaveBeenCalledTimes(1); | ||
expect(coreSetupMock.http.anonymousPaths.register).toHaveBeenCalledWith( | ||
'/internal/security/capture-url' | ||
); | ||
|
||
expect(coreSetupMock.application.register).toHaveBeenCalledTimes(1); | ||
|
||
const [[appRegistration]] = coreSetupMock.application.register.mock.calls; | ||
expect(appRegistration).toEqual({ | ||
id: 'security_capture_url', | ||
chromeless: true, | ||
appRoute: '/internal/security/capture-url', | ||
title: 'Capture URL', | ||
mount: expect.any(Function), | ||
}); | ||
}); | ||
|
||
it('properly handles captured URL', async () => { | ||
window.location.href = `https://host.com/mock-base-path/internal/security/capture-url?next=${encodeURIComponent( | ||
'/mock-base-path/app/home' | ||
)}&providerType=saml&providerName=saml1#/?_g=()`; | ||
|
||
const coreSetupMock = coreMock.createSetup(); | ||
coreSetupMock.http.post.mockResolvedValue({ location: '/mock-base-path/app/home#/?_g=()' }); | ||
|
||
captureURLApp.create(coreSetupMock); | ||
|
||
const [[{ mount }]] = coreSetupMock.application.register.mock.calls; | ||
await (mount as AppMount)({ | ||
element: document.createElement('div'), | ||
appBasePath: '', | ||
onAppLeave: jest.fn(), | ||
history: (scopedHistoryMock.create() as unknown) as ScopedHistory, | ||
}); | ||
|
||
expect(coreSetupMock.http.post).toHaveBeenCalledTimes(1); | ||
expect(coreSetupMock.http.post).toHaveBeenCalledWith('/internal/security/login', { | ||
body: JSON.stringify({ | ||
providerType: 'saml', | ||
providerName: 'saml1', | ||
currentURL: `https://host.com/mock-base-path/internal/security/capture-url?next=${encodeURIComponent( | ||
'/mock-base-path/app/home' | ||
)}&providerType=saml&providerName=saml1#/?_g=()`, | ||
}), | ||
}); | ||
|
||
expect(window.location.href).toBe('/mock-base-path/app/home#/?_g=()'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { parse } from 'url'; | ||
import { ApplicationSetup, FatalErrorsSetup, HttpSetup } from 'src/core/public'; | ||
|
||
interface CreateDeps { | ||
application: ApplicationSetup; | ||
http: HttpSetup; | ||
fatalErrors: FatalErrorsSetup; | ||
} | ||
|
||
/** | ||
* Some authentication providers need to know current user URL to, for example, restore it after a | ||
* complex authentication handshake. But most of the Kibana URLs include hash fragment that is never | ||
* sent to the server. To capture that authentication provider can redirect user to this app putting | ||
* path segment into the `next` query string parameter (so that it's not lost during redirect). And | ||
* since browsers preserve hash fragments during redirects (assuming redirect location doesn't | ||
* specify its own hash fragment, which is true in our case) this app can capture both path and | ||
* hash URL segments and send them back to the authentication provider via login endpoint. | ||
* | ||
* The flow can look like this: | ||
* 1. User visits `/app/kibana#/management/elasticsearch` that initiates authentication. | ||
* 2. Provider redirect user to `/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1`. | ||
* 3. Browser preserves hash segment and users ends up at `/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1#/management/elasticsearch`. | ||
* 4. The app captures full URL and sends it back as is via login endpoint: | ||
* { | ||
* providerType: 'saml', | ||
* providerName: 'saml1', | ||
* currentURL: 'https://kibana.com/internal/security/capture-url?next=%2Fapp%2Fkibana&providerType=saml&providerName=saml1#/management/elasticsearch' | ||
* } | ||
* 5. Login endpoint handler parses and validates `next` parameter, joins it with the hash segment | ||
* and finally passes it to the provider that initiated capturing. | ||
*/ | ||
export const captureURLApp = Object.freeze({ | ||
id: 'security_capture_url', | ||
create({ application, fatalErrors, http }: CreateDeps) { | ||
http.anonymousPaths.register('/internal/security/capture-url'); | ||
application.register({ | ||
id: this.id, | ||
title: 'Capture URL', | ||
chromeless: true, | ||
appRoute: '/internal/security/capture-url', | ||
async mount() { | ||
try { | ||
const { providerName, providerType } = parse(window.location.href, true).query ?? {}; | ||
if (!providerName || !providerType) { | ||
fatalErrors.add(new Error('Provider to capture URL for is not specified.')); | ||
return () => {}; | ||
} | ||
|
||
const { location } = await http.post<{ location: string }>('/internal/security/login', { | ||
body: JSON.stringify({ providerType, providerName, currentURL: window.location.href }), | ||
}); | ||
|
||
window.location.href = location; | ||
} catch (err) { | ||
fatalErrors.add(new Error('Cannot login with captured URL.')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than failing with a fatal error, what do you think about rendering a custom error page? The fatal error page gives you an option to clear your session, which doesn't actually have anything to do with your user session. I'd love to have a page that helps users fix their problem (or somehow retry), but that part might not be feasible given the number of potential causes for a failed login. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, it's kind of similar to #61232 as well. I thought we could approach them together and involve Core UI Team as well. For all cases there are basically 2 generic actions we can propose:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That seems reasonable, feel free to defer this then |
||
} | ||
|
||
return () => {}; | ||
}, | ||
}); | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export { captureURLApp } from './capture_url_app'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: there are couple of drawbacks of this approach (slower than rendering custom simple HTML and if hash isn't specified we'll have unnecessary redirect), but for now benefits outweight them for me (same approach for all providers, using core's HTTP and App services).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree!