fix: nil-guard bug and add group ACL tests for BBC

The 4 Mux permission helpers wrote double responses when input was
nil due to missing returns. Extract shared muxPermissions helper
and add test coverage for project/repo group-based access.

Author: tricktron

pkg/provider/bitbucketdatacenter/acl_test.go

@@ -36,6 +36,7 @@ func TestIsAllowed(t *testing.T) {
 		repoMembers               []*bbv1test.UserPermission
 		projGroups                []*bbv1test.ProjGroup
 		repoGroups                []*bbv1test.ProjGroup
+		groupMembers              map[string][]bbv1test.GroupMember
 		activities                []*bbv1test.Activity
 		filescontents             map[string]string
 		defaultBranchLatestCommit string
@@ -138,6 +139,69 @@ func TestIsAllowed(t *testing.T) {
 			},
 			isAllowed: false,
 		},
+		{
+			name:  "allowed/user is in project group",
+			event: bbv1test.MakeEvent(&info.Event{Sender: "sender", AccountID: fmt.Sprintf("%d", otherAccountID)}),
+			fields: fields{
+				projGroups: []*bbv1test.ProjGroup{
+					{
+						Group:      bbv1test.Group{Name: "project-devs"},
+						Permission: "PROJECT_WRITE",
+					},
+				},
+				groupMembers: map[string][]bbv1test.GroupMember{
+					"project-devs": {
+						{Name: "sender", Slug: "sender"},
+					},
+				},
+			},
+			isAllowed: true,
+		},
+		{
+			name:  "allowed/user is in repo group",
+			event: bbv1test.MakeEvent(&info.Event{Sender: "sender", AccountID: fmt.Sprintf("%d", otherAccountID)}),
+			fields: fields{
+				repoGroups: []*bbv1test.ProjGroup{
+					{
+						Group:      bbv1test.Group{Name: "repo-devs"},
+						Permission: "REPO_WRITE",
+					},
+				},
+				groupMembers: map[string][]bbv1test.GroupMember{
+					"repo-devs": {
+						{Name: "sender", Slug: "sender"},
+					},
+				},
+			},
+			isAllowed: true,
+		},
+		{
+			name:  "disallowed/user not in any group members",
+			event: bbv1test.MakeEvent(&info.Event{Sender: "outsider", AccountID: fmt.Sprintf("%d", otherAccountID)}),
+			fields: fields{
+				projGroups: []*bbv1test.ProjGroup{
+					{
+						Group:      bbv1test.Group{Name: "project-devs"},
+						Permission: "PROJECT_WRITE",
+					},
+				},
+				repoGroups: []*bbv1test.ProjGroup{
+					{
+						Group:      bbv1test.Group{Name: "repo-devs"},
+						Permission: "REPO_WRITE",
+					},
+				},
+				groupMembers: map[string][]bbv1test.GroupMember{
+					"project-devs": {
+						{Name: "someone-else", Slug: "someone-else"},
+					},
+					"repo-devs": {
+						{Name: "another-person", Slug: "another-person"},
+					},
+				},
+			},
+			isAllowed: false,
+		},
 		{
 			name:  "disallowed/same nickname different account id",
 			event: bbv1test.MakeEvent(&info.Event{Sender: "Bouffon", AccountID: "6666"}),
@@ -215,6 +279,7 @@ func TestIsAllowed(t *testing.T) {
 			bbv1test.MuxRepoMemberShip(t, mux, tt.event, tt.fields.repoMembers)
 			bbv1test.MuxProjectGroupMembership(t, mux, tt.event, tt.fields.projGroups)
 			bbv1test.MuxRepoGroupMembership(t, mux, tt.event, tt.fields.repoGroups)
+			bbv1test.MuxGroupMembers(t, mux, tt.fields.groupMembers)
 			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

@@ -218,62 +218,53 @@ func MuxCreateAndTestCommitStatus(t *testing.T, mux *http.ServeMux, event *info.
 	})
 }
 
-func MuxProjectMemberShip(t *testing.T, mux *http.ServeMux, event *info.Event, userperms []*UserPermission) {
-	path := fmt.Sprintf("/projects/%s/permissions/users", event.Organization)
+func muxPermissions(t *testing.T, mux *http.ServeMux, path string, values any) {
+	t.Helper()
 	mux.HandleFunc(path, func(rw http.ResponseWriter, _ *http.Request) {
-		if userperms == nil {
-			fmt.Fprintf(rw, "{\"values\": []}")
-		}
 		resp := map[string]any{
-			"values": userperms,
+			"values":     values,
+			"isLastPage": true,
 		}
 		b, err := json.Marshal(resp)
 		assert.NilError(t, err)
-
 		fmt.Fprint(rw, string(b))
 	})
 }
 
+func MuxProjectMemberShip(t *testing.T, mux *http.ServeMux, event *info.Event, userperms []*UserPermission) {
+	path := fmt.Sprintf("/projects/%s/permissions/users", event.Organization)
+	muxPermissions(t, mux, path, userperms)
+}
+
 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))
-	})
+	muxPermissions(t, mux, path, groups)
 }
 
 func MuxRepoGroupMembership(t *testing.T, mux *http.ServeMux, event *info.Event, groups []*ProjGroup) {
 	path := fmt.Sprintf("/projects/%s/repos/%s/permissions/groups", event.Organization, event.Repository)
-	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))
-	})
+	muxPermissions(t, mux, path, groups)
 }
 
 func MuxRepoMemberShip(t *testing.T, mux *http.ServeMux, event *info.Event, userperms []*UserPermission) {
 	path := fmt.Sprintf("/projects/%s/repos/%s/permissions/users", event.Organization, event.Repository)
-	mux.HandleFunc(path, func(rw http.ResponseWriter, _ *http.Request) {
-		if userperms == nil {
-			fmt.Fprintf(rw, "{\"values\": []}")
+	muxPermissions(t, mux, path, userperms)
+}
+
+// MuxGroupMembers mocks the /admin/groups/more-members endpoint used by go-scm
+// to resolve group names to individual users. groupMembers maps group name to
+// the list of members in that group.
+func MuxGroupMembers(t *testing.T, mux *http.ServeMux, groupMembers map[string][]GroupMember) {
+	t.Helper()
+	mux.HandleFunc("/admin/groups/more-members", func(rw http.ResponseWriter, r *http.Request) {
+		groupName := r.URL.Query().Get("context")
+		members, ok := groupMembers[groupName]
+		if !ok {
+			members = []GroupMember{}
 		}
 		resp := map[string]any{
-			"values": userperms,
+			"values":     members,
+			"isLastPage": true,
 		}
 		b, err := json.Marshal(resp)
 		assert.NilError(t, err)

pkg/provider/bitbucketdatacenter/test/test_types.go

@@ -34,6 +34,13 @@ type Group struct {
 	Name string `json:"name"`
 }
 
+// GroupMember matches the member struct in go-scm's stash driver,
+// returned by the /admin/groups/more-members endpoint.
+type GroupMember struct {
+	Name string `json:"name"`
+	Slug string `json:"slug"`
+}
+
 // kept these structs here because these are only used in tests
 
 type Comment struct {