fix: make the fork API respect CanCreateOrgRepo policy (#12442)
When a forking target organization was supplied, the API handler only verified org membership. This is asymmetric with the rest of the codebase, as CanCreateOrgRepo is used everywhere else. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes (can be removed for JavaScript 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. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. Co-authored-by: jvoisin <julien.voisin@dustri.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12442 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
This commit is contained in:
committed by
Mathieu Fenniak
co-authored by
jvoisin
parent
0211c1eace
commit
d0f35bd1ba
@@ -140,6 +140,17 @@ func CreateFork(ctx *context.APIContext) {
|
||||
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()
|
||||
}
|
||||
|
||||
|
||||
@@ -50,6 +50,7 @@ func TestAPIForkAsAdminIgnoringLimits(t *testing.T) {
|
||||
IncludesAllRepositories: true,
|
||||
Permission: "write",
|
||||
Units: []string{"repo.code", "repo.issues"},
|
||||
CanCreateOrgRepo: true,
|
||||
}
|
||||
|
||||
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", orgName), &teamToCreate).AddTokenAuth(adminToken)
|
||||
@@ -86,6 +87,69 @@ func TestCreateForkNoLogin(t *testing.T) {
|
||||
MakeRequest(t, req, http.StatusUnauthorized)
|
||||
}
|
||||
|
||||
func TestAPIForkOrgCanCreateOrgRepoRequired(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
// user5 will be the regular org member without repo-creation permission.
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user5"})
|
||||
userSession := loginUser(t, user.Name)
|
||||
userToken := getTokenForLoggedInUser(t, userSession,
|
||||
auth_model.AccessTokenScopeWriteRepository,
|
||||
auth_model.AccessTokenScopeWriteOrganization)
|
||||
|
||||
adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true})
|
||||
adminSession := loginUser(t, adminUser.Name)
|
||||
adminToken := getTokenForLoggedInUser(t, adminSession,
|
||||
auth_model.AccessTokenScopeWriteRepository,
|
||||
auth_model.AccessTokenScopeWriteOrganization)
|
||||
|
||||
orgName := "fork-cancreaterepo-org"
|
||||
|
||||
// Create an organization
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/orgs", &api.CreateOrgOption{
|
||||
UserName: orgName,
|
||||
}).AddTokenAuth(adminToken)
|
||||
MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
// Create a team with CanCreateOrgRepo = false (the default)
|
||||
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", orgName), &api.CreateTeamOption{
|
||||
Name: "no-create-repo",
|
||||
IncludesAllRepositories: true,
|
||||
Permission: "write",
|
||||
Units: []string{"repo.code", "repo.issues"},
|
||||
// CanCreateOrgRepo is intentionally omitted (defaults to false)
|
||||
}).AddTokenAuth(adminToken)
|
||||
resp := MakeRequest(t, req, http.StatusCreated)
|
||||
var team api.Team
|
||||
DecodeJSON(t, resp, &team)
|
||||
assert.False(t, team.CanCreateOrgRepo)
|
||||
|
||||
// Add user5 to the team
|
||||
req = NewRequestf(t, "PUT", "/api/v1/teams/%d/members/%s", team.ID, user.Name).AddTokenAuth(adminToken)
|
||||
MakeRequest(t, req, http.StatusNoContent)
|
||||
|
||||
originForkURL := "/api/v1/repos/user2/repo1/forks"
|
||||
|
||||
t.Run("member without CanCreateOrgRepo is rejected", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
req := NewRequestWithJSON(t, "POST", originForkURL, &api.CreateForkOption{
|
||||
Organization: &orgName,
|
||||
}).AddTokenAuth(userToken)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "User is not allowed to create repos in Organisation")
|
||||
})
|
||||
|
||||
t.Run("admin can still fork into the org", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
req := NewRequestWithJSON(t, "POST", originForkURL, &api.CreateForkOption{
|
||||
Organization: &orgName,
|
||||
}).AddTokenAuth(adminToken)
|
||||
MakeRequest(t, req, http.StatusAccepted)
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIDisabledForkRepo(t *testing.T) {
|
||||
defer test.MockVariableValue(&setting.Repository.DisableForks, true)()
|
||||
defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
|
||||
|
||||
Reference in New Issue
Block a user