diff --git a/routers/api/v1/repo/fork.go b/routers/api/v1/repo/fork.go index 16bfce9f19..38a3952879 100644 --- a/routers/api/v1/repo/fork.go +++ b/routers/api/v1/repo/fork.go @@ -6,7 +6,6 @@ package repo import ( "errors" - "fmt" "net/http" "code.gitea.io/gitea/models/organization" @@ -14,6 +13,7 @@ import ( 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() + forkOwner = org.AsUser() } - var name string - if form.Name == nil { - name = repo.Name - } else { - name = *form.Name - } - - 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, diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 81618fd25d..4a5091fded 100644 --- a/routers/api/v1/repo/repo.go +++ b/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) - } + orgName := ctx.PathParam("org") + org := prepareDoerCreateRepoInOrg(ctx, orgName) + if ctx.Written() { return } - - if !organization.HasOrgOrUserVisible(ctx, org.AsUser(), ctx.Doer) { - ctx.APIErrorNotFound("HasOrgOrUserVisible", nil) - 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) } diff --git a/services/context/org.go b/services/context/org.go index 4c64ff72a9..bd20d807ef 100644 --- a/services/context/org.go +++ b/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 { diff --git a/tests/integration/api_fork_test.go b/tests/integration/api_fork_test.go index bd2d38ef4f..f99cc29fab 100644 --- a/tests/integration/api_fork_test.go +++ b/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,9 +95,7 @@ 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")) @@ -88,28 +103,22 @@ func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) { 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) @@ -117,8 +126,7 @@ func TestGetPrivateReposForks(t *testing.T) { 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) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index dae2c1c3c5..d60f66e785 100644 --- a/tests/integration/integration_test.go +++ b/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 }