fix: allow Actions tokens to access repos readable by signed in users (#8889)

This is an alternate take on https://codeberg.org/forgejo/forgejo/pulls/8808 which allows Actions tokens to access other repos which are readable by by signed in users. In practise this means public repos belonging to public or limited owners.

This PR is split into two commits to aid review of the security-sensitive changes I've made:

* The first commit is a refactor _which is not intended to change behaviour_. It extracts the permission logic for Actions tokens from the githttp handler and moves it to `repo_permission.go` alongside the permission logic for regular users. The new function, `GetActionRepoPermission` returns a `Permission` object just like `GetUserRepoPermission`. Only code unit access is currently allowed in the interest of keeping this commit from changing any access logic.

* The second commit is the broadening of access: this commit changes the logic in `GetActionRepoPermission` to give actions tokens access to repos readable by signed in users.

cc @earl-warren
ref https://codeberg.org/forgejo/forgejo/issues/5877

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/8889): <!--number 8889 --><!--line 0 --><!--description YWxsb3cgQWN0aW9ucyB0b2tlbnMgdG8gYWNjZXNzIHJlcG9zIHJlYWRhYmxlIGJ5IHNpZ25lZCBpbiB1c2Vycw==-->allow Actions tokens to access repos readable by signed in users<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8889
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Hailey Somerville <hailey@hails.org>
Co-committed-by: Hailey Somerville <hailey@hails.org>

models/perm/access/repo_permission.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 
+	actions_model "forgejo.org/models/actions"
 	"forgejo.org/models/db"
 	"forgejo.org/models/organization"
 	perm_model "forgejo.org/models/perm"
@@ -136,6 +137,33 @@ func (p *Permission) LogString() string {
 	return fmt.Sprintf(format, args...)
 }
 
+func GetActionRepoPermission(ctx context.Context, repo *repo_model.Repository, task *actions_model.ActionTask) (Permission, error) {
+	// straight forward case: an actions task is attempting to access its own repo
+	if task.RepoID == repo.ID {
+		var mode perm_model.AccessMode
+
+		// determine default access mode for repo:
+		if task.IsForkPullRequest {
+			mode = perm_model.AccessModeRead
+		} else {
+			mode = perm_model.AccessModeWrite
+		}
+
+		if err := repo.LoadUnits(ctx); err != nil {
+			return Permission{}, err
+		}
+
+		perm := Permission{
+			AccessMode: mode,
+			Units:      repo.Units,
+		}
+
+		return perm, nil
+	}
+
+	return GetUserRepoPermission(ctx, repo, user_model.NewActionsUser())
+}
+
 // GetUserRepoPermission returns the user permissions to the repository
 func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (Permission, error) {
 	var perm Permission

models/perm/access/repo_permission_test.go

@@ -0,0 +1,78 @@
+package access_test
+
+import (
+	"testing"
+
+	actions_model "forgejo.org/models/actions"
+	"forgejo.org/models/db"
+	perm_model "forgejo.org/models/perm"
+	"forgejo.org/models/perm/access"
+	repo_model "forgejo.org/models/repo"
+	"forgejo.org/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func assertAccess(t *testing.T, expectedMode perm_model.AccessMode, perm *access.Permission) {
+	assert.Equal(t, expectedMode, perm.AccessMode)
+
+	for _, unit := range perm.Units {
+		assert.Equal(t, expectedMode, perm.UnitAccessMode(unit.Type))
+	}
+}
+
+func TestActionTaskCanAccessOwnRepo(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: actionTask.RepoID})
+
+	perm, err := access.GetActionRepoPermission(db.DefaultContext, repo, actionTask)
+	require.NoError(t, err)
+	assertAccess(t, perm_model.AccessModeWrite, &perm)
+}
+
+func TestActionTaskCanAccessPublicRepo(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	perm, err := access.GetActionRepoPermission(db.DefaultContext, repo, actionTask)
+	require.NoError(t, err)
+	assertAccess(t, perm_model.AccessModeRead, &perm)
+}
+
+func TestActionTaskCanAccessPublicRepoOfLimitedOrg(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 38})
+
+	perm, err := access.GetActionRepoPermission(db.DefaultContext, repo, actionTask)
+	require.NoError(t, err)
+	assertAccess(t, perm_model.AccessModeRead, &perm)
+}
+
+func TestActionTaskNoAccessPublicRepoOfPrivateOrg(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 40})
+
+	perm, err := access.GetActionRepoPermission(db.DefaultContext, repo, actionTask)
+	require.NoError(t, err)
+	assertAccess(t, perm_model.AccessModeNone, &perm)
+}
+
+func TestActionTaskNoAccessPrivateRepo(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+
+	perm, err := access.GetActionRepoPermission(db.DefaultContext, repo, actionTask)
+	require.NoError(t, err)
+	assertAccess(t, perm_model.AccessModeNone, &perm)
+}

routers/web/repo/githttp.go

@@ -192,24 +192,19 @@ func httpBase(ctx *context.Context) *serviceHandler {
 					ctx.ServerError("GetTaskByID", err)
 					return nil
 				}
-				if task.RepoID != repo.ID {
+
+				p, err := access_model.GetActionRepoPermission(ctx, repo, task)
+				if err != nil {
+					ctx.ServerError("GetActionRepoPermission", err)
+					return nil
+				}
+
+				if !p.CanAccess(accessMode, unitType) {
 					ctx.PlainText(http.StatusForbidden, "User permission denied")
 					return nil
 				}
 
-				if task.IsForkPullRequest {
+				environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, p.AccessMode))
-					if accessMode > perm.AccessModeRead {
-						ctx.PlainText(http.StatusForbidden, "User permission denied")
-						return nil
-					}
-					environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeRead))
-				} else {
-					if accessMode > perm.AccessModeWrite {
-						ctx.PlainText(http.StatusForbidden, "User permission denied")
-						return nil
-					}
-					environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeWrite))
-				}
 			} else {
 				p, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
 				if err != nil {

tests/integration/git_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 	"time"
 
+	actions_model "forgejo.org/models/actions"
 	auth_model "forgejo.org/models/auth"
 	"forgejo.org/models/db"
 	git_model "forgejo.org/models/git"
@@ -51,6 +52,33 @@ func TestGit(t *testing.T) {
 	onGiteaRun(t, testGit)
 }
 
+func TestActionsTokenAuth(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
+		task.GenerateToken()
+		actions_model.UpdateTask(db.DefaultContext, task)
+		u.User = url.UserPassword("token", task.Token)
+
+		t.Run("clone task's own repo", func(t *testing.T) {
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: task.RepoID})
+			u.Path = fmt.Sprintf("/%s/%s.git", repo.OwnerName, repo.Name)
+			doGitClone(t.TempDir(), u)(t)
+		})
+
+		t.Run("clone public repo of limited owner", func(t *testing.T) {
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 38})
+			u.Path = fmt.Sprintf("/%s/%s.git", repo.OwnerName, repo.Name)
+			doGitClone(t.TempDir(), u)(t)
+		})
+
+		t.Run("cannot clone private repo", func(t *testing.T) {
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+			u.Path = fmt.Sprintf("/%s/%s.git", repo.OwnerName, repo.Name)
+			doGitCloneFail(u)(t)
+		})
+	})
+}
+
 func testGit(t *testing.T, u *url.URL) {
 	username := "user2"
 	baseAPITestContext := NewAPITestContext(t, username, "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)