Skip to content

Commit 9004a55

Browse files
rchiodorchiodo
authored
Support not needing a password on a jupyter connection (microsoft#12456)
* Support not needing a password on a jupyter connection * Fix unit tests
1 parent 23b9f5a commit 9004a55

File tree

2 files changed

+114
-139
lines changed

2 files changed

+114
-139
lines changed

‎src/client/datascience/jupyter/jupyterPasswordConnect.ts‎

Lines changed: 72 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,18 @@ import { Telemetry } from './../constants';
1313

1414
@injectable()
1515
export class JupyterPasswordConnect implements IJupyterPasswordConnect {
16+
private savedConnectInfo = new Map<string, Promise<IJupyterPasswordConnectInfo | undefined>>();
17+
1618
constructor(@inject(IApplicationShell) private appShell: IApplicationShell) {}
1719

1820
@captureTelemetry(Telemetry.GetPasswordAttempt)
19-
public async getPasswordConnectionInfo(
21+
public getPasswordConnectionInfo(
2022
url: string,
2123
allowUnauthorized: boolean,
2224
fetchFunction?: (url: nodeFetch.RequestInfo, init?: nodeFetch.RequestInit) => Promise<nodeFetch.Response>
2325
): Promise<IJupyterPasswordConnectInfo | undefined> {
24-
// For testing allow for our fetch function to be overridden
25-
if (!fetchFunction) {
26-
fetchFunction = nodeFetch.default;
27-
}
28-
29-
let xsrfCookie: string | undefined;
30-
let sessionCookieName: string | undefined;
31-
let sessionCookieValue: string | undefined;
32-
3326
if (!url || url.length < 1) {
34-
return undefined;
27+
return Promise.resolve(undefined);
3528
}
3629

3730
// Add on a trailing slash to our URL if it's not there already
@@ -40,31 +33,60 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
4033
newUrl = `${newUrl}/`;
4134
}
4235

43-
// Get password first
44-
let userPassword = await this.getUserPassword();
45-
46-
if (userPassword) {
47-
// First get the xsrf cookie by hitting the initial login page
48-
xsrfCookie = await this.getXSRFToken(url, allowUnauthorized, fetchFunction);
49-
50-
// Then get the session cookie by hitting that same page with the xsrftoken and the password
51-
if (xsrfCookie) {
52-
const sessionResult = await this.getSessionCookie(
53-
url,
54-
allowUnauthorized,
55-
xsrfCookie,
56-
userPassword,
57-
fetchFunction
58-
);
59-
sessionCookieName = sessionResult.sessionCookieName;
60-
sessionCookieValue = sessionResult.sessionCookieValue;
36+
// See if we already have this data. Don't need to ask for a password more than once. (This can happen in remote when listing kernels)
37+
let result = this.savedConnectInfo.get(newUrl);
38+
if (!result) {
39+
result = this.getNonCachedPasswordConnectionInfo(newUrl, allowUnauthorized, fetchFunction);
40+
this.savedConnectInfo.set(newUrl, result);
41+
}
42+
43+
return result;
44+
}
45+
46+
private async getNonCachedPasswordConnectionInfo(
47+
url: string,
48+
allowUnauthorized: boolean,
49+
fetchFunction?: (url: nodeFetch.RequestInfo, init?: nodeFetch.RequestInit) => Promise<nodeFetch.Response>
50+
) {
51+
// For testing allow for our fetch function to be overridden
52+
if (!fetchFunction) {
53+
fetchFunction = nodeFetch.default;
54+
}
55+
56+
let xsrfCookie: string | undefined;
57+
let sessionCookieName: string | undefined;
58+
let sessionCookieValue: string | undefined;
59+
60+
// First determine if we need a password. A request for the base URL with /tree? should return a 302 if we do.
61+
if (await this.needPassword(url, allowUnauthorized, fetchFunction)) {
62+
// Get password first
63+
let userPassword = await this.getUserPassword();
64+
65+
if (userPassword) {
66+
xsrfCookie = await this.getXSRFToken(url, allowUnauthorized, fetchFunction);
67+
68+
// Then get the session cookie by hitting that same page with the xsrftoken and the password
69+
if (xsrfCookie) {
70+
const sessionResult = await this.getSessionCookie(
71+
url,
72+
allowUnauthorized,
73+
xsrfCookie,
74+
userPassword,
75+
fetchFunction
76+
);
77+
sessionCookieName = sessionResult.sessionCookieName;
78+
sessionCookieValue = sessionResult.sessionCookieValue;
79+
}
80+
} else {
81+
// If userPassword is undefined or '' then the user didn't pick a password. In this case return back that we should just try to connect
82+
// like a standard connection. Might be the case where there is no token and no password
83+
return { emptyPassword: true, xsrfCookie: '', sessionCookieName: '', sessionCookieValue: '' };
6184
}
85+
userPassword = undefined;
6286
} else {
63-
// If userPassword is undefined or '' then the user didn't pick a password. In this case return back that we should just try to connect
64-
// like a standard connection. Might be the case where there is no token and no password
87+
// If no password needed, act like empty password and no cookie
6588
return { emptyPassword: true, xsrfCookie: '', sessionCookieName: '', sessionCookieValue: '' };
6689
}
67-
userPassword = undefined;
6890

6991
// If we found everything return it all back if not, undefined as partial is useless
7092
if (xsrfCookie && sessionCookieName && sessionCookieValue) {
@@ -125,6 +147,24 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
125147
return xsrfCookie;
126148
}
127149

150+
private async needPassword(
151+
url: string,
152+
allowUnauthorized: boolean,
153+
fetchFunction: (url: nodeFetch.RequestInfo, init?: nodeFetch.RequestInit) => Promise<nodeFetch.Response>
154+
): Promise<boolean> {
155+
// A jupyter server will redirect if you ask for the tree when a login is required
156+
const response = await fetchFunction(
157+
`${url}tree?`,
158+
this.addAllowUnauthorized(url, allowUnauthorized, {
159+
method: 'get',
160+
redirect: 'manual',
161+
headers: { Connection: 'keep-alive' }
162+
})
163+
);
164+
165+
return response.status !== 200;
166+
}
167+
128168
// Jupyter uses a session cookie to validate so by hitting the login page with the password we can get that cookie and use it ourselves
129169
// This workflow can be seen by running fiddler and hitting the login page with a browser
130170
// First you need a get at the login page to get the xsrf token, then you send back that token along with the password in a post

‎src/test/datascience/jupyterPasswordConnect.unit.test.ts‎

Lines changed: 42 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -24,53 +24,57 @@ suite('JupyterPasswordConnect', () => {
2424
jupyterPasswordConnect = new JupyterPasswordConnect(appShell.object);
2525
});
2626

27-
test('getPasswordConnectionInfo', async () => {
27+
function createMockSetup(secure: boolean, ok: boolean) {
2828
// Set up our fake node fetch
2929
const fetchMock: typemoq.IMock<typeof nodeFetch.default> = typemoq.Mock.ofInstance(nodeFetch.default);
3030

31+
//tslint:disable-next-line:no-http-string
32+
const rootUrl = secure ? 'https://TESTNAME:8888/' : 'http://TESTNAME:8888/';
33+
3134
// Mock our first call to get xsrf cookie
3235
const mockXsrfResponse = typemoq.Mock.ofType(nodeFetch.Response);
3336
const mockXsrfHeaders = typemoq.Mock.ofType(nodeFetch.Headers);
34-
mockXsrfHeaders
35-
.setup((mh) => mh.get('set-cookie'))
36-
.returns(() => `_xsrf=${xsrfValue}`)
37-
.verifiable(typemoq.Times.once());
38-
mockXsrfResponse
39-
.setup((mr) => mr.ok)
40-
.returns(() => true)
41-
.verifiable(typemoq.Times.once());
42-
mockXsrfResponse
43-
.setup((mr) => mr.headers)
44-
.returns(() => mockXsrfHeaders.object)
45-
.verifiable(typemoq.Times.once());
37+
mockXsrfHeaders.setup((mh) => mh.get('set-cookie')).returns(() => `_xsrf=${xsrfValue}`);
38+
mockXsrfResponse.setup((mr) => mr.ok).returns(() => ok);
39+
mockXsrfResponse.setup((mr) => mr.status).returns(() => 302);
40+
mockXsrfResponse.setup((mr) => mr.headers).returns(() => mockXsrfHeaders.object);
4641

42+
fetchMock
43+
.setup((fm) =>
44+
fm(
45+
`${rootUrl}login?`,
46+
typemoq.It.isObjectWith({
47+
method: 'get',
48+
headers: { Connection: 'keep-alive' }
49+
})
50+
)
51+
)
52+
.returns(() => Promise.resolve(mockXsrfResponse.object));
4753
fetchMock
4854
.setup((fm) =>
4955
//tslint:disable-next-line:no-http-string
50-
fm('http://TESTNAME:8888/login?', {
51-
method: 'get',
52-
redirect: 'manual',
53-
headers: { Connection: 'keep-alive' }
54-
})
56+
fm(
57+
`${rootUrl}tree?`,
58+
typemoq.It.isObjectWith({
59+
method: 'get',
60+
headers: { Connection: 'keep-alive' }
61+
})
62+
)
5563
)
56-
.returns(() => Promise.resolve(mockXsrfResponse.object))
57-
.verifiable(typemoq.Times.once());
64+
.returns(() => Promise.resolve(mockXsrfResponse.object));
65+
66+
return { fetchMock, mockXsrfHeaders, mockXsrfResponse };
67+
}
68+
69+
test('getPasswordConnectionInfo', async () => {
70+
const { fetchMock, mockXsrfHeaders, mockXsrfResponse } = createMockSetup(false, true);
5871

5972
// Mock our second call to get session cookie
6073
const mockSessionResponse = typemoq.Mock.ofType(nodeFetch.Response);
6174
const mockSessionHeaders = typemoq.Mock.ofType(nodeFetch.Headers);
62-
mockSessionHeaders
63-
.setup((mh) => mh.get('set-cookie'))
64-
.returns(() => `${sessionName}=${sessionValue}`)
65-
.verifiable(typemoq.Times.once());
66-
mockSessionResponse
67-
.setup((mr) => mr.status)
68-
.returns(() => 302)
69-
.verifiable(typemoq.Times.once());
70-
mockSessionResponse
71-
.setup((mr) => mr.headers)
72-
.returns(() => mockSessionHeaders.object)
73-
.verifiable(typemoq.Times.once());
75+
mockSessionHeaders.setup((mh) => mh.get('set-cookie')).returns(() => `${sessionName}=${sessionValue}`);
76+
mockSessionResponse.setup((mr) => mr.status).returns(() => 302);
77+
mockSessionResponse.setup((mr) => mr.headers).returns(() => mockSessionHeaders.object);
7478

7579
// typemoq doesn't love this comparison, so generalize it a bit
7680
fetchMock
@@ -88,8 +92,7 @@ suite('JupyterPasswordConnect', () => {
8892
})
8993
)
9094
)
91-
.returns(() => Promise.resolve(mockSessionResponse.object))
92-
.verifiable(typemoq.Times.once());
95+
.returns(() => Promise.resolve(mockSessionResponse.object));
9396

9497
const result = await jupyterPasswordConnect.getPasswordConnectionInfo(
9598
//tslint:disable-next-line:no-http-string
@@ -113,38 +116,7 @@ suite('JupyterPasswordConnect', () => {
113116
});
114117

115118
test('getPasswordConnectionInfo allowUnauthorized', async () => {
116-
// Set up our fake node fetch
117-
const fetchMock: typemoq.IMock<typeof nodeFetch.default> = typemoq.Mock.ofInstance(nodeFetch.default);
118-
119-
// Mock our first call to get xsrf cookie
120-
const mockXsrfResponse = typemoq.Mock.ofType(nodeFetch.Response);
121-
const mockXsrfHeaders = typemoq.Mock.ofType(nodeFetch.Headers);
122-
mockXsrfHeaders
123-
.setup((mh) => mh.get('set-cookie'))
124-
.returns(() => `_xsrf=${xsrfValue}`)
125-
.verifiable(typemoq.Times.once());
126-
mockXsrfResponse
127-
.setup((mr) => mr.ok)
128-
.returns(() => true)
129-
.verifiable(typemoq.Times.once());
130-
mockXsrfResponse
131-
.setup((mr) => mr.headers)
132-
.returns(() => mockXsrfHeaders.object)
133-
.verifiable(typemoq.Times.once());
134-
135-
//tslint:disable-next-line:no-http-string
136-
fetchMock
137-
.setup((fm) =>
138-
fm(
139-
'https://TESTNAME:8888/login?',
140-
typemoq.It.isObjectWith({
141-
method: 'get',
142-
headers: { Connection: 'keep-alive' }
143-
})
144-
)
145-
)
146-
.returns(() => Promise.resolve(mockXsrfResponse.object))
147-
.verifiable(typemoq.Times.once());
119+
const { fetchMock, mockXsrfHeaders, mockXsrfResponse } = createMockSetup(true, true);
148120

149121
// Mock our second call to get session cookie
150122
const mockSessionResponse = typemoq.Mock.ofType(nodeFetch.Response);
@@ -153,14 +125,8 @@ suite('JupyterPasswordConnect', () => {
153125
.setup((mh) => mh.get('set-cookie'))
154126
.returns(() => `${sessionName}=${sessionValue}`)
155127
.verifiable(typemoq.Times.once());
156-
mockSessionResponse
157-
.setup((mr) => mr.status)
158-
.returns(() => 302)
159-
.verifiable(typemoq.Times.once());
160-
mockSessionResponse
161-
.setup((mr) => mr.headers)
162-
.returns(() => mockSessionHeaders.object)
163-
.verifiable(typemoq.Times.once());
128+
mockSessionResponse.setup((mr) => mr.status).returns(() => 302);
129+
mockSessionResponse.setup((mr) => mr.headers).returns(() => mockSessionHeaders.object);
164130

165131
// typemoq doesn't love this comparison, so generalize it a bit
166132
//tslint:disable-next-line:no-http-string
@@ -178,8 +144,7 @@ suite('JupyterPasswordConnect', () => {
178144
})
179145
)
180146
)
181-
.returns(() => Promise.resolve(mockSessionResponse.object))
182-
.verifiable(typemoq.Times.once());
147+
.returns(() => Promise.resolve(mockSessionResponse.object));
183148

184149
//tslint:disable-next-line:no-http-string
185150
const result = await jupyterPasswordConnect.getPasswordConnectionInfo(
@@ -203,37 +168,7 @@ suite('JupyterPasswordConnect', () => {
203168
});
204169

205170
test('getPasswordConnectionInfo failure', async () => {
206-
// Set up our fake node fetch
207-
const fetchMock: typemoq.IMock<typeof nodeFetch.default> = typemoq.Mock.ofInstance(nodeFetch.default);
208-
209-
// Mock our first call to get xsrf cookie
210-
const mockXsrfResponse = typemoq.Mock.ofType(nodeFetch.Response);
211-
const mockXsrfHeaders = typemoq.Mock.ofType(nodeFetch.Headers);
212-
mockXsrfHeaders
213-
.setup((mh) => mh.get('set-cookie'))
214-
.returns(() => `_xsrf=${xsrfValue}`)
215-
.verifiable(typemoq.Times.never());
216-
// Status set to not ok and header fetch should not be hit
217-
mockXsrfResponse
218-
.setup((mr) => mr.ok)
219-
.returns(() => false)
220-
.verifiable(typemoq.Times.once());
221-
mockXsrfResponse
222-
.setup((mr) => mr.headers)
223-
.returns(() => mockXsrfHeaders.object)
224-
.verifiable(typemoq.Times.never());
225-
226-
fetchMock
227-
.setup((fm) =>
228-
//tslint:disable-next-line:no-http-string
229-
fm('http://TESTNAME:8888/login?', {
230-
method: 'get',
231-
redirect: 'manual',
232-
headers: { Connection: 'keep-alive' }
233-
})
234-
)
235-
.returns(() => Promise.resolve(mockXsrfResponse.object))
236-
.verifiable(typemoq.Times.once());
171+
const { fetchMock, mockXsrfHeaders, mockXsrfResponse } = createMockSetup(false, false);
237172

238173
const result = await jupyterPasswordConnect.getPasswordConnectionInfo(
239174
//tslint:disable-next-line:no-http-string

0 commit comments

Comments
 (0)