mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-07-13 13:07:57 +00:00
chore: add coverage for REST API fork destination permission check (#13151)
- extract permission check from `routers/api/v1/repo/fork.go` into the `checkForkDestination()` middleware with no functional change - add the `checkForkDestination()` middleware to the `CreateFork` route - add test scenarios covering `checkForkDestination()`, except for unrecoverable errors ### Tests for Go changes - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### 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 - [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13151 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
This commit is contained in:
parent
a48683f064
commit
a3fa6fe41d
4 changed files with 145 additions and 21 deletions
|
|
@ -449,6 +449,14 @@ func mustEnableAttachments() func(ctx *context.APIContext) {
|
|||
return checkPermission(apiv1_permissions.MustEnableAttachments)
|
||||
}
|
||||
|
||||
func checkForkDestination() func(*context.APIContext) {
|
||||
apiv1_permissions_testhelpers.RecordSignature(apiv1_permissions.CheckForkDestination)
|
||||
return func(ctx *context.APIContext) {
|
||||
form := web.GetForm(ctx).(*api.CreateForkOption)
|
||||
apiv1_permissions.CheckForkDestination(ctx, form.Organization)
|
||||
}
|
||||
}
|
||||
|
||||
// bind binding an obj to a func(ctx *context.APIContext)
|
||||
func bind[T any](_ T) any {
|
||||
return func(ctx *context.APIContext) {
|
||||
|
|
@ -847,7 +855,7 @@ func Routes() *web.Route {
|
|||
m.Get("/archive/*", reqRepoReader(unit.TypeCode), repo.GetArchive)
|
||||
if !setting.Repository.DisableForks {
|
||||
m.Combo("/forks").Get(repo.ListForks).
|
||||
Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
|
||||
Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), checkForkDestination(), repo.CreateFork)
|
||||
}
|
||||
m.Group("/branches", func() {
|
||||
m.Get("", repo.ListBranches)
|
||||
|
|
|
|||
45
routers/api/v1/permissions/check_fork_destination.go
Normal file
45
routers/api/v1/permissions/check_fork_destination.go
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
// Copyright 2026 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package permissions
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"forgejo.org/models/organization"
|
||||
)
|
||||
|
||||
func CheckForkDestination(ctx Context, organizationName *string) {
|
||||
if organizationName == nil {
|
||||
return
|
||||
}
|
||||
org, err := organization.GetOrgByName(ctx.GetContext(), *organizationName)
|
||||
if err != nil {
|
||||
if organization.IsErrOrgNotExist(err) {
|
||||
ctx.Error(http.StatusUnprocessableEntity, "", err)
|
||||
} else {
|
||||
ctx.Error(http.StatusInternalServerError, "GetOrgByName", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
isMember, err := org.IsOrgMember(ctx.GetContext(), ctx.GetDoer().ID)
|
||||
if err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "IsOrgMember", err)
|
||||
return
|
||||
} else if !isMember {
|
||||
ctx.Error(http.StatusForbidden, "isMemberNot", fmt.Sprintf("User is no Member of Organisation '%s'", org.Name))
|
||||
return
|
||||
}
|
||||
if !IsUserSiteAdmin(ctx) {
|
||||
canCreate, err := org.CanCreateOrgRepo(ctx.GetContext(), ctx.GetDoer().ID)
|
||||
if err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "CanCreateOrgRepo", err)
|
||||
return
|
||||
}
|
||||
if !canCreate {
|
||||
ctx.Error(http.StatusForbidden, "CanCreateOrgRepo", fmt.Sprintf("User is not allowed to create repos in Organisation '%s'", org.Name))
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
91
routers/api/v1/permissions/tests/check_fork_destination.go
Normal file
91
routers/api/v1/permissions/tests/check_fork_destination.go
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
// Copyright 2026 The Forgejo Authors.
|
||||
// SPDX-License-Identifier: GPLv3-or-later
|
||||
|
||||
package tests
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
org_model "forgejo.org/models/organization"
|
||||
"forgejo.org/models/perm"
|
||||
user_model "forgejo.org/models/user"
|
||||
apiv1_permissions "forgejo.org/routers/api/v1/permissions"
|
||||
"forgejo.org/tests/forgery"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
var _ = registerFunctionTestWithCall(apiv1_permissions.CheckForkDestination, functionTest{
|
||||
interpret: func(t *testing.T, permissions *apiv1_permissions.Permissions, data *fixtureData) {
|
||||
require.True(t, data.Has("forkOrg"))
|
||||
if data.Get("forkOrg") == "unknownOrg" {
|
||||
return
|
||||
}
|
||||
require.True(t, data.Has("forkOrgOwner"))
|
||||
name := data.Get("forkOrg")
|
||||
owner := data.Get("forkOrgOwner")
|
||||
org := fixtureCreateOrg(t, &org_model.Organization{Name: name}, &user_model.User{Name: owner})
|
||||
|
||||
if data.Has("team") {
|
||||
fixtureCreateTeam(t, org, data.Get("doer"), &forgery.CreateTeamOptions{
|
||||
Name: data.Get("team"),
|
||||
CanCreateOrgRepo: data.Get("teamCanCreateOrgRepo") != "false",
|
||||
|
||||
Mode: perm.AccessModeWrite,
|
||||
})
|
||||
}
|
||||
},
|
||||
call: func(t *testing.T, ctx apiv1_permissions.Context, data *fixtureData, _ []any) {
|
||||
forkOrg := data.Get("forkOrg")
|
||||
t.Logf("calling CheckForkDestination(ctx, %s)", forkOrg)
|
||||
apiv1_permissions.CheckForkDestination(ctx, &forkOrg)
|
||||
},
|
||||
fixtures: []*fixtureType{
|
||||
{
|
||||
data: newFixtureData(map[string]string{
|
||||
"doer": "regularorgowner",
|
||||
"repository": "userowner/repositorypublic",
|
||||
"forkOrg": "regularorg1",
|
||||
"forkOrgOwner": "regularorgowner",
|
||||
}),
|
||||
},
|
||||
{
|
||||
data: newFixtureData(map[string]string{
|
||||
"doer": "regularuser",
|
||||
"repository": "regularuser/repositorypublic",
|
||||
"forkOrg": "regularorg1",
|
||||
"forkOrgOwner": "regularorgowner",
|
||||
"team": "team1",
|
||||
"teamCanCreateOrgRepo": "true",
|
||||
}),
|
||||
},
|
||||
{
|
||||
data: newFixtureData(map[string]string{
|
||||
"doer": "regularuser",
|
||||
"repository": "regularuser/repositorypublic",
|
||||
"forkOrg": "regularorg1",
|
||||
"forkOrgOwner": "regularorgowner",
|
||||
"team": "team1",
|
||||
"teamCanCreateOrgRepo": "false",
|
||||
}),
|
||||
error: "User is not allowed to create repos in Organisation",
|
||||
},
|
||||
{
|
||||
data: newFixtureData(map[string]string{
|
||||
"doer": "doerregular",
|
||||
"repository": "userowner/repositorypublic",
|
||||
"forkOrg": "regularorg2",
|
||||
"forkOrgOwner": "regularorgowner",
|
||||
}),
|
||||
error: "User is no Member of Organisation 'regularorg2'",
|
||||
},
|
||||
{
|
||||
data: newFixtureData(map[string]string{
|
||||
"doer": "regularorgowner",
|
||||
"repository": "userowner/repositorypublic",
|
||||
"forkOrg": "unknownOrg",
|
||||
}),
|
||||
error: "org does not exist",
|
||||
},
|
||||
},
|
||||
})
|
||||
|
|
@ -6,7 +6,6 @@ package repo
|
|||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"forgejo.org/models/organization"
|
||||
|
|
@ -132,25 +131,6 @@ func CreateFork(ctx *context.APIContext) {
|
|||
}
|
||||
return
|
||||
}
|
||||
isMember, err := org.IsOrgMember(ctx, ctx.Doer().ID)
|
||||
if err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "IsOrgMember", err)
|
||||
return
|
||||
} else if !isMember {
|
||||
ctx.Error(http.StatusForbidden, "isMemberNot", fmt.Sprintf("User is no Member of Organisation '%s'", org.Name))
|
||||
return
|
||||
}
|
||||
if !ctx.IsUserSiteAdmin() {
|
||||
canCreate, err := org.CanCreateOrgRepo(ctx, ctx.Doer().ID)
|
||||
if err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "CanCreateOrgRepo", err)
|
||||
return
|
||||
}
|
||||
if !canCreate {
|
||||
ctx.Error(http.StatusForbidden, "CanCreateOrgRepo", fmt.Sprintf("User is not allowed to create repos in Organisation '%s'", org.Name))
|
||||
return
|
||||
}
|
||||
}
|
||||
forker = org.AsUser()
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue