Skip to content

[bbs] check webhooks permission in scm itself #18575

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

Merged
merged 1 commit into from
Aug 22, 2023
Merged
Changes from all commits
Commits
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
25 changes: 2 additions & 23 deletions components/server/src/prebuilds/bitbucket-server-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,29 +63,8 @@ export class BitbucketServerService extends RepositoryService {
console.log(`BBS: could not read webhooks.`, error, { error, cloneUrl });
return false;
}

if (repoKind === "users") {
const ownProfile = await this.api.getUserProfile(user, identity.authName);
if (owner === ownProfile.slug) {
return true;
}
}

let permission = await this.api.getPermission(user, { username: identity.authName, repoKind, owner, repoName });
if (!permission && repoKind === "projects") {
permission = await this.api.getPermission(user, { username: identity.authName, repoKind, owner });
}

if (this.hasPermissionToCreateWebhooks(permission)) {
return true;
}

console.log(`BBS: Not allowed to install webhooks.`, { permission });
return false;
}

protected hasPermissionToCreateWebhooks(permission: string | undefined) {
return permission && ["REPO_ADMIN", "PROJECT_ADMIN"].indexOf(permission) !== -1;
// return true once it can get webhooks, fallback to let SCM itself to check permission
Copy link
Member

Choose a reason for hiding this comment

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

where fallback happens?

It seems the code before that we have create permissions, not only read. Is it because it is not so fine grained in bbs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akosyakov We will add webhooks when creating the project. So BBS will go check it as test result shows #18575 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fallback because our permission checking for webhook add is not align to what BBS does

  • Admin of BBS has all permission
  • Admin can create webhook for all repo/project
  • Admin don't have project_admin and repo_admin permission in its open APIs if they don't configure it manually in BBS projects/repos

return true;
}

async installAutomatedPrebuilds(user: User, cloneUrl: string): Promise<void> {
Expand Down