Fix a bug when forking a repository in an organization

routers/api/v1/repo/fork.go

@@ -6,14 +6,14 @@ package repo
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
 
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/optional"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
@@ -83,6 +83,35 @@ func ListForks(ctx *context.APIContext) {
 	ctx.JSON(http.StatusOK, apiForks)
 }
 
+func prepareDoerCreateRepoInOrg(ctx *context.APIContext, orgName string) *organization.Organization {
+	org, err := organization.GetOrgByName(ctx, orgName)
+	if errors.Is(err, util.ErrNotExist) {
+		ctx.APIErrorNotFound()
+		return nil
+	} else if err != nil {
+		ctx.APIErrorInternal(err)
+		return nil
+	}
+
+	if !organization.HasOrgOrUserVisible(ctx, org.AsUser(), ctx.Doer) {
+		ctx.APIErrorNotFound()
+		return nil
+	}
+
+	if !ctx.Doer.IsAdmin {
+		canCreate, err := org.CanCreateOrgRepo(ctx, ctx.Doer.ID)
+		if err != nil {
+			ctx.APIErrorInternal(err)
+			return nil
+		}
+		if !canCreate {
+			ctx.APIError(http.StatusForbidden, "User is not allowed to create repositories in this organization.")
+			return nil
+		}
+	}
+	return org
+}
+
 // CreateFork create a fork of a repo
 func CreateFork(ctx *context.APIContext) {
 	// swagger:operation POST /repos/{owner}/{repo}/forks repository createFork
@@ -118,41 +147,18 @@ func CreateFork(ctx *context.APIContext) {
 	//     "$ref": "#/responses/validationError"
 
 	form := web.GetForm(ctx).(*api.CreateForkOption)
-	repo := ctx.Repo.Repository
-	var forker *user_model.User // user/org that will own the fork
-	if form.Organization == nil {
-		forker = ctx.Doer
-	} else {
-		org, err := organization.GetOrgByName(ctx, *form.Organization)
-		if err != nil {
-			if organization.IsErrOrgNotExist(err) {
-				ctx.APIError(http.StatusUnprocessableEntity, err)
-			} else {
-				ctx.APIErrorInternal(err)
-			}
+	forkOwner := ctx.Doer // user/org that will own the fork
+	if form.Organization != nil {
+		org := prepareDoerCreateRepoInOrg(ctx, *form.Organization)
+		if ctx.Written() {
 			return
 		}
-		if !ctx.Doer.IsAdmin {
-			isMember, err := org.IsOrgMember(ctx, ctx.Doer.ID)
-			if err != nil {
-				ctx.APIErrorInternal(err)
-				return
-			} else if !isMember {
-				ctx.APIError(http.StatusForbidden, fmt.Sprintf("User is no Member of Organisation '%s'", org.Name))
-				return
-			}
-		}
-		forker = org.AsUser()
-	}
-
-	var name string
-	if form.Name == nil {
-		name = repo.Name
-	} else {
-		name = *form.Name
+		forkOwner = org.AsUser()
 	}
 
-	fork, err := repo_service.ForkRepository(ctx, ctx.Doer, forker, repo_service.ForkRepoOptions{
+	repo := ctx.Repo.Repository
+	name := optional.FromPtr(form.Name).ValueOrDefault(repo.Name)
+	fork, err := repo_service.ForkRepository(ctx, ctx.Doer, forkOwner, repo_service.ForkRepoOptions{
 		BaseRepo:    repo,
 		Name:        name,
 		Description: repo.Description,

routers/api/v1/repo/repo.go

@@ -498,31 +498,11 @@ func CreateOrgRepo(ctx *context.APIContext) {
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 	opt := web.GetForm(ctx).(*api.CreateRepoOption)
-	org, err := organization.GetOrgByName(ctx, ctx.PathParam("org"))
-	if err != nil {
-		if organization.IsErrOrgNotExist(err) {
-			ctx.APIError(http.StatusUnprocessableEntity, err)
-		} else {
-			ctx.APIErrorInternal(err)
-		}
-		return
-	}
-
-	if !organization.HasOrgOrUserVisible(ctx, org.AsUser(), ctx.Doer) {
-		ctx.APIErrorNotFound("HasOrgOrUserVisible", nil)
+	orgName := ctx.PathParam("org")
+	org := prepareDoerCreateRepoInOrg(ctx, orgName)
+	if ctx.Written() {
 		return
 	}
-
-	if !ctx.Doer.IsAdmin {
-		canCreate, err := org.CanCreateOrgRepo(ctx, ctx.Doer.ID)
-		if err != nil {
-			ctx.APIErrorInternal(err)
-			return
-		} else if !canCreate {
-			ctx.APIError(http.StatusForbidden, "Given user is not allowed to create repository in organization.")
-			return
-		}
-	}
 	CreateUserRepo(ctx, org.AsUser(), *opt)
 }
 

services/context/org.go

@@ -132,10 +132,12 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) {
 					ctx.ServerError("IsOrgMember", err)
 					return
 				}
-				ctx.Org.CanCreateOrgRepo, err = org.CanCreateOrgRepo(ctx, ctx.Doer.ID)
-				if err != nil {
-					ctx.ServerError("CanCreateOrgRepo", err)
-					return
+				if ctx.Org.IsMember {
+					ctx.Org.CanCreateOrgRepo, err = org.CanCreateOrgRepo(ctx, ctx.Doer.ID)
+					if err != nil {
+						ctx.ServerError("CanCreateOrgRepo", err)
+						return
+					}
 				}
 			}
 		} else {

tests/integration/api_fork_test.go

@@ -19,15 +19,35 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCreateForkNoLogin(t *testing.T) {
+func TestAPIFork(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
+	t.Run("CreateForkNoLogin", testCreateForkNoLogin)
+	t.Run("CreateForkOrgNoCreatePermission", testCreateForkOrgNoCreatePermission)
+	t.Run("APIForkListLimitedAndPrivateRepos", testAPIForkListLimitedAndPrivateRepos)
+	t.Run("GetPrivateReposForks", testGetPrivateReposForks)
+}
+
+func testCreateForkNoLogin(t *testing.T) {
 	req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{})
 	MakeRequest(t, req, http.StatusUnauthorized)
 }
 
-func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
+func testCreateForkOrgNoCreatePermission(t *testing.T) {
+	user4Sess := loginUser(t, "user4")
+	org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
+
+	canCreate, err := org_model.OrgFromUser(org).CanCreateOrgRepo(t.Context(), 4)
+	assert.NoError(t, err)
+	assert.False(t, canCreate)
+
+	user4Token := getTokenForLoggedInUser(t, user4Sess, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization)
+	req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{
+		Organization: &org.Name,
+	}).AddTokenAuth(user4Token)
+	MakeRequest(t, req, http.StatusForbidden)
+}
 
+func testAPIForkListLimitedAndPrivateRepos(t *testing.T) {
 	user1Sess := loginUser(t, "user1")
 	user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user1"})
 
@@ -64,10 +84,7 @@ func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) {
 
 		req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks")
 		resp := MakeRequest(t, req, http.StatusOK)
-
-		var forks []*api.Repository
-		DecodeJSON(t, resp, &forks)
-
+		forks := DecodeJSON(t, resp, []*api.Repository{})
 		assert.Empty(t, forks)
 		assert.Equal(t, "0", resp.Header().Get("X-Total-Count"))
 	})
@@ -78,47 +95,38 @@ func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) {
 		req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks").AddTokenAuth(user1Token)
 		resp := MakeRequest(t, req, http.StatusOK)
 
-		var forks []*api.Repository
-		DecodeJSON(t, resp, &forks)
-
+		forks := DecodeJSON(t, resp, []*api.Repository{})
 		assert.Len(t, forks, 2)
 		assert.Equal(t, "2", resp.Header().Get("X-Total-Count"))
 
 		assert.NoError(t, org_service.AddTeamMember(t.Context(), ownerTeam2, user1))
 
 		req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks").AddTokenAuth(user1Token)
 		resp = MakeRequest(t, req, http.StatusOK)
-
-		forks = []*api.Repository{}
-		DecodeJSON(t, resp, &forks)
-
+		forks = DecodeJSON(t, resp, []*api.Repository{})
 		assert.Len(t, forks, 2)
 		assert.Equal(t, "2", resp.Header().Get("X-Total-Count"))
 	})
 }
 
-func TestGetPrivateReposForks(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
+func testGetPrivateReposForks(t *testing.T) {
 	user1Sess := loginUser(t, "user1")
 	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) // private repository
 	privateOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23})
 	user1Token := getTokenForLoggedInUser(t, user1Sess, auth_model.AccessTokenScopeWriteRepository)
 
-	forkedRepoName := "forked-repo"
 	// create fork from a private repository
 	req := NewRequestWithJSON(t, "POST", "/api/v1/repos/"+repo2.FullName()+"/forks", &api.CreateForkOption{
 		Organization: &privateOrg.Name,
-		Name:         &forkedRepoName,
+		Name:         new("forked-repo"),
 	}).AddTokenAuth(user1Token)
 	MakeRequest(t, req, http.StatusAccepted)
 
 	// test get a private fork without clear permissions
 	req = NewRequest(t, "GET", "/api/v1/repos/"+repo2.FullName()+"/forks").AddTokenAuth(user1Token)
 	resp := MakeRequest(t, req, http.StatusOK)
 
-	forks := []*api.Repository{}
-	DecodeJSON(t, resp, &forks)
+	forks := DecodeJSON(t, resp, []*api.Repository{})
 	assert.Len(t, forks, 1)
 	assert.Equal(t, "1", resp.Header().Get("X-Total-Count"))
 	assert.Equal(t, "forked-repo", forks[0].Name)

tests/integration/integration_test.go

@@ -415,7 +415,7 @@ func DecodeJSON[T any](t testing.TB, resp *httptest.ResponseRecorder, v T) (ret
 
 	// FIXME: JSON-KEY-CASE: for testing purpose only, because many structs don't provide `json` tags, they just use capitalized field names
 	decoder := json.NewDecoderCaseInsensitive(resp.Body)
-	require.NoError(t, decoder.Decode(v))
+	require.NoError(t, decoder.Decode(&v))
 	return v
 }