Skip to content

Commit 10229e5

Browse files
TylerLeonhardtTylerLeonhardt
authored
Add some more logging for MCP Auth (microsoft#285770)
* Add some more logging for MCP Auth * Change the log level so users can self-service configuration issues better * Add a telemetry event
1 parent fee65f1 commit 10229e5

File tree

5 files changed

+144
-38
lines changed

5 files changed

+144
-38
lines changed

‎src/vs/base/common/oauth.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,7 +1112,7 @@ export async function fetchResourceMetadata(
11121112
targetResource: string,
11131113
resourceMetadataUrl: string | undefined,
11141114
options: IFetchResourceMetadataOptions = {}
1115-
): Promise<{ metadata: IAuthorizationProtectedResourceMetadata; errors: Error[] }> {
1115+
): Promise<{ metadata: IAuthorizationProtectedResourceMetadata; discoveryUrl: string; errors: Error[] }> {
11161116
const {
11171117
sameOriginHeaders = {},
11181118
fetch: fetchImpl = fetch
@@ -1164,7 +1164,7 @@ export async function fetchResourceMetadata(
11641164
if (resourceMetadataUrl) {
11651165
try {
11661166
const metadata = await fetchPrm(resourceMetadataUrl, targetResource);
1167-
return { metadata, errors };
1167+
return { metadata, discoveryUrl: resourceMetadataUrl, errors };
11681168
} catch (e) {
11691169
errors.push(e instanceof Error ? e : new Error(String(e)));
11701170
}
@@ -1178,7 +1178,7 @@ export async function fetchResourceMetadata(
11781178
const pathAppendedUrl = `${rootUrl}${targetResourceUrlObj.pathname}`;
11791179
try {
11801180
const metadata = await fetchPrm(pathAppendedUrl, targetResource);
1181-
return { metadata, errors };
1181+
return { metadata, discoveryUrl: pathAppendedUrl, errors };
11821182
} catch (e) {
11831183
errors.push(e instanceof Error ? e : new Error(String(e)));
11841184
}
@@ -1187,7 +1187,7 @@ export async function fetchResourceMetadata(
11871187
// Finally, try root discovery
11881188
try {
11891189
const metadata = await fetchPrm(rootUrl, targetResourceUrlObj.origin);
1190-
return { metadata, errors };
1190+
return { metadata, discoveryUrl: rootUrl, errors };
11911191
} catch (e) {
11921192
errors.push(e instanceof Error ? e : new Error(String(e)));
11931193
}
@@ -1261,7 +1261,7 @@ async function getErrText(res: CommonResponse): Promise<string> {
12611261
export async function fetchAuthorizationServerMetadata(
12621262
authorizationServer: string,
12631263
options: IFetchAuthorizationServerMetadataOptions = {}
1264-
): Promise<IAuthorizationServerMetadata> {
1264+
): Promise<{ metadata: IAuthorizationServerMetadata; discoveryUrl: string; errors: Error[] }> {
12651265
const {
12661266
additionalHeaders = {},
12671267
fetch: fetchImpl = fetch
@@ -1301,7 +1301,7 @@ export async function fetchAuthorizationServerMetadata(
13011301
const pathToFetch = new URL(AUTH_SERVER_METADATA_DISCOVERY_PATH, authorizationServer).toString() + extraPath;
13021302
let metadata = await doFetch(pathToFetch);
13031303
if (metadata) {
1304-
return metadata;
1304+
return { metadata, discoveryUrl: pathToFetch, errors };
13051305
}
13061306

13071307
// Try fetching the OpenID Connect Discovery with path insertion.
@@ -1310,7 +1310,7 @@ export async function fetchAuthorizationServerMetadata(
13101310
const openidPathInsertionUrl = new URL(OPENID_CONNECT_DISCOVERY_PATH, authorizationServer).toString() + extraPath;
13111311
metadata = await doFetch(openidPathInsertionUrl);
13121312
if (metadata) {
1313-
return metadata;
1313+
return { metadata, discoveryUrl: openidPathInsertionUrl, errors };
13141314
}
13151315

13161316
// Try fetching the other discovery URL. For the openid metadata discovery
@@ -1321,7 +1321,7 @@ export async function fetchAuthorizationServerMetadata(
13211321
: authorizationServer + OPENID_CONNECT_DISCOVERY_PATH;
13221322
metadata = await doFetch(openidPathAdditionUrl);
13231323
if (metadata) {
1324-
return metadata;
1324+
return { metadata, discoveryUrl: openidPathAdditionUrl, errors };
13251325
}
13261326

13271327
// If we've tried all URLs and none worked, throw the error(s)

‎src/vs/base/test/common/oauth.test.ts‎

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,8 @@ suite('OAuth', () => {
880880
);
881881

882882
assert.deepStrictEqual(result.metadata, expectedMetadata);
883+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
884+
assert.deepStrictEqual(result.errors, []);
883885
assert.strictEqual(fetchStub.callCount, 1);
884886
assert.strictEqual(fetchStub.firstCall.args[0], resourceMetadataUrl);
885887
assert.strictEqual(fetchStub.firstCall.args[1].method, 'GET');
@@ -903,12 +905,13 @@ suite('OAuth', () => {
903905
text: async () => JSON.stringify(expectedMetadata)
904906
});
905907

906-
await fetchResourceMetadata(
908+
const result = await fetchResourceMetadata(
907909
targetResource,
908910
resourceMetadataUrl,
909911
{ fetch: fetchStub, sameOriginHeaders }
910912
);
911913

914+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
912915
const headers = fetchStub.firstCall.args[1].headers;
913916
assert.strictEqual(headers['Accept'], 'application/json');
914917
assert.strictEqual(headers['X-Test-Header'], 'test-value');
@@ -931,12 +934,13 @@ suite('OAuth', () => {
931934
text: async () => JSON.stringify(expectedMetadata)
932935
});
933936

934-
await fetchResourceMetadata(
937+
const result = await fetchResourceMetadata(
935938
targetResource,
936939
resourceMetadataUrl,
937940
{ fetch: fetchStub, sameOriginHeaders }
938941
);
939942

943+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
940944
const headers = fetchStub.firstCall.args[1].headers;
941945
assert.strictEqual(headers['Accept'], 'application/json');
942946
assert.strictEqual(headers['X-Test-Header'], undefined);
@@ -1024,6 +1028,7 @@ suite('OAuth', () => {
10241028
// URL normalization should handle hostname case differences
10251029
const result = await fetchResourceMetadata(targetResource, resourceMetadataUrl, { fetch: fetchStub });
10261030
assert.deepStrictEqual(result.metadata, metadata);
1031+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
10271032
});
10281033

10291034
test('should throw error when response is not valid resource metadata', async () => {
@@ -1116,6 +1121,7 @@ suite('OAuth', () => {
11161121
const result = await fetchResourceMetadata(targetResource, resourceMetadataUrl);
11171122

11181123
assert.deepStrictEqual(result.metadata, metadata);
1124+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
11191125
assert.strictEqual(globalFetchStub.callCount, 1);
11201126
});
11211127

@@ -1135,12 +1141,13 @@ suite('OAuth', () => {
11351141
text: async () => JSON.stringify(metadata)
11361142
});
11371143

1138-
await fetchResourceMetadata(
1144+
const result = await fetchResourceMetadata(
11391145
targetResource,
11401146
resourceMetadataUrl,
11411147
{ fetch: fetchStub, sameOriginHeaders }
11421148
);
11431149

1150+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
11441151
// Different ports mean different origins
11451152
const headers = fetchStub.firstCall.args[1].headers;
11461153
assert.strictEqual(headers['X-Test-Header'], undefined);
@@ -1162,12 +1169,13 @@ suite('OAuth', () => {
11621169
text: async () => JSON.stringify(metadata)
11631170
});
11641171

1165-
await fetchResourceMetadata(
1172+
const result = await fetchResourceMetadata(
11661173
targetResource,
11671174
resourceMetadataUrl,
11681175
{ fetch: fetchStub, sameOriginHeaders }
11691176
);
11701177

1178+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
11711179
// Different protocols mean different origins
11721180
const headers = fetchStub.firstCall.args[1].headers;
11731181
assert.strictEqual(headers['X-Test-Header'], undefined);
@@ -1219,6 +1227,7 @@ suite('OAuth', () => {
12191227
);
12201228

12211229
assert.deepStrictEqual(result.metadata, expectedMetadata);
1230+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource/api/v1');
12221231
assert.strictEqual(fetchStub.callCount, 1);
12231232
// Should try path-appended version first
12241233
assert.strictEqual(fetchStub.firstCall.args[0], 'https://example.com/.well-known/oauth-protected-resource/api/v1');
@@ -1251,6 +1260,8 @@ suite('OAuth', () => {
12511260
);
12521261

12531262
assert.deepStrictEqual(result.metadata, expectedMetadata);
1263+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource');
1264+
assert.strictEqual(result.errors.length, 1);
12541265
assert.strictEqual(fetchStub.callCount, 2);
12551266
// First attempt with path
12561267
assert.strictEqual(fetchStub.firstCall.args[0], 'https://example.com/.well-known/oauth-protected-resource/api/v1');
@@ -1299,6 +1310,7 @@ suite('OAuth', () => {
12991310
);
13001311

13011312
assert.deepStrictEqual(result.metadata, expectedMetadata);
1313+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource');
13021314
assert.strictEqual(fetchStub.callCount, 1);
13031315
// Both URLs should be the same when path is /
13041316
assert.strictEqual(fetchStub.firstCall.args[0], 'https://example.com/.well-known/oauth-protected-resource');
@@ -1320,12 +1332,13 @@ suite('OAuth', () => {
13201332
text: async () => JSON.stringify(expectedMetadata)
13211333
});
13221334

1323-
await fetchResourceMetadata(
1335+
const result = await fetchResourceMetadata(
13241336
targetResource,
13251337
undefined,
13261338
{ fetch: fetchStub, sameOriginHeaders }
13271339
);
13281340

1341+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource/api');
13291342
const headers = fetchStub.firstCall.args[1].headers;
13301343
assert.strictEqual(headers['Accept'], 'application/json');
13311344
assert.strictEqual(headers['X-Test-Header'], 'test-value');
@@ -1355,6 +1368,9 @@ suite('OAuth', () => {
13551368
);
13561369

13571370
assert.deepStrictEqual(result.metadata, expectedMetadata);
1371+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource');
1372+
assert.strictEqual(result.errors.length, 1);
1373+
assert.ok(/Network connection failed/.test(result.errors[0].message));
13581374
assert.strictEqual(fetchStub.callCount, 2);
13591375
// First attempt with path should have thrown error
13601376
assert.strictEqual(fetchStub.firstCall.args[0], 'https://example.com/.well-known/oauth-protected-resource/api/v1');
@@ -1532,6 +1548,8 @@ suite('OAuth', () => {
15321548
const result = await fetchResourceMetadata(targetResource, undefined, { fetch: fetchStub });
15331549

15341550
assert.deepStrictEqual(result.metadata, invalidMetadata);
1551+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource');
1552+
assert.strictEqual(result.errors.length, 1);
15351553
assert.strictEqual(fetchStub.callCount, 2);
15361554
// Verify both URLs were tried
15371555
assert.strictEqual(fetchStub.firstCall.args[0], 'https://example.com/.well-known/oauth-protected-resource/api/v1');
@@ -1560,6 +1578,7 @@ suite('OAuth', () => {
15601578
);
15611579

15621580
assert.deepStrictEqual(result.metadata, validMetadata);
1581+
assert.strictEqual(result.discoveryUrl, resourceMetadataUrl);
15631582
assert.strictEqual(fetchStub.callCount, 1);
15641583
assert.strictEqual(fetchStub.firstCall.args[0], resourceMetadataUrl);
15651584
});
@@ -1583,6 +1602,8 @@ suite('OAuth', () => {
15831602
// Should succeed on root discovery fallback
15841603
const result = await fetchResourceMetadata(targetResource, resourceMetadataUrl, { fetch: fetchStub });
15851604
assert.deepStrictEqual(result.metadata, invalidMetadata);
1605+
assert.strictEqual(result.discoveryUrl, 'https://example.com/.well-known/oauth-protected-resource');
1606+
assert.ok(result.errors.length >= 1);
15861607
// Should have tried explicit URL, path-appended, then succeeded on root
15871608
assert.ok(fetchStub.callCount >= 2);
15881609
});
@@ -1639,7 +1660,9 @@ suite('OAuth', () => {
16391660

16401661
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
16411662

1642-
assert.deepStrictEqual(result, expectedMetadata);
1663+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1664+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
1665+
assert.deepStrictEqual(result.errors, []);
16431666
assert.strictEqual(fetchStub.callCount, 1);
16441667
// Should try OAuth discovery with path insertion: https://auth.example.com/.well-known/oauth-authorization-server/tenant
16451668
assert.strictEqual(fetchStub.firstCall.args[0], 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
@@ -1672,7 +1695,9 @@ suite('OAuth', () => {
16721695

16731696
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
16741697

1675-
assert.deepStrictEqual(result, expectedMetadata);
1698+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1699+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/openid-configuration/tenant');
1700+
assert.strictEqual(result.errors.length, 1);
16761701
assert.strictEqual(fetchStub.callCount, 2);
16771702
// First attempt: OAuth discovery
16781703
assert.strictEqual(fetchStub.firstCall.args[0], 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
@@ -1713,7 +1738,9 @@ suite('OAuth', () => {
17131738

17141739
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
17151740

1716-
assert.deepStrictEqual(result, expectedMetadata);
1741+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1742+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/tenant/.well-known/openid-configuration');
1743+
assert.strictEqual(result.errors.length, 2);
17171744
assert.strictEqual(fetchStub.callCount, 3);
17181745
// First attempt: OAuth discovery
17191746
assert.strictEqual(fetchStub.firstCall.args[0], 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
@@ -1741,7 +1768,9 @@ suite('OAuth', () => {
17411768

17421769
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
17431770

1744-
assert.deepStrictEqual(result, expectedMetadata);
1771+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1772+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server');
1773+
assert.deepStrictEqual(result.errors, []);
17451774
assert.strictEqual(fetchStub.callCount, 1);
17461775
// For root URLs, no extra path is added
17471776
assert.strictEqual(fetchStub.firstCall.args[0], 'https://auth.example.com/.well-known/oauth-authorization-server');
@@ -1765,7 +1794,9 @@ suite('OAuth', () => {
17651794

17661795
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
17671796

1768-
assert.deepStrictEqual(result, expectedMetadata);
1797+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1798+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server/tenant/');
1799+
assert.deepStrictEqual(result.errors, []);
17691800
assert.strictEqual(fetchStub.callCount, 1);
17701801
});
17711802

@@ -1787,8 +1818,9 @@ suite('OAuth', () => {
17871818
statusText: 'OK'
17881819
});
17891820

1790-
await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub, additionalHeaders });
1821+
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub, additionalHeaders });
17911822

1823+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
17921824
const headers = fetchStub.firstCall.args[1].headers;
17931825
assert.strictEqual(headers['X-Custom-Header'], 'custom-value');
17941826
assert.strictEqual(headers['Authorization'], 'Bearer token123');
@@ -1847,7 +1879,8 @@ suite('OAuth', () => {
18471879

18481880
// Should succeed on second attempt
18491881
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
1850-
assert.deepStrictEqual(result, expectedMetadata);
1882+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1883+
assert.strictEqual(result.errors.length, 1);
18511884
assert.strictEqual(fetchStub.callCount, 2);
18521885
});
18531886

@@ -1944,7 +1977,9 @@ suite('OAuth', () => {
19441977

19451978
const result = await fetchAuthorizationServerMetadata(authorizationServer);
19461979

1947-
assert.deepStrictEqual(result, expectedMetadata);
1980+
assert.deepStrictEqual(result.metadata, expectedMetadata);
1981+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server');
1982+
assert.deepStrictEqual(result.errors, []);
19481983
assert.strictEqual(globalFetchStub.callCount, 1);
19491984
});
19501985

@@ -1966,7 +2001,9 @@ suite('OAuth', () => {
19662001

19672002
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
19682003

1969-
assert.deepStrictEqual(result, expectedMetadata);
2004+
assert.deepStrictEqual(result.metadata, expectedMetadata);
2005+
assert.strictEqual(result.errors.length, 1);
2006+
assert.ok(/Network error/.test(result.errors[0].message));
19702007
// Should have tried two endpoints
19712008
assert.strictEqual(fetchStub.callCount, 2);
19722009
});
@@ -2022,7 +2059,8 @@ suite('OAuth', () => {
20222059

20232060
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
20242061

2025-
assert.deepStrictEqual(result, expectedMetadata);
2062+
assert.deepStrictEqual(result.metadata, expectedMetadata);
2063+
assert.strictEqual(result.errors.length, 2);
20262064
// Should have tried all three endpoints
20272065
assert.strictEqual(fetchStub.callCount, 3);
20282066
});
@@ -2082,7 +2120,9 @@ suite('OAuth', () => {
20822120

20832121
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
20842122

2085-
assert.deepStrictEqual(result, expectedMetadata);
2123+
assert.deepStrictEqual(result.metadata, expectedMetadata);
2124+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/tenant/.well-known/openid-configuration');
2125+
assert.strictEqual(result.errors.length, 2);
20862126
assert.strictEqual(fetchStub.callCount, 3);
20872127
// Third attempt should correctly handle trailing slash (not double-slash)
20882128
assert.strictEqual(fetchStub.thirdCall.args[0], 'https://auth.example.com/tenant/.well-known/openid-configuration');
@@ -2104,7 +2144,9 @@ suite('OAuth', () => {
21042144

21052145
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
21062146

2107-
assert.deepStrictEqual(result, expectedMetadata);
2147+
assert.deepStrictEqual(result.metadata, expectedMetadata);
2148+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server/tenant/org/sub');
2149+
assert.deepStrictEqual(result.errors, []);
21082150
assert.strictEqual(fetchStub.callCount, 1);
21092151
// Should correctly insert well-known path with nested paths
21102152
assert.strictEqual(fetchStub.firstCall.args[0], 'https://auth.example.com/.well-known/oauth-authorization-server/tenant/org/sub');
@@ -2162,7 +2204,9 @@ suite('OAuth', () => {
21622204

21632205
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
21642206

2165-
assert.deepStrictEqual(result, validMetadata);
2207+
assert.deepStrictEqual(result.metadata, validMetadata);
2208+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server');
2209+
assert.deepStrictEqual(result.errors, []);
21662210
assert.strictEqual(fetchStub.callCount, 1);
21672211
});
21682212

@@ -2182,7 +2226,10 @@ suite('OAuth', () => {
21822226

21832227
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub });
21842228

2185-
assert.deepStrictEqual(result, expectedMetadata);
2229+
assert.deepStrictEqual(result.metadata, expectedMetadata);
2230+
// Query parameters are not included in the discovery URL (only pathname is extracted)
2231+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server/tenant');
2232+
assert.deepStrictEqual(result.errors, []);
21862233
assert.strictEqual(fetchStub.callCount, 1);
21872234
});
21882235

@@ -2200,8 +2247,9 @@ suite('OAuth', () => {
22002247
statusText: 'OK'
22012248
});
22022249

2203-
await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub, additionalHeaders: {} });
2250+
const result = await fetchAuthorizationServerMetadata(authorizationServer, { fetch: fetchStub, additionalHeaders: {} });
22042251

2252+
assert.strictEqual(result.discoveryUrl, 'https://auth.example.com/.well-known/oauth-authorization-server');
22052253
const headers = fetchStub.firstCall.args[1].headers;
22062254
assert.strictEqual(headers['Accept'], 'application/json');
22072255
});

0 commit comments

Comments
 (0)