development/vscode
development/vscode
1
0
Fork 0
mirror of https://github.com/Microsoft/vscode synced 2024-07-17 02:57:19 +00:00
Code Issues Actions 9 Packages Projects Releases Wiki Activity

Use email for label & use label to group results in Account menu (#193727)

Browse source 
So, when you make a new session in the Microsoft Identity stack, depending on the scopes you pass in you might get:
* A token with a name & email
* A token with just a name
Additionally, Microsoft has like 3-4 concepts of essentially an "id" but depending on what you're trying to access, you might get a different value.

This historical behavior leads to 2 awkward things:
1. The account menu shows two accounts, one with name & email, one with just email.
2. The account menu shows two of the same accounts, but their underlying id is different

So, to fix this, we're just gonna lean on the labels. In other words, if there are two accounts that share the same label, then they will be grouped together.

The previous behavior was hurting the Azure Account folks and the Q# folks and with this change, I believe they both should be happy.

Interestingly enough, when I inherited this code, it use to do this, but I changed it to use the id as that seemed "more correct"... so it a way, this is reverting a change I did a while back.

Fixes https://github.com/microsoft/vscode/issues/184218
This commit is contained in:
Tyler James Leonhardt 2023-09-21 14:09:25 -07:00 committed by GitHub
parent f5e97242d0
commit d5c2817e69
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
extensions/microsoft-authentication/src/AADHelper.ts
View file

@ -524,13 +524,6 @@ export class AzureActiveDirectoryService {
throw e;
}
let label;
if (claims.name && claims.email) {
label = `${claims.name} - ${claims.email}`;
} else {
label = claims.email ?? claims.unique_name ?? claims.preferred_username ?? 'user@example.com';
}
const id = `${claims.tid}/${(claims.oid ?? (claims.altsecid ?? '' + claims.ipd ?? ''))}`;
const sessionId = existingId || `${id}/${randomUUID()}`;
this._logger.trace(`[${scopeData.scopeStr}] '${sessionId}' Token response parsed successfully.`);
@ -543,7 +536,7 @@ export class AzureActiveDirectoryService {
scope: scopeData.scopeStr,
sessionId,
account: {
label,
label: claims.email ?? claims.preferred_username ?? claims.unique_name ?? 'user@example.com',
id,
type: claims.tid === MSA_TID || claims.tid === MSA_PASSTHRU_TID ? MicrosoftAccountType.MSA : MicrosoftAccountType.AAD
}

45
src/vs/workbench/browser/parts/activitybar/activitybarActions.ts
View file

@ -351,7 +351,7 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
if (account.canSignOut) {
const signOutAction = disposables.add(new Action('signOut', localize('signOut', "Sign Out"), undefined, true, async () => {
const allSessions = await this.authenticationService.getSessions(providerId);
const sessionsForAccount = allSessions.filter(s => s.account.id === account.id);
const sessionsForAccount = allSessions.filter(s => s.account.label === account.label);
return await this.authenticationService.removeAccountSessions(providerId, account.label, sessionsForAccount);
}));
providerSubMenuActions.push(signOutAction);
@ -399,34 +399,35 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
private async addOrUpdateAccount(providerId: string, account: AuthenticationSessionAccount): Promise<void> {
let accounts = this.groupedAccounts.get(providerId);
if (accounts) {
const existingAccount = accounts.find(a => a.id === account.id);
if (existingAccount) {
// Update the label if it has changed
if (existingAccount.label !== account.label) {
existingAccount.label = account.label;
}
return;
}
} else {
if (!accounts) {
accounts = [];
this.groupedAccounts.set(providerId, accounts);
}
const sessionFromEmbedder = await this.sessionFromEmbedder.value;
// If the session stored from the embedder allows sign out, then we can treat it and all others as sign out-able
let canSignOut = !!sessionFromEmbedder?.canSignOut;
if (!canSignOut) {
if (sessionFromEmbedder?.id) {
const sessions = (await this.authenticationService.getSessions(providerId)).filter(s => s.account.id === account.id);
canSignOut = !sessions.some(s => s.id === sessionFromEmbedder.id);
} else {
// The default if we don't have a session from the embedder is to allow sign out
canSignOut = true;
}
let canSignOut = true;
if (
sessionFromEmbedder // if we have a session from the embedder
&& !sessionFromEmbedder.canSignOut // and that session says we can't sign out
&& (await this.authenticationService.getSessions(providerId)) // and that session is associated with the account we are adding/updating
.some(s =>
s.id === sessionFromEmbedder.id
&& s.account.id === account.id
)
) {
canSignOut = false;
}
accounts.push({ ...account, canSignOut });
const existingAccount = accounts.find(a => a.label === account.label);
if (existingAccount) {
// if we have an existing account and we discover that we
// can't sign out of it, update the account to mark it as "can't sign out"
if (!canSignOut) {
existingAccount.canSignOut = canSignOut;
}
} else {
accounts.push({ ...account, canSignOut });
}
}
private removeAccount(providerId: string, account: AuthenticationSessionAccount): void {