-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat(bitbucket-server): support bearer token authentication #26522
feat(bitbucket-server): support bearer token authentication #26522
Conversation
Add support for project/repository http access token
|
@@ -302,7 +316,9 @@ export async function getPrList(refreshCache?: boolean): Promise<Pr[]> { | |||
}; | |||
if (!config.ignorePrAuthor) { | |||
searchParams['role.1'] = 'AUTHOR'; | |||
searchParams['username.1'] = config.username; | |||
if(config.username){ |
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.
Can the username be "learned" using the API? It's possible in other platforms to do a "whoami" type query to the API
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.
no, afaik the point is to be anonymous. the tokens are for a repository and not for a user. They are no PAT.
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.
please check what's the author for PRs created by such a token.
@@ -17,13 +17,16 @@ export function getNoVerify(): GitNoVerifyOption[] { | |||
return noVerify; | |||
} | |||
|
|||
export function simpleGitConfig(): Partial<SimpleGitOptions> { | |||
export function simpleGitConfig(extraConfig: SimpleGitOptions['config'] = []): Partial<SimpleGitOptions> { |
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.
Isn't it possible to pass token in the same way other platforms do?
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.
yes, revert
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.
the token can't be passed as an URL argument
@@ -63,7 +63,7 @@ export const id = 'bitbucket-server'; | |||
|
|||
let config: BbsConfig = {} as any; | |||
|
|||
const bitbucketServerHttp = new BitbucketServerHttp(); | |||
let bitbucketServerHttp: BitbucketServerHttp; |
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.
nope, revert
let options: HttpOptions | undefined = undefined; | ||
if (token) { | ||
options = { | ||
headers: { | ||
'authorization': `Bearer ${token}`, | ||
}, | ||
}; | ||
} | ||
|
||
bitbucketServerHttp = new BitbucketServerHttp(options) |
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.
nope, revert. the check above should be enough to allow token auth , because the token is later converted to hostrule and then implicit used for bearer auth.
@@ -205,6 +218,7 @@ export async function initRepo({ | |||
url, | |||
cloneSubmodules, | |||
fullClone: true, | |||
authorization: opts.token, |
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.
no, will already be handled via hostrule and is already added to opts.
you need to update the getRepoGitUrl
function above to handle token auth.
@@ -17,13 +17,16 @@ export function getNoVerify(): GitNoVerifyOption[] { | |||
return noVerify; | |||
} | |||
|
|||
export function simpleGitConfig(): Partial<SimpleGitOptions> { | |||
export function simpleGitConfig(extraConfig: SimpleGitOptions['config'] = []): Partial<SimpleGitOptions> { |
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.
yes, revert
@@ -232,7 +232,7 @@ export async function initRepo(args: StorageConfig): Promise<void> { | |||
config.ignoredAuthors = []; | |||
config.additionalBranches = []; | |||
config.branchIsModified = {}; | |||
git = simpleGit(GlobalConfig.get('localDir'), simpleGitConfig()).env({ | |||
git = simpleGit(GlobalConfig.get('localDir'), simpleGitConfig(config.authorization ? [`http.extraHeader=Authorization: Bearer ${config.authorization}`] : undefined)).env({ |
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.
revert
@@ -17,6 +17,7 @@ export type LongCommitSha = string & { __longCommitSha: never }; | |||
export interface StorageConfig { | |||
currentBranch?: string; | |||
url: string; | |||
authorization?: string, |
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.
revert.
config: [ | ||
...extraConfig, | ||
'core.quotePath=false', | ||
], |
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.
revert
I'm confused: is this PR closed because the feature will not be added or is it not possible? |
it seems the author is not able to implement the suggestions or can't sign the cla |
I can't find the suggested places required to be changed. |
Add support for project/repository http access token
Changes
allowed token authentication by passing the authentication to the rest api and simple-git as extra-header
Context
fixes #14900
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: