fix(gitea): check write/admin permission instead of collaborator only

Replace IsCollaborator (which returns true for read-only collaborators)
with CollaboratorPermission to verify the sender has write or admin
or owner access before allowing pipeline runs. Also fix
CreateForkPullRequest to grant access to SecondUserName instead of
TargetRefName.

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>

Author: zakisk

pkg/provider/gitea/acl.go

@@ -226,8 +226,17 @@ func (v *Provider) listOrgTeams(org, sender string) ([]*forgejo.Team, error) {
 }
 
 func (v *Provider) checkSenderRepoMembership(_ context.Context, runevent *info.Event) (bool, error) {
-	ret, _, err := v.Client().IsCollaborator(runevent.Organization, runevent.Repository, runevent.Sender)
-	return ret, err
+	permissionResult, _, err := v.Client().CollaboratorPermission(runevent.Organization, runevent.Repository, runevent.Sender)
+	if err != nil {
+		return false, err
+	}
+	if permissionResult != nil &&
+		(permissionResult.Permission == forgejo.AccessModeOwner ||
+			permissionResult.Permission == forgejo.AccessModeAdmin ||
+			permissionResult.Permission == forgejo.AccessModeWrite) {
+		return true, nil
+	}
+	return false, nil
 }
 
 // getFileFromDefaultBranch will get a file directly from the Default BaseBranch as

pkg/provider/gitea/acl_test.go

@@ -228,7 +228,7 @@ func TestOkToTestComment(t *testing.T) {
 			runevent: info.Event{
 				Organization: "owner",
 				Repository:   "repo",
-				Sender:       "nonowner",
+				Sender:       "notowner",
 				EventType:    "issue_comment",
 				Event:        issueCommentPayload,
 			},
@@ -298,7 +298,7 @@ func TestOkToTestComment(t *testing.T) {
 			runevent: info.Event{
 				Organization: "owner",
 				Repository:   "repo",
-				Sender:       "nonowner",
+				Sender:       "notowner",
 				EventType:    "issue_comment",
 				Event:        issueCommentPayload,
 			},
@@ -326,8 +326,9 @@ func TestOkToTestComment(t *testing.T) {
 				func(rw http.ResponseWriter, _ *http.Request) {
 					fmt.Fprint(rw, tt.commentsReply)
 				})
-			mux.HandleFunc("/repos/owner/collaborators", func(rw http.ResponseWriter, _ *http.Request) {
-				fmt.Fprint(rw, "[]")
+			mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/collaborators/%s/permission", tt.runevent.Organization,
+				tt.runevent.Repository, tt.runevent.Sender), func(rw http.ResponseWriter, _ *http.Request) {
+				fmt.Fprint(rw, `{"permission": "none"}`)
 			})
 			ctx, _ := rtesting.SetupFakeContext(t)
 			gprovider := Provider{
@@ -354,7 +355,9 @@ func TestOkToTestComment(t *testing.T) {
 func TestAclCheckAll(t *testing.T) {
 	type allowedRules struct {
 		ownerFile bool
+		read      bool
 		collabo   bool
+		admin     bool
 	}
 	tests := []struct {
 		name         string
@@ -364,7 +367,7 @@ func TestAclCheckAll(t *testing.T) {
 		allowed      bool
 	}{
 		{
-			name: "allowed_from_org/sender allowed_from_org in collabo",
+			name: "allowed when sender has repository write collaborator permission",
 			runevent: info.Event{
 				Organization: "collabo",
 				Repository:   "repo",
@@ -375,7 +378,18 @@ func TestAclCheckAll(t *testing.T) {
 			wantErr:      false,
 		},
 		{
-			name: "allowed_from_org/sender allowed_from_org from owner file",
+			name: "allowed when sender has repository admin permission",
+			runevent: info.Event{
+				Organization: "collabo",
+				Repository:   "repo",
+				Sender:       "login_allowed",
+			},
+			allowedRules: allowedRules{admin: true},
+			allowed:      true,
+			wantErr:      false,
+		},
+		{
+			name: "allowed when sender is approver in OWNERS file",
 			runevent: info.Event{
 				Organization:  "collabo",
 				Repository:    "repo",
@@ -388,7 +402,7 @@ func TestAclCheckAll(t *testing.T) {
 			wantErr:      false,
 		},
 		{
-			name: "disallowed/sender not allowed_from_org in collabo",
+			name: "disallowed when sender has no collaborator or OWNERS approval",
 			runevent: info.Event{
 				Organization: "denied",
 				Repository:   "denied",
@@ -397,6 +411,17 @@ func TestAclCheckAll(t *testing.T) {
 			allowed: false,
 			wantErr: false,
 		},
+		{
+			name: "allowed when sender has repository read permission",
+			runevent: info.Event{
+				Organization: "denied",
+				Repository:   "denied",
+				Sender:       "notallowed",
+			},
+			allowedRules: allowedRules{read: true},
+			allowed:      false,
+			wantErr:      false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -411,12 +436,20 @@ func TestAclCheckAll(t *testing.T) {
 				Logger:      logger,
 			}
 
-			if tt.allowedRules.collabo {
-				mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/collaborators/%s", tt.runevent.Organization,
-					tt.runevent.Repository, tt.runevent.Sender), func(rw http.ResponseWriter, _ *http.Request) {
-					rw.WriteHeader(http.StatusNoContent)
-				})
-			}
+			mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/collaborators/%s/permission", tt.runevent.Organization,
+				tt.runevent.Repository, tt.runevent.Sender), func(rw http.ResponseWriter, _ *http.Request) {
+				rw.WriteHeader(http.StatusOK)
+				permission := "none"
+				switch {
+				case tt.allowedRules.admin:
+					permission = "admin"
+				case tt.allowedRules.collabo:
+					permission = "write"
+				case tt.allowedRules.read:
+					permission = "read"
+				}
+				fmt.Fprintf(rw, `{"permission": "%s"}`, permission)
+			})
 			if tt.allowedRules.ownerFile {
 				url := fmt.Sprintf("/repos/%s/%s/contents/OWNERS", tt.runevent.Organization, tt.runevent.Repository)
 				mux.HandleFunc(url, func(rw http.ResponseWriter, r *http.Request) {

test/gitea_access_control_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm"
 	twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
+	"github.com/tektoncd/pipeline/pkg/names"
 	"gotest.tools/v3/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -189,12 +190,44 @@ func TestGiteaACLOrgAllowed(t *testing.T) {
 	secondcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.TargetRefName, topts.GiteaPassword)
 	assert.NilError(t, err)
 
-	tgitea.CreateForkPullRequest(t, topts, secondcnx, "read")
+	tgitea.CreateForkPullRequest(t, topts, secondcnx, "write")
 	topts.CheckForStatus = "success"
 	tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false)
 	topts.GiteaCNX = adminCnx
 }
 
+func TestGiteaACLOrgWriteAndAdminAccess(t *testing.T) {
+	topts := &tgitea.TestOpts{
+		TargetEvent: triggertype.PullRequest.String(),
+		YAMLFiles: map[string]string{
+			".tekton/pr.yaml": "testdata/pipelinerun.yaml",
+		},
+		ExpectEvents:         false,
+		CheckForNumberStatus: 2,
+	}
+	_, f := tgitea.TestPR(t, topts)
+	defer f()
+	adminCnx := topts.GiteaCNX
+
+	topts.SecondUserName = topts.TargetRefName
+	secondcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.SecondUserName, topts.GiteaPassword)
+	assert.NilError(t, err)
+	tgitea.CreateForkPullRequest(t, topts, secondcnx, "write")
+	topts.CheckForStatus = "success"
+	tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false)
+
+	// third user setup to give it admin access to the repo and check the pipeline run are created.
+	thirdUserName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
+	topts.SecondUserName = thirdUserName
+	thirdcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.SecondUserName, topts.GiteaPassword)
+	assert.NilError(t, err)
+	tgitea.CreateForkPullRequest(t, topts, thirdcnx, "admin")
+	topts.CheckForStatus = "success"
+	topts.CheckForNumberStatus = 3
+	tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false)
+	topts.GiteaCNX = adminCnx
+}
+
 // TestGiteaACLOrgPendingApproval tests when non authorized user sends a PR the status of CI shows as pending.
 func TestGiteaACLOrgPendingApproval(t *testing.T) {
 	topts := &tgitea.TestOpts{

test/pkg/gitea/scm.go

@@ -253,7 +253,11 @@ func CreateForkPullRequest(t *testing.T, topts *TestOpts, secondcnx pgitea.Provi
 	topts.ParamsRun.Clients.Log.Infof("Forked repository %s has been created", forkrepo.CloneURL)
 
 	if accessMode != "" {
-		assert.NilError(t, CreateAccess(topts, topts.TargetRefName, accessMode))
+		userName := topts.SecondUserName
+		if userName == "" {
+			userName = topts.TargetRefName
+		}
+		assert.NilError(t, CreateAccess(topts, userName, accessMode))
 	}
 
 	pr, _, err := secondcnx.Client().CreatePullRequest(topts.Opts.Organization, topts.TargetRefName,

test/pkg/gitea/test.go

@@ -67,6 +67,7 @@ type TestOpts struct {
 	FileChanges           []scm.FileChange
 	CreateSecret          []corev1.Secret
 	ProviderType          string // defaults to "forgejo" if empty
+	SecondUserName        string
 }
 
 func PostCommentOnPullRequest(t *testing.T, topt *TestOpts, body string) {