Pass session down so we can avoid a network call (#181570)

We don't need to fetch the user details when the Auth Session already has the info we need. This PR removes that extraneous web request.

I've also changed the `User-Agent` that we pass to be more specific so we can narrow down any future issues with spamming web requests.

Fixes #173645
This commit is contained in:
Tyler James Leonhardt 2023-05-04 11:59:25 -07:00 committed by GitHub
parent e4bb8fa827
commit f9b4b4c6a3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 27 deletions

View file

@ -110,7 +110,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
// We only want to fire a telemetry if we haven't seen this account yet in this session.
if (!this._accountsSeen.has(session.account.id)) {
this._accountsSeen.add(session.account.id);
this._githubServer.sendAdditionalTelemetryInfo(session.accessToken);
this._githubServer.sendAdditionalTelemetryInfo(session);
}
}

View file

@ -31,7 +31,7 @@ const REDIRECT_URL_INSIDERS = 'https://insiders.vscode.dev/redirect';
export interface IGitHubServer {
login(scopes: string): Promise<string>;
getUserInfo(token: string): Promise<{ id: string; accountName: string }>;
sendAdditionalTelemetryInfo(token: string): Promise<void>;
sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise<void>;
friendlyName: string;
}
@ -48,7 +48,7 @@ async function getScopes(token: string, serverUri: vscode.Uri, logger: Log): Pro
const result = await fetching(serverUri.toString(), {
headers: {
Authorization: `token ${token}`,
'User-Agent': 'Visual-Studio-Code'
'User-Agent': `${vscode.env.appName} (${vscode.env.appHost})`
}
});
@ -510,7 +510,7 @@ export class GitHubServer implements IGitHubServer {
result = await fetching(this.getServerUri('/user').toString(), {
headers: {
Authorization: `token ${token}`,
'User-Agent': 'Visual-Studio-Code'
'User-Agent': `${vscode.env.appName} (${vscode.env.appHost})`
}
});
} catch (ex) {
@ -543,7 +543,7 @@ export class GitHubServer implements IGitHubServer {
}
}
public async sendAdditionalTelemetryInfo(token: string): Promise<void> {
public async sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise<void> {
if (!vscode.env.isTelemetryEnabled) {
return;
}
@ -554,22 +554,22 @@ export class GitHubServer implements IGitHubServer {
}
if (this._type === AuthProviderType.github) {
return await this.checkUserDetails(token);
return await this.checkUserDetails(session);
}
// GHES
await this.checkEnterpriseVersion(token);
await this.checkEnterpriseVersion(session.accessToken);
}
private async checkUserDetails(token: string): Promise<void> {
private async checkUserDetails(session: vscode.AuthenticationSession): Promise<void> {
let edu: string | undefined;
try {
const result = await fetching('https://education.github.com/api/user', {
headers: {
Authorization: `token ${token}`,
Authorization: `token ${session.accessToken}`,
'faculty-check-preview': 'true',
'User-Agent': 'Visual-Studio-Code'
'User-Agent': `${vscode.env.appName} (${vscode.env.appHost})`
}
});
@ -580,22 +580,11 @@ export class GitHubServer implements IGitHubServer {
: json.faculty
? 'faculty'
: 'none';
} else {
edu = 'unknown';
}
} catch (e) {
// No-op
}
let managed: string | undefined;
try {
const user = await this.getUserInfo(token);
// Apparently, this is how you tell if a user is an EMU...
managed = user.accountName.includes('_') ? 'true' : 'false';
} catch (e) {
// No-op
}
if (edu === undefined && managed === undefined) {
return;
edu = 'unknown';
}
/* __GDPR__
@ -606,8 +595,9 @@ export class GitHubServer implements IGitHubServer {
}
*/
this._telemetryReporter.sendTelemetryEvent('session', {
isEdu: edu ?? 'unknown',
isManaged: managed ?? 'unknown'
isEdu: edu,
// Apparently, this is how you tell if a user is an EMU...
isManaged: session.account.label.includes('_') ? 'true' : 'false'
});
}
@ -618,7 +608,7 @@ export class GitHubServer implements IGitHubServer {
const result = await fetching(this.getServerUri('/meta').toString(), {
headers: {
Authorization: `token ${token}`,
'User-Agent': 'Visual-Studio-Code'
'User-Agent': `${vscode.env.appName} (${vscode.env.appHost})`
}
});