-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
base: apply-overview-patch
Are you sure you want to change the base?
WIP Apply auth patch #454
Conversation
e7f52e1
to
b28701a
Compare
@@ -292,4 +305,285 @@ describe('LoginManager', () => { | |||
); | |||
}); | |||
}); | |||
|
|||
describe('extractTokenFromGitRemoteRegex', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later
}); | ||
}); | ||
|
||
describe('getAuthTokenFromHomeGitCredentials', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later
}); | ||
}); | ||
|
||
describe('authenticateWithBitbucketToken', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later
@@ -853,7 +857,7 @@ | |||
"properties": { | |||
"atlascode.outputLevel": { | |||
"type": "string", | |||
"default": "silent", | |||
"default": "debug", |
There was a problem hiding this comment.
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.
@@ -223,21 +223,6 @@ describe('CredentialManager', () => { | |||
site: mockJiraSite, | |||
}); | |||
}); | |||
|
|||
it("should not save to secret storage if auth info hasn't changed", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this test?
/** | ||
* 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> { |
There was a problem hiding this comment.
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.
@@ -0,0 +1,188 @@ | |||
import * as vscode from 'vscode'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this test file not belong to a previous PR?
@@ -228,7 +228,7 @@ export class Container { | |||
)), | |||
); | |||
this._context.subscriptions.push((this._jiraActiveIssueStatusBar = new JiraActiveIssueStatusBar(bbCtx))); | |||
|
|||
Container.authenticateWithBitbucketToken(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduce a new config: isdevsphere=true
and only then call this function. In upstream, we don't want to trigger this flow by default.
And add a comment that this whole flow is catered towards devsphere and not general public.
return true; | ||
} catch (e) { | ||
Logger.error(e, 'Error authenticating with Bitbucket token'); | ||
vscode.window.showErrorMessage(`Error authenticating with Bitbucket token: ${e}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this log: it is removed in a later patch 58
What Is This Change?
Applied patches for auth changes from fork for PRs 1,16,27,28,34,39,48,49, 52
How Has This Been Tested?
Basic checks:
npm run lint
npm run test
Advanced checks:
Recommendations: