feat(bitbucketdatacenter): Implement missing OrgMembership method

[henkka]

This pull request implements the "OrgMembership" method to Bitbucket Datacenter forge. Previously, users of the Bitbucket Data Center forge could not utilize organization secrets or properly list repositories within the organization due to missing functionality.

Unfortunately, the Bitbucket API doesn't provide good support for this use case, so the implementation is somewhat hackish. We derive the "Member/Admin" information based on whether the user has admin or write permissions to any repositories in the Bitbucket project (that equals to the Woodpecker Organization in the terminology).

[Levy-Tal]

Hey, thank you for implementing it. I'm not sure that every repository admin should be considered an organization admin. It makes more sense that if you are a project admin, then you should be considered an organization admin. you can look at a reference here-

neticdk/go-bitbucket#5

[henkka]

Hey, thank you for implementing it. I'm not sure that every repository admin should be considered an organization admin. It makes more sense that if you are a project admin, then you should be considered an organization admin. you can look at a reference here-

neticdk/go-bitbucket#5

Yeah I agree the proposed solution is not perfect, but unfortunately, the endpoint that you linked doesn't work with Oauth2 applications at least in our Bitbucket datacenter installation, as the Oauth2 application receives 401 whenever it tries to use it even though the user has PROJECT_ADMIN permissions in the specific project.

[Levy-Tal]

Yes, i can see it in the docs now-

"Forge and OAuth2 apps cannot access this REST resource."

https://developer.atlassian.com/server/bitbucket/rest/v906/api-group-project/#api-api-latest-projects-projectkey-permissions-search-get

[henkka]

Yes, i can see it in the docs now-

"Forge and OAuth2 apps cannot access this REST resource."

https://developer.atlassian.com/server/bitbucket/rest/v906/api-group-project/#api-api-latest-projects-projectkey-permissions-search-get

Yep :/ Unfortunately, this restriction appears to be actually enforced for this endpoint, unlike other API endpoints that have the same documentation clause but still work with OAuth2 apps. For example, the repository listing endpoint has identical documentation but functions correctly with OAuth2 apps (as we're already using it here).

[Levy-Tal]

Is it possible that you got the 401 because the scope here needs to include PROJECT_ADMIN (and also in the application link client in Bitbucket)?

At the end of the page here you can see all the possible scopes that a client can request, and currently we don’t request PROJECT_ADMIN.

This would be a significant change, since it forces users to grant the PROJECT_ADMIN permission to the client in order for it to work. However, I think this is still much better than giving Org Admin permissions to users who are not project admins.

Can you confirm if that works for you?

[henkka]

Is it possible that you got the 401 because the scope here needs to include PROJECT_ADMIN (and also in the application link client in Bitbucket)?

At the end of the page here you can see all the possible scopes that a client can request, and currently we don’t request PROJECT_ADMIN.

This would be a significant change, since it forces users to grant the PROJECT_ADMIN permission to the client in order for it to work. However, I think this is still much better than giving Org Admin permissions to users who are not project admins.

Can you confirm if that works for you?

That seems to work, good catch! However, as you said, it required our organizations Bitbucket admins to tweak the application link permissions for Woodpecker, so if we move forward with that idea it'll be modification that will require changes for all Woodpecker users using Bitbucket DC forge type.

For us either should work, so hopefully maintainers can chime in what's the approach the project wants to take.~~

Tested with the following modifications: main...henkka:woodpecker:testing

[Levy-Tal]

Cool, I think it is more correct to give Org Admin if the user is a Project Admin and not a Repo Admin.
A Project Admin has admin access to all repos in the project, while a Repo Admin may not even have read access to other repos in the same project. Because of that, I don’t think a Repo Admin should have the ability to change Org secrets, since that can affect other repos in the Org.

For Woodpecker to check if a user is a Project Admin, it needs the PROJECT_ADMIN scope in the Bitbucket OAuth2 app.
This means that users upgrading to the new version will need to update their application link in Bitbucket and add the PROJECT_ADMIN scope.

If we go with changing the scope permission, I think we should add it as a note in the release. It would also be good to document the required permissions in the Bitbucket Data Center integration docs.

@langecode @qwerty287 what do you think?

[qwerty287]

For us either should work, so hopefully maintainers can chime in what's the approach the project wants to take

Using the proper method would be what I prefer. We can ship that in a major update as breaking change.

[henkka]

Sorry for the delay on this. We've been swamped with migrating our organization to Woodpecker CI and had to focus on getting that sorted first.

Using the proper method would be what I prefer. We can ship that in a major update as breaking change.

I've refactored this to use the proper method like you suggested. Since we don't want to run our own fork while waiting for the next major release and this feature shipped in that, I added a feature flag so it's not a breaking change anymore. This way people can opt into the new behavior if they want it, but existing setups won't break.

(ps. sorry for force pushing, it's a habit of mine and I am aware this project doesn't prefer it, so I try to remember it in the future)

[qwerty287]

No problem. Feature flag is a good solution 👍

[qwerty287]

Besides these points it looks fine if there's no way around them…

[woodpecker-bot]

Surge PR preview deployment succeeded. View it at https://woodpecker-ci-woodpecker-pr-5476.surge.sh

[codecov]

Codecov Report

❌ Patch coverage is 20.51282% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.40%. Comparing base (68d690a) to head (f2a7dcc).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...r/forge/bitbucketdatacenter/bitbucketdatacenter.go	24.61%	48 Missing and 1 partial ⚠️
server/forge/setup/setup.go	0.00%	12 Missing ⚠️
server/services/setup.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5476      +/-   ##
==========================================
- Coverage   26.43%   26.40%   -0.03%     
==========================================
  Files         405      405              
  Lines       28878    28939      +61     
==========================================
+ Hits         7633     7641       +8     
- Misses      20546    20598      +52     
- Partials      699      700       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.