Skip to content

WIP Apply auth patch #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: apply-overview-patch
Choose a base branch
from
Open

WIP Apply auth patch #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d93e915
added tests for coverage
sn2912 Jun 9, 2025
a347bff
Applied auth patch
sn2912 Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
},
"contributes": {
"commands": [
{
"command": "atlascode.authenticateWithBitbucketToken",
"title": "Atlasian: Authenticate with Bitbucket Token"
},
{
"command": "atlascode.jira.todoIssue",
"title": "TO DO"
Expand Down Expand Up @@ -853,7 +857,7 @@
"properties": {
"atlascode.outputLevel": {
"type": "string",
"default": "silent",
"default": "debug",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove for upstream. This is only for devboxes.

"enum": [
"silent",
"errors",
Expand Down Expand Up @@ -1324,6 +1328,7 @@
"@material-ui/styles": "^4.11.5",
"@segment/analytics-node": "^2.1.3",
"@vscode/webview-ui-toolkit": "^1.4.0",
"async-mutex": "^0.5.0",
"awesome-debounce-promise": "^2.1.0",
"axios": "1.9.0",
"axios-curlirize": "^2.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src/atlclients/authInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export interface AuthInfo {

export interface OAuthInfo extends AuthInfo {
access: string;
refresh: string;
refresh?: string;
expirationDate?: number;
iat?: number;
recievedAt: number;
Expand Down
15 changes: 0 additions & 15 deletions src/atlclients/authStore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,21 +223,6 @@ describe('CredentialManager', () => {
site: mockJiraSite,
});
});

it("should not save to secret storage if auth info hasn't changed", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this test?

// Setup existing auth info
const memStore = (credentialManager as any)._memStore;
const jiraStore = memStore.get(ProductJira.key);
jiraStore.set(mockJiraSite.credentialId, mockAuthInfo);

// Mock getAuthInfo to return the existing info
jest.spyOn(credentialManager, 'getAuthInfo').mockResolvedValue(mockAuthInfo);

await credentialManager.saveAuthInfo(mockJiraSite, mockAuthInfo);

expect(Container.context.secrets.store).not.toHaveBeenCalled();
expect(mockFireEvent).not.toHaveBeenCalled();
});
});

describe('refreshAccessToken', () => {
Expand Down
75 changes: 62 additions & 13 deletions src/atlclients/authStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Mutex } from 'async-mutex';
import crypto from 'crypto';
import PQueue from 'p-queue';
import { Disposable, Event, EventEmitter, version, window } from 'vscode';
Expand Down Expand Up @@ -34,10 +35,15 @@ enum Priority {
Write,
}

function getKeyForSecrets(productKey: string, credentialId: string): string {
return `${productKey}-${credentialId}`;
}

export class CredentialManager implements Disposable {
private _memStore: Map<string, Map<string, AuthInfo>> = new Map<string, Map<string, AuthInfo>>();
private _queue = new PQueue({ concurrency: 1 });
private _refresher = new OAuthRefesher();
private mutex = new Mutex();

constructor(private _analyticsClient: AnalyticsClient) {
this._memStore.set(ProductJira.key, new Map<string, AuthInfo>());
Expand All @@ -59,21 +65,40 @@ export class CredentialManager implements Disposable {
* it's available, otherwise will return the value in the secretstorage.
*/
public async getAuthInfo(site: DetailedSiteInfo, allowCache = true): Promise<AuthInfo | undefined> {
return this.getAuthInfoForProductAndCredentialId(site, allowCache);
return this.mutex.runExclusive(() => {
return this.getAuthInfoForProductAndCredentialId(site, allowCache);
});
}

/**
* Saves the auth info to both the in-memory store and the secretstorage.
*/
public async saveAuthInfo(site: DetailedSiteInfo, info: AuthInfo): Promise<void> {
Logger.debug(`Saving auth info for site: ${site.baseApiUrl} credentialID: ${site.credentialId}`);
return this.mutex.runExclusive(() => {
return this.saveAuthInfoForProductAndCredentialId(site, info);
});
}

/**
* Removes an auth item from both the in-memory store and the secretstorage.
*/
public async removeAuthInfo(site: DetailedSiteInfo): Promise<boolean> {
return this.mutex.runExclusive(() => {
return this.removeAuthInfoForProductAndCredentialId(site);
});
}

/**
* Saves the auth info to both the in-memory store and the secretstorage.
*/
public async saveAuthInfoForProductAndCredentialId(site: DetailedSiteInfo, info: AuthInfo): Promise<void> {
let productAuths = this._memStore.get(site.product.key);

if (!productAuths) {
productAuths = new Map<string, AuthInfo>();
}

const existingInfo = await this.getAuthInfo(site, false);
const existingInfo = await this.getAuthInfoForProductAndCredentialId(site, false);

this._memStore.set(site.product.key, productAuths.set(site.credentialId, info));

Expand All @@ -100,6 +125,29 @@ export class CredentialManager implements Disposable {
}
}

/**
* Saves the auth info to both the in-memory store and the secretstorage for Bitbucket API key login
*/
public async saveAuthInfoBBToken(site: DetailedSiteInfo, info: AuthInfo, id: number): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used anywhere in the fork. Can be safely removed.

Logger.debug(
`[id=${id}] Saving auth info for site: ${site.baseApiUrl} credentialID: ${site.credentialId} using BB token`,
);
const productAuths = this._memStore.get(site.product.key);
Logger.debug(`[id=${id}] productAuths: ${productAuths}`);
if (!productAuths) {
this._memStore.set(site.product.key, new Map<string, AuthInfo>().set(site.credentialId, info));
Logger.debug(`[id=${id}] productAuths is empty so setting it to the new auth info in memstore`);
} else {
productAuths.set(site.credentialId, info);
this._memStore.set(site.product.key, productAuths);
Logger.debug(
`[id=${id}] productAuths is not empty so setting it to productAuth: ${productAuths}in memstore`,
);
}
Logger.debug(`[id=${id}] Calling saveAuthInfo for site: ${site.baseApiUrl} credentialID: ${site.credentialId}`);
await this.saveAuthInfo(site, info);
}

private async getAuthInfoForProductAndCredentialId(
site: DetailedSiteInfo,
allowCache: boolean,
Expand Down Expand Up @@ -202,7 +250,10 @@ export class CredentialManager implements Disposable {
await this._queue.add(
async () => {
try {
await Container.context.secrets.store(`${productKey}-${credentialId}`, JSON.stringify(info));
await Container.context.secrets.store(
getKeyForSecrets(productKey, credentialId),
JSON.stringify(info),
);
} catch (e) {
Logger.error(e, `Error writing to secretstorage`);
}
Expand All @@ -217,7 +268,7 @@ export class CredentialManager implements Disposable {
let info: string | undefined = undefined;
await this._queue.add(
async () => {
info = await Container.context.secrets.get(`${productKey}-${credentialId}`);
info = await Container.context.secrets.get(getKeyForSecrets(productKey, credentialId));
},
{ priority: Priority.Read },
);
Expand All @@ -227,9 +278,9 @@ export class CredentialManager implements Disposable {
let wasKeyDeleted = false;
await this._queue.add(
async () => {
const storedInfo = await Container.context.secrets.get(`${productKey}-${credentialId}`);
const storedInfo = await Container.context.secrets.get(getKeyForSecrets(productKey, credentialId));
if (storedInfo) {
await Container.context.secrets.delete(`${productKey}-${credentialId}`);
await Container.context.secrets.delete(getKeyForSecrets(productKey, credentialId));
wasKeyDeleted = true;
}
},
Expand All @@ -244,7 +295,7 @@ export class CredentialManager implements Disposable {
if (keychain) {
wasKeyDeleted = await keychain.deletePassword(
keychainServiceNameV3,
`${productKey}-${credentialId}`,
getKeyForSecrets(productKey, credentialId),
);
}
},
Expand All @@ -256,9 +307,7 @@ export class CredentialManager implements Disposable {
private async getAuthInfoFromSecretStorage(
productKey: string,
credentialId: string,
serviceName?: string,
): Promise<AuthInfo | undefined> {
Logger.debug(`Retrieving secretstorage info for product: ${productKey} credentialID: ${credentialId}`);
let authInfo: string | undefined = undefined;
authInfo = await this.getSiteInformationFromSecretStorage(productKey, credentialId);
if (!authInfo) {
Expand Down Expand Up @@ -288,7 +337,7 @@ export class CredentialManager implements Disposable {
await this._queue.add(
async () => {
if (keychain) {
authInfo = await keychain.getPassword(svcName, `${productKey}-${credentialId}`);
authInfo = await keychain.getPassword(svcName, getKeyForSecrets(productKey, credentialId));
}
},
{ priority: Priority.Read },
Expand Down Expand Up @@ -320,7 +369,7 @@ export class CredentialManager implements Disposable {

const provider: OAuthProvider | undefined = oauthProviderForSite(site);
const newTokens = undefined;
if (provider && credentials) {
if (provider && credentials && credentials.refresh) {
const tokenResponse = await this._refresher.getNewTokens(provider, credentials.refresh);
if (tokenResponse.tokens) {
const newTokens = tokenResponse.tokens;
Expand All @@ -344,7 +393,7 @@ export class CredentialManager implements Disposable {
/**
* Removes an auth item from both the in-memory store and the secretstorage.
*/
public async removeAuthInfo(site: DetailedSiteInfo): Promise<boolean> {
public async removeAuthInfoForProductAndCredentialId(site: DetailedSiteInfo): Promise<boolean> {
const productAuths = this._memStore.get(site.product.key);
let wasKeyDeleted = false;
let wasMemDeleted = false;
Expand Down
Loading