refact: Refactor IsAllowed func in Bitbucket Data Center

pkg/provider/bitbucketdatacenter/acl.go

@@ -4,11 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strconv"
 	"strings"
 
 	bbv1 "github.com/gfleury/go-bitbucket-v1"
-	"github.com/mitchellh/mapstructure"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/acl"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 )
@@ -53,6 +51,8 @@ func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, e
 		if nextPage > 0 {
 			localVarOptionals["start"] = int(nextPage)
 		}
+		// will replace this API call with jenkins-x/go-scm after my PR on go-scm is merged
+		// https://github.com/jenkins-x/go-scm/pull/494
 		return v.Client.DefaultApi.GetActivities(v.projectKey, event.Repository, v.pullRequestNumber, localVarOptionals)
 	})
 	if err != nil {
@@ -94,66 +94,27 @@ func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, e
 	return false, nil
 }
 
-func (v *Provider) checkMemberShipResults(results []any, event *info.Event) (bool, error) {
-	accountintid, err := strconv.Atoi(event.AccountID)
-	if err != nil {
-		return false, err
-	}
-	for _, row := range results {
-		user := &bbv1.UserPermission{}
-		err := mapstructure.Decode(row, user)
-		if err != nil {
-			return false, err
-		}
-
-		if user.User.ID == accountintid {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 func (v *Provider) checkMemberShip(ctx context.Context, event *info.Event) (bool, error) {
 	// Get permissions from project
-	allValues, err := paginate(func(nextPage int) (*bbv1.APIResponse, error) {
-		localVarOptionals := map[string]any{}
-		if nextPage > 0 {
-			localVarOptionals["start"] = int(nextPage)
-		}
-		return v.Client.DefaultApi.GetUsersWithAnyPermission_23(v.projectKey, localVarOptionals)
-	})
-	if err != nil {
-		return false, err
-	}
-	allowed, err := v.checkMemberShipResults(allValues, event)
+	allowed, _, err := v.ScmClient.Organizations.IsMember(ctx, event.Organization, event.Sender)
 	if err != nil {
 		return false, err
 	}
 	if allowed {
 		return true, nil
 	}
 
+	orgAndRepo := fmt.Sprintf("%s/%s", event.Organization, event.Repository)
 	// Get permissions from repo
-	allValues, err = paginate(func(nextPage int) (*bbv1.APIResponse, error) {
-		localVarOptionals := map[string]any{}
-		if nextPage > 0 {
-			localVarOptionals["start"] = int(nextPage)
-		}
-		return v.Client.DefaultApi.GetUsersWithAnyPermission_24(v.projectKey, event.Repository, localVarOptionals)
-	})
-	if err != nil {
-		return false, err
-	}
-
-	allowed, err = v.checkMemberShipResults(allValues, event)
+	allowed, _, err = v.ScmClient.Repositories.IsCollaborator(ctx, orgAndRepo, event.Sender)
 	if err != nil {
 		return false, err
 	}
 	if allowed {
 		return true, nil
 	}
 
-	// Check if sender (which in bitbucket-datacenter mean the accountID) is inside the Owner file
+	// Check if sender is inside the Owner file
 	// in the 'main' branch Silently ignore error, which should be fine it
 	// probably means the OWNERS file is not created. If we had another error
 	// (ie: like API) we probably would have hit it already.

pkg/provider/bitbucketdatacenter/acl_test.go

@@ -33,6 +33,7 @@ func TestIsAllowed(t *testing.T) {
 	type fields struct {
 		projectMembers            []*bbv1.UserPermission
 		repoMembers               []*bbv1.UserPermission
+		projGroups                []*bbv1test.ProjGroup
 		activities                []*bbv1.Activity
 		filescontents             map[string]string
 		defaultBranchLatestCommit string
@@ -52,7 +53,7 @@ func TestIsAllowed(t *testing.T) {
 				projectMembers: []*bbv1.UserPermission{
 					{
 						User: bbv1.User{
-							ID: ownerAccountID,
+							Slug: "member",
 						},
 					},
 				},
@@ -70,7 +71,7 @@ func TestIsAllowed(t *testing.T) {
 				projectMembers: []*bbv1.UserPermission{
 					{
 						User: bbv1.User{
-							ID: ownerAccountID,
+							Slug: "member",
 						},
 					},
 				},
@@ -79,7 +80,7 @@ func TestIsAllowed(t *testing.T) {
 						Comment: bbv1.ActivityComment{
 							Text: "/ok-to-test",
 							Author: bbv1.User{
-								ID: ownerAccountID,
+								Slug: "member",
 							},
 						},
 					},
@@ -207,6 +208,7 @@ func TestIsAllowed(t *testing.T) {
 			defer tearDown()
 			bbv1test.MuxProjectMemberShip(t, mux, tt.event, tt.fields.projectMembers)
 			bbv1test.MuxRepoMemberShip(t, mux, tt.event, tt.fields.repoMembers)
+			bbv1test.MuxProjectGroupMembership(t, mux, tt.event, tt.fields.projGroups)
 			bbv1test.MuxPullRequestActivities(t, mux, tt.event, tt.fields.pullRequestNumber, tt.fields.activities)
 			bbv1test.MuxFiles(t, mux, tt.event, tt.fields.defaultBranchLatestCommit, "", tt.fields.filescontents, false)
 

pkg/provider/bitbucketdatacenter/test/test.go

@@ -240,6 +240,22 @@ func MuxProjectMemberShip(t *testing.T, mux *http.ServeMux, event *info.Event, u
 	})
 }
 
+func MuxProjectGroupMembership(t *testing.T, mux *http.ServeMux, event *info.Event, groups []*ProjGroup) {
+	path := fmt.Sprintf("/projects/%s/permissions/groups", event.Organization)
+	mux.HandleFunc(path, func(rw http.ResponseWriter, _ *http.Request) {
+		if groups == nil {
+			fmt.Fprintf(rw, "{\"values\": []}")
+		}
+		resp := map[string]any{
+			"values": groups,
+		}
+		b, err := json.Marshal(resp)
+		assert.NilError(t, err)
+
+		fmt.Fprint(rw, string(b))
+	})
+}
+
 func MuxRepoMemberShip(t *testing.T, mux *http.ServeMux, event *info.Event, userperms []*bbv1.UserPermission) {
 	path := fmt.Sprintf("/projects/%s/repos/%s/permissions/users", event.Organization, event.Repository)
 	mux.HandleFunc(path, func(rw http.ResponseWriter, _ *http.Request) {

pkg/provider/bitbucketdatacenter/test/test_types.go

@@ -22,3 +22,12 @@ type DiffStats struct {
 	Pagination
 	Values []*DiffStat
 }
+
+type ProjGroup struct {
+	Group      Group  `json:"group"`
+	Permission string `json:"permission"`
+}
+
+type Group struct {
+	Name string `json:"name"`
+}