Skip to content

Commit 6add2d6

Browse files
sdzh-atlassiansdzh-atlassian
committedMar 5, 2025
Revert "[AXON-17] (WIP) fix weird site list behaviour"
This reverts commit 94a4c7b.
1 parent 7a6cfbf commit 6add2d6

File tree

3 files changed

+42
-38
lines changed

3 files changed

+42
-38
lines changed
 

‎src/atlclients/authInfo.ts

-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export interface RemoveAuthInfoEvent extends AuthInfoEvent {
1919
type: AuthChangeType.Remove;
2020
product: Product;
2121
credentialId: string;
22-
host: string;
2322
}
2423

2524
export interface Product {
@@ -122,10 +121,6 @@ export interface DetailedSiteInfo extends SiteInfo {
122121
credentialId: string;
123122
}
124123

125-
export function getSiteInfoKey(site: DetailedSiteInfo): string {
126-
return `${site.product.key} - ${site.host} - ${site.credentialId}`;
127-
}
128-
129124
// You MUST send source
130125
// You SHOULD send both AAID and Anonymous ID when available (if only one is available, send that)
131126
// Anonymous ID should match the ID sent to amplitude for analytics events

‎src/atlclients/authStore.ts

+41-30
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
UpdateAuthInfoEvent,
1313
emptyAuthInfo,
1414
getSecretForAuthInfo,
15-
getSiteInfoKey,
1615
isOAuthInfo,
1716
oauthProviderForSite,
1817
} from './authInfo';
@@ -92,7 +91,7 @@ export class CredentialManager implements Disposable {
9291
}
9392
}
9493

95-
this._memStore.set(site.product.key, productAuths.set(getSiteInfoKey(site), info));
94+
this._memStore.set(site.product.key, productAuths.set(site.credentialId, info));
9695

9796
const hasNewInfo =
9897
!existingInfo ||
@@ -108,7 +107,7 @@ export class CredentialManager implements Disposable {
108107
}
109108

110109
try {
111-
this.addSiteInformationToSecretStorage(site, info);
110+
this.addSiteInformationToSecretStorage(site.product.key, site.credentialId, info);
112111
const updateEvent: UpdateAuthInfoEvent = { type: AuthChangeType.Update, site: site };
113112
this._onDidAuthChange.fire(updateEvent);
114113
} catch (e) {
@@ -125,8 +124,8 @@ export class CredentialManager implements Disposable {
125124
let foundInfo: AuthInfo | undefined = undefined;
126125
const productAuths = this._memStore.get(site.product.key);
127126

128-
if (allowCache && productAuths && productAuths.has(getSiteInfoKey(site))) {
129-
foundInfo = productAuths.get(getSiteInfoKey(site));
127+
if (allowCache && productAuths && productAuths.has(site.credentialId)) {
128+
foundInfo = productAuths.get(site.credentialId);
130129
if (foundInfo) {
131130
// clone the object so editing it and saving it back doesn't trip up equality checks
132131
// in saveAuthInfo
@@ -136,20 +135,24 @@ export class CredentialManager implements Disposable {
136135

137136
if (!foundInfo) {
138137
try {
139-
let infoEntry = await this.getAuthInfoFromSecretStorage(site);
138+
let infoEntry = await this.getAuthInfoFromSecretStorage(site.product.key, site.credentialId);
140139
// if no authinfo found in secretstorage
141140
if (!infoEntry) {
142141
// we first check if keychain exists and if it does then we migrate users from keychain to secretstorage
143142
// without them having to relogin manually
144143
if (keychain) {
145-
infoEntry = await this.getAuthInfoFromKeychain(site);
144+
infoEntry = await this.getAuthInfoFromKeychain(site.product.key, site.credentialId);
146145
if (infoEntry) {
147146
Logger.debug(
148147
`adding info from keychain to secretstorage for product: ${site.product.key} credentialID: ${site.credentialId}`,
149148
);
150-
await this.addSiteInformationToSecretStorage(site, infoEntry);
149+
await this.addSiteInformationToSecretStorage(
150+
site.product.key,
151+
site.credentialId,
152+
infoEntry,
153+
);
151154
// Once authinfo has been stored in the secretstorage, info in keychain is no longer needed so removing it
152-
await this.removeSiteInformationFromKeychain(site);
155+
await this.removeSiteInformationFromKeychain(site.product.key, site.credentialId);
153156
} else if (Container.siteManager.getSiteForId(site.product, site.id)) {
154157
// if keychain does not have any auth info for the current site but the site has been saved, we need to remove it
155158
Logger.debug(
@@ -176,7 +179,7 @@ export class CredentialManager implements Disposable {
176179
}
177180
}
178181
if (infoEntry && productAuths) {
179-
this._memStore.set(site.product.key, productAuths.set(getSiteInfoKey(site), infoEntry));
182+
this._memStore.set(site.product.key, productAuths.set(site.credentialId, infoEntry));
180183

181184
foundInfo = infoEntry;
182185
}
@@ -206,48 +209,54 @@ export class CredentialManager implements Disposable {
206209
}
207210
}
208211

209-
private async addSiteInformationToSecretStorage(site: DetailedSiteInfo, info: AuthInfo) {
212+
private async addSiteInformationToSecretStorage(productKey: string, credentialId: string, info: AuthInfo) {
210213
await this._queue.add(
211214
async () => {
212215
try {
213-
await Container.context.secrets.store(getSiteInfoKey(site), JSON.stringify(info));
216+
await Container.context.secrets.store(`${productKey}-${credentialId}`, JSON.stringify(info));
214217
} catch (e) {
215218
Logger.error(e, `Error writing to secretstorage`);
216219
}
217220
},
218221
{ priority: Priority.Write },
219222
);
220223
}
221-
private async getSiteInformationFromSecretStorage(site: DetailedSiteInfo): Promise<string | undefined> {
224+
private async getSiteInformationFromSecretStorage(
225+
productKey: string,
226+
credentialId: string,
227+
): Promise<string | undefined> {
222228
let info: string | undefined = undefined;
223229
await this._queue.add(
224230
async () => {
225-
info = await Container.context.secrets.get(getSiteInfoKey(site));
231+
info = await Container.context.secrets.get(`${productKey}-${credentialId}`);
226232
},
227233
{ priority: Priority.Read },
228234
);
229235
return info;
230236
}
231-
private async removeSiteInformationFromSecretStorage(site: DetailedSiteInfo): Promise<boolean> {
237+
private async removeSiteInformationFromSecretStorage(productKey: string, credentialId: string): Promise<boolean> {
232238
let wasKeyDeleted = false;
233239
await this._queue.add(
234240
async () => {
235-
const storedInfo = await Container.context.secrets.get(getSiteInfoKey(site));
241+
const storedInfo = await Container.context.secrets.get(`${productKey}-${credentialId}`);
236242
if (storedInfo) {
237-
await Container.context.secrets.delete(getSiteInfoKey(site));
243+
await Container.context.secrets.delete(`${productKey}-${credentialId}`);
238244
wasKeyDeleted = true;
239245
}
240246
},
241247
{ priority: Priority.Write },
242248
);
243249
return wasKeyDeleted;
244250
}
245-
private async removeSiteInformationFromKeychain(site: DetailedSiteInfo): Promise<boolean> {
251+
private async removeSiteInformationFromKeychain(productKey: string, credentialId: string): Promise<boolean> {
246252
let wasKeyDeleted = false;
247253
await this._queue.add(
248254
async () => {
249255
if (keychain) {
250-
wasKeyDeleted = await keychain.deletePassword(keychainServiceNameV3, getSiteInfoKey(site));
256+
wasKeyDeleted = await keychain.deletePassword(
257+
keychainServiceNameV3,
258+
`${productKey}-${credentialId}`,
259+
);
251260
}
252261
},
253262
{ priority: Priority.Write },
@@ -256,14 +265,13 @@ export class CredentialManager implements Disposable {
256265
}
257266

258267
private async getAuthInfoFromSecretStorage(
259-
site: DetailedSiteInfo,
268+
productKey: string,
269+
credentialId: string,
260270
serviceName?: string,
261271
): Promise<AuthInfo | undefined> {
262-
Logger.debug(
263-
`Retrieving secretstorage info for product: ${site.product.key} credentialID: ${site.credentialId}`,
264-
);
272+
Logger.debug(`Retrieving secretstorage info for product: ${productKey} credentialID: ${credentialId}`);
265273
let authInfo: string | undefined = undefined;
266-
authInfo = await this.getSiteInformationFromSecretStorage(site);
274+
authInfo = await this.getSiteInformationFromSecretStorage(productKey, credentialId);
267275
if (!authInfo) {
268276
return undefined;
269277
}
@@ -275,8 +283,12 @@ export class CredentialManager implements Disposable {
275283
}
276284
return info;
277285
}
278-
private async getAuthInfoFromKeychain(site: DetailedSiteInfo, serviceName?: string): Promise<AuthInfo | undefined> {
279-
Logger.debug(`Retrieving keychain info for product: ${site.product.key} credentialID: ${site.credentialId}`);
286+
private async getAuthInfoFromKeychain(
287+
productKey: string,
288+
credentialId: string,
289+
serviceName?: string,
290+
): Promise<AuthInfo | undefined> {
291+
Logger.debug(`Retrieving keychain info for product: ${productKey} credentialID: ${credentialId}`);
280292
let svcName = keychainServiceNameV3;
281293

282294
if (serviceName) {
@@ -287,7 +299,7 @@ export class CredentialManager implements Disposable {
287299
await this._queue.add(
288300
async () => {
289301
if (keychain) {
290-
authInfo = await keychain.getPassword(svcName, getSiteInfoKey(site));
302+
authInfo = await keychain.getPassword(svcName, `${productKey}-${credentialId}`);
291303
}
292304
},
293305
{ priority: Priority.Read },
@@ -348,11 +360,11 @@ export class CredentialManager implements Disposable {
348360
let wasKeyDeleted = false;
349361
let wasMemDeleted = false;
350362
if (productAuths) {
351-
wasMemDeleted = productAuths.delete(getSiteInfoKey(site));
363+
wasMemDeleted = productAuths.delete(site.credentialId);
352364
this._memStore.set(site.product.key, productAuths);
353365
}
354366

355-
wasKeyDeleted = await this.removeSiteInformationFromSecretStorage(site);
367+
wasKeyDeleted = await this.removeSiteInformationFromSecretStorage(site.product.key, site.credentialId);
356368
if (wasMemDeleted || wasKeyDeleted) {
357369
const cmdctx = this.commandContextFor(site.product);
358370
if (cmdctx) {
@@ -365,7 +377,6 @@ export class CredentialManager implements Disposable {
365377
type: AuthChangeType.Remove,
366378
product: site.product,
367379
credentialId: site.credentialId,
368-
host: site.host,
369380
};
370381
this._onDidAuthChange.fire(removeEvent);
371382

‎src/siteManager.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,7 @@ export class SiteManager extends Disposable {
111111

112112
onDidAuthChange(e: AuthInfoEvent) {
113113
if (isRemoveAuthEvent(e)) {
114-
const deadSites = this.getSitesAvailable(e.product).filter(
115-
(site) => site.credentialId === e.credentialId && site.host === e.host,
116-
);
114+
const deadSites = this.getSitesAvailable(e.product).filter((site) => site.credentialId === e.credentialId);
117115
deadSites.forEach((s) => this.removeSite(s));
118116
if (deadSites.length > 0) {
119117
this._onDidSitesAvailableChange.fire({

0 commit comments

Comments
 (0)
Failed to load comments.